From 156da29ceade106176e2288ef391c4cb2006d800 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?=
<skjnldsv@protonmail.com>
Date: Mon, 28 May 2018 09:44:10 +0200
Subject: [PATCH] Avatar imagick bump
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
---
core/Controller/AvatarController.php | 62 +++++--------------
lib/private/Avatar.php | 53 +++++++++++-----
lib/public/IAvatar.php | 8 ---
.../Core/Controller/AvatarControllerTest.php | 58 +++--------------
tests/lib/AvatarTest.php | 2 +-
5 files changed, 63 insertions(+), 120 deletions(-)
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 = '<?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
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('<svg></svg>');
- $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);
+ $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('<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');
- }
-
}
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 = '
<svg width="64" height="64" version="1.1" viewBox="0 0 500 500" xmlns="http://www.w3.org/2000/svg">
--
GitLab