From b3eab7db01f8e6dc4c98a1f89171052d8bf762b5 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 11 Aug 2016 23:25:41 +0200 Subject: Theming: Add dynamic icon and favicon endpoints Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 13 ++ apps/theming/lib/Controller/IconController.php | 241 +++++++++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 apps/theming/lib/Controller/IconController.php diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index 4a8d4bac5bc..1894d810287 100644 --- a/apps/theming/appinfo/routes.php +++ b/apps/theming/appinfo/routes.php @@ -60,5 +60,18 @@ return ['routes' => [ 'url' => '/js/theming', 'verb' => 'GET', ], + [ + 'name' => 'Icon#getFavicon', + 'url' => '/favicon/{app}', + 'verb' => 'GET', + 'defaults' => array("app" => "core"), + ], + [ + 'name' => 'Icon#getThemedIcon', + 'url' => '/image/{app}/{image}', + 'verb' => 'GET', + 'defaults' => array("app" => "core"), + 'requirements' => array('image' => '.+') + ], ]]; diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php new file mode 100644 index 00000000000..e1690c039d3 --- /dev/null +++ b/apps/theming/lib/Controller/IconController.php @@ -0,0 +1,241 @@ + + * + * @author Julius Haertl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Theming\Controller; + +use OCA\Theming\Template; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\AppFramework\Http\StreamResponse; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Files\IRootFolder; +use OCP\IConfig; +use OCP\IL10N; +use OCP\IRequest; +use OCA\Theming\Util; +use OCP\IURLGenerator; +use Imagick; +use ImagickPixel; + +class IconController extends Controller { + /** @var Template */ + private $template; + /** @var Util */ + private $util; + /** @var ITimeFactory */ + private $timeFactory; + /** @var IL10N */ + private $l; + /** @var IConfig */ + private $config; + /** @var IRootFolder */ + private $rootFolder; + + /** + * IconController constructor. + * + * @param string $appName + * @param IRequest $request + * @param IConfig $config + * @param Template $template + * @param Util $util + * @param ITimeFactory $timeFactory + * @param IL10N $l + * @param IRootFolder $rootFolder + */ + public function __construct( + $appName, + IRequest $request, + IConfig $config, + Template $template, + Util $util, + ITimeFactory $timeFactory, + IL10N $l, + IRootFolder $rootFolder + ) { + parent::__construct($appName, $request); + + $this->template = $template; + $this->util = $util; + $this->timeFactory = $timeFactory; + $this->l = $l; + $this->config = $config; + $this->rootFolder = $rootFolder; + } + + /** + * @PublicPage + * @NoCSRFRequired + * + * @param $app app name + * @param $image image file name + * @return StreamResponse|DataResponse + */ + public function getThemedIcon($app, $image) { + $image = $this->getAppImage($app, $image); + $svg = file_get_contents($image); + $color = $this->template->getMailHeaderColor(); + $svg = $this->colorizeSvg($svg, $color); + return new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + } + + /** + * Return a 32x32 favicon as png + * + * @PublicPage + * @NoCSRFRequired + * + * @param $app app name + * @return StreamResponse|DataResponse + */ + public function getFavicon($app) { + // TODO: we need caching here + $icon = $this->renderAppIcon($app); + $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); + $icon->setImageFormat("png24"); + + $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response->cacheFor(3600); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + return $response; + } + + private function renderAppIcon($app) { + $appIcon = $this->getAppIcon($app); + $color = $this->config->getAppValue($this->appName, 'color'); + if ($color === "") { + $color = '#0082c9'; + } + $svg = file_get_contents($appIcon); + if ($this->util->invertTextColor($color)) { + $svg = $this->svgInvert($svg); + } + + // generate background image with rounded corners + $background = '' . + '' . + '' . + ''; + + $tmp = new Imagick(); + $tmp->readImageBlob($svg); + $x = $tmp->getImageWidth(); + $y = $tmp->getImageHeight(); + $res = $tmp->getImageResolution(); + $tmp->destroy(); + + // convert svg to resized image + $appIconFile = new Imagick(); + $resX = (int)(512 * $res['x'] / $x * 2.53); + $resY = (int)(512 * $res['y'] / $y * 2.53); + $appIconFile->setResolution($resX, $resY); + $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); + $appIconFile->readImageBlob($svg); + $appIconFile->setImageFormat("png24"); + + // offset for icon positioning + $offset_w = (int)($appIconFile->getImageWidth() * 0.05); + $offset_h = (int)($appIconFile->getImageHeight() * 0.05); + // center icon if it is not square + if ($x > $y) { + $offset_h += 512 / 2 - $appIconFile->getImageHeight() / 2; + } + if ($y > $x) { + $offset_h += 512 / 2 - $appIconFile->getImageHeight() / 2; + } + + $innerWidth = (int)($appIconFile->getImageWidth() - $offset_w * 2); + $innerHeight = (int)($appIconFile->getImageHeight() - $offset_h * 2); + $appIconFile->adaptiveResizeImage($innerWidth, $innerHeight); + + $finalIconFile = new Imagick(); + $finalIconFile->readImageBlob($background); + $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); + $finalIconFile->setImageArtifact('compose:args', "1,0,-0.5,0.5"); + $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); + $appIconFile->destroy(); + return $finalIconFile; + } + + private function getAppIcon($app) { + $appPath = \OC_App::getAppPath($app); + + $icon = $appPath . '/img/' . $app . '.svg'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/app.svg'; + if(file_exists($icon)) { + return $icon; + } + + return \OC::$SERVERROOT . '/core/img/logo.svg'; + } + + private function getAppImage($app, $image) { + // TODO: add support for images in core/img/ + $appPath = \OC_App::getAppPath($app); + + if($app==="core") { + $icon = \OC::$SERVERROOT . '/core/img/' . $image; + if(file_exists($icon)) { + return $icon; + } + } + + $icon = $appPath . '/img/' . $image; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.svg'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.png'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.gif'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.jpg'; + if(file_exists($icon)) { + return $icon; + } + } + + private function svgInvert($svg) { + $svg = preg_replace('/#(f{3,6})/i', '#REPLACECOLOR', $svg); + $svg = preg_replace('/#(0{3,6})/i', '#ffffff', $svg); + $svg = preg_replace('/#(REPLACECOLOR)/i', '#000000', $svg); + return $svg; + } + + private function colorizeSvg($svg, $color) { + $svg = preg_replace('/#0082c9/i', $color, $svg); + return $svg; + } + +} \ No newline at end of file -- cgit v1.2.3 From e5f8f28d04df4ca046c80fa10275f0341d9ea25c Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 11 Aug 2016 23:27:26 +0200 Subject: Core: Load mimetype icons from theming app if available Signed-off-by: Julius Haertl --- core/js/mimetype.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/js/mimetype.js b/core/js/mimetype.js index 0d30da26c26..b830e673151 100644 --- a/core/js/mimetype.js +++ b/core/js/mimetype.js @@ -91,6 +91,11 @@ OC.MimeType = { path += icon; } } + if(OCA.Theming) { + path = OC.generateUrl('/apps/theming/image/core/filetypes/'); + path += OC.MimeType._getFile(mimeType, OC.MimeTypeList.files); + gotIcon = true; + } // If we do not yet have an icon fall back to the default if (gotIcon === null) { -- cgit v1.2.3 From da6285b84f175640622a020f2d9258cee86a3c14 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 11 Aug 2016 23:29:53 +0200 Subject: Core: load favicon from theming app Signed-off-by: Julius Haertl --- lib/private/URLGenerator.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 93517dc9f7e..e730bf7aedd 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -157,7 +157,9 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; - if (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { + if(\OCP\App::isEnabled('theming') && $image === "favicon.ico" && $app !== "") { + $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]); + } elseif (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { $path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image"; } elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.svg") && file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.png")) { -- cgit v1.2.3 From 2d65b8c600580cd64aa599a791a467101b873c5a Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 14 Aug 2016 12:24:51 +0200 Subject: Theming: Add favicon-touch and fix icon creation with non svg images Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 8 ++- apps/theming/lib/Controller/IconController.php | 96 +++++++++++++++++--------- lib/private/URLGenerator.php | 4 +- 3 files changed, 75 insertions(+), 33 deletions(-) diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index 1894d810287..a6511668d14 100644 --- a/apps/theming/appinfo/routes.php +++ b/apps/theming/appinfo/routes.php @@ -66,9 +66,15 @@ return ['routes' => [ 'verb' => 'GET', 'defaults' => array("app" => "core"), ], + [ + 'name' => 'Icon#getTouchIcon', + 'url' => '/icon/{app}', + 'verb' => 'GET', + 'defaults' => array("app" => "core"), + ], [ 'name' => 'Icon#getThemedIcon', - 'url' => '/image/{app}/{image}', + 'url' => '/img/{app}/{image}', 'verb' => 'GET', 'defaults' => array("app" => "core"), 'requirements' => array('image' => '.+') diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index e1690c039d3..d7c27a88134 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -121,53 +121,87 @@ class IconController extends Controller { return $response; } + /** + * Return a 512x512 icon for touch devices + * + * @PublicPage + * @NoCSRFRequired + * + * @param $app app name + * @return StreamResponse|DataResponse + */ + public function getTouchIcon($app) { + // TODO: we need caching here + $icon = $this->renderAppIcon($app); + $icon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); + $icon->setImageFormat("png24"); + + $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $response->cacheFor(3600); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + return $response; + } + + /** + * Render app icon on themed background color + * fallback to logo + * + * @param $app app name + * @return Imagick + */ private function renderAppIcon($app) { $appIcon = $this->getAppIcon($app); $color = $this->config->getAppValue($this->appName, 'color'); + $mime = mime_content_type($appIcon); if ($color === "") { $color = '#0082c9'; } - $svg = file_get_contents($appIcon); - if ($this->util->invertTextColor($color)) { - $svg = $this->svgInvert($svg); - } - // generate background image with rounded corners $background = '' . '' . '' . ''; - $tmp = new Imagick(); - $tmp->readImageBlob($svg); - $x = $tmp->getImageWidth(); - $y = $tmp->getImageHeight(); - $res = $tmp->getImageResolution(); - $tmp->destroy(); - - // convert svg to resized image - $appIconFile = new Imagick(); - $resX = (int)(512 * $res['x'] / $x * 2.53); - $resY = (int)(512 * $res['y'] / $y * 2.53); - $appIconFile->setResolution($resX, $resY); - $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $appIconFile->readImageBlob($svg); - $appIconFile->setImageFormat("png24"); - // offset for icon positioning - $offset_w = (int)($appIconFile->getImageWidth() * 0.05); - $offset_h = (int)($appIconFile->getImageHeight() * 0.05); - // center icon if it is not square - if ($x > $y) { - $offset_h += 512 / 2 - $appIconFile->getImageHeight() / 2; - } - if ($y > $x) { - $offset_h += 512 / 2 - $appIconFile->getImageHeight() / 2; + // resize svg magic as this seems broken in Imagemagick + if($mime === "image/svg+xml") { + $svg = file_get_contents($appIcon); + if ($this->util->invertTextColor($color)) { + $svg = $this->svgInvert($svg); + } + + $tmp = new Imagick(); + $tmp->readImageBlob($svg); + $x = $tmp->getImageWidth(); + $y = $tmp->getImageHeight(); + $res = $tmp->getImageResolution(); + $tmp->destroy(); + + // convert svg to resized image + $appIconFile = new Imagick(); + $resX = (int)(512 * $res['x'] / $x * 2.53); + $resY = (int)(512 * $res['y'] / $y * 2.53); + $appIconFile->setResolution($resX, $resY); + $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); + $appIconFile->readImageBlob($svg); + } else { + $appIconFile = new Imagick(); + $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); + $appIconFile->readImageBlob(file_get_contents($appIcon)); + $appIconFile->scaleImage(512, 512, true); } - $innerWidth = (int)($appIconFile->getImageWidth() - $offset_w * 2); - $innerHeight = (int)($appIconFile->getImageHeight() - $offset_h * 2); + // offset for icon positioning + $border_w = (int)($appIconFile->getImageWidth() * 0.05); + $border_h = (int)($appIconFile->getImageHeight() * 0.05); + $innerWidth = (int)($appIconFile->getImageWidth() - $border_w * 2); + $innerHeight = (int)($appIconFile->getImageHeight() - $border_h * 2); $appIconFile->adaptiveResizeImage($innerWidth, $innerHeight); + // center icon + $offset_w = 512 / 2 - $innerWidth / 2; + $offset_h = 512 / 2 - $innerHeight / 2; + + $appIconFile->setImageFormat("png24"); $finalIconFile = new Imagick(); $finalIconFile->readImageBlob($background); diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index e730bf7aedd..8972f1602e3 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -157,8 +157,10 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; - if(\OCP\App::isEnabled('theming') && $image === "favicon.ico" && $app !== "") { + if(\OCP\App::isEnabled('theming') && $image === "favicon.ico") { $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]); + } elseif(\OCP\App::isEnabled('theming') && $image === "favicon-touch.png") { + $path = $this->linkToRoute('theming.Icon.getTouchIcon', [ 'app' => $app ]); } elseif (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { $path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image"; } elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.svg") -- cgit v1.2.3 From 64510bd87aaca44af3048121aff38d65c7712ae3 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 14 Aug 2016 12:26:05 +0200 Subject: Theming: Add PHPdoc and icon fallback to theming logo Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 27 +++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index d7c27a88134..7ceaa599232 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -212,6 +212,10 @@ class IconController extends Controller { return $finalIconFile; } + /** + * @param $app app name + * @return string path to app icon / logo + */ private function getAppIcon($app) { $appPath = \OC_App::getAppPath($app); @@ -224,11 +228,19 @@ class IconController extends Controller { return $icon; } + $icon = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; + if(file_exists($icon)) { + return $icon; + } return \OC::$SERVERROOT . '/core/img/logo.svg'; } + /** + * @param $app app name + * @param $image relative path to image in app folder + * @return string absolute path to image + */ private function getAppImage($app, $image) { - // TODO: add support for images in core/img/ $appPath = \OC_App::getAppPath($app); if($app==="core") { @@ -260,6 +272,12 @@ class IconController extends Controller { } } + /** + * replace black with white and white with black + * + * @param $svg content of a svg file + * @return string + */ private function svgInvert($svg) { $svg = preg_replace('/#(f{3,6})/i', '#REPLACECOLOR', $svg); $svg = preg_replace('/#(0{3,6})/i', '#ffffff', $svg); @@ -267,6 +285,13 @@ class IconController extends Controller { return $svg; } + /** + * replace default color with a custom one + * + * @param $svg content of a svg file + * @param $color color to match + * @return string + */ private function colorizeSvg($svg, $color) { $svg = preg_replace('/#0082c9/i', $color, $svg); return $svg; -- cgit v1.2.3 From c7adcb85aefc384a59c6081015537a603a675c3e Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 14 Aug 2016 12:53:16 +0200 Subject: Theming: Fix default parameters in icon routes Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 1 - apps/theming/lib/Controller/IconController.php | 4 ++-- lib/private/URLGenerator.php | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index a6511668d14..a8436ab254f 100644 --- a/apps/theming/appinfo/routes.php +++ b/apps/theming/appinfo/routes.php @@ -76,7 +76,6 @@ return ['routes' => [ 'name' => 'Icon#getThemedIcon', 'url' => '/img/{app}/{image}', 'verb' => 'GET', - 'defaults' => array("app" => "core"), 'requirements' => array('image' => '.+') ], ]]; diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 7ceaa599232..6e51f10c4cc 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -109,7 +109,7 @@ class IconController extends Controller { * @param $app app name * @return StreamResponse|DataResponse */ - public function getFavicon($app) { + public function getFavicon($app="core") { // TODO: we need caching here $icon = $this->renderAppIcon($app); $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); @@ -130,7 +130,7 @@ class IconController extends Controller { * @param $app app name * @return StreamResponse|DataResponse */ - public function getTouchIcon($app) { + public function getTouchIcon($app="core") { // TODO: we need caching here $icon = $this->renderAppIcon($app); $icon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 8972f1602e3..f8b79dedd8e 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -158,8 +158,10 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; if(\OCP\App::isEnabled('theming') && $image === "favicon.ico") { + if($app==="") { $app = "core"; } $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]); } elseif(\OCP\App::isEnabled('theming') && $image === "favicon-touch.png") { + if($app==="") { $app = "core"; } $path = $this->linkToRoute('theming.Icon.getTouchIcon', [ 'app' => $app ]); } elseif (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { $path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image"; -- cgit v1.2.3 From d1fcfe8e7dd7f6acf06e6060cbedbca53a6eca0a Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 14 Aug 2016 12:54:06 +0200 Subject: Theming: Add colorized icon css from icons.css Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/ThemingController.php | 9 +++++++ .../tests/Controller/ThemingControllerTest.php | 31 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 09b4a14f2b0..7ba4feb62dd 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -403,6 +403,15 @@ class ThemingController extends Controller { $responseCss .= '.nc-theming-contrast {color: #ffffff}' . "\n"; } + if($logo !== '' or $color !== '') { + $responseCss .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v='.$cacheBusterValue.'\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v='.$cacheBusterValue.'\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v='.$cacheBusterValue.'\')!important;' . "}\n"; + } + $response = new DataDownloadResponse($responseCss, 'style', 'text/css'); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $response->addHeader('Pragma', 'cache'); diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index d9d5005e25f..5de16a7abdf 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -475,6 +475,12 @@ class ThemingControllerTest extends TestCase { $expectedData .= sprintf('.nc-theming-main-background {background-color: %s}' . "\n", $color); $expectedData .= sprintf('.nc-theming-main-text {color: %s}' . "\n", $color); $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; + $expectedData .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); @@ -569,6 +575,12 @@ class ThemingControllerTest extends TestCase { $expectedData .= '#body-login input.login { background-image: url(\'' . \OC::$WEBROOT . '/core/img/actions/confirm.svg?v=2\'); }' . "\n"; $expectedData .= '.nc-theming-contrast {color: #000000}' . "\n"; $expectedData .= '.ui-widget-header { color: #000000; }' . "\n"; + $expectedData .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); @@ -614,6 +626,12 @@ class ThemingControllerTest extends TestCase { 'background-size: contain;' . '}' . "\n"; $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; + $expectedData .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); @@ -756,6 +774,12 @@ class ThemingControllerTest extends TestCase { 'background-image: url(\'./loginbackground?v=0\');' . '}' . "\n"; $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; + $expectedData .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); $expected->cacheFor(3600); @@ -867,6 +891,13 @@ class ThemingControllerTest extends TestCase { $expectedData .= '#body-login input.login { background-image: url(\'' . \OC::$WEBROOT . '/core/img/actions/confirm.svg?v=2\'); }' . "\n"; $expectedData .= '.nc-theming-contrast {color: #000000}' . "\n"; $expectedData .= '.ui-widget-header { color: #000000; }' . "\n"; + $expectedData .= '.icon-file,.icon-filetype-text {' . + 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . + '.icon-folder, .icon-filetype-folder ' . + 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . + '.icon-filetype-folder-drag-accept {' . + 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; + $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); $expected->cacheFor(3600); -- cgit v1.2.3 From 6c7ebb120485a95f500fc3c3a1c565872980b54f Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 16 Aug 2016 10:30:56 +0200 Subject: Theming: Add icon caching Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 6e51f10c4cc..bbfa73b29ca 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -97,7 +97,11 @@ class IconController extends Controller { $svg = file_get_contents($image); $color = $this->template->getMailHeaderColor(); $svg = $this->colorizeSvg($svg, $color); - return new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + + $response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + return $response; } /** @@ -110,7 +114,6 @@ class IconController extends Controller { * @return StreamResponse|DataResponse */ public function getFavicon($app="core") { - // TODO: we need caching here $icon = $this->renderAppIcon($app); $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); $icon->setImageFormat("png24"); @@ -131,7 +134,6 @@ class IconController extends Controller { * @return StreamResponse|DataResponse */ public function getTouchIcon($app="core") { - // TODO: we need caching here $icon = $this->renderAppIcon($app); $icon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); $icon->setImageFormat("png24"); -- cgit v1.2.3 From 4945592a86ad3bfc71d87b60621830c3ca1c21ee Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 25 Aug 2016 16:18:54 +0200 Subject: Fix IconController after rebase Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 24 ++++++++++++------------ core/js/mimetype.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index bbfa73b29ca..716d590fe55 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -23,6 +23,7 @@ namespace OCA\Theming\Controller; use OCA\Theming\Template; +use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -39,8 +40,8 @@ use Imagick; use ImagickPixel; class IconController extends Controller { - /** @var Template */ - private $template; + /** @var ThemingDefaults */ + private $themingDefaults; /** @var Util */ private $util; /** @var ITimeFactory */ @@ -58,7 +59,7 @@ class IconController extends Controller { * @param string $appName * @param IRequest $request * @param IConfig $config - * @param Template $template + * @param ThemingDefaults $themingDefaults * @param Util $util * @param ITimeFactory $timeFactory * @param IL10N $l @@ -68,7 +69,7 @@ class IconController extends Controller { $appName, IRequest $request, IConfig $config, - Template $template, + ThemingDefaults $themingDefaults, Util $util, ITimeFactory $timeFactory, IL10N $l, @@ -76,7 +77,7 @@ class IconController extends Controller { ) { parent::__construct($appName, $request); - $this->template = $template; + $this->themingDefaults = $themingDefaults; $this->util = $util; $this->timeFactory = $timeFactory; $this->l = $l; @@ -95,7 +96,7 @@ class IconController extends Controller { public function getThemedIcon($app, $image) { $image = $this->getAppImage($app, $image); $svg = file_get_contents($image); - $color = $this->template->getMailHeaderColor(); + $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); $svg = $this->colorizeSvg($svg, $color); $response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); @@ -119,7 +120,7 @@ class IconController extends Controller { $icon->setImageFormat("png24"); $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); - $response->cacheFor(3600); + $response->cacheFor(86400); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); return $response; } @@ -139,7 +140,7 @@ class IconController extends Controller { $icon->setImageFormat("png24"); $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); - $response->cacheFor(3600); + $response->cacheFor(86400); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); return $response; } @@ -153,7 +154,7 @@ class IconController extends Controller { */ private function renderAppIcon($app) { $appIcon = $this->getAppIcon($app); - $color = $this->config->getAppValue($this->appName, 'color'); + $color = $this->themingDefaults->getMailHeaderColor(); $mime = mime_content_type($appIcon); if ($color === "") { $color = '#0082c9'; @@ -230,9 +231,8 @@ class IconController extends Controller { return $icon; } - $icon = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; - if(file_exists($icon)) { - return $icon; + if($this->rootFolder->nodeExists('/themedinstancelogo')) { + return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; } return \OC::$SERVERROOT . '/core/img/logo.svg'; } diff --git a/core/js/mimetype.js b/core/js/mimetype.js index b830e673151..3ce4ceccbb0 100644 --- a/core/js/mimetype.js +++ b/core/js/mimetype.js @@ -92,7 +92,7 @@ OC.MimeType = { } } if(OCA.Theming) { - path = OC.generateUrl('/apps/theming/image/core/filetypes/'); + path = OC.generateUrl('/apps/theming/img/core/filetypes/'); path += OC.MimeType._getFile(mimeType, OC.MimeTypeList.files); gotIcon = true; } -- cgit v1.2.3 From 9ac5476c4eb4d286ae1c48996e916180e3a54035 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 25 Aug 2016 16:19:22 +0200 Subject: Add cachebuster to favicons Signed-off-by: Julius Haertl --- lib/private/URLGenerator.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index f8b79dedd8e..8290fcb1337 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -158,11 +158,13 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; if(\OCP\App::isEnabled('theming') && $image === "favicon.ico") { + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); if($app==="") { $app = "core"; } - $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]); + $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]) . '?v='. $cacheBusterValue; } elseif(\OCP\App::isEnabled('theming') && $image === "favicon-touch.png") { + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); if($app==="") { $app = "core"; } - $path = $this->linkToRoute('theming.Icon.getTouchIcon', [ 'app' => $app ]); + $path = $this->linkToRoute('theming.Icon.getTouchIcon', [ 'app' => $app ]) . '?v='. $cacheBusterValue; } elseif (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { $path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image"; } elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.svg") -- cgit v1.2.3 From ac7f852ed996079785e6b5f1c1789f76a7a26386 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Thu, 25 Aug 2016 16:26:28 +0200 Subject: Theming: Add IconController tests Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 25 +-- .../tests/Controller/IconControllerTest.php | 201 +++++++++++++++++++++ 2 files changed, 207 insertions(+), 19 deletions(-) create mode 100644 apps/theming/tests/Controller/IconControllerTest.php diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 716d590fe55..56782992ab3 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -136,7 +136,6 @@ class IconController extends Controller { */ public function getTouchIcon($app="core") { $icon = $this->renderAppIcon($app); - $icon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); $icon->setImageFormat("png24"); $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); @@ -156,6 +155,7 @@ class IconController extends Controller { $appIcon = $this->getAppIcon($app); $color = $this->themingDefaults->getMailHeaderColor(); $mime = mime_content_type($appIcon); + // FIXME: test if we need this if ($color === "") { $color = '#0082c9'; } @@ -169,10 +169,7 @@ class IconController extends Controller { // resize svg magic as this seems broken in Imagemagick if($mime === "image/svg+xml") { $svg = file_get_contents($appIcon); - if ($this->util->invertTextColor($color)) { - $svg = $this->svgInvert($svg); - } - + $tmp = new Imagick(); $tmp->readImageBlob($svg); $x = $tmp->getImageWidth(); @@ -211,6 +208,8 @@ class IconController extends Controller { $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); $finalIconFile->setImageArtifact('compose:args', "1,0,-0.5,0.5"); $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); + $finalIconFile->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); + $appIconFile->destroy(); return $finalIconFile; } @@ -272,21 +271,9 @@ class IconController extends Controller { if(file_exists($icon)) { return $icon; } + return false; } - - /** - * replace black with white and white with black - * - * @param $svg content of a svg file - * @return string - */ - private function svgInvert($svg) { - $svg = preg_replace('/#(f{3,6})/i', '#REPLACECOLOR', $svg); - $svg = preg_replace('/#(0{3,6})/i', '#ffffff', $svg); - $svg = preg_replace('/#(REPLACECOLOR)/i', '#000000', $svg); - return $svg; - } - + /** * replace default color with a custom one * diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php new file mode 100644 index 00000000000..e22369aa744 --- /dev/null +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -0,0 +1,201 @@ + + * + * @author Julius Haertl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Theming\Tests\Controller; + +use OCA\Theming\Controller\IconController; +use OCA\Theming\Util; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\Files\IRootFolder; +use OCP\IConfig; +use OCP\IL10N; +use OCP\IRequest; +use Test\TestCase; +use OCA\Theming\ThemingDefaults; +use \Imagick; + +class ThemingControllerTest extends TestCase { + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; + /** @var ThemingDefaults|\PHPUnit_Framework_MockObject_MockObject */ + private $themingDefaults; + /** @var Util */ + private $util; + /** @var \OCP\AppFramework\Utility\ITimeFactory */ + private $timeFactory; + /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ + private $l10n; + /** @var IconController */ + private $iconController; + /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ + private $rootFolder; + + public function setUp() { + $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); + $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); + $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') + ->disableOriginalConstructor()->getMock(); + $this->util = new Util(); + $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') + ->disableOriginalConstructor() + ->getMock(); + $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(123); + + $this->iconController = new IconController( + 'theming', + $this->request, + $this->config, + $this->themingDefaults, + $this->util, + $this->timeFactory, + $this->l10n, + $this->rootFolder + ); + + return parent::setUp(); + } + + public function testGetThemedIcon() { + $this->themingDefaults + ->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn('#000000'); + $svg = " + + + + + +"; + $expected = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + @$this->assertEquals($expected, $this->iconController->getThemedIcon('core','filetypes/folder.svg')); + } + + public function testGetFaviconDefault() { + $favicon = $this->iconController->getFavicon(); + + $expectedIcon = $this->invokePrivate($this->iconController, 'renderAppIcon', ["core"]); + $expectedIcon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); + $expectedIcon->setImageFormat("png24"); + + $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $this->assertEquals($expected, $favicon); + } + public function testGetTouchIconDefault() { + $favicon = $this->iconController->getTouchIcon(); + + $expectedIcon = $this->invokePrivate($this->iconController, 'renderAppIcon', ["core"]); + $expectedIcon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); + $expectedIcon->setImageFormat("png24"); + + $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $this->assertEquals($expected, $favicon); + } + + public function testRenderAppIcon() { + $this->themingDefaults->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn('#000000'); + + $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', ['core']); + $this->assertEquals(true, $icon->valid()); + $this->assertEquals(512, $icon->getImageWidth()); + $this->assertEquals(512, $icon->getImageHeight()); + + } + public function testRenderAppIconColor() { + $this->themingDefaults->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn('#0082c9'); + + $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', ['core']); + $this->assertEquals(true, $icon->valid()); + $this->assertEquals(512, $icon->getImageWidth()); + $this->assertEquals(512, $icon->getImageHeight()); + + } + + + /** + * @dataProvider dataGetAppIcon + */ + public function testGetAppIcon($app, $expected) { + $icon = $this->invokePrivate($this->iconController, 'getAppIcon', [$app]); + $this->assertEquals($expected, $icon); + } + + public function dataGetAppIcon() { + return [ + ['user_ldap', \OC_App::getAppPath('user_ldap') . '/img/app.svg'], + ['noapplikethis', \OC::$SERVERROOT . '/core/img/logo.svg'], + ['comments', \OC_App::getAppPath('comments') . '/img/comments.svg'], + ]; + } + + public function testGetAppIconThemed() { + $this->rootFolder->expects($this->once()) + ->method('nodeExists') + ->with('/themedinstancelogo') + ->willReturn(true); + $expected = '/themedinstancelogo'; + $icon = $this->invokePrivate($this->iconController, 'getAppIcon', ['noapplikethis']); + $this->assertEquals($expected, $icon); + } + + /** + * @dataProvider dataGetAppImage + */ + public function testGetAppImage($app, $image, $expected) { + $this->assertEquals($expected, $this->invokePrivate($this->iconController, 'getAppImage', [$app, $image])); + } + public function dataGetAppImage() { + return [ + ['core', 'logo.svg', \OC::$SERVERROOT . '/core/img/logo.svg'], + ['files', 'external', \OC::$SERVERROOT . '/apps/files/img/external.svg'], + ['files', 'external.svg', \OC::$SERVERROOT . '/apps/files/img/external.svg'], + ['noapplikethis', 'foobar.svg', false], + ]; + } + + public function testColorizeSvg() { + $input = "#0082c9 #0082C9 #000000 #FFFFFF"; + $expected = "#AAAAAA #AAAAAA #000000 #FFFFFF"; + $result = $this->invokePrivate($this->iconController, 'colorizeSvg', [$input, '#AAAAAA']); + $this->assertEquals($expected, $result); + } + +} -- cgit v1.2.3 From b466628bfddee71b3c4d9f8d903e327269f57b4a Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Mon, 29 Aug 2016 11:53:44 +0200 Subject: Improve unit tests for image generation Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 84 +------------- apps/theming/lib/Util.php | 84 ++++++++++++++ .../tests/Controller/IconControllerTest.php | 127 ++++++++++----------- .../tests/Controller/ThemingControllerTest.php | 2 +- apps/theming/tests/UtilTest.php | 61 +++++++++- apps/theming/tests/data/favicon-original.ico | Bin 0 -> 1618 bytes apps/theming/tests/data/touch-comments.png | Bin 0 -> 21814 bytes apps/theming/tests/data/touch-core-red.png | Bin 0 -> 23415 bytes apps/theming/tests/data/touch-original-png.png | Bin 0 -> 32785 bytes apps/theming/tests/data/touch-original.png | Bin 0 -> 29078 bytes apps/theming/tests/data/touch-testing-red.png | Bin 0 -> 23438 bytes 11 files changed, 207 insertions(+), 151 deletions(-) create mode 100644 apps/theming/tests/data/favicon-original.ico create mode 100644 apps/theming/tests/data/touch-comments.png create mode 100644 apps/theming/tests/data/touch-core-red.png create mode 100644 apps/theming/tests/data/touch-original-png.png create mode 100644 apps/theming/tests/data/touch-original.png create mode 100644 apps/theming/tests/data/touch-testing-red.png diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 56782992ab3..5770bd20742 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -53,6 +53,7 @@ class IconController extends Controller { /** @var IRootFolder */ private $rootFolder; + /** * IconController constructor. * @@ -94,10 +95,10 @@ class IconController extends Controller { * @return StreamResponse|DataResponse */ public function getThemedIcon($app, $image) { - $image = $this->getAppImage($app, $image); + $image = $this->util->getAppImage($app, $image); $svg = file_get_contents($image); $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); - $svg = $this->colorizeSvg($svg, $color); + $svg = $this->util->colorizeSvg($svg, $color); $response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $response->cacheFor(86400); @@ -152,24 +153,19 @@ class IconController extends Controller { * @return Imagick */ private function renderAppIcon($app) { - $appIcon = $this->getAppIcon($app); + $appIcon = $this->util->getAppIcon($app); $color = $this->themingDefaults->getMailHeaderColor(); $mime = mime_content_type($appIcon); - // FIXME: test if we need this - if ($color === "") { - $color = '#0082c9'; - } // generate background image with rounded corners $background = '' . '' . '' . ''; - // resize svg magic as this seems broken in Imagemagick if($mime === "image/svg+xml") { $svg = file_get_contents($appIcon); - + $tmp = new Imagick(); $tmp->readImageBlob($svg); $x = $tmp->getImageWidth(); @@ -214,76 +210,6 @@ class IconController extends Controller { return $finalIconFile; } - /** - * @param $app app name - * @return string path to app icon / logo - */ - private function getAppIcon($app) { - $appPath = \OC_App::getAppPath($app); - - $icon = $appPath . '/img/' . $app . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/app.svg'; - if(file_exists($icon)) { - return $icon; - } - - if($this->rootFolder->nodeExists('/themedinstancelogo')) { - return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; - } - return \OC::$SERVERROOT . '/core/img/logo.svg'; - } - - /** - * @param $app app name - * @param $image relative path to image in app folder - * @return string absolute path to image - */ - private function getAppImage($app, $image) { - $appPath = \OC_App::getAppPath($app); - - if($app==="core") { - $icon = \OC::$SERVERROOT . '/core/img/' . $image; - if(file_exists($icon)) { - return $icon; - } - } - $icon = $appPath . '/img/' . $image; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.png'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.gif'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.jpg'; - if(file_exists($icon)) { - return $icon; - } - return false; - } - - /** - * replace default color with a custom one - * - * @param $svg content of a svg file - * @param $color color to match - * @return string - */ - private function colorizeSvg($svg, $color) { - $svg = preg_replace('/#0082c9/i', $color, $svg); - return $svg; - } } \ No newline at end of file diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 71ed0958e42..8aa5f50ede2 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -23,8 +23,19 @@ namespace OCA\Theming; +use OCP\IConfig; +use OCP\Files\IRootFolder; + class Util { + private $config; + private $rootFolder; + + public function __construct(IConfig $config, IRootFolder $rootFolder) { + $this->config = $config; + $this->rootFolder = $rootFolder; + } + /** * @param string $color rgb color value * @return bool @@ -81,4 +92,77 @@ class Util { return base64_encode($radioButtonIcon); } + + /** + * @param $app app name + * @return string path to app icon / logo + */ + public function getAppIcon($app) { + $appPath = \OC_App::getAppPath($app); + + $icon = $appPath . '/img/' . $app . '.svg'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/app.svg'; + if(file_exists($icon)) { + return $icon; + } + + if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) { + return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; + } + return \OC::$SERVERROOT . '/core/img/logo.svg'; + } + + /** + * @param $app app name + * @param $image relative path to image in app folder + * @return string absolute path to image + */ + public function getAppImage($app, $image) { + $appPath = \OC_App::getAppPath($app); + + if($app==="core") { + $icon = \OC::$SERVERROOT . '/core/img/' . $image; + if(file_exists($icon)) { + return $icon; + } + } + + $icon = $appPath . '/img/' . $image; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.svg'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.png'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.gif'; + if(file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.jpg'; + if(file_exists($icon)) { + return $icon; + } + return false; + } + + /** + * replace default color with a custom one + * + * @param $svg content of a svg file + * @param $color color to match + * @return string + */ + public function colorizeSvg($svg, $color) { + $svg = preg_replace('/#0082c9/i', $color, $svg); + return $svg; + } + } diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index e22369aa744..22d4ae343a1 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -35,7 +35,7 @@ use Test\TestCase; use OCA\Theming\ThemingDefaults; use \Imagick; -class ThemingControllerTest extends TestCase { +class IconControllerTest extends TestCase { /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ private $request; /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ @@ -54,11 +54,21 @@ class ThemingControllerTest extends TestCase { private $rootFolder; public function setUp() { + + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Tests skipped as Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present'); + } + $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') ->disableOriginalConstructor()->getMock(); - $this->util = new Util(); + $this->util = $this->getMockBuilder('\OCA\Theming\Util')->disableOriginalConstructor() + ->setMethods(['getAppImage', 'getAppIcon', 'elementColor'])->getMock(); $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') ->disableOriginalConstructor() ->getMock(); @@ -84,10 +94,19 @@ class ThemingControllerTest extends TestCase { } public function testGetThemedIcon() { + $this->util->expects($this->once()) + ->method('getAppImage') + ->with('core','filetypes/folder.svg') + ->willReturn(\OC::$SERVERROOT . "/core/img/filetypes/folder.svg"); $this->themingDefaults ->expects($this->once()) ->method('getMailHeaderColor') ->willReturn('#000000'); + $this->util + ->expects($this->once()) + ->method('elementColor') + ->willReturn('#000000'); + $svg = " @@ -102,23 +121,27 @@ class ThemingControllerTest extends TestCase { } public function testGetFaviconDefault() { - $favicon = $this->iconController->getFavicon(); - $expectedIcon = $this->invokePrivate($this->iconController, 'renderAppIcon', ["core"]); - $expectedIcon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); - $expectedIcon->setImageFormat("png24"); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with('core') + ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); + $favicon = $this->iconController->getFavicon(); + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/favicon-original.ico'); $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $this->assertEquals($expected, $favicon); } public function testGetTouchIconDefault() { - $favicon = $this->iconController->getTouchIcon(); - $expectedIcon = $this->invokePrivate($this->iconController, 'renderAppIcon', ["core"]); - $expectedIcon->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); - $expectedIcon->setImageFormat("png24"); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with('core') + ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); + $favicon = $this->iconController->getTouchIcon(); + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/touch-original.png'); $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); @@ -126,76 +149,42 @@ class ThemingControllerTest extends TestCase { $this->assertEquals($expected, $favicon); } - public function testRenderAppIcon() { - $this->themingDefaults->expects($this->once()) - ->method('getMailHeaderColor') - ->willReturn('#000000'); + /** + * @dataProvider dataRenderAppIcon + * @param $appicon + * @param $color + * @param $file + */ + public function testRenderAppIcon($app, $appicon, $color, $file) { - $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', ['core']); - $this->assertEquals(true, $icon->valid()); - $this->assertEquals(512, $icon->getImageWidth()); - $this->assertEquals(512, $icon->getImageHeight()); - - } - public function testRenderAppIconColor() { + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app) + ->willReturn(\OC::$SERVERROOT . "/" . $appicon); $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') - ->willReturn('#0082c9'); + ->willReturn($color); + + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/../data/" . $file); + + $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', [$app]); - $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', ['core']); $this->assertEquals(true, $icon->valid()); $this->assertEquals(512, $icon->getImageWidth()); $this->assertEquals(512, $icon->getImageHeight()); - - } - - - /** - * @dataProvider dataGetAppIcon - */ - public function testGetAppIcon($app, $expected) { - $icon = $this->invokePrivate($this->iconController, 'getAppIcon', [$app]); - $this->assertEquals($expected, $icon); - } - - public function dataGetAppIcon() { - return [ - ['user_ldap', \OC_App::getAppPath('user_ldap') . '/img/app.svg'], - ['noapplikethis', \OC::$SERVERROOT . '/core/img/logo.svg'], - ['comments', \OC_App::getAppPath('comments') . '/img/comments.svg'], - ]; - } - - public function testGetAppIconThemed() { - $this->rootFolder->expects($this->once()) - ->method('nodeExists') - ->with('/themedinstancelogo') - ->willReturn(true); - $expected = '/themedinstancelogo'; - $icon = $this->invokePrivate($this->iconController, 'getAppIcon', ['noapplikethis']); - $this->assertEquals($expected, $icon); + $this->assertEquals($icon, $expectedIcon); + //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); + } - /** - * @dataProvider dataGetAppImage - */ - public function testGetAppImage($app, $image, $expected) { - $this->assertEquals($expected, $this->invokePrivate($this->iconController, 'getAppImage', [$app, $image])); - } - public function dataGetAppImage() { + public function dataRenderAppIcon() { return [ - ['core', 'logo.svg', \OC::$SERVERROOT . '/core/img/logo.svg'], - ['files', 'external', \OC::$SERVERROOT . '/apps/files/img/external.svg'], - ['files', 'external.svg', \OC::$SERVERROOT . '/apps/files/img/external.svg'], - ['noapplikethis', 'foobar.svg', false], + ['core','core/img/logo.svg', '#0082c9', 'touch-original.png'], + ['core','core/img/logo.svg', '#FF0000', 'touch-core-red.png'], + ['testing','apps/testing/img/app.svg', '#FF0000', 'touch-testing-red.png'], + ['comments','apps/comments/img/comments.svg', '#0082c9', 'touch-comments.png'], + ['core','core/img/logo.png', '#0082c9', 'touch-original-png.png'], ]; } - public function testColorizeSvg() { - $input = "#0082c9 #0082C9 #000000 #FFFFFF"; - $expected = "#AAAAAA #AAAAAA #000000 #FFFFFF"; - $result = $this->invokePrivate($this->iconController, 'colorizeSvg', [$input, '#AAAAAA']); - $this->assertEquals($expected, $result); - } - } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 5de16a7abdf..b065046fdba 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -61,12 +61,12 @@ class ThemingControllerTest extends TestCase { $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); $this->template = $this->getMockBuilder('OCA\Theming\ThemingDefaults') ->disableOriginalConstructor()->getMock(); - $this->util = new Util(); $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') ->disableOriginalConstructor() ->getMock(); $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->util = new Util($this->config, $this->rootFolder); $this->timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index c7fc385d25d..499a3ffc45d 100644 --- a/apps/theming/tests/UtilTest.php +++ b/apps/theming/tests/UtilTest.php @@ -23,16 +23,22 @@ namespace OCA\Theming\Tests; use OCA\Theming\Util; +use OCP\IConfig; +use OCP\Files\IRootFolder; use Test\TestCase; class UtilTest extends TestCase { /** @var Util */ protected $util; - + /** @var IConfig */ + protected $config; + protected $rootFolder; protected function setUp() { parent::setUp(); - $this->util = new Util(); + $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->util = new Util($this->config, $this->rootFolder); } public function testInvertTextColorLight() { @@ -94,4 +100,55 @@ class UtilTest extends TestCase { $expected = 'PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMTYiIHdpZHRoPSIxNiI+PHBhdGggZD0iTTggMWE3IDcgMCAwIDAtNyA3IDcgNyAwIDAgMCA3IDcgNyA3IDAgMCAwIDctNyA3IDcgMCAwIDAtNy03em0wIDFhNiA2IDAgMCAxIDYgNiA2IDYgMCAwIDEtNiA2IDYgNiAwIDAgMS02LTYgNiA2IDAgMCAxIDYtNnptMCAyYTQgNCAwIDEgMCAwIDggNCA0IDAgMCAwIDAtOHoiIGZpbGw9IiMwMDAwMDAiLz48L3N2Zz4='; $this->assertEquals($expected, $button); } + + + + /** + * @dataProvider dataGetAppIcon + */ + public function testGetAppIcon($app, $expected) { + $icon = $this->util->getAppIcon($app); + $this->assertEquals($expected, $icon); + } + + public function dataGetAppIcon() { + return [ + ['user_ldap', \OC_App::getAppPath('user_ldap') . '/img/app.svg'], + ['noapplikethis', \OC::$SERVERROOT . '/core/img/logo.svg'], + ['comments', \OC_App::getAppPath('comments') . '/img/comments.svg'], + ]; + } + + public function testGetAppIconThemed() { + $this->rootFolder->expects($this->once()) + ->method('nodeExists') + ->with('/themedinstancelogo') + ->willReturn(true); + $expected = '/themedinstancelogo'; + $icon = $this->util->getAppIcon('noapplikethis'); + $this->assertEquals($expected, $icon); + } + + /** + * @dataProvider dataGetAppImage + */ + public function testGetAppImage($app, $image, $expected) { + $this->assertEquals($expected, $this->util->getAppImage($app, $image)); + } + public function dataGetAppImage() { + return [ + ['core', 'logo.svg', \OC::$SERVERROOT . '/core/img/logo.svg'], + ['files', 'external', \OC::$SERVERROOT . '/apps/files/img/external.svg'], + ['files', 'external.svg', \OC::$SERVERROOT . '/apps/files/img/external.svg'], + ['noapplikethis', 'foobar.svg', false], + ]; + } + + public function testColorizeSvg() { + $input = "#0082c9 #0082C9 #000000 #FFFFFF"; + $expected = "#AAAAAA #AAAAAA #000000 #FFFFFF"; + $result = $this->util->colorizeSvg($input, '#AAAAAA'); + $this->assertEquals($expected, $result); + } + } diff --git a/apps/theming/tests/data/favicon-original.ico b/apps/theming/tests/data/favicon-original.ico new file mode 100644 index 00000000000..fab2f7f0231 Binary files /dev/null and b/apps/theming/tests/data/favicon-original.ico differ diff --git a/apps/theming/tests/data/touch-comments.png b/apps/theming/tests/data/touch-comments.png new file mode 100644 index 00000000000..af0d2ca579b Binary files /dev/null and b/apps/theming/tests/data/touch-comments.png differ diff --git a/apps/theming/tests/data/touch-core-red.png b/apps/theming/tests/data/touch-core-red.png new file mode 100644 index 00000000000..7a492f10e21 Binary files /dev/null and b/apps/theming/tests/data/touch-core-red.png differ diff --git a/apps/theming/tests/data/touch-original-png.png b/apps/theming/tests/data/touch-original-png.png new file mode 100644 index 00000000000..6997dfc6b88 Binary files /dev/null and b/apps/theming/tests/data/touch-original-png.png differ diff --git a/apps/theming/tests/data/touch-original.png b/apps/theming/tests/data/touch-original.png new file mode 100644 index 00000000000..ac39872aa6e Binary files /dev/null and b/apps/theming/tests/data/touch-original.png differ diff --git a/apps/theming/tests/data/touch-testing-red.png b/apps/theming/tests/data/touch-testing-red.png new file mode 100644 index 00000000000..4cbdfbbb42c Binary files /dev/null and b/apps/theming/tests/data/touch-testing-red.png differ -- cgit v1.2.3 From af8976ab03d976bba04a2442957a650728de7ecb Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 30 Aug 2016 09:03:06 +0200 Subject: Add IconBuilder class to encapsulate icon generation Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 117 +++++----------- apps/theming/lib/IconBuilder.php | 140 +++++++++++++++++++ apps/theming/lib/ThemingDefaults.php | 17 +++ .../tests/Controller/IconControllerTest.php | 44 +----- apps/theming/tests/IconBuilderTest.php | 150 +++++++++++++++++++++ 5 files changed, 341 insertions(+), 127 deletions(-) create mode 100644 apps/theming/lib/IconBuilder.php create mode 100644 apps/theming/tests/IconBuilderTest.php diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 5770bd20742..78d41d621a0 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -22,6 +22,7 @@ */ namespace OCA\Theming\Controller; +use OCA\Theming\IconBuilder; use OCA\Theming\Template; use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; @@ -35,9 +36,6 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCA\Theming\Util; -use OCP\IURLGenerator; -use Imagick; -use ImagickPixel; class IconController extends Controller { /** @var ThemingDefaults */ @@ -52,7 +50,8 @@ class IconController extends Controller { private $config; /** @var IRootFolder */ private $rootFolder; - + /** @var IconBuilder */ + private $iconBuilder; /** * IconController constructor. @@ -84,6 +83,9 @@ class IconController extends Controller { $this->l = $l; $this->config = $config; $this->rootFolder = $rootFolder; + if(extension_loaded('imagick')) { + $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); + } } /** @@ -91,7 +93,7 @@ class IconController extends Controller { * @NoCSRFRequired * * @param $app app name - * @param $image image file name + * @param $image image file name (svg required) * @return StreamResponse|DataResponse */ public function getThemedIcon($app, $image) { @@ -99,10 +101,10 @@ class IconController extends Controller { $svg = file_get_contents($image); $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); $svg = $this->util->colorizeSvg($svg, $color); - $response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $response->cacheFor(86400); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); return $response; } @@ -116,14 +118,21 @@ class IconController extends Controller { * @return StreamResponse|DataResponse */ public function getFavicon($app="core") { - $icon = $this->renderAppIcon($app); - $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); - $icon->setImageFormat("png24"); + if($this->themingDefaults->shouldReplaceIcons()) { + $icon = $this->iconBuilder->getFavicon($app); + $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + return $response; + } else { + $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + return $response; + } + - $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - return $response; } /** @@ -136,80 +145,20 @@ class IconController extends Controller { * @return StreamResponse|DataResponse */ public function getTouchIcon($app="core") { - $icon = $this->renderAppIcon($app); - $icon->setImageFormat("png24"); - - $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - return $response; - } - - /** - * Render app icon on themed background color - * fallback to logo - * - * @param $app app name - * @return Imagick - */ - private function renderAppIcon($app) { - $appIcon = $this->util->getAppIcon($app); - $color = $this->themingDefaults->getMailHeaderColor(); - $mime = mime_content_type($appIcon); - // generate background image with rounded corners - $background = '' . - '' . - '' . - ''; - - // resize svg magic as this seems broken in Imagemagick - if($mime === "image/svg+xml") { - $svg = file_get_contents($appIcon); - - $tmp = new Imagick(); - $tmp->readImageBlob($svg); - $x = $tmp->getImageWidth(); - $y = $tmp->getImageHeight(); - $res = $tmp->getImageResolution(); - $tmp->destroy(); - - // convert svg to resized image - $appIconFile = new Imagick(); - $resX = (int)(512 * $res['x'] / $x * 2.53); - $resY = (int)(512 * $res['y'] / $y * 2.53); - $appIconFile->setResolution($resX, $resY); - $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $appIconFile->readImageBlob($svg); + if($this->themingDefaults->shouldReplaceIcons()) { + $icon = $this->iconBuilder->getTouchIcon($app); + $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + return $response; } else { - $appIconFile = new Imagick(); - $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $appIconFile->readImageBlob(file_get_contents($appIcon)); - $appIconFile->scaleImage(512, 512, true); + $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + return $response; } - - // offset for icon positioning - $border_w = (int)($appIconFile->getImageWidth() * 0.05); - $border_h = (int)($appIconFile->getImageHeight() * 0.05); - $innerWidth = (int)($appIconFile->getImageWidth() - $border_w * 2); - $innerHeight = (int)($appIconFile->getImageHeight() - $border_h * 2); - $appIconFile->adaptiveResizeImage($innerWidth, $innerHeight); - // center icon - $offset_w = 512 / 2 - $innerWidth / 2; - $offset_h = 512 / 2 - $innerHeight / 2; - - $appIconFile->setImageFormat("png24"); - - $finalIconFile = new Imagick(); - $finalIconFile->readImageBlob($background); - $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); - $finalIconFile->setImageArtifact('compose:args', "1,0,-0.5,0.5"); - $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); - $finalIconFile->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); - - $appIconFile->destroy(); - return $finalIconFile; } - } \ No newline at end of file diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php new file mode 100644 index 00000000000..b61e12d9236 --- /dev/null +++ b/apps/theming/lib/IconBuilder.php @@ -0,0 +1,140 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Theming; + +use Imagick; +use ImagickPixel; + +class IconBuilder { + /** @var ThemingDefaults */ + private $themingDefaults; + /** @var Util */ + private $util; + + /** + * IconBuilder constructor. + * + * @param ThemingDefaults $themingDefaults + * @param Util $util + */ + public function __construct( + ThemingDefaults $themingDefaults, + Util $util + ) { + $this->themingDefaults = $themingDefaults; + $this->util = $util; + } + + /** + * @param $app app name + * @return string image blob + */ + public function getFavicon($app) { + $icon = $this->renderAppIcon($app); + $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); + $icon->setImageFormat("png24"); + $data = $icon->getImageBlob(); + $icon->destroy(); + return $data; + } + + /** + * @param $app app name + * @return string image blob + */ + public function getTouchIcon($app) { + $icon = $this->renderAppIcon($app); + $icon->setImageFormat("png24"); + $data = $icon->getImageBlob(); + $icon->destroy(); + return $data; + } + + /** + * Render app icon on themed background color + * fallback to logo + * + * @param $app app name + * @return Imagick + */ + public function renderAppIcon($app) { + $appIcon = $this->util->getAppIcon($app); + + $color = $this->themingDefaults->getMailHeaderColor(); + $mime = mime_content_type($appIcon); + // generate background image with rounded corners + $background = '' . + '' . + '' . + ''; + + // resize svg magic as this seems broken in Imagemagick + if($mime === "image/svg+xml") { + $svg = file_get_contents($appIcon); + + $tmp = new Imagick(); + $tmp->readImageBlob($svg); + $x = $tmp->getImageWidth(); + $y = $tmp->getImageHeight(); + $res = $tmp->getImageResolution(); + $tmp->destroy(); + + // convert svg to resized image + $appIconFile = new Imagick(); + $resX = (int)(512 * $res['x'] / $x * 2.53); + $resY = (int)(512 * $res['y'] / $y * 2.53); + $appIconFile->setResolution($resX, $resY); + $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); + $appIconFile->readImageBlob($svg); + } else { + $appIconFile = new Imagick(); + $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); + $appIconFile->readImageBlob(file_get_contents($appIcon)); + $appIconFile->scaleImage(512, 512, true); + } + + // offset for icon positioning + $border_w = (int)($appIconFile->getImageWidth() * 0.05); + $border_h = (int)($appIconFile->getImageHeight() * 0.05); + $innerWidth = (int)($appIconFile->getImageWidth() - $border_w * 2); + $innerHeight = (int)($appIconFile->getImageHeight() - $border_h * 2); + $appIconFile->adaptiveResizeImage($innerWidth, $innerHeight); + // center icon + $offset_w = 512 / 2 - $innerWidth / 2; + $offset_h = 512 / 2 - $innerHeight / 2; + + $appIconFile->setImageFormat("png24"); + + $finalIconFile = new Imagick(); + $finalIconFile->readImageBlob($background); + $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); + $finalIconFile->setImageArtifact('compose:args', "1,0,-0.5,0.5"); + $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); + $finalIconFile->resizeImage(512, 512, Imagick::FILTER_LANCZOS, 1); + + $appIconFile->destroy(); + return $finalIconFile; + } + +} \ No newline at end of file diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 9139dd56247..b7968d0073f 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -144,6 +144,23 @@ class ThemingDefaults extends \OC_Defaults { } } + /** + * Check if Imagemagick is enabled and if SVG is supported + * otherwise we can't render custom icons + * + * @return bool + */ + public function shouldReplaceIcons() { + if(extension_loaded('imagick')) { + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) >= 1) { + return true; + } + $checkImagick->clear(); + } + return false; + } + /** * Increases the cache buster key */ diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 22d4ae343a1..3443be60712 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -55,13 +55,7 @@ class IconControllerTest extends TestCase { public function setUp() { - if(!extension_loaded('imagick')) { - $this->markTestSkipped('Tests skipped as Imagemagick is required for dynamic icon generation.'); - } - $checkImagick = new \Imagick(); - if (count($checkImagick->queryFormats('SVG')) < 1) { - $this->markTestSkipped('No SVG provider present'); - } + $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); @@ -149,42 +143,6 @@ class IconControllerTest extends TestCase { $this->assertEquals($expected, $favicon); } - /** - * @dataProvider dataRenderAppIcon - * @param $appicon - * @param $color - * @param $file - */ - public function testRenderAppIcon($app, $appicon, $color, $file) { - - $this->util->expects($this->once()) - ->method('getAppIcon') - ->with($app) - ->willReturn(\OC::$SERVERROOT . "/" . $appicon); - $this->themingDefaults->expects($this->once()) - ->method('getMailHeaderColor') - ->willReturn($color); - - $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/../data/" . $file); - - $icon = $this->invokePrivate($this->iconController, 'renderAppIcon', [$app]); - $this->assertEquals(true, $icon->valid()); - $this->assertEquals(512, $icon->getImageWidth()); - $this->assertEquals(512, $icon->getImageHeight()); - $this->assertEquals($icon, $expectedIcon); - //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); - - } - - public function dataRenderAppIcon() { - return [ - ['core','core/img/logo.svg', '#0082c9', 'touch-original.png'], - ['core','core/img/logo.svg', '#FF0000', 'touch-core-red.png'], - ['testing','apps/testing/img/app.svg', '#FF0000', 'touch-testing-red.png'], - ['comments','apps/comments/img/comments.svg', '#0082c9', 'touch-comments.png'], - ['core','core/img/logo.png', '#0082c9', 'touch-original-png.png'], - ]; - } } diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php new file mode 100644 index 00000000000..ffabb31df79 --- /dev/null +++ b/apps/theming/tests/IconBuilderTest.php @@ -0,0 +1,150 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Theming\Tests; + +use OCA\Theming\IconBuilder; +use OCA\Theming\ThemingDefaults; +use OCA\Theming\Util; +use OCP\Files\IRootFolder; +use OCP\IConfig; +use Test\TestCase; + +class IconBuilderTest extends TestCase { + + /** @var IConfig */ + protected $config; + /** @var IRootFolder */ + protected $rootFolder; + /** @var ThemingDefaults */ + protected $themingDefaults; + /** @var Util */ + protected $util; + /** @var IconBuilder */ + protected $iconBuilder; + + protected function setUp() { + parent::setUp(); + + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present.'); + } + + $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') + ->disableOriginalConstructor()->getMock(); + $this->util = new Util($this->config, $this->rootFolder); + $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); + } + + public function dataRenderAppIcon() { + return [ + ['core', '#0082c9', 'touch-original.png'], + ['core', '#FF0000', 'touch-core-red.png'], + ['testing', '#FF0000', 'touch-testing-red.png'], + ['comments', '#0082c9', 'touch-comments.png'], + ['core', '#0082c9', 'touch-original-png.png'], + ]; + } + + /** + * @dataProvider dataRenderAppIcon + * @param $app + * @param $color + * @param $file + */ + public function testRenderAppIcon($app, $color, $file) { + + $this->themingDefaults->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn($color); + + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/data/" . $file); + $icon = $this->iconBuilder->renderAppIcon($app); + + $this->assertEquals(true, $icon->valid()); + $this->assertEquals(512, $icon->getImageWidth()); + $this->assertEquals(512, $icon->getImageHeight()); + $this->assertEquals($icon, $expectedIcon); + $icon->destroy(); + $expectedIcon->destroy(); + //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); + + } + + /** + * @dataProvider dataRenderAppIcon + * @param $app + * @param $color + * @param $file + */ + public function testGetTouchIcon($app, $color, $file) { + + $this->themingDefaults->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn($color); + + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/data/" . $file); + $icon = new \Imagick(); + $icon->readImageBlob($this->iconBuilder->getTouchIcon($app)); + + $this->assertEquals(true, $icon->valid()); + $this->assertEquals(512, $icon->getImageWidth()); + $this->assertEquals(512, $icon->getImageHeight()); + $this->assertEquals($icon, $expectedIcon); + $icon->destroy(); + $expectedIcon->destroy(); + //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); + + } + + /** + * @dataProvider dataRenderAppIcon + * @param $app + * @param $color + * @param $file + */ + public function testGetFavicon($app, $color, $file) { + + $this->themingDefaults->expects($this->once()) + ->method('getMailHeaderColor') + ->willReturn($color); + + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/data/" . $file); + $icon = new \Imagick(); + $icon->readImageBlob($this->iconBuilder->getFavicon($app)); + + $this->assertEquals(true, $icon->valid()); + $this->assertEquals(32, $icon->getImageWidth()); + $this->assertEquals(32, $icon->getImageHeight()); + $icon->destroy(); + $expectedIcon->destroy(); + //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); + + } + +} -- cgit v1.2.3 From 237034818dd3425116ef3db04dabbc95a5d10125 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 30 Aug 2016 09:03:42 +0200 Subject: Check if dynamic icons can be used Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 34 ++++---- .../tests/Controller/IconControllerTest.php | 93 ++++++++++++++++++---- lib/private/URLGenerator.php | 4 +- 3 files changed, 92 insertions(+), 39 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 78d41d621a0..6f97fdcdaba 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -73,7 +73,8 @@ class IconController extends Controller { Util $util, ITimeFactory $timeFactory, IL10N $l, - IRootFolder $rootFolder + IRootFolder $rootFolder, + IconBuilder $iconBuilder ) { parent::__construct($appName, $request); @@ -83,9 +84,10 @@ class IconController extends Controller { $this->l = $l; $this->config = $config; $this->rootFolder = $rootFolder; - if(extension_loaded('imagick')) { - $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); - } + $this->iconBuilder = $iconBuilder; + //if(extension_loaded('imagick')) { + // $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); + //} } /** @@ -121,18 +123,13 @@ class IconController extends Controller { if($this->themingDefaults->shouldReplaceIcons()) { $icon = $this->iconBuilder->getFavicon($app); $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); - return $response; } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - return $response; } - - + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + return $response; } /** @@ -148,16 +145,13 @@ class IconController extends Controller { if($this->themingDefaults->shouldReplaceIcons()) { $icon = $this->iconBuilder->getTouchIcon($app); $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); - return $response; } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - return $response; } + $response->cacheFor(86400); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + return $response; } diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 3443be60712..8c916c6cfe6 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -23,6 +23,7 @@ namespace OCA\Theming\Tests\Controller; use OCA\Theming\Controller\IconController; +use OCA\Theming\IconBuilder; use OCA\Theming\Util; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -52,11 +53,10 @@ class IconControllerTest extends TestCase { private $iconController; /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ private $rootFolder; + /** @var IconBuilder */ + private $iconBuilder; public function setUp() { - - - $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') @@ -68,6 +68,8 @@ class IconControllerTest extends TestCase { ->getMock(); $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->iconBuilder = $this->getMockBuilder('OCA\Theming\IconBuilder') + ->disableOriginalConstructor()->getMock(); $this->timeFactory->expects($this->any()) ->method('getTime') @@ -81,10 +83,11 @@ class IconControllerTest extends TestCase { $this->util, $this->timeFactory, $this->l10n, - $this->rootFolder + $this->rootFolder, + $this->iconBuilder ); - return parent::setUp(); + parent::setUp(); } public function testGetThemedIcon() { @@ -111,38 +114,94 @@ class IconControllerTest extends TestCase { $expected = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->iconController->getThemedIcon('core','filetypes/folder.svg')); } public function testGetFaviconDefault() { - - $this->util->expects($this->once()) - ->method('getAppIcon') + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present.'); + } + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(true); + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/favicon-original.ico'); + $this->iconBuilder->expects($this->once()) + ->method('getFavicon') ->with('core') - ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); - + ->willReturn($expectedIcon); $favicon = $this->iconController->getFavicon(); - $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/favicon-original.ico'); + $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $favicon); } public function testGetTouchIconDefault() { - - $this->util->expects($this->once()) - ->method('getAppIcon') + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present.'); + } + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(true); + $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/touch-original.png'); + $this->iconBuilder->expects($this->once()) + ->method('getTouchIcon') ->with('core') - ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); + ->willReturn($expectedIcon); $favicon = $this->iconController->getTouchIcon(); - $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/touch-original.png'); $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $favicon); } - + public function testGetFaviconFail() { + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present.'); + } + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(false); + $favicon = $this->iconController->getFavicon(); + $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); + $this->assertEquals($expected, $favicon); + } + public function testGetTouchIconFail() { + if(!extension_loaded('imagick')) { + $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); + } + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats('SVG')) < 1) { + $this->markTestSkipped('No SVG provider present.'); + } + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(false); + $favicon = $this->iconController->getTouchIcon(); + $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); + $this->assertEquals($expected, $favicon); + } } diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 8290fcb1337..bdf7bdafae3 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -157,11 +157,11 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; - if(\OCP\App::isEnabled('theming') && $image === "favicon.ico") { + if(\OCP\App::isEnabled('theming') && $image === "favicon.ico" && \OC::$server->getThemingDefaults()->shouldReplaceIcons()) { $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); if($app==="") { $app = "core"; } $path = $this->linkToRoute('theming.Icon.getFavicon', [ 'app' => $app ]) . '?v='. $cacheBusterValue; - } elseif(\OCP\App::isEnabled('theming') && $image === "favicon-touch.png") { + } elseif(\OCP\App::isEnabled('theming') && $image === "favicon-touch.png" && \OC::$server->getThemingDefaults()->shouldReplaceIcons()) { $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); if($app==="") { $app = "core"; } $path = $this->linkToRoute('theming.Icon.getTouchIcon', [ 'app' => $app ]) . '?v='. $cacheBusterValue; -- cgit v1.2.3 From 9e28a3ba120356b03063e44445a9401c3aa205f3 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 30 Aug 2016 11:51:48 +0200 Subject: Theming: Code cleanup and cache buster for mime icons Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 40 +++++----------------- apps/theming/lib/Controller/ThemingController.php | 2 ++ apps/theming/lib/IconBuilder.php | 6 ++-- apps/theming/lib/Util.php | 14 +++----- .../tests/Controller/IconControllerTest.php | 27 +++------------ .../tests/Controller/ThemingControllerTest.php | 2 ++ apps/theming/tests/IconBuilderTest.php | 15 ++++---- apps/theming/tests/UtilTest.php | 2 ++ core/js/mimetype.js | 4 +++ 9 files changed, 37 insertions(+), 75 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 6f97fdcdaba..f2355fe3f82 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -23,17 +23,11 @@ namespace OCA\Theming\Controller; use OCA\Theming\IconBuilder; -use OCA\Theming\Template; use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; -use OCP\AppFramework\Http\StreamResponse; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\Files\IRootFolder; -use OCP\IConfig; -use OCP\IL10N; use OCP\IRequest; use OCA\Theming\Util; @@ -44,12 +38,6 @@ class IconController extends Controller { private $util; /** @var ITimeFactory */ private $timeFactory; - /** @var IL10N */ - private $l; - /** @var IConfig */ - private $config; - /** @var IRootFolder */ - private $rootFolder; /** @var IconBuilder */ private $iconBuilder; @@ -58,22 +46,17 @@ class IconController extends Controller { * * @param string $appName * @param IRequest $request - * @param IConfig $config * @param ThemingDefaults $themingDefaults * @param Util $util * @param ITimeFactory $timeFactory - * @param IL10N $l - * @param IRootFolder $rootFolder + * @param IconBuilder $iconBuilder */ public function __construct( $appName, IRequest $request, - IConfig $config, ThemingDefaults $themingDefaults, Util $util, ITimeFactory $timeFactory, - IL10N $l, - IRootFolder $rootFolder, IconBuilder $iconBuilder ) { parent::__construct($appName, $request); @@ -81,22 +64,16 @@ class IconController extends Controller { $this->themingDefaults = $themingDefaults; $this->util = $util; $this->timeFactory = $timeFactory; - $this->l = $l; - $this->config = $config; - $this->rootFolder = $rootFolder; $this->iconBuilder = $iconBuilder; - //if(extension_loaded('imagick')) { - // $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); - //} } /** * @PublicPage * @NoCSRFRequired * - * @param $app app name - * @param $image image file name (svg required) - * @return StreamResponse|DataResponse + * @param $app string app name + * @param $image string image file name (svg required) + * @return DataDisplayResponse */ public function getThemedIcon($app, $image) { $image = $this->util->getAppImage($app, $image); @@ -116,8 +93,8 @@ class IconController extends Controller { * @PublicPage * @NoCSRFRequired * - * @param $app app name - * @return StreamResponse|DataResponse + * @param $app string app name + * @return DataDisplayResponse */ public function getFavicon($app="core") { if($this->themingDefaults->shouldReplaceIcons()) { @@ -138,8 +115,8 @@ class IconController extends Controller { * @PublicPage * @NoCSRFRequired * - * @param $app app name - * @return StreamResponse|DataResponse + * @param $app string app name + * @return DataDisplayResponse */ public function getTouchIcon($app="core") { if($this->themingDefaults->shouldReplaceIcons()) { @@ -154,5 +131,4 @@ class IconController extends Controller { return $response; } - } \ No newline at end of file diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 7ba4feb62dd..c908f0e5782 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -425,6 +425,7 @@ class ThemingController extends Controller { * @return DataDownloadResponse */ public function getJavascript() { + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); $responseJS = '(function() { OCA.Theming = { name: ' . json_encode($this->template->getName()) . ', @@ -432,6 +433,7 @@ class ThemingController extends Controller { slogan: ' . json_encode($this->template->getSlogan()) . ', color: ' . json_encode($this->template->getMailHeaderColor()) . ', inverted: ' . json_encode($this->util->invertTextColor($this->template->getMailHeaderColor())) . ', + cacheBuster: ' . json_encode($cacheBusterValue). ' }; })();'; $response = new Http\DataDisplayResponse($responseJS); diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index b61e12d9236..edd7602a2e4 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -47,7 +47,7 @@ class IconBuilder { } /** - * @param $app app name + * @param $app string app name * @return string image blob */ public function getFavicon($app) { @@ -60,7 +60,7 @@ class IconBuilder { } /** - * @param $app app name + * @param $app string app name * @return string image blob */ public function getTouchIcon($app) { @@ -75,7 +75,7 @@ class IconBuilder { * Render app icon on themed background color * fallback to logo * - * @param $app app name + * @param $app string app name * @return Imagick */ public function renderAppIcon($app) { diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 8aa5f50ede2..84c631092a8 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -94,12 +94,11 @@ class Util { /** - * @param $app app name + * @param $app string app name * @return string path to app icon / logo */ public function getAppIcon($app) { $appPath = \OC_App::getAppPath($app); - $icon = $appPath . '/img/' . $app . '.svg'; if(file_exists($icon)) { return $icon; @@ -108,7 +107,6 @@ class Util { if(file_exists($icon)) { return $icon; } - if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) { return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; } @@ -116,20 +114,18 @@ class Util { } /** - * @param $app app name - * @param $image relative path to image in app folder + * @param $app string app name + * @param $image string relative path to image in app folder * @return string absolute path to image */ public function getAppImage($app, $image) { $appPath = \OC_App::getAppPath($app); - if($app==="core") { $icon = \OC::$SERVERROOT . '/core/img/' . $image; if(file_exists($icon)) { return $icon; } } - $icon = $appPath . '/img/' . $image; if(file_exists($icon)) { return $icon; @@ -156,8 +152,8 @@ class Util { /** * replace default color with a custom one * - * @param $svg content of a svg file - * @param $color color to match + * @param $svg string content of a svg file + * @param $color string color to match * @return string */ public function colorizeSvg($svg, $color) { diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 8c916c6cfe6..09cb41088de 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -2,7 +2,7 @@ /** * @copyright Copyright (c) 2016 Julius Härtl * - * @author Julius Haertl + * @author Julius Härtl * * @license GNU AGPL version 3 or any later version * @@ -22,43 +22,31 @@ */ namespace OCA\Theming\Tests\Controller; -use OCA\Theming\Controller\IconController; -use OCA\Theming\IconBuilder; -use OCA\Theming\Util; + use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; -use OCP\Files\IRootFolder; -use OCP\IConfig; -use OCP\IL10N; use OCP\IRequest; use Test\TestCase; -use OCA\Theming\ThemingDefaults; -use \Imagick; +use OCA\Theming\Util; +use OCA\Theming\Controller\IconController; + class IconControllerTest extends TestCase { /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ private $request; /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ - private $config; - /** @var ThemingDefaults|\PHPUnit_Framework_MockObject_MockObject */ private $themingDefaults; /** @var Util */ private $util; /** @var \OCP\AppFramework\Utility\ITimeFactory */ private $timeFactory; /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ - private $l10n; - /** @var IconController */ private $iconController; /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ - private $rootFolder; - /** @var IconBuilder */ private $iconBuilder; public function setUp() { $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); - $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') ->disableOriginalConstructor()->getMock(); $this->util = $this->getMockBuilder('\OCA\Theming\Util')->disableOriginalConstructor() @@ -66,8 +54,6 @@ class IconControllerTest extends TestCase { $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') ->disableOriginalConstructor() ->getMock(); - $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); - $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); $this->iconBuilder = $this->getMockBuilder('OCA\Theming\IconBuilder') ->disableOriginalConstructor()->getMock(); @@ -78,12 +64,9 @@ class IconControllerTest extends TestCase { $this->iconController = new IconController( 'theming', $this->request, - $this->config, $this->themingDefaults, $this->util, $this->timeFactory, - $this->l10n, - $this->rootFolder, $this->iconBuilder ); diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index b065046fdba..b7bce2d6ec0 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -932,6 +932,7 @@ class ThemingControllerTest extends TestCase { slogan: "", color: "#000", inverted: false, + cacheBuster: null }; })();'; $expected = new Http\DataDisplayResponse($expectedResponse); @@ -966,6 +967,7 @@ class ThemingControllerTest extends TestCase { slogan: "awesome", color: "#ffffff", inverted: true, + cacheBuster: null }; })();'; $expected = new Http\DataDisplayResponse($expectedResponse); diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index ffabb31df79..03054367210 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -78,7 +78,6 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testRenderAppIcon($app, $color, $file) { - $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); @@ -92,8 +91,8 @@ class IconBuilderTest extends TestCase { $this->assertEquals($icon, $expectedIcon); $icon->destroy(); $expectedIcon->destroy(); - //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); - + // FIXME: We may need some comparison of the generated and the test images + // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } /** @@ -103,7 +102,6 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testGetTouchIcon($app, $color, $file) { - $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); @@ -118,8 +116,8 @@ class IconBuilderTest extends TestCase { $this->assertEquals($icon, $expectedIcon); $icon->destroy(); $expectedIcon->destroy(); - //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); - + // FIXME: We may need some comparison of the generated and the test images + // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } /** @@ -129,7 +127,6 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testGetFavicon($app, $color, $file) { - $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); @@ -143,8 +140,8 @@ class IconBuilderTest extends TestCase { $this->assertEquals(32, $icon->getImageHeight()); $icon->destroy(); $expectedIcon->destroy(); - //$this->assertLessThan(0.0005, $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]); - + // FIXME: We may need some comparison of the generated and the test images + // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } } diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index 499a3ffc45d..9a53858598c 100644 --- a/apps/theming/tests/UtilTest.php +++ b/apps/theming/tests/UtilTest.php @@ -33,7 +33,9 @@ class UtilTest extends TestCase { protected $util; /** @var IConfig */ protected $config; + /** @var IRootFolder */ protected $rootFolder; + protected function setUp() { parent::setUp(); $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); diff --git a/core/js/mimetype.js b/core/js/mimetype.js index 3ce4ceccbb0..8920fe09a7e 100644 --- a/core/js/mimetype.js +++ b/core/js/mimetype.js @@ -105,6 +105,10 @@ OC.MimeType = { path += '.svg'; + if(OCA.Theming) { + path += "?v=" + OCA.Theming.cacheBuster; + } + // Cache the result OC.MimeType._mimeTypeIcons[mimeType] = path; return path; -- cgit v1.2.3 From 492d0b9f9bd72591183ff8bf4e8d5f9b4aecba35 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 18 Sep 2016 20:15:06 +0200 Subject: Caching for icon files using AppData Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 53 ++++-- apps/theming/lib/IconBuilder.php | 18 +- apps/theming/lib/ImageManager.php | 118 +++++++++++++ .../tests/Controller/IconControllerTest.php | 133 +++++++-------- apps/theming/tests/IconBuilderTest.php | 19 ++- apps/theming/tests/ImageManagerTest.php | 185 +++++++++++++++++++++ 6 files changed, 433 insertions(+), 93 deletions(-) create mode 100644 apps/theming/lib/ImageManager.php create mode 100644 apps/theming/tests/ImageManagerTest.php diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index f2355fe3f82..eb65ae54f1a 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -23,13 +23,16 @@ namespace OCA\Theming\Controller; use OCA\Theming\IconBuilder; +use OCA\Theming\ImageManager; use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; use OCA\Theming\Util; +use OCP\IConfig; class IconController extends Controller { /** @var ThemingDefaults */ @@ -38,8 +41,12 @@ class IconController extends Controller { private $util; /** @var ITimeFactory */ private $timeFactory; + /** @var IConfig */ + private $config; /** @var IconBuilder */ private $iconBuilder; + /** @var ImageManager */ + private $imageManager; /** * IconController constructor. @@ -49,7 +56,9 @@ class IconController extends Controller { * @param ThemingDefaults $themingDefaults * @param Util $util * @param ITimeFactory $timeFactory + * @param IConfig $config * @param IconBuilder $iconBuilder + * @param ImageManager $imageManager */ public function __construct( $appName, @@ -57,14 +66,18 @@ class IconController extends Controller { ThemingDefaults $themingDefaults, Util $util, ITimeFactory $timeFactory, - IconBuilder $iconBuilder + IConfig $config, + IconBuilder $iconBuilder, + ImageManager $imageManager ) { parent::__construct($appName, $request); $this->themingDefaults = $themingDefaults; $this->util = $util; $this->timeFactory = $timeFactory; + $this->config = $config; $this->iconBuilder = $iconBuilder; + $this->imageManager = $imageManager; } /** @@ -73,14 +86,15 @@ class IconController extends Controller { * * @param $app string app name * @param $image string image file name (svg required) - * @return DataDisplayResponse + * @return FileDisplayResponse */ public function getThemedIcon($app, $image) { - $image = $this->util->getAppImage($app, $image); - $svg = file_get_contents($image); - $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); - $svg = $this->util->colorizeSvg($svg, $color); - $response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + $iconFile = $this->imageManager->getCachedImage("icon-" . $app . '-' . str_replace("/","_",$image)); + if ($iconFile === null) { + $icon = $this->iconBuilder->colorSvg($app, $image); + $iconFile = $this->imageManager->setCachedImage("icon-" . $app . '-' . str_replace("/","_",$image), $icon); + } + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $response->cacheFor(86400); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $response->addHeader('Pragma', 'cache'); @@ -94,12 +108,16 @@ class IconController extends Controller { * @NoCSRFRequired * * @param $app string app name - * @return DataDisplayResponse + * @return FileDisplayResponse|DataDisplayResponse */ - public function getFavicon($app="core") { - if($this->themingDefaults->shouldReplaceIcons()) { + public function getFavicon($app = "core") { + $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); + if($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { $icon = $this->iconBuilder->getFavicon($app); - $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); + } + if ($this->themingDefaults->shouldReplaceIcons()) { + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); } @@ -116,12 +134,16 @@ class IconController extends Controller { * @NoCSRFRequired * * @param $app string app name - * @return DataDisplayResponse + * @return FileDisplayResponse|DataDisplayResponse */ - public function getTouchIcon($app="core") { - if($this->themingDefaults->shouldReplaceIcons()) { + public function getTouchIcon($app = "core") { + $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); + if ($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { $icon = $this->iconBuilder->getTouchIcon($app); - $response = new DataDisplayResponse($icon, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); + } + if ($this->themingDefaults->shouldReplaceIcons()) { + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); } @@ -130,5 +152,4 @@ class IconController extends Controller { $response->addHeader('Pragma', 'cache'); return $response; } - } \ No newline at end of file diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index edd7602a2e4..10ba3cacc43 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -80,19 +80,21 @@ class IconBuilder { */ public function renderAppIcon($app) { $appIcon = $this->util->getAppIcon($app); + $appIconContent = file_get_contents($appIcon); $color = $this->themingDefaults->getMailHeaderColor(); $mime = mime_content_type($appIcon); + // generate background image with rounded corners $background = '' . '' . '' . ''; - // resize svg magic as this seems broken in Imagemagick - if($mime === "image/svg+xml") { - $svg = file_get_contents($appIcon); - + if($mime === "image/svg+xml" || substr($appIconContent, 0, 4) === "".$appIconContent; + } $tmp = new Imagick(); $tmp->readImageBlob($svg); $x = $tmp->getImageWidth(); @@ -137,4 +139,12 @@ class IconBuilder { return $finalIconFile; } + public function colorSvg($app, $image) { + $imageFile = $this->util->getAppImage($app, $image); + $svg = file_get_contents($imageFile); + $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); + $svg = $this->util->colorizeSvg($svg, $color); + return $svg; + } + } \ No newline at end of file diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php new file mode 100644 index 00000000000..5e1b61e3a92 --- /dev/null +++ b/apps/theming/lib/ImageManager.php @@ -0,0 +1,118 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\Theming; + +use OCP\IConfig; +use OCP\Files\IAppData; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; + +class ImageManager { + + /** @var IConfig */ + private $config; + /** @var IAppData */ + private $appData; + + /** + * ImageManager constructor. + * + * @param IConfig $config + * @param IAppData $appData + */ + public function __construct(IConfig $config, + IAppData $appData + ) { + $this->config = $config; + $this->appData = $appData; + } + + /** + * Get folder for current theming files + * + * @return \OCP\Files\SimpleFS\ISimpleFolder + * @throws NotPermittedException + * @throws \RuntimeException + */ + public function getCacheFolder() { + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); + try { + $folder = $this->appData->getFolder($cacheBusterValue); + } catch (NotFoundException $e) { + $folder = $this->appData->newFolder($cacheBusterValue); + $this->cleanup(); + } + return $folder; + } + + /** + * Get a file from AppData + * + * @param string $filename + * @return null|\OCP\Files\SimpleFS\ISimpleFile + */ + public function getCachedImage($filename) { + $currentFolder = $this->getCacheFolder(); + if($currentFolder->fileExists($filename)) { + return $currentFolder->getFile($filename); + } else { + return null; + } + } + + /** + * Store a file for theming in AppData + * + * @param string $filename + * @param string $data + * @return \OCP\Files\SimpleFS\ISimpleFile + */ + public function setCachedImage($filename, $data) { + $currentFolder = $this->getCacheFolder(); + if ($currentFolder->fileExists($filename)) { + $file = $currentFolder->getFile($filename); + } else { + $file = $currentFolder->newFile($filename); + } + $file->putContent($data); + return $file; + } + + /** + * remove cached files that are not required any longer + */ + public function cleanup() { + $currentFolder = $this->getCacheFolder(); + $folders = $this->appData->getDirectoryListing(); + foreach ($folders as $folder) { + if ($folder->getName() !== $currentFolder->getName()) { + $folder->delete(); + } + } + } + + + +} diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 09cb41088de..82a937c3b91 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -23,12 +23,18 @@ namespace OCA\Theming\Tests\Controller; +use OC\Files\SimpleFS\SimpleFile; +use OCA\Theming\ImageManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\Files\IRootFolder; +use OCP\IConfig; +use OCP\IL10N; use OCP\IRequest; use Test\TestCase; use OCA\Theming\Util; use OCA\Theming\Controller\IconController; +use OCP\AppFramework\Http\FileDisplayResponse; class IconControllerTest extends TestCase { @@ -42,8 +48,12 @@ class IconControllerTest extends TestCase { private $timeFactory; /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ private $iconController; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ private $iconBuilder; + /** @var ImageManager */ + private $imageManager; public function setUp() { $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); @@ -54,9 +64,10 @@ class IconControllerTest extends TestCase { $this->timeFactory = $this->getMockBuilder('OCP\AppFramework\Utility\ITimeFactory') ->disableOriginalConstructor() ->getMock(); + $this->config = $this->getMockBuilder('OCP\IConfig')->getMock(); $this->iconBuilder = $this->getMockBuilder('OCA\Theming\IconBuilder') ->disableOriginalConstructor()->getMock(); - + $this->imageManager = $this->getMockBuilder('OCA\Theming\ImageManager')->disableOriginalConstructor()->getMock(); $this->timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); @@ -67,124 +78,114 @@ class IconControllerTest extends TestCase { $this->themingDefaults, $this->util, $this->timeFactory, - $this->iconBuilder + $this->config, + $this->iconBuilder, + $this->imageManager ); parent::setUp(); } + private function iconFileMock($filename, $data) { + $icon = $this->getMockBuilder('OCP\Files\File')->getMock(); + $icon->expects($this->any())->method('getContent')->willReturn($data); + $icon->expects($this->any())->method('getMimeType')->willReturn('image type'); + $icon->expects($this->any())->method('getEtag')->willReturn('my etag'); + $icon->method('getName')->willReturn($filename); + return new SimpleFile($icon); + } + public function testGetThemedIcon() { - $this->util->expects($this->once()) - ->method('getAppImage') - ->with('core','filetypes/folder.svg') - ->willReturn(\OC::$SERVERROOT . "/core/img/filetypes/folder.svg"); - $this->themingDefaults - ->expects($this->once()) - ->method('getMailHeaderColor') - ->willReturn('#000000'); - $this->util - ->expects($this->once()) - ->method('elementColor') - ->willReturn('#000000'); - - $svg = " - - - - - -"; - $expected = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + $file = $this->iconFileMock('icon-core-filetypes_folder.svg', 'filecontent'); + $this->imageManager->expects($this->once()) + ->method('getCachedImage') + ->with('icon-core-filetypes_folder.svg') + ->willReturn($file); + $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $expected->addHeader('Pragma', 'cache'); - @$this->assertEquals($expected, $this->iconController->getThemedIcon('core','filetypes/folder.svg')); + @$this->assertEquals($expected, $this->iconController->getThemedIcon('core', 'filetypes/folder.svg')); + + } public function testGetFaviconDefault() { - if(!extension_loaded('imagick')) { + if (!extension_loaded('imagick')) { $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); } $checkImagick = new \Imagick(); if (count($checkImagick->queryFormats('SVG')) < 1) { $this->markTestSkipped('No SVG provider present.'); } - $this->themingDefaults->expects($this->once()) + $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(true); - $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/favicon-original.ico'); + $this->iconBuilder->expects($this->once()) ->method('getFavicon') ->with('core') - ->willReturn($expectedIcon); - $favicon = $this->iconController->getFavicon(); + ->willReturn('filecontent'); + $file = $this->iconFileMock('filename', 'filecontent'); + $this->imageManager->expects($this->once()) + ->method('setCachedImage') + ->willReturn($file); - $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $expected->addHeader('Pragma', 'cache'); - $this->assertEquals($expected, $favicon); + $this->assertEquals($expected, $this->iconController->getFavicon()); } + + public function testGetFaviconFail() { + $this->themingDefaults->expects($this->any()) + ->method('shouldReplaceIcons') + ->willReturn(false); + $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $expected->cacheFor(86400); + $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); + $this->assertEquals($expected, $this->iconController->getFavicon()); + } + public function testGetTouchIconDefault() { - if(!extension_loaded('imagick')) { + if (!extension_loaded('imagick')) { $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); } $checkImagick = new \Imagick(); if (count($checkImagick->queryFormats('SVG')) < 1) { $this->markTestSkipped('No SVG provider present.'); } - $this->themingDefaults->expects($this->once()) + $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(true); - $expectedIcon = new \Imagick(realpath(dirname(__FILE__)) . '/../data/touch-original.png'); + $this->iconBuilder->expects($this->once()) ->method('getTouchIcon') ->with('core') - ->willReturn($expectedIcon); - $favicon = $this->iconController->getTouchIcon(); + ->willReturn('filecontent'); + $file = $this->iconFileMock('filename', 'filecontent'); + $this->imageManager->expects($this->once()) + ->method('setCachedImage') + ->willReturn($file); - $expected = new DataDisplayResponse($expectedIcon, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $expected->addHeader('Pragma', 'cache'); - $this->assertEquals($expected, $favicon); + $this->assertEquals($expected, $this->iconController->getTouchIcon()); } - public function testGetFaviconFail() { - if(!extension_loaded('imagick')) { - $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); - } - $checkImagick = new \Imagick(); - if (count($checkImagick->queryFormats('SVG')) < 1) { - $this->markTestSkipped('No SVG provider present.'); - } - $this->themingDefaults->expects($this->once()) - ->method('shouldReplaceIcons') - ->willReturn(false); - $favicon = $this->iconController->getFavicon(); - $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $expected->addHeader('Pragma', 'cache'); - $this->assertEquals($expected, $favicon); - } public function testGetTouchIconFail() { - if(!extension_loaded('imagick')) { - $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); - } - $checkImagick = new \Imagick(); - if (count($checkImagick->queryFormats('SVG')) < 1) { - $this->markTestSkipped('No SVG provider present.'); - } - $this->themingDefaults->expects($this->once()) + $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(false); - $favicon = $this->iconController->getTouchIcon(); $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); $expected->cacheFor(86400); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $expected->addHeader('Pragma', 'cache'); - $this->assertEquals($expected, $favicon); + $this->assertEquals($expected, $this->iconController->getTouchIcon()); } } diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index 03054367210..529518b30de 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -45,6 +45,15 @@ class IconBuilderTest extends TestCase { protected function setUp() { parent::setUp(); + $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') + ->disableOriginalConstructor()->getMock(); + $this->util = new Util($this->config, $this->rootFolder); + $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); + } + + private function checkImagick() { if(!extension_loaded('imagick')) { $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); } @@ -52,13 +61,6 @@ class IconBuilderTest extends TestCase { if (count($checkImagick->queryFormats('SVG')) < 1) { $this->markTestSkipped('No SVG provider present.'); } - - $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); - $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); - $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') - ->disableOriginalConstructor()->getMock(); - $this->util = new Util($this->config, $this->rootFolder); - $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); } public function dataRenderAppIcon() { @@ -78,6 +80,7 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testRenderAppIcon($app, $color, $file) { + $this->checkImagick(); $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); @@ -102,6 +105,7 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testGetTouchIcon($app, $color, $file) { + $this->checkImagick(); $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); @@ -127,6 +131,7 @@ class IconBuilderTest extends TestCase { * @param $file */ public function testGetFavicon($app, $color, $file) { + $this->checkImagick(); $this->themingDefaults->expects($this->once()) ->method('getMailHeaderColor') ->willReturn($color); diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php new file mode 100644 index 00000000000..b52d557ab67 --- /dev/null +++ b/apps/theming/tests/ImageManagerTest.php @@ -0,0 +1,185 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Theming\Tests; + +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\IConfig; +use Test\TestCase; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\Files\IAppData; +use OCP\Files\NotFoundException; + +class ImageManager extends TestCase { + + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + protected $config; + /** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */ + protected $appData; + /** @var ImageManager */ + protected $imageManager; + + protected function setUp() { + parent::setUp(); + $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); + $this->appData = $this->getMockBuilder('OCP\Files\IAppData')->getMock(); + $this->imageManager = new \OCA\Theming\ImageManager( + $this->config, + $this->appData + ); + } + + public function testGetCacheFolder() { + $folder = $this->createMock(ISimpleFolder::class); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('theming', 'cachebuster', '0') + ->willReturn('0'); + $this->appData->expects($this->at(0)) + ->method('getFolder') + ->with('0') + ->willReturn($folder); + $this->assertEquals($folder, $this->imageManager->getCacheFolder()); + } + public function testGetCacheFolderCreate() { + $folder = $this->createMock(ISimpleFolder::class); + $this->config->expects($this->exactly(2)) + ->method('getAppValue') + ->with('theming', 'cachebuster', '0') + ->willReturn('0'); + $this->appData->expects($this->at(0)) + ->method('getFolder') + ->willThrowException(new NotFoundException()); + $this->appData->expects($this->at(1)) + ->method('newFolder') + ->with('0') + ->willReturn($folder); + $this->appData->expects($this->at(2)) + ->method('getFolder') + ->with('0') + ->willReturn($folder); + $this->appData->expects($this->once()) + ->method('getDirectoryListing') + ->willReturn([]); + $this->assertEquals($folder, $this->imageManager->getCacheFolder()); + } + + public function testGetCachedImage() { + $folder = $this->setupCacheFolder(); + $folder->expects($this->once()) + ->method('fileExists') + ->with('filename') + ->willReturn(true); + $folder->expects($this->once()) + ->method('getFile') + ->with('filename') + ->willReturn('filecontent'); + $expected = 'filecontent'; + $this->assertEquals($expected, $this->imageManager->getCachedImage('filename')); + } + + public function testGetCachedImageNotFound() { + $folder = $this->setupCacheFolder(); + $folder->expects($this->once()) + ->method('fileExists') + ->with('filename') + ->willReturn(false); + $expected = null; + $this->assertEquals($expected, $this->imageManager->getCachedImage('filename')); + } + + public function testSetCachedImage() { + $folder = $this->setupCacheFolder(); + $file = $this->createMock(ISimpleFile::class); + $folder->expects($this->once()) + ->method('fileExists') + ->with('filename') + ->willReturn(true); + $folder->expects($this->once()) + ->method('getFile') + ->with('filename') + ->willReturn($file); + $file->expects($this->once()) + ->method('putContent') + ->with('filecontent'); + $this->assertEquals($file, $this->imageManager->setCachedImage('filename', 'filecontent')); + } + + public function testSetCachedImageCreate() { + $folder = $this->setupCacheFolder(); + $file = $this->createMock(ISimpleFile::class); + $folder->expects($this->once()) + ->method('fileExists') + ->with('filename') + ->willReturn(false); + $folder->expects($this->once()) + ->method('newFile') + ->with('filename') + ->willReturn($file); + $file->expects($this->once()) + ->method('putContent') + ->with('filecontent'); + $this->assertEquals($file, $this->imageManager->setCachedImage('filename', 'filecontent')); + } + + private function setupCacheFolder() { + $folder = $this->createMock(ISimpleFolder::class); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('theming', 'cachebuster', '0') + ->willReturn('0'); + $this->appData->expects($this->at(0)) + ->method('getFolder') + ->with('0') + ->willReturn($folder); + return $folder; + } + + public function testCleanup() { + $folders = [ + $this->createMock(ISimpleFolder::class), + $this->createMock(ISimpleFolder::class), + $this->createMock(ISimpleFolder::class) + ]; + foreach ($folders as $index=>$folder) { + $folder->expects($this->any()) + ->method('getName') + ->willReturn($index); + } + $folders[0]->expects($this->once())->method('delete'); + $folders[1]->expects($this->once())->method('delete'); + $folders[2]->expects($this->never())->method('delete'); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('theming','cachebuster','0') + ->willReturn('2'); + $this->appData->expects($this->once()) + ->method('getDirectoryListing') + ->willReturn($folders); + $this->appData->expects($this->once()) + ->method('getFolder') + ->with('2') + ->willReturn($folders[2]); + $this->imageManager->cleanup(); + } + +} -- cgit v1.2.3 From 2e8dd218157123cdb7f1741980e12dc22b95f320 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 14 Oct 2016 14:57:58 +0200 Subject: Improve caching Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 47 ++++++++++++++-------- apps/theming/lib/IconBuilder.php | 4 +- apps/theming/lib/ThemingDefaults.php | 17 ++++++-- .../tests/Controller/IconControllerTest.php | 25 +++++++----- apps/theming/tests/ThemingDefaultsTest.php | 7 +++- lib/private/Server.php | 3 +- 6 files changed, 71 insertions(+), 32 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index eb65ae54f1a..887dba8a68a 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -96,7 +96,10 @@ class IconController extends Controller { } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); return $response; } @@ -111,19 +114,24 @@ class IconController extends Controller { * @return FileDisplayResponse|DataDisplayResponse */ public function getFavicon($app = "core") { - $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); - if($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { - $icon = $this->iconBuilder->getFavicon($app); - $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); - } if ($this->themingDefaults->shouldReplaceIcons()) { + $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); + if($iconFile === null) { + $icon = $this->iconBuilder->getFavicon($app); + $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); + } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(0); + $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); return $response; } @@ -137,19 +145,24 @@ class IconController extends Controller { * @return FileDisplayResponse|DataDisplayResponse */ public function getTouchIcon($app = "core") { - $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); - if ($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { - $icon = $this->iconBuilder->getTouchIcon($app); - $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); - } if ($this->themingDefaults->shouldReplaceIcons()) { + $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); + if ($iconFile === null) { + $icon = $this->iconBuilder->getTouchIcon($app); + $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); + } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(0); + $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); return $response; } } \ No newline at end of file diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 10ba3cacc43..fac8cad430b 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -94,6 +94,8 @@ class IconBuilder { if($mime === "image/svg+xml" || substr($appIconContent, 0, 4) === "".$appIconContent; + } else { + $svg = $appIconContent; } $tmp = new Imagick(); $tmp->readImageBlob($svg); @@ -147,4 +149,4 @@ class IconBuilder { return $svg; } -} \ No newline at end of file +} diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index b7968d0073f..e10870685a5 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -23,6 +23,7 @@ namespace OCA\Theming; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -38,6 +39,8 @@ class ThemingDefaults extends \OC_Defaults { private $urlGenerator; /** @var IRootFolder */ private $rootFolder; + /** @var ICacheFactory */ + private $cacheFactory; /** @var string */ private $name; /** @var string */ @@ -60,13 +63,15 @@ class ThemingDefaults extends \OC_Defaults { IL10N $l, IURLGenerator $urlGenerator, \OC_Defaults $defaults, - IRootFolder $rootFolder + IRootFolder $rootFolder, + ICacheFactory $cacheFactory ) { parent::__construct(); $this->config = $config; $this->l = $l; $this->urlGenerator = $urlGenerator; $this->rootFolder = $rootFolder; + $this->cacheFactory = $cacheFactory; $this->name = $defaults->getName(); $this->url = $defaults->getBaseUrl(); @@ -151,14 +156,20 @@ class ThemingDefaults extends \OC_Defaults { * @return bool */ public function shouldReplaceIcons() { + $cache = $this->cacheFactory->create('theming'); + if($cache->hasKey('shouldReplaceIcons')) { + return (bool)$cache->get('shouldReplaceIcons'); + } + $value = false; if(extension_loaded('imagick')) { $checkImagick = new \Imagick(); if (count($checkImagick->queryFormats('SVG')) >= 1) { - return true; + $value = true; } $checkImagick->clear(); } - return false; + $cache->set('shouldReplaceIcons', $value); + return $value; } /** diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 82a937c3b91..98fa4f1a243 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -103,7 +103,10 @@ class IconControllerTest extends TestCase { ->willReturn($file); $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->iconController->getThemedIcon('core', 'filetypes/folder.svg')); @@ -133,7 +136,10 @@ class IconControllerTest extends TestCase { $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -143,9 +149,8 @@ class IconControllerTest extends TestCase { ->method('shouldReplaceIcons') ->willReturn(false); $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $expected->addHeader('Pragma', 'cache'); + $expected->cacheFor(0); + $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -172,7 +177,10 @@ class IconControllerTest extends TestCase { $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $this->iconController->getTouchIcon()); } @@ -182,9 +190,8 @@ class IconControllerTest extends TestCase { ->method('shouldReplaceIcons') ->willReturn(false); $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $expected->addHeader('Pragma', 'cache'); + $expected->cacheFor(0); + $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getTouchIcon()); } diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index 204c96d86d5..cd3a90e760a 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -24,6 +24,7 @@ namespace OCA\Theming\Tests; use OCA\Theming\ThemingDefaults; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -43,6 +44,8 @@ class ThemingDefaultsTest extends TestCase { private $template; /** @var IRootFolder */ private $rootFolder; + /** @var ICacheFactory */ + private $cacheFactory; public function setUp() { parent::setUp(); @@ -52,6 +55,7 @@ class ThemingDefaultsTest extends TestCase { $this->rootFolder = $this->getMockBuilder(IRootFolder::class) ->disableOriginalConstructor() ->getMock(); + $this->cacheFactory = $this->getMockBuilder(ICacheFactory::class)->getMock(); $this->defaults = $this->getMockBuilder(\OC_Defaults::class) ->disableOriginalConstructor() ->getMock(); @@ -76,7 +80,8 @@ class ThemingDefaultsTest extends TestCase { $this->l10n, $this->urlGenerator, $this->defaults, - $this->rootFolder + $this->rootFolder, + $this->cacheFactory ); } diff --git a/lib/private/Server.php b/lib/private/Server.php index c6755357a1d..c6cfa018be5 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -703,7 +703,8 @@ class Server extends ServerContainer implements IServerContainer { $c->getL10N('theming'), $c->getURLGenerator(), new \OC_Defaults(), - $c->getLazyRootFolder() + $c->getLazyRootFolder(), + $c->getMemCacheFactory() ); } return new \OC_Defaults(); -- cgit v1.2.3 From 43097eabeea450fd23d5c07c3df92d63abeb3e24 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 14 Oct 2016 21:42:25 +0200 Subject: Fix svg resizing and remove deprecated method call Signed-off-by: Julius Haertl --- apps/theming/lib/IconBuilder.php | 11 +++++++++-- apps/theming/lib/ThemingDefaults.php | 4 ++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index fac8cad430b..3819a2be4cb 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -104,13 +104,20 @@ class IconBuilder { $res = $tmp->getImageResolution(); $tmp->destroy(); + if($x>$y) { + $max = $x; + } else { + $max = $y; + } + // convert svg to resized image $appIconFile = new Imagick(); - $resX = (int)(512 * $res['x'] / $x * 2.53); - $resY = (int)(512 * $res['y'] / $y * 2.53); + $resX = (int)(512 * $res['x'] / $max * 2.53); + $resY = (int)(512 * $res['y'] / $max * 2.53); $appIconFile->setResolution($resX, $resY); $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); $appIconFile->readImageBlob($svg); + $appIconFile->scaleImage(512, 512, true); } else { $appIconFile = new Imagick(); $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index e10870685a5..2c344172127 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -157,8 +157,8 @@ class ThemingDefaults extends \OC_Defaults { */ public function shouldReplaceIcons() { $cache = $this->cacheFactory->create('theming'); - if($cache->hasKey('shouldReplaceIcons')) { - return (bool)$cache->get('shouldReplaceIcons'); + if($value = $cache->get('shouldReplaceIcons')) { + return (bool)$value; } $value = false; if(extension_loaded('imagick')) { -- cgit v1.2.3 From cc8b1d382910e13c0825e5a3fdb33276f65cf2bd Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Mon, 17 Oct 2016 14:51:31 +0200 Subject: Fix icon-folder css Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/ThemingController.php | 2 +- apps/theming/tests/Controller/ThemingControllerTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index c908f0e5782..c0661409d05 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -406,7 +406,7 @@ class ThemingController extends Controller { if($logo !== '' or $color !== '') { $responseCss .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v='.$cacheBusterValue.'\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v='.$cacheBusterValue.'\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v='.$cacheBusterValue.'\')!important;' . "}\n"; diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index b7bce2d6ec0..3bf7709b0de 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -477,7 +477,7 @@ class ThemingControllerTest extends TestCase { $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; $expectedData .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; @@ -577,7 +577,7 @@ class ThemingControllerTest extends TestCase { $expectedData .= '.ui-widget-header { color: #000000; }' . "\n"; $expectedData .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; @@ -628,7 +628,7 @@ class ThemingControllerTest extends TestCase { $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; $expectedData .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; @@ -776,7 +776,7 @@ class ThemingControllerTest extends TestCase { $expectedData .= '.nc-theming-contrast {color: #ffffff}' . "\n"; $expectedData .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; @@ -893,7 +893,7 @@ class ThemingControllerTest extends TestCase { $expectedData .= '.ui-widget-header { color: #000000; }' . "\n"; $expectedData .= '.icon-file,.icon-filetype-text {' . 'background-image: url(\'./img/core/filetypes/text.svg?v=0\');' . "}\n" . - '.icon-folder, .icon-filetype-folder ' . + '.icon-folder, .icon-filetype-folder {' . 'background-image: url(\'./img/core/filetypes/folder.svg?v=0\');' . "}\n" . '.icon-filetype-folder-drag-accept {' . 'background-image: url(\'./img/core/filetypes/folder-drag-accept.svg?v=0\')!important;' . "}\n"; -- cgit v1.2.3 From 3a400f92d1936b2b752d813cbb27632d6acb9904 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Mon, 17 Oct 2016 16:31:07 +0200 Subject: Replace null return with NotFoundException Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 18 +++++++++++------- apps/theming/lib/ImageManager.php | 9 +++------ apps/theming/tests/Controller/IconControllerTest.php | 7 +++++++ apps/theming/tests/ImageManagerTest.php | 14 ++++++-------- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 887dba8a68a..08d5b50120f 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -30,6 +30,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Files\NotFoundException; use OCP\IRequest; use OCA\Theming\Util; use OCP\IConfig; @@ -89,8 +90,9 @@ class IconController extends Controller { * @return FileDisplayResponse */ public function getThemedIcon($app, $image) { - $iconFile = $this->imageManager->getCachedImage("icon-" . $app . '-' . str_replace("/","_",$image)); - if ($iconFile === null) { + try { + $iconFile = $this->imageManager->getCachedImage("icon-" . $app . '-' . str_replace("/","_",$image)); + } catch (NotFoundException $exception) { $icon = $this->iconBuilder->colorSvg($app, $image); $iconFile = $this->imageManager->setCachedImage("icon-" . $app . '-' . str_replace("/","_",$image), $icon); } @@ -115,8 +117,9 @@ class IconController extends Controller { */ public function getFavicon($app = "core") { if ($this->themingDefaults->shouldReplaceIcons()) { - $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); - if($iconFile === null) { + try { + $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); + } catch (NotFoundException $exception) { $icon = $this->iconBuilder->getFavicon($app); $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); } @@ -146,8 +149,9 @@ class IconController extends Controller { */ public function getTouchIcon($app = "core") { if ($this->themingDefaults->shouldReplaceIcons()) { - $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); - if ($iconFile === null) { + try { + $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); + } catch (NotFoundException $exception) { $icon = $this->iconBuilder->getTouchIcon($app); $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); } @@ -165,4 +169,4 @@ class IconController extends Controller { } return $response; } -} \ No newline at end of file +} diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 5e1b61e3a92..e7dcfa92190 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -71,15 +71,12 @@ class ImageManager { * Get a file from AppData * * @param string $filename - * @return null|\OCP\Files\SimpleFS\ISimpleFile + * @throws NotFoundException + * @return \OCP\Files\SimpleFS\ISimpleFile */ public function getCachedImage($filename) { $currentFolder = $this->getCacheFolder(); - if($currentFolder->fileExists($filename)) { - return $currentFolder->getFile($filename); - } else { - return null; - } + return $currentFolder->getFile($filename); } /** diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 98fa4f1a243..69bbfa0e45e 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -28,6 +28,7 @@ use OCA\Theming\ImageManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -130,6 +131,9 @@ class IconControllerTest extends TestCase { ->with('core') ->willReturn('filecontent'); $file = $this->iconFileMock('filename', 'filecontent'); + $this->imageManager->expects($this->once()) + ->method('getCachedImage') + ->will($this->throwException(new NotFoundException())); $this->imageManager->expects($this->once()) ->method('setCachedImage') ->willReturn($file); @@ -171,6 +175,9 @@ class IconControllerTest extends TestCase { ->with('core') ->willReturn('filecontent'); $file = $this->iconFileMock('filename', 'filecontent'); + $this->imageManager->expects($this->once()) + ->method('getCachedImage') + ->will($this->throwException(new NotFoundException())); $this->imageManager->expects($this->once()) ->method('setCachedImage') ->willReturn($file); diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php index b52d557ab67..4df49633d80 100644 --- a/apps/theming/tests/ImageManagerTest.php +++ b/apps/theming/tests/ImageManagerTest.php @@ -85,10 +85,6 @@ class ImageManager extends TestCase { public function testGetCachedImage() { $folder = $this->setupCacheFolder(); - $folder->expects($this->once()) - ->method('fileExists') - ->with('filename') - ->willReturn(true); $folder->expects($this->once()) ->method('getFile') ->with('filename') @@ -97,14 +93,16 @@ class ImageManager extends TestCase { $this->assertEquals($expected, $this->imageManager->getCachedImage('filename')); } + /** + * @expectedException \OCP\Files\NotFoundException + */ public function testGetCachedImageNotFound() { $folder = $this->setupCacheFolder(); $folder->expects($this->once()) - ->method('fileExists') + ->method('getFile') ->with('filename') - ->willReturn(false); - $expected = null; - $this->assertEquals($expected, $this->imageManager->getCachedImage('filename')); + ->will($this->throwException(new \OCP\Files\NotFoundException())); + $image = $this->imageManager->getCachedImage('filename'); } public function testSetCachedImage() { -- cgit v1.2.3 From 78de213b8582f160b9e3acd1d921a6dd1ccd88d9 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 4 Nov 2016 18:55:00 +0100 Subject: Sanitize input and small fixes Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 4 +- apps/theming/lib/Controller/IconController.php | 7 +- apps/theming/lib/ImageManager.php | 3 - apps/theming/lib/ThemingDefaults.php | 1 + apps/theming/lib/Util.php | 76 +++++++++++++--------- .../tests/Controller/IconControllerTest.php | 11 ++-- apps/theming/tests/UtilTest.php | 4 +- 7 files changed, 61 insertions(+), 45 deletions(-) diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index a8436ab254f..f4aa2f93162 100644 --- a/apps/theming/appinfo/routes.php +++ b/apps/theming/appinfo/routes.php @@ -64,13 +64,13 @@ return ['routes' => [ 'name' => 'Icon#getFavicon', 'url' => '/favicon/{app}', 'verb' => 'GET', - 'defaults' => array("app" => "core"), + 'defaults' => array('app' => 'core'), ], [ 'name' => 'Icon#getTouchIcon', 'url' => '/icon/{app}', 'verb' => 'GET', - 'defaults' => array("app" => "core"), + 'defaults' => array('app' => 'core'), ], [ 'name' => 'Icon#getThemedIcon', diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 08d5b50120f..519c52f5fa9 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -27,6 +27,7 @@ use OCA\Theming\ImageManager; use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; @@ -131,7 +132,8 @@ class IconController extends Controller { $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); } else { - $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response = new Response(); + $response->setStatus(Http::STATUS_NOT_FOUND); $response->cacheFor(0); $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } @@ -163,7 +165,8 @@ class IconController extends Controller { $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); } else { - $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response = new Response(); + $response->setStatus(Http::STATUS_NOT_FOUND); $response->cacheFor(0); $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index e7dcfa92190..4cd43e02054 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -109,7 +109,4 @@ class ImageManager { } } } - - - } diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 2c344172127..36f19157637 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -58,6 +58,7 @@ class ThemingDefaults extends \OC_Defaults { * @param IURLGenerator $urlGenerator * @param \OC_Defaults $defaults * @param IRootFolder $rootFolder + * @param ICacheFactory $cacheFactory */ public function __construct(IConfig $config, IL10N $l, diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 84c631092a8..963cf56633b 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -28,9 +28,18 @@ use OCP\Files\IRootFolder; class Util { + /** @var IConfig */ private $config; + + /** @var IRootFolder */ private $rootFolder; + /** + * Util constructor. + * + * @param IConfig $config + * @param IRootFolder $rootFolder + */ public function __construct(IConfig $config, IRootFolder $rootFolder) { $this->config = $config; $this->rootFolder = $rootFolder; @@ -98,14 +107,17 @@ class Util { * @return string path to app icon / logo */ public function getAppIcon($app) { + $app = str_replace(array('\0', '/', '\\', '..'), '', $app); $appPath = \OC_App::getAppPath($app); - $icon = $appPath . '/img/' . $app . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/app.svg'; - if(file_exists($icon)) { - return $icon; + if ($appPath !== false) { + $icon = $appPath . '/img/' . $app . '.svg'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/app.svg'; + if (file_exists($icon)) { + return $icon; + } } if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) { return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; @@ -119,32 +131,36 @@ class Util { * @return string absolute path to image */ public function getAppImage($app, $image) { + $app = str_replace(array('\0', '/', '\\', '..'), '', $app); + $image = str_replace(array('\0', '\\', '..'), '', $image); $appPath = \OC_App::getAppPath($app); - if($app==="core") { - $icon = \OC::$SERVERROOT . '/core/img/' . $image; - if(file_exists($icon)) { + if ($app === "core") { + $icon = \OC::$SERVERROOT . '/core/img/' . $image; + if (file_exists($icon)) { + return $icon; + } + } + if ($appPath !== false) { + $icon = $appPath . '/img/' . $image; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.svg'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.png'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.gif'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.jpg'; + if (file_exists($icon)) { return $icon; } - } - $icon = $appPath . '/img/' . $image; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.png'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.gif'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.jpg'; - if(file_exists($icon)) { - return $icon; } return false; } diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 69bbfa0e45e..b1742db018f 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -30,7 +30,6 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IConfig; -use OCP\IL10N; use OCP\IRequest; use Test\TestCase; use OCA\Theming\Util; @@ -47,7 +46,7 @@ class IconControllerTest extends TestCase { private $util; /** @var \OCP\AppFramework\Utility\ITimeFactory */ private $timeFactory; - /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IconController|\PHPUnit_Framework_MockObject_MockObject */ private $iconController; /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ private $config; @@ -110,8 +109,6 @@ class IconControllerTest extends TestCase { $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->iconController->getThemedIcon('core', 'filetypes/folder.svg')); - - } public function testGetFaviconDefault() { @@ -152,7 +149,8 @@ class IconControllerTest extends TestCase { $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(false); - $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $expected = new Http\Response(); + $expected->setStatus(Http::STATUS_NOT_FOUND); $expected->cacheFor(0); $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getFavicon()); @@ -196,7 +194,8 @@ class IconControllerTest extends TestCase { $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(false); - $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $expected = new Http\Response(); + $expected->setStatus(Http::STATUS_NOT_FOUND); $expected->cacheFor(0); $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getTouchIcon()); diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index 9a53858598c..d82ec8eeb85 100644 --- a/apps/theming/tests/UtilTest.php +++ b/apps/theming/tests/UtilTest.php @@ -97,14 +97,13 @@ class UtilTest extends TestCase { $expected = 'PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMTYiIHdpZHRoPSIxNiI+PHBhdGggZD0iTTggMWE3IDcgMCAwIDAtNyA3IDcgNyAwIDAgMCA3IDcgNyA3IDAgMCAwIDctNyA3IDcgMCAwIDAtNy03em0wIDFhNiA2IDAgMCAxIDYgNiA2IDYgMCAwIDEtNiA2IDYgNiAwIDAgMS02LTYgNiA2IDAgMCAxIDYtNnptMCAyYTQgNCAwIDEgMCAwIDggNCA0IDAgMCAwIDAtOHoiIGZpbGw9IiNmZmZmZmYiLz48L3N2Zz4='; $this->assertEquals($expected, $button); } + public function testGenerateRadioButtonBlack() { $button = $this->util->generateRadioButton('#000000'); $expected = 'PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMTYiIHdpZHRoPSIxNiI+PHBhdGggZD0iTTggMWE3IDcgMCAwIDAtNyA3IDcgNyAwIDAgMCA3IDcgNyA3IDAgMCAwIDctNyA3IDcgMCAwIDAtNy03em0wIDFhNiA2IDAgMCAxIDYgNiA2IDYgMCAwIDEtNiA2IDYgNiAwIDAgMS02LTYgNiA2IDAgMCAxIDYtNnptMCAyYTQgNCAwIDEgMCAwIDggNCA0IDAgMCAwIDAtOHoiIGZpbGw9IiMwMDAwMDAiLz48L3N2Zz4='; $this->assertEquals($expected, $button); } - - /** * @dataProvider dataGetAppIcon */ @@ -137,6 +136,7 @@ class UtilTest extends TestCase { public function testGetAppImage($app, $image, $expected) { $this->assertEquals($expected, $this->util->getAppImage($app, $image)); } + public function dataGetAppImage() { return [ ['core', 'logo.svg', \OC::$SERVERROOT . '/core/img/logo.svg'], -- cgit v1.2.3 From d409fe1c525ba05f342d52a9686ae395a0dac465 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sat, 5 Nov 2016 20:11:49 +0100 Subject: Error handling and tests if file was not found Signed-off-by: Julius Haertl --- apps/theming/lib/Controller/IconController.php | 77 +++++++++++----------- apps/theming/lib/IconBuilder.php | 25 +++++-- .../tests/Controller/IconControllerTest.php | 9 +-- apps/theming/tests/IconBuilderTest.php | 36 ++++++++++ 4 files changed, 96 insertions(+), 51 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 519c52f5fa9..4c25d911e5e 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -27,8 +27,7 @@ use OCA\Theming\ImageManager; use OCA\Theming\ThemingDefaults; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\Response; -use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\NotFoundException; @@ -88,7 +87,7 @@ class IconController extends Controller { * * @param $app string app name * @param $image string image file name (svg required) - * @return FileDisplayResponse + * @return FileDisplayResponse|NotFoundResponse */ public function getThemedIcon($app, $image) { try { @@ -97,14 +96,18 @@ class IconController extends Controller { $icon = $this->iconBuilder->colorSvg($app, $image); $iconFile = $this->imageManager->setCachedImage("icon-" . $app . '-' . str_replace("/","_",$image), $icon); } - $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); - $response->cacheFor(86400); - $expires = new \DateTime(); - $expires->setTimestamp($this->timeFactory->getTime()); - $expires->add(new \DateInterval('PT24H')); - $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); - $response->addHeader('Pragma', 'cache'); - return $response; + if ($iconFile !== false) { + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); + return $response; + } else { + return new NotFoundResponse(); + } } /** @@ -114,7 +117,7 @@ class IconController extends Controller { * @NoCSRFRequired * * @param $app string app name - * @return FileDisplayResponse|DataDisplayResponse + * @return FileDisplayResponse|NotFoundResponse */ public function getFavicon($app = "core") { if ($this->themingDefaults->shouldReplaceIcons()) { @@ -124,20 +127,18 @@ class IconController extends Controller { $icon = $this->iconBuilder->getFavicon($app); $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); } - $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); - $response->cacheFor(86400); - $expires = new \DateTime(); - $expires->setTimestamp($this->timeFactory->getTime()); - $expires->add(new \DateInterval('PT24H')); - $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); - $response->addHeader('Pragma', 'cache'); - } else { - $response = new Response(); - $response->setStatus(Http::STATUS_NOT_FOUND); - $response->cacheFor(0); - $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); + if ($iconFile !== false) { + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); + return $response; + } } - return $response; + return new NotFoundResponse(); } /** @@ -147,7 +148,7 @@ class IconController extends Controller { * @NoCSRFRequired * * @param $app string app name - * @return FileDisplayResponse|DataDisplayResponse + * @return FileDisplayResponse|NotFoundResponse */ public function getTouchIcon($app = "core") { if ($this->themingDefaults->shouldReplaceIcons()) { @@ -157,19 +158,17 @@ class IconController extends Controller { $icon = $this->iconBuilder->getTouchIcon($app); $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); } - $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); - $response->cacheFor(86400); - $expires = new \DateTime(); - $expires->setTimestamp($this->timeFactory->getTime()); - $expires->add(new \DateInterval('PT24H')); - $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); - $response->addHeader('Pragma', 'cache'); - } else { - $response = new Response(); - $response->setStatus(Http::STATUS_NOT_FOUND); - $response->cacheFor(0); - $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); + if ($iconFile !== false) { + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); + return $response; + } } - return $response; + return new NotFoundResponse(); } } diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 3819a2be4cb..be3035eb0d8 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -48,10 +48,13 @@ class IconBuilder { /** * @param $app string app name - * @return string image blob + * @return string|false image blob */ public function getFavicon($app) { $icon = $this->renderAppIcon($app); + if($icon === false) { + return false; + } $icon->resizeImage(32, 32, Imagick::FILTER_LANCZOS, 1); $icon->setImageFormat("png24"); $data = $icon->getImageBlob(); @@ -61,10 +64,13 @@ class IconBuilder { /** * @param $app string app name - * @return string image blob + * @return string|false image blob */ public function getTouchIcon($app) { $icon = $this->renderAppIcon($app); + if($icon === false) { + return false; + } $icon->setImageFormat("png24"); $data = $icon->getImageBlob(); $icon->destroy(); @@ -76,11 +82,14 @@ class IconBuilder { * fallback to logo * * @param $app string app name - * @return Imagick + * @return Imagick|false */ public function renderAppIcon($app) { $appIcon = $this->util->getAppIcon($app); $appIconContent = file_get_contents($appIcon); + if($appIconContent === false) { + return false; + } $color = $this->themingDefaults->getMailHeaderColor(); $mime = mime_content_type($appIcon); @@ -151,9 +160,13 @@ class IconBuilder { public function colorSvg($app, $image) { $imageFile = $this->util->getAppImage($app, $image); $svg = file_get_contents($imageFile); - $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); - $svg = $this->util->colorizeSvg($svg, $color); - return $svg; + if ($svg !== false) { + $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); + $svg = $this->util->colorizeSvg($svg, $color); + return $svg; + } else { + return false; + } } } diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index b1742db018f..591c1075492 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -27,6 +27,7 @@ use OC\Files\SimpleFS\SimpleFile; use OCA\Theming\ImageManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\AppFramework\Http\NotFoundResponse; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IConfig; @@ -153,7 +154,7 @@ class IconControllerTest extends TestCase { $expected->setStatus(Http::STATUS_NOT_FOUND); $expected->cacheFor(0); $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); - $this->assertEquals($expected, $this->iconController->getFavicon()); + $this->assertInstanceOf(NotFoundResponse::class, $this->iconController->getFavicon()); } public function testGetTouchIconDefault() { @@ -194,11 +195,7 @@ class IconControllerTest extends TestCase { $this->themingDefaults->expects($this->any()) ->method('shouldReplaceIcons') ->willReturn(false); - $expected = new Http\Response(); - $expected->setStatus(Http::STATUS_NOT_FOUND); - $expected->cacheFor(0); - $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); - $this->assertEquals($expected, $this->iconController->getTouchIcon()); + $this->assertInstanceOf(NotFoundResponse::class, $this->iconController->getTouchIcon()); } } diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index 529518b30de..a13d4b9476d 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -25,6 +25,7 @@ namespace OCA\Theming\Tests; use OCA\Theming\IconBuilder; use OCA\Theming\ThemingDefaults; use OCA\Theming\Util; +use OCP\AppFramework\Http\NotFoundResponse; use OCP\Files\IRootFolder; use OCP\IConfig; use Test\TestCase; @@ -149,4 +150,39 @@ class IconBuilderTest extends TestCase { // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } + /** + * @expectedException \PHPUnit_Framework_Error_Warning + */ + public function testGetFaviconNotFound() { + $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); + $iconBuilder = new IconBuilder($this->themingDefaults, $util); + $util->expects($this->once()) + ->method('getAppIcon') + ->willReturn('notexistingfile'); + $this->assertFalse($iconBuilder->getFavicon('noapp')); + } + + /** + * @expectedException \PHPUnit_Framework_Error_Warning + */ + public function testGetTouchIconNotFound() { + $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); + $iconBuilder = new IconBuilder($this->themingDefaults, $util); + $util->expects($this->once()) + ->method('getAppIcon') + ->willReturn('notexistingfile'); + $this->assertFalse($iconBuilder->getTouchIcon('noapp')); + } + + /** + * @expectedException \PHPUnit_Framework_Error_Warning + */ + public function testColorSvgNotFound() { + $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); + $iconBuilder = new IconBuilder($this->themingDefaults, $util); + $util->expects($this->once()) + ->method('getAppImage') + ->willReturn('notexistingfile'); + $this->assertFalse($iconBuilder->colorSvg('noapp','noimage')); + } } -- cgit v1.2.3 From 2ab4d1e0a3f15af2b8f04edcf18b7fe3fc0be262 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 18 Nov 2016 10:49:03 +0100 Subject: Use IAppManager instead of OC_App Signed-off-by: Julius Haertl --- apps/theming/lib/IconBuilder.php | 16 ++++- apps/theming/lib/Util.php | 73 +++++++++++++--------- .../tests/Controller/ThemingControllerTest.php | 6 +- apps/theming/tests/IconBuilderTest.php | 6 +- apps/theming/tests/UtilTest.php | 16 ++++- 5 files changed, 81 insertions(+), 36 deletions(-) diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index be3035eb0d8..d8161051ebb 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -25,6 +25,7 @@ namespace OCA\Theming; use Imagick; use ImagickPixel; +use OCP\App\AppPathNotFoundException; class IconBuilder { /** @var ThemingDefaults */ @@ -85,8 +86,13 @@ class IconBuilder { * @return Imagick|false */ public function renderAppIcon($app) { - $appIcon = $this->util->getAppIcon($app); - $appIconContent = file_get_contents($appIcon); + try { + $appIcon = $this->util->getAppIcon($app); + $appIconContent = file_get_contents($appIcon); + } catch (AppPathNotFoundException $e) { + return false; + } + if($appIconContent === false) { return false; } @@ -158,7 +164,11 @@ class IconBuilder { } public function colorSvg($app, $image) { - $imageFile = $this->util->getAppImage($app, $image); + try { + $imageFile = $this->util->getAppImage($app, $image); + } catch (AppPathNotFoundException $e) { + return false; + } $svg = file_get_contents($imageFile); if ($svg !== false) { $color = $this->util->elementColor($this->themingDefaults->getMailHeaderColor()); diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 963cf56633b..9fea56838ad 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -23,6 +23,8 @@ namespace OCA\Theming; +use OCP\App\AppPathNotFoundException; +use OCP\App\IAppManager; use OCP\IConfig; use OCP\Files\IRootFolder; @@ -34,15 +36,20 @@ class Util { /** @var IRootFolder */ private $rootFolder; + /** @var IAppManager */ + private $appManager; + /** * Util constructor. * * @param IConfig $config * @param IRootFolder $rootFolder + * @param IAppManager $appManager */ - public function __construct(IConfig $config, IRootFolder $rootFolder) { + public function __construct(IConfig $config, IRootFolder $rootFolder, IAppManager $appManager) { $this->config = $config; $this->rootFolder = $rootFolder; + $this->appManager = $appManager; } /** @@ -108,8 +115,8 @@ class Util { */ public function getAppIcon($app) { $app = str_replace(array('\0', '/', '\\', '..'), '', $app); - $appPath = \OC_App::getAppPath($app); - if ($appPath !== false) { + try { + $appPath = $this->appManager->getAppPath($app); $icon = $appPath . '/img/' . $app . '.svg'; if (file_exists($icon)) { return $icon; @@ -118,7 +125,8 @@ class Util { if (file_exists($icon)) { return $icon; } - } + } catch (AppPathNotFoundException $e) {} + if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) { return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo'; } @@ -128,40 +136,45 @@ class Util { /** * @param $app string app name * @param $image string relative path to image in app folder - * @return string absolute path to image + * @return string|false absolute path to image */ public function getAppImage($app, $image) { $app = str_replace(array('\0', '/', '\\', '..'), '', $app); $image = str_replace(array('\0', '\\', '..'), '', $image); - $appPath = \OC_App::getAppPath($app); - if ($app === "core") { - $icon = \OC::$SERVERROOT . '/core/img/' . $image; - if (file_exists($icon)) { - return $icon; - } - } - if ($appPath !== false) { - $icon = $appPath . '/img/' . $image; - if (file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.svg'; - if (file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.png'; - if (file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.gif'; - if (file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.jpg'; + if ($app === "core") { + $icon = \OC::$SERVERROOT . '/core/img/' . $image; if (file_exists($icon)) { return $icon; } } + + try { + $appPath = $this->appManager->getAppPath($app); + } catch (AppPathNotFoundException $e) { + return false; + } + + $icon = $appPath . '/img/' . $image; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.svg'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.png'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.gif'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.jpg'; + if (file_exists($icon)) { + return $icon; + } + return false; } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 3bf7709b0de..9ce4fb86b52 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -26,6 +26,7 @@ namespace OCA\Theming\Tests\Controller; use OCA\Theming\Controller\ThemingController; use OCA\Theming\Util; +use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\Files\IRootFolder; @@ -55,6 +56,8 @@ class ThemingControllerTest extends TestCase { private $rootFolder; /** @var ITempManager */ private $tempManager; + /** @var IAppManager */ + private $appManager; public function setUp() { $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); @@ -66,7 +69,8 @@ class ThemingControllerTest extends TestCase { ->getMock(); $this->l10n = $this->getMockBuilder('OCP\IL10N')->getMock(); $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); - $this->util = new Util($this->config, $this->rootFolder); + $this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock(); + $this->util = new Util($this->config, $this->rootFolder, $this->appManager); $this->timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index a13d4b9476d..54850c8f3c2 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -25,6 +25,7 @@ namespace OCA\Theming\Tests; use OCA\Theming\IconBuilder; use OCA\Theming\ThemingDefaults; use OCA\Theming\Util; +use OCP\App\IAppManager; use OCP\AppFramework\Http\NotFoundResponse; use OCP\Files\IRootFolder; use OCP\IConfig; @@ -42,6 +43,8 @@ class IconBuilderTest extends TestCase { protected $util; /** @var IconBuilder */ protected $iconBuilder; + /** @var IAppManager */ + protected $appManager; protected function setUp() { parent::setUp(); @@ -50,7 +53,8 @@ class IconBuilderTest extends TestCase { $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); $this->themingDefaults = $this->getMockBuilder('OCA\Theming\ThemingDefaults') ->disableOriginalConstructor()->getMock(); - $this->util = new Util($this->config, $this->rootFolder); + $this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock(); + $this->util = new Util($this->config, $this->rootFolder, $this->appManager); $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util); } diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index d82ec8eeb85..83895208fea 100644 --- a/apps/theming/tests/UtilTest.php +++ b/apps/theming/tests/UtilTest.php @@ -23,6 +23,7 @@ namespace OCA\Theming\Tests; use OCA\Theming\Util; +use OCP\App\IAppManager; use OCP\IConfig; use OCP\Files\IRootFolder; use Test\TestCase; @@ -35,12 +36,15 @@ class UtilTest extends TestCase { protected $config; /** @var IRootFolder */ protected $rootFolder; + /** @var IAppManager */ + protected $appManager; protected function setUp() { parent::setUp(); $this->config = $this->getMockBuilder('\OCP\IConfig')->getMock(); $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); - $this->util = new Util($this->config, $this->rootFolder); + $this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock(); + $this->util = new Util($this->config, $this->rootFolder, $this->appManager); } public function testInvertTextColorLight() { @@ -108,6 +112,10 @@ class UtilTest extends TestCase { * @dataProvider dataGetAppIcon */ public function testGetAppIcon($app, $expected) { + $this->appManager->expects($this->once()) + ->method('getAppPath') + ->with($app) + ->willReturn(\OC_App::getAppPath($app)); $icon = $this->util->getAppIcon($app); $this->assertEquals($expected, $icon); } @@ -134,6 +142,12 @@ class UtilTest extends TestCase { * @dataProvider dataGetAppImage */ public function testGetAppImage($app, $image, $expected) { + if($app !== 'core') { + $this->appManager->expects($this->once()) + ->method('getAppPath') + ->with($app) + ->willReturn(\OC_App::getAppPath($app)); + } $this->assertEquals($expected, $this->util->getAppImage($app, $image)); } -- cgit v1.2.3