Theming: Code cleanup and cache buster for mime icons

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

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 IconBuilder $iconBuilder
-	 * @param IRootFolder $rootFolder
 	 */
 	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 $app string app name
-	 * @param $image image file name (svg required)
+	 * @param $image string image file name (svg required)
-	 * @return StreamResponse|DataResponse
+	 * @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
+	 * @param $app string app name
-	 * @return StreamResponse|DataResponse
+	 * @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
+	 * @param $app string app name
-	 * @return StreamResponse|DataResponse
+	 * @return DataDisplayResponse
 	 */
 	public function getTouchIcon($app="core") {
 		if($this->themingDefaults->shouldReplaceIcons()) {
@@ -154,5 +131,4 @@ class IconController extends Controller {
 		return $response;
 	}
 
-
 }

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);

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) {

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 $app string app name
-	 * @param $image relative path to image in app folder
+	 * @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 $svg string content of a svg file
-	 * @param $color color to match
+	 * @param $color string color to match
 	 * @return string
 	 */
 	public function colorizeSvg($svg, $color) {

apps/theming/tests/Controller/IconControllerTest.php

@@ -2,7 +2,7 @@
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
- * @author Julius Haertl <jus@bitgrid.net>
+ * @author Julius Härtl <jus@bitgrid.net>
  *
  * @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 OCA\Theming\Util;
-use \Imagick;
+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
 		);
 

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);

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])
 	}
 
 }

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();

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;