Check if dynamic icons can be used

Signed-off-by: Julius Haertl <jus@bitgrid.net>

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 = $iconBuilder;
-			$this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util);
+		//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;
 	}
 
 

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() {
-
+		if(!extension_loaded('imagick')) {
-		$this->util->expects($this->once())
+			$this->markTestSkipped('Imagemagick is required for dynamic icon generation.');
-			->method('getAppIcon')
+		}
-			->with('core')
+		$checkImagick = new \Imagick();
-			->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg");
+		if (count($checkImagick->queryFormats('SVG')) < 1) {
-
+			$this->markTestSkipped('No SVG provider present.');
-		$favicon = $this->iconController->getFavicon();
+		}
+		$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() {
-
+		if(!extension_loaded('imagick')) {
-		$this->util->expects($this->once())
+			$this->markTestSkipped('Imagemagick is required for dynamic icon generation.');
-			->method('getAppIcon')
+		}
-			->with('core')
+		$checkImagick = new \Imagick();
-			->willReturn(\OC::$SERVERROOT . "/core/img/logo.svg");
+		if (count($checkImagick->queryFormats('SVG')) < 1) {
-		$favicon = $this->iconController->getTouchIcon();
+			$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);
+	}
 
 }

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;