Theming: Code cleanup and cache buster for mime icons

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

diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php
index 6f97fdcdaba248ace182e93975c0ca400ebd6f77..f2355fe3f825e7e719c8c2e71be02ae1d5f31cb2 100644 (file)
--- 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 7ba4feb62ddba39745f51d9386995d46595d9d97..c908f0e5782b38282191f8d5fd12dcb1fbe779d8 100644 (file)
--- 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 b61e12d9236f23f05f608f6ba02460377386d2ac..edd7602a2e429411008c59fefd1cdd5736c1a80a 100644 (file)
--- 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 8aa5f50ede2ab295f69d6c04bcc76f0e9aac5bb8..84c631092a8eb6ef30f22e779fc0f7ef53f5b3ed 100644 (file)
--- 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 8c916c6cfe6e2d9bc5ad00547949d5a8501bd70d..09cb41088de8637ed72f63c144ae0f2431a2662a 100644 (file)
--- 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 <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 \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 b065046fdba2ce2c7ddcc3b41dd1d51b1675cd91..b7bce2d6ec01c3f8adb8fb6def0b73b26314572e 100644 (file)
--- 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 ffabb31df7956d9b7aa7f4b71739206ae385553e..030543672102bec6cc10aa1dc37d98c708707249 100644 (file)
--- 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 499a3ffc45d6186d3e037daf2981d535cb4e9438..9a53858598c3bebadef80d5d7060e022c469b224 100644 (file)
--- 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 3ce4ceccbb0b6bd1a9e4ec11748e4cc8ac4055e3..8920fe09a7eeed60c19ffc9066f936f0274d8db9 100644 (file)
--- 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;