From b49ab065b783b3ec041ca395739d747d20e2e187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 11 Sep 2017 15:03:47 +0200 Subject: [PATCH 1/5] Move theming related imagePath logic to ThemingDefaults Signed-off-by: Julius Haertl --- apps/theming/lib/ThemingDefaults.php | 30 ++++++++++++++++++++ apps/theming/tests/ThemingDefaultsTest.php | 32 ++++++++++++++++++++++ lib/private/URLGenerator.php | 15 +++++----- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 8200957edc..0a9d5a6cc1 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -248,6 +248,36 @@ class ThemingDefaults extends \OC_Defaults { return $variables; } + /** + * Check if the image should be replaced by the theming app + * and return the new image location then + * + * @param string $app name of the app + * @param string $image filename of the image + * @return bool|string false if image should not replaced, otherwise the location of the image + */ + public function replaceImagePath($app, $image) { + if($app==='') { + $app = 'core'; + } + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); + + if ($image === 'favicon.ico' && $this->shouldReplaceIcons()) { + return $this->urlGenerator->linkToRoute('theming.Icon.getFavicon', ['app' => $app]) . '?v=' . $cacheBusterValue; + } + if ($image === 'favicon-touch.png' && $this->shouldReplaceIcons()) { + return $this->urlGenerator->linkToRoute('theming.Icon.getTouchIcon', ['app' => $app]) . '?v=' . $cacheBusterValue; + } + if ($image === 'manifest.json') { + $appPath = \OC_App::getAppPath($app); + if ($appPath && file_exists($appPath . '/img/manifest.json')) { + return false; + } + return $this->urlGenerator->linkToRoute('theming.Theming.getManifest') . '?v=' . $cacheBusterValue; + } + return false; + } + /** * Check if Imagemagick is enabled and if SVG is supported * otherwise we can't render custom icons diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index c6d1fec91d..48099e8be0 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -607,4 +607,36 @@ class ThemingDefaultsTest extends TestCase { $this->assertEquals('1234567890', $this->template->getiTunesAppId()); } + public function dataReplaceImagePath() { + return [ + ['core', 'test.png', false], + ['core', 'manifest.json'], + ['core', 'favicon.ico'], + ['core', 'favicon-touch.png'] + ]; + } + + /** @dataProvider dataReplaceImagePath */ + public function testReplaceImagePath($app, $image, $result = 'themingRoute?v=0') { + $cache = $this->createMock(ICache::class); + $cache->expects($this->any()) + ->method('get') + ->with('shouldReplaceIcons') + ->willReturn(true); + $this->cacheFactory->expects($this->any()) + ->method('create') + ->with('theming') + ->willReturn($cache); + $this->config + ->expects($this->any()) + ->method('getAppValue') + ->with('theming', 'cachebuster', '0') + ->willReturn('0'); + $this->urlGenerator + ->expects($this->any()) + ->method('linkToRoute') + ->willReturn('themingRoute'); + $this->assertEquals($result, $this->template->replaceImagePath($app, $image)); + } + } diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 9c73ba4cbc..ee75f8b21b 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -166,6 +166,11 @@ class URLGenerator implements IURLGenerator { // Check if the app is in the app folder $path = ''; $themingEnabled = $this->config->getSystemValue('installed', false) && \OCP\App::isEnabled('theming') && \OC_App::isAppLoaded('theming'); + $themingImagePath = false; + if($themingEnabled) { + $themingImagePath = \OC::$server->getThemingDefaults()->replaceImagePath($app, $image); + } + if (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) { $path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image"; } elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.svg") @@ -181,14 +186,8 @@ class URLGenerator implements IURLGenerator { } elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/core/img/$basename.svg") && file_exists(\OC::$SERVERROOT . "/themes/$theme/core/img/$basename.png")) { $path = \OC::$WEBROOT . "/themes/$theme/core/img/$basename.png"; - } elseif($themingEnabled && $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($themingEnabled && $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; + } elseif($themingEnabled && $themingImagePath) { + $path = $themingImagePath; } elseif ($appPath && file_exists($appPath . "/img/$image")) { $path = \OC_App::getAppWebPath($app) . "/img/$image"; } elseif ($appPath && !file_exists($appPath . "/img/$basename.svg") From 770aae42f6c57e3a273c7b09a91d43a366175234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 11 Sep 2017 15:04:26 +0200 Subject: [PATCH 2/5] Add themed manifest.json to theming app Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 6 +++ .../lib/Controller/ThemingController.php | 35 +++++++++++++ .../Controller/ThemingControllerTest.php | 49 +++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index f4aa2f9316..530e13f53d 100644 --- a/apps/theming/appinfo/routes.php +++ b/apps/theming/appinfo/routes.php @@ -60,6 +60,12 @@ return ['routes' => [ 'url' => '/js/theming', 'verb' => 'GET', ], + [ + 'name' => 'Theming#getManifest', + 'url' => '/manifest/{app}', + 'verb' => 'GET', + 'defaults' => array('app' => 'core') + ], [ 'name' => 'Icon#getFavicon', 'url' => '/favicon/{app}', diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index b409d309f4..06c2c430b7 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -423,4 +423,39 @@ class ThemingController extends Controller { $response->cacheFor(3600); return $response; } + + /** + * @NoCSRFRequired + * @PublicPage + * + * @return Http\JSONResponse + */ + public function getManifest($app) { + $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); + $responseJS = [ + 'name' => $this->themingDefaults->getName(), + 'start_url' => $this->urlGenerator->getBaseUrl(), + 'icons' => + [ + [ + 'src' => $this->urlGenerator->linkToRoute('theming.Icon.getTouchIcon', + ['app' => $app]) . '?v=' . $cacheBusterValue, + 'type'=> 'image/png', + 'sizes'=> '128x128' + ], + [ + 'src' => $this->urlGenerator->linkToRoute('theming.Icon.getFavicon', + ['app' => $app]) . '?v=' . $cacheBusterValue, + 'type' => 'image/svg+xml', + 'sizes' => '16x16' + ] + ], + 'display' => 'standalone' + ]; + $response = new Http\JSONResponse($responseJS); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + $response->cacheFor(3600); + return $response; + } } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 5e6e43ca3c..c03eccb6ee 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -729,4 +729,53 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); @$this->assertEquals($expected, $this->themingController->getJavascript()); } + + public function testGetManifest() { + $this->config + ->expects($this->once()) + ->method('getAppValue') + ->with('theming', 'cachebuster', '0') + ->willReturn('0'); + $this->themingDefaults + ->expects($this->any()) + ->method('getName') + ->willReturn('Nextcloud'); + $this->urlGenerator + ->expects($this->at(0)) + ->method('getBaseUrl') + ->willReturn('localhost'); + $this->urlGenerator + ->expects($this->at(1)) + ->method('linkToRoute') + ->with('theming.Icon.getTouchIcon', ['app' => 'core']) + ->willReturn('touchicon'); + $this->urlGenerator + ->expects($this->at(2)) + ->method('linkToRoute') + ->with('theming.Icon.getFavicon', ['app' => 'core']) + ->willReturn('favicon'); + $response = new Http\JSONResponse([ + 'name' => 'Nextcloud', + 'start_url' => 'localhost', + 'icons' => + [ + [ + 'src' => 'touchicon?v=0', + 'type'=> 'image/png', + 'sizes'=> '128x128' + ], + [ + 'src' => 'favicon?v=0', + 'type' => 'image/svg+xml', + 'sizes' => '16x16' + ] + ], + 'display' => 'standalone' + ]); + $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); + $response->cacheFor(3600); + $this->assertEquals($response, $this->themingController->getManifest('core')); + } + } From 699c64c7504ecd1518827e031eac202da1dcd228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 11 Sep 2017 15:07:45 +0200 Subject: [PATCH 3/5] Add manifest.json to the login page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/templates/layout.guest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/core/templates/layout.guest.php b/core/templates/layout.guest.php index 9a4f9c3719..ce0eccb971 100644 --- a/core/templates/layout.guest.php +++ b/core/templates/layout.guest.php @@ -13,6 +13,7 @@ + From 8391ca8792dace13ba4cf5ebd43d9ea0931b7b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 12 Sep 2017 09:09:45 +0200 Subject: [PATCH 4/5] Use IAppManager instead of private API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/theming/lib/ThemingDefaults.php | 24 +++++++++++++++------- apps/theming/tests/ThemingDefaultsTest.php | 7 ++++++- lib/private/Server.php | 3 ++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 0a9d5a6cc1..5dd22fb632 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -23,6 +23,8 @@ namespace OCA\Theming; +use OCP\App\AppPathNotFoundException; +use OCP\App\IAppManager; use OCP\Files\IAppData; use OCP\ICacheFactory; use OCP\IConfig; @@ -41,6 +43,10 @@ class ThemingDefaults extends \OC_Defaults { private $appData; /** @var ICacheFactory */ private $cacheFactory; + /** @var Util */ + private $util; + /** @var IAppManager */ + private $appManager; /** @var string */ private $name; /** @var string */ @@ -49,8 +55,7 @@ class ThemingDefaults extends \OC_Defaults { private $slogan; /** @var string */ private $color; - /** @var Util */ - private $util; + /** @var string */ private $iTunesAppId; /** @var string */ @@ -68,13 +73,15 @@ class ThemingDefaults extends \OC_Defaults { * @param IAppData $appData * @param ICacheFactory $cacheFactory * @param Util $util + * @param IAppManager $appManager */ public function __construct(IConfig $config, IL10N $l, IURLGenerator $urlGenerator, IAppData $appData, ICacheFactory $cacheFactory, - Util $util + Util $util, + IAppManager $appManager ) { parent::__construct(); $this->config = $config; @@ -83,6 +90,7 @@ class ThemingDefaults extends \OC_Defaults { $this->appData = $appData; $this->cacheFactory = $cacheFactory; $this->util = $util; + $this->appManager = $appManager; $this->name = parent::getName(); $this->url = parent::getBaseUrl(); @@ -269,10 +277,12 @@ class ThemingDefaults extends \OC_Defaults { return $this->urlGenerator->linkToRoute('theming.Icon.getTouchIcon', ['app' => $app]) . '?v=' . $cacheBusterValue; } if ($image === 'manifest.json') { - $appPath = \OC_App::getAppPath($app); - if ($appPath && file_exists($appPath . '/img/manifest.json')) { - return false; - } + try { + $appPath = $this->appManager->getAppPath($app); + if (file_exists($appPath . '/img/manifest.json')) { + return false; + } + } catch (AppPathNotFoundException $e) {} return $this->urlGenerator->linkToRoute('theming.Theming.getManifest') . '?v=' . $cacheBusterValue; } return false; diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index 48099e8be0..b1d86bff43 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -24,6 +24,7 @@ namespace OCA\Theming\Tests; use OCA\Theming\ThemingDefaults; +use OCP\App\IAppManager; use OCP\Files\IAppData; use OCA\Theming\Util; use OCP\Files\NotFoundException; @@ -55,6 +56,8 @@ class ThemingDefaultsTest extends TestCase { private $util; /** @var ICache|\PHPUnit_Framework_MockObject_MockObject */ private $cache; + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + private $appManager; public function setUp() { parent::setUp(); @@ -65,6 +68,7 @@ class ThemingDefaultsTest extends TestCase { $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->cache = $this->createMock(ICache::class); $this->util = $this->createMock(Util::class); + $this->appManager = $this->createMock(IAppManager::class); $this->defaults = new \OC_Defaults(); $this->cacheFactory ->expects($this->any()) @@ -77,7 +81,8 @@ class ThemingDefaultsTest extends TestCase { $this->urlGenerator, $this->appData, $this->cacheFactory, - $this->util + $this->util, + $this->appManager ); } diff --git a/lib/private/Server.php b/lib/private/Server.php index fb0aa76cd1..a20d9ccfc0 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -892,7 +892,8 @@ class Server extends ServerContainer implements IServerContainer { $c->getURLGenerator(), $c->getAppDataDir('theming'), $c->getMemCacheFactory(), - new Util($c->getConfig(), $this->getAppManager(), $this->getAppDataDir('theming')) + new Util($c->getConfig(), $this->getAppManager(), $this->getAppDataDir('theming')), + $this->getAppManager() ); } return new \OC_Defaults(); From 81ef116bfb9e69f361ded5c6dd8a8ae472909fd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 12 Sep 2017 09:47:38 +0200 Subject: [PATCH 5/5] Fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/theming/tests/ThemingDefaultsTest.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index b1d86bff43..abd85a612c 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -623,15 +623,10 @@ class ThemingDefaultsTest extends TestCase { /** @dataProvider dataReplaceImagePath */ public function testReplaceImagePath($app, $image, $result = 'themingRoute?v=0') { - $cache = $this->createMock(ICache::class); - $cache->expects($this->any()) + $this->cache->expects($this->any()) ->method('get') ->with('shouldReplaceIcons') ->willReturn(true); - $this->cacheFactory->expects($this->any()) - ->method('create') - ->with('theming') - ->willReturn($cache); $this->config ->expects($this->any()) ->method('getAppValue')