Avatar imagick bump

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
2018-05-28 09:44:10 +02:00 · 2018-05-28 09:44:10 +02:00 · 156da29cea
commit 156da29cea
parent 4432cfd445
5 changed files with 63 additions and 120 deletions
--- a/core/Controller/AvatarController.php
+++ b/core/Controller/AvatarController.php
@ -113,22 +113,6 @@ class AvatarController extends Controller {
 	}


-
-
-
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @NoSameSiteCookieRequired
-	 * @PublicPage
-	 * 
-	 * Shortcut to getAvatar
-	 */
-	public function getAvatarPng($userId, $size) {
-		return $this->getAvatar($userId, $size, true);
-	}
-
-
 	/**
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
@ -137,46 +121,28 @@ class AvatarController extends Controller {
 	 *
 	 * @param string $userId
 	 * @param int $size
-	 * @param bool $png return png or not
 	 * @return JSONResponse|FileDisplayResponse
 	 */
-	public function getAvatar($userId, $size, bool $png = false) {
+	public function getAvatar($userId, $size) {
+		// min/max size
 		if ($size > 2048) {
 			$size = 2048;
 		} elseif ($size <= 0) {
 			$size = 64;
 		}

-		// Serve png as a fallback only
-		if ($png === false) {
-
-			try {
-				$avatar = $this->avatarManager->getAvatar($userId)->getAvatarVector($size);
-				$resp = new DataDisplayResponse(
-					$avatar,
-					Http::STATUS_OK,
-					['Content-Type' => 'image/svg+xml'
-				]);
-			} catch (\Exception $e) {
-				$resp = new Http\Response();
-				$resp->setStatus(Http::STATUS_NOT_FOUND);
-				return $resp;
-			}
-
-		} else {
-
-			try {
-				$avatar = $this->avatarManager->getAvatar($userId)->getFile($size);
-				$resp = new FileDisplayResponse(
-					$avatar,
-					Http::STATUS_OK,
-					['Content-Type' => $avatar->getMimeType()
-				]);
-			} catch (\Exception $e) {
-				$resp = new Http\Response();
-				$resp->setStatus(Http::STATUS_NOT_FOUND);
-				return $resp;
-			}
+		try {
+			$avatar = $this->avatarManager->getAvatar($userId)->getFile($size);
+			$resp = new FileDisplayResponse(
+				$avatar,
+				Http::STATUS_OK,
+				['Content-Type' => $avatar->getMimeType()
+			]);
+		} catch (\Exception $e) {
+			var_dump($e);
+			$resp = new Http\Response();
+			$resp->setStatus(Http::STATUS_NOT_FOUND);
+			return $resp;
 		}

 		// Cache for 30 minutes
--- a/lib/private/Avatar.php
+++ b/lib/private/Avatar.php
@ -42,6 +42,7 @@ use OCP\IL10N;
 use OCP\ILogger;
 use OC\User\User;
 use OC_Image;
+use Imagick;

 /**
 * This class gets and sets users avatars.
@ -66,16 +67,8 @@ class Avatar implements IAvatar {
 	 * 
 	 * @var string 
 	 */
-	private $svgTemplate = '
+	private $svgTemplate = '<?xml version="1.0" encoding="UTF-8" standalone="no"?>
 		<svg width="{size}" height="{size}" version="1.1" viewBox="0 0 500 500" xmlns="http://www.w3.org/2000/svg">
-		<defs>
-			<style type="text/css">
-				@font-face {
-					font-family: Open Sans;
-					src: url({font});
-				}
-			</style>
-		</defs>
 			<rect width="100%" height="100%" fill="#{fill}"></rect>
 			<text x="50%" y="350" style="font-weight:600;font-size:278px;font-family:\'Open Sans\';text-anchor:middle;fill:#fff">{letter}</text>
 		</svg>';
@ -213,7 +206,9 @@ class Avatar implements IAvatar {
 		try {
 			$ext = $this->getExtension();
 		} catch (NotFoundException $e) {
-			$data = $this->generateAvatar($this->user->getDisplayName(), 1024);
+			if (!$data = $this->generateAvatarFromSvg(1024)) {
+				$data = $this->generateAvatar($this->user->getDisplayName(), 1024);
+			}
 			$avatar = $this->folder->newFile('avatar.png');
 			$avatar->putContent($data);
 			$ext = 'png';
@ -236,7 +231,9 @@ class Avatar implements IAvatar {
 			}

 			if ($this->folder->fileExists('generated')) {
-				$data = $this->generateAvatar($this->user->getDisplayName(), $size);
+				if (!$data = $this->generateAvatarFromSvg($size)) {
+					$data = $this->generateAvatar($this->user->getDisplayName(), $size);
+				}

 			} else {
 				$avatar = new OC_Image();
@ -289,19 +286,45 @@ class Avatar implements IAvatar {
 	 * @return string
 	 * 
 	 */
-	public function getAvatarVector(int $size): string {
+	private function getAvatarVector(int $size): string {
 		$userDisplayName = $this->user->getDisplayName();

 		$bgRGB = $this->avatarBackgroundColor($userDisplayName);
 		$bgHEX = sprintf("%02x%02x%02x", $bgRGB->r, $bgRGB->g, $bgRGB->b);
 		$letter = mb_strtoupper(mb_substr($userDisplayName, 0, 1), 'UTF-8');
-		$font = \OC::$WEBROOT.'/core/fonts/OpenSans-Semibold.ttf';
 		
-		$toReplace = ['{size}', '{fill}', '{letter}', '{font}'];
-		return str_replace($toReplace, [$size, $bgHEX, $letter, $font], $this->svgTemplate);
+		$toReplace = ['{size}', '{fill}', '{letter}'];
+		return str_replace($toReplace, [$size, $bgHEX, $letter], $this->svgTemplate);
 	}

 	/**
+	 * Generate png avatar from svg with Imagick
+	 * 
+	 * @param int $size
+	 * @return string
+	 */
+	private function generateAvatarFromSvg(int $size) {
+		if (!extension_loaded('imagick')) {
+			return false;
+		}
+		try {
+			$font = __DIR__ . '/../../core/fonts/OpenSans-Semibold.ttf';
+			$svg = $this->getAvatarVector($size);
+			$avatar = new Imagick();
+			$avatar->setFont($font);
+			$avatar->readImageBlob($svg);
+			$avatar->setImageFormat('png');
+			$image = new OC_Image();
+			$image->loadFromData($avatar);
+			return $image->data();
+		} catch (\Exception $e) {
+			return false;
+		}
+	}
+
+	/**
+	 * Generate png avatar with GD
+	 * 
 	 * @param string $userDisplayName
 	 * @param int $size
 	 * @return string
--- a/lib/public/IAvatar.php
+++ b/lib/public/IAvatar.php
@ -80,14 +80,6 @@ interface IAvatar {
     */
    public function getFile($size);

-    /**
-     * Generate SVG avatar
-     * @param int $size -1 can be used to not scale the image
-     * @return string
-     * @since 14.0.0
-     */
-    public function getAvatarVector(int $size): string;
-
 	/**
 	 * @param string $text
 	 * @return Color Object containting r g b int in the range [0, 255]
--- a/tests/Core/Controller/AvatarControllerTest.php
+++ b/tests/Core/Controller/AvatarControllerTest.php
@ -94,7 +94,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->timeFactory = $this->getMockBuilder('OC\AppFramework\Utility\TimeFactory')->getMock();

 		$this->avatarMock = $this->getMockBuilder('OCP\IAvatar')->getMock();
-		$this->color = new \OC\Color(0, 130, 201);
 		$this->userMock = $this->getMockBuilder(IUser::class)->getMock();

 		$this->avatarController = new AvatarController(
@ -120,8 +119,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->avatarFile->method('getContent')->willReturn('image data');
 		$this->avatarFile->method('getMimeType')->willReturn('image type');
 		$this->avatarFile->method('getEtag')->willReturn('my etag');
-
-		$this->avatarMock->method('avatarBackgroundColor')->willReturn($this->color);
 	}

 	public function tearDown() {
@ -133,21 +130,10 @@ class AvatarControllerTest extends \Test\TestCase {
 	 */
 	public function testGetAvatarNoAvatar() {
 		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);
-		$this->avatarMock->method('getAvatarVector')->willReturn('<svg></svg>');
+		$this->avatarMock->method('getFile')->will($this->throwException(new NotFoundException()));
 		$response = $this->avatarController->getAvatar('userId', 32);

-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertEquals('<svg></svg>', $response->getData());
-	}
-
-	/**
-	 * Fetch a png avatar if a user has no avatar
-	 */
-	public function testGetPngAvatarNoAvatar() {
-		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);
-		$this->avatarMock->method('getFile')->will($this->throwException(new NotFoundException()));
-		$response = $this->avatarController->getAvatar('userId', 32, true);
-
+		//Comment out until JS is fixed
 		$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
 	}

@ -155,26 +141,10 @@ class AvatarControllerTest extends \Test\TestCase {
 	 * Fetch the user's avatar
 	 */
 	public function testGetAvatar() {
-		$this->avatarMock->method('getAvatarVector')->willReturn('<svg></svg>');
-		$this->avatarManager->method('getAvatar')->with('userId')->willReturn($this->avatarMock);
-
-		$response = $this->avatarController->getAvatar('userId', 32);
-
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertArrayHasKey('Content-Type', $response->getHeaders());
-		$this->assertEquals('image/svg+xml', $response->getHeaders()['Content-Type']);
-
-		$this->assertEquals('<svg></svg>', $response->getData());
-	}
-
-	/**
-	 * Fetch the user's avatar
-	 */
-	public function testGetPngAvatar() {
 		$this->avatarMock->method('getFile')->willReturn($this->avatarFile);
 		$this->avatarManager->method('getAvatar')->with('userId')->willReturn($this->avatarMock);

-		$response = $this->avatarController->getAvatar('userId', 32, true);
+		$response = $this->avatarController->getAvatar('userId', 32);

 		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
 		$this->assertArrayHasKey('Content-Type', $response->getHeaders());
@ -201,7 +171,7 @@ class AvatarControllerTest extends \Test\TestCase {
 	/**
 	 * Make sure we get the correct size
 	 */
-	public function testGetPngAvatarSize() {
+	public function testGetAvatarSize() {
 		$this->avatarMock->expects($this->once())
 			->method('getFile')
 			->with($this->equalTo(32))
@ -209,13 +179,13 @@ class AvatarControllerTest extends \Test\TestCase {

 		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);

-		$this->avatarController->getAvatar('userId', 32, true);
+		$this->avatarController->getAvatar('userId', 32);
 	}

 	/**
 	 * We cannot get avatars that are 0 or negative
 	 */
-	public function testGetPngAvatarSizeMin() {
+	public function testGetAvatarSizeMin() {
 		$this->avatarMock->expects($this->once())
 			->method('getFile')
 			->with($this->equalTo(64))
@ -223,13 +193,13 @@ class AvatarControllerTest extends \Test\TestCase {

 		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);

-		$this->avatarController->getAvatar('userId', 0, true);
+		$this->avatarController->getAvatar('userId', 0);
 	}

 	/**
 	 * We do not support avatars larger than 2048*2048
 	 */
-	public function testGetPngAvatarSizeMax() {
+	public function testGetAvatarSizeMax() {
 		$this->avatarMock->expects($this->once())
 			->method('getFile')
 			->with($this->equalTo(2048))
@ -237,7 +207,7 @@ class AvatarControllerTest extends \Test\TestCase {

 		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);

-		$this->avatarController->getAvatar('userId', 2049, true);
+		$this->avatarController->getAvatar('userId', 2049);
 	}

 	/**
@ -516,6 +486,7 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]));
 	}

+
 	/**
 	 * Check for proper reply on proper crop argument
 	 */
@ -530,13 +501,4 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->assertEquals('File is too big', $response->getData()['data']['message']);
 	}

-	/**
-	 * Test get Avatar BG colour algorithm
-	 */
-	public function testAvatarBackgroundColor() {
-		$bgRGB = $this->avatarMock->avatarBackgroundColor('TestBlue');
-		$this->assertEquals($bgRGB, $this->color);
-		$this->assertEquals(sprintf("%02x%02x%02x", $bgRGB->r, $bgRGB->g, $bgRGB->b), '0082c9');
-	}
-
 }
--- a/tests/lib/AvatarTest.php
+++ b/tests/lib/AvatarTest.php
@ -230,7 +230,7 @@ class AvatarTest extends \Test\TestCase {
 	}

 	public function testGenerateSvgAvatar() {
-		$avatar = $this->avatar->getAvatarVector(64);
+		$avatar = $this->invokePrivate($this->avatar, 'getAvatarVector', [64]);
 		
 		$svg = '
 		<svg width="64" height="64" version="1.1" viewBox="0 0 500 500" xmlns="http://www.w3.org/2000/svg">