From 237034818dd3425116ef3db04dabbc95a5d10125 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 30 Aug 2016 09:03:42 +0200 Subject: [PATCH] Check if dynamic icons can be used Signed-off-by: Julius Haertl --- .../theming/lib/Controller/IconController.php | 34 +++---- .../tests/Controller/IconControllerTest.php | 97 +++++++++++++++---- lib/private/URLGenerator.php | 4 +- 3 files changed, 94 insertions(+), 41 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 78d41d621a..6f97fdcdab 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 3443be6071..8c916c6cfe 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') - ->with('core') - ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); - - $favicon = $this->iconController->getFavicon(); + 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($expectedIcon); + $favicon = $this->iconController->getFavicon(); + $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') - ->with('core') - ->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg"); - $favicon = $this->iconController->getTouchIcon(); + 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($expectedIcon); + $favicon = $this->iconController->getTouchIcon(); $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 8290fcb133..bdf7bdafae 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;