From 03d6be087748a4d640253d2614fafa313675d9ff Mon Sep 17 00:00:00 2001 From: "John Molakvoæ (skjnldsv)" Date: Mon, 7 May 2018 14:09:31 +0200 Subject: Added new tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- tests/lib/AvatarTest.php | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'tests/lib') diff --git a/tests/lib/AvatarTest.php b/tests/lib/AvatarTest.php index 4914c02bd14..a9b798fe5d7 100644 --- a/tests/lib/AvatarTest.php +++ b/tests/lib/AvatarTest.php @@ -48,6 +48,9 @@ class AvatarTest extends \Test\TestCase { $this->createMock(ILogger::class), $this->config ); + + // abcdefghi is a convenient name that our algorithm convert to our nextcloud blue 0082c9 + $this->user->method('getDisplayName')->willReturn('abcdefghi'); } public function testGetNoAvatar() { @@ -226,4 +229,37 @@ class AvatarTest extends \Test\TestCase { $this->avatar->set($image->data()); } + public function testGenerateSvgAvatar() { + $avatar = $this->avatar->getAvatarVector(64); + + $svg = ' + + + A + '; + $this->assertEquals($avatar, $svg); + } + + public function testHashToInt() { + $hashToInt = $this->invokePrivate($this->avatar, 'hashToInt', ['abcdef', 18]); + $this->assertTrue(gettype($hashToInt) === 'integer'); + } + + public function testMixPalette() { + $colorFrom = new \OC\Color(0,0,0); + $colorTo = new \OC\Color(6,12,18); + $steps = 6; + $palette = $this->invokePrivate($this->avatar, 'mixPalette', [$steps, $colorFrom, $colorTo]); + foreach($palette as $j => $color) { + // calc increment + $incR = $colorTo->r / $steps * $j; + $incG = $colorTo->g / $steps * $j; + $incB = $colorTo->b / $steps * $j; + // ensure everything is equal + $this->assertEquals($color, new \OC\Color($incR, $incG,$incB)); + } + $hashToInt = $this->invokePrivate($this->avatar, 'hashToInt', ['abcdef', 18]); + $this->assertTrue(gettype($hashToInt) === 'integer'); + } + } -- cgit v1.2.3 From 156da29ceade106176e2288ef391c4cb2006d800 Mon Sep 17 00:00:00 2001 From: "John Molakvoæ (skjnldsv)" Date: Mon, 28 May 2018 09:44:10 +0200 Subject: Avatar imagick bump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- core/Controller/AvatarController.php | 62 ++++++-------------------- lib/private/Avatar.php | 53 +++++++++++++++------- lib/public/IAvatar.php | 8 ---- tests/Core/Controller/AvatarControllerTest.php | 58 +++++------------------- tests/lib/AvatarTest.php | 2 +- 5 files changed, 63 insertions(+), 120 deletions(-) (limited to 'tests/lib') diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index e5a8e4fe29c..94c837888dc 100644 --- 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 diff --git a/lib/private/Avatar.php b/lib/private/Avatar.php index 0164d73a2e3..270f69a96ad 100644 --- 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 = ' - - - {letter} '; @@ -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 diff --git a/lib/public/IAvatar.php b/lib/public/IAvatar.php index 6b4f00d480b..54d703a502f 100644 --- 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] diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index 6d52a2c7ebf..3194d671908 100644 --- 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() { @@ -132,22 +129,11 @@ class AvatarControllerTest extends \Test\TestCase { * Fetch an avatar if a user has no avatar */ public function testGetAvatarNoAvatar() { - $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - $this->avatarMock->method('getAvatarVector')->willReturn(''); - $response = $this->avatarController->getAvatar('userId', 32); - - $this->assertEquals(Http::STATUS_OK, $response->getStatus()); - $this->assertEquals('', $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); + $response = $this->avatarController->getAvatar('userId', 32); + //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(''); - $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('', $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'); - } - } diff --git a/tests/lib/AvatarTest.php b/tests/lib/AvatarTest.php index a9b798fe5d7..67fde694b8e 100644 --- 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 = ' -- cgit v1.2.3 From 16ec9d96708793cc6d7016743d6d8ba9182088bd Mon Sep 17 00:00:00 2001 From: "John Molakvoæ (skjnldsv)" Date: Mon, 28 May 2018 11:06:33 +0200 Subject: Removed old route, fix tests and fix var type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- core/routes.php | 1 - lib/private/Avatar.php | 2 +- tests/lib/AvatarTest.php | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) (limited to 'tests/lib') diff --git a/core/routes.php b/core/routes.php index dd35638a7ee..cc1bd34d898 100644 --- a/core/routes.php +++ b/core/routes.php @@ -42,7 +42,6 @@ $application->registerRoutes($this, [ ['name' => 'lost#setPassword', 'url' => '/lostpassword/set/{token}/{userId}', 'verb' => 'POST'], ['name' => 'user#getDisplayNames', 'url' => '/displaynames', 'verb' => 'POST'], ['name' => 'avatar#getAvatar', 'url' => '/avatar/{userId}/{size}', 'verb' => 'GET'], - ['name' => 'avatar#getAvatarPng', 'url' => '/avatar/{userId}/{size}/png', 'verb' => 'GET'], ['name' => 'avatar#deleteAvatar', 'url' => '/avatar/', 'verb' => 'DELETE'], ['name' => 'avatar#postCroppedAvatar', 'url' => '/avatar/cropped', 'verb' => 'POST'], ['name' => 'avatar#getTmpAvatar', 'url' => '/avatar/tmp', 'verb' => 'GET'], diff --git a/lib/private/Avatar.php b/lib/private/Avatar.php index 270f69a96ad..9dbeb4ac745 100644 --- a/lib/private/Avatar.php +++ b/lib/private/Avatar.php @@ -301,7 +301,7 @@ class Avatar implements IAvatar { * Generate png avatar from svg with Imagick * * @param int $size - * @return string + * @return string|boolean */ private function generateAvatarFromSvg(int $size) { if (!extension_loaded('imagick')) { diff --git a/tests/lib/AvatarTest.php b/tests/lib/AvatarTest.php index 67fde694b8e..759dd385564 100644 --- a/tests/lib/AvatarTest.php +++ b/tests/lib/AvatarTest.php @@ -232,7 +232,7 @@ class AvatarTest extends \Test\TestCase { public function testGenerateSvgAvatar() { $avatar = $this->invokePrivate($this->avatar, 'getAvatarVector', [64]); - $svg = ' + $svg = ' A -- cgit v1.2.3