From 9e28a3ba120356b03063e44445a9401c3aa205f3 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Tue, 30 Aug 2016 11:51:48 +0200 Subject: [PATCH] Theming: Code cleanup and cache buster for mime icons Signed-off-by: Julius Haertl --- .../theming/lib/Controller/IconController.php | 40 ++++--------------- .../lib/Controller/ThemingController.php | 2 + apps/theming/lib/IconBuilder.php | 6 +-- apps/theming/lib/Util.php | 14 +++---- .../tests/Controller/IconControllerTest.php | 27 +++---------- .../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 6f97fdcdab..f2355fe3f8 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 7ba4feb62d..c908f0e578 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 b61e12d923..edd7602a2e 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 8aa5f50ede..84c631092a 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 8c916c6cfe..09cb41088d 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 b065046fdb..b7bce2d6ec 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 ffabb31df7..0305436721 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 499a3ffc45..9a53858598 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 3ce4ceccbb..8920fe09a7 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;