From 50a9beb4bbcfc7baf2a832fef2f6fdf601d8219e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 12:10:36 +0100 Subject: [PATCH 1/5] Move base url from global cache prefix to frontend related ones 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 | 2 +- lib/private/Server.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 94abb4e288..79995f52ef 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -231,7 +231,7 @@ class ThemingDefaults extends \OC_Defaults { * @return array scss variables to overwrite */ public function getScssVariables() { - $cache = $this->cacheFactory->createDistributed('theming'); + $cache = $this->cacheFactory->createDistributed('theming-' . $this->urlGenerator->getBaseUrl()); if ($value = $cache->get('getScssVariables')) { return $value; } diff --git a/lib/private/Server.php b/lib/private/Server.php index d586bab15b..ed4eb7eb39 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -498,7 +498,7 @@ class Server extends ServerContainer implements IServerContainer { $version = implode(',', $v); $instanceId = \OC_Util::getInstanceId(); $path = \OC::$SERVERROOT; - $prefix = md5($instanceId . '-' . $version . '-' . $path . '-' . $urlGenerator->getBaseUrl()); + $prefix = md5($instanceId . '-' . $version . '-' . $path); return new \OC\Memcache\Factory($prefix, $c->getLogger(), $config->getSystemValue('memcache.local', null), $config->getSystemValue('memcache.distributed', null), @@ -965,7 +965,7 @@ class Server extends ServerContainer implements IServerContainer { $c->getConfig(), $c->getThemingDefaults(), \OC::$SERVERROOT, - $cacheFactory->createDistributed('SCSS') + $cacheFactory->createDistributed('SCSS-' . md5($this->getURLGenerator()->getBaseUrl())) ); }); $this->registerService(JSCombiner::class, function (Server $c) { @@ -974,7 +974,7 @@ class Server extends ServerContainer implements IServerContainer { return new JSCombiner( $c->getAppDataDir('js'), $c->getURLGenerator(), - $cacheFactory->createDistributed('JS'), + $cacheFactory->createDistributed('JS-' . md5($this->getURLGenerator()->getBaseUrl())), $c->getSystemConfig(), $c->getLogger() ); From f652c28b2105792908dc73a414ff21cc135a906d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 12:11:03 +0100 Subject: [PATCH 2/5] Use JSCombiner from server container when searching for JS files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/TemplateLayout.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index 0f2eb48bef..c378396c72 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -306,13 +306,7 @@ class TemplateLayout extends \OC_Template { $theme, array( \OC::$SERVERROOT => \OC::$WEBROOT ), array( \OC::$SERVERROOT => \OC::$WEBROOT ), - new JSCombiner( - \OC::$server->getAppDataDir('js'), - \OC::$server->getURLGenerator(), - \OC::$server->getMemCacheFactory()->createDistributed('JS'), - \OC::$server->getSystemConfig(), - \OC::$server->getLogger() - ) + \OC::$server->query(JSCombiner::class) ); $locator->find($scripts); return $locator->getResources(); From 63e261b4446996d9ba42f09b1c1b17c98f34390f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 14:13:36 +0100 Subject: [PATCH 3/5] Actually clear cache values for all base urls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Server.php | 4 ++-- lib/private/Template/JSCombiner.php | 13 +++++++++---- lib/private/Template/SCSSCacher.php | 13 +++++++++---- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/private/Server.php b/lib/private/Server.php index ed4eb7eb39..e0bd21b035 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -965,7 +965,7 @@ class Server extends ServerContainer implements IServerContainer { $c->getConfig(), $c->getThemingDefaults(), \OC::$SERVERROOT, - $cacheFactory->createDistributed('SCSS-' . md5($this->getURLGenerator()->getBaseUrl())) + $this->getMemCacheFactory() ); }); $this->registerService(JSCombiner::class, function (Server $c) { @@ -974,7 +974,7 @@ class Server extends ServerContainer implements IServerContainer { return new JSCombiner( $c->getAppDataDir('js'), $c->getURLGenerator(), - $cacheFactory->createDistributed('JS-' . md5($this->getURLGenerator()->getBaseUrl())), + $this->getMemCacheFactory(), $c->getSystemConfig(), $c->getLogger() ); diff --git a/lib/private/Template/JSCombiner.php b/lib/private/Template/JSCombiner.php index bc548c22fd..9368375389 100644 --- a/lib/private/Template/JSCombiner.php +++ b/lib/private/Template/JSCombiner.php @@ -30,6 +30,7 @@ use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\ICacheFactory; use OCP\ILogger; use OCP\IURLGenerator; @@ -50,21 +51,25 @@ class JSCombiner { /** @var ILogger */ protected $logger; + /** @var ICacheFactory */ + private $cacheFactory; + /** * @param IAppData $appData * @param IURLGenerator $urlGenerator - * @param ICache $depsCache + * @param ICacheFactory $cacheFactory * @param SystemConfig $config * @param ILogger $logger */ public function __construct(IAppData $appData, IURLGenerator $urlGenerator, - ICache $depsCache, + ICacheFactory $cacheFactory, SystemConfig $config, ILogger $logger) { $this->appData = $appData; $this->urlGenerator = $urlGenerator; - $this->depsCache = $depsCache; + $this->cacheFactory = $cacheFactory; + $this->depsCache = $this->cacheFactory->createDistributed('JS-' . md5($this->urlGenerator->getBaseUrl())); $this->config = $config; $this->logger = $logger; } @@ -243,7 +248,7 @@ class JSCombiner { * @throws NotFoundException */ public function resetCache() { - $this->depsCache->clear(); + $this->cacheFactory->createDistributed('JS-')->clear(); $appDirectory = $this->appData->getDirectoryListing(); foreach ($appDirectory as $folder) { foreach ($folder->getDirectoryListing() as $file) { diff --git a/lib/private/Template/SCSSCacher.php b/lib/private/Template/SCSSCacher.php index 433d5e5d0b..0f5be9643b 100644 --- a/lib/private/Template/SCSSCacher.php +++ b/lib/private/Template/SCSSCacher.php @@ -39,6 +39,7 @@ use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\ILogger; use OCP\IURLGenerator; @@ -69,6 +70,9 @@ class SCSSCacher { /** @var null|string */ private $injectedVariables; + /** @var ICacheFactory */ + private $cacheFactory; + /** * @param ILogger $logger * @param Factory $appDataFactory @@ -76,7 +80,7 @@ class SCSSCacher { * @param IConfig $config * @param \OC_Defaults $defaults * @param string $serverRoot - * @param ICache $depsCache + * @param ICacheFactory $cacheFactory */ public function __construct(ILogger $logger, Factory $appDataFactory, @@ -84,14 +88,15 @@ class SCSSCacher { IConfig $config, \OC_Defaults $defaults, $serverRoot, - ICache $depsCache) { + ICacheFactory $cacheFactory) { $this->logger = $logger; $this->appData = $appDataFactory->get('css'); $this->urlGenerator = $urlGenerator; $this->config = $config; $this->defaults = $defaults; $this->serverRoot = $serverRoot; - $this->depsCache = $depsCache; + $this->cacheFactory = $cacheFactory; + $this->depsCache = $cacheFactory->createDistributed('SCSS-' . md5($this->urlGenerator->getBaseUrl())); } /** @@ -263,7 +268,7 @@ class SCSSCacher { */ public function resetCache() { $this->injectedVariables = null; - $this->depsCache->clear(); + $this->cacheFactory->createDistributed('SCSS-')->clear(); $appDirectory = $this->appData->getDirectoryListing(); foreach ($appDirectory as $folder) { foreach ($folder->getDirectoryListing() as $file) { From 95366bbd1fe5fc93d767721233b2a38a455e6f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 14:51:43 +0100 Subject: [PATCH 4/5] Fix tests to use ICacheFactory 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 | 4 ++-- apps/theming/tests/ThemingDefaultsTest.php | 2 +- tests/lib/Template/CSSResourceLocatorTest.php | 10 +++++----- tests/lib/Template/JSCombinerTest.php | 16 ++++++++++++++-- tests/lib/Template/JSResourceLocatorTest.php | 10 +++++----- tests/lib/Template/SCSSCacherTest.php | 15 +++++++++++++-- 6 files changed, 40 insertions(+), 17 deletions(-) diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 79995f52ef..986de4b1e8 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -298,7 +298,7 @@ class ThemingDefaults extends \OC_Defaults { * @return bool */ public function shouldReplaceIcons() { - $cache = $this->cacheFactory->createDistributed('theming'); + $cache = $this->cacheFactory->createDistributed('theming-'); if($value = $cache->get('shouldReplaceIcons')) { return (bool)$value; } @@ -320,7 +320,7 @@ class ThemingDefaults extends \OC_Defaults { private function increaseCacheBuster() { $cacheBusterKey = $this->config->getAppValue('theming', 'cachebuster', '0'); $this->config->setAppValue('theming', 'cachebuster', (int)$cacheBusterKey+1); - $this->cacheFactory->createDistributed('theming')->clear('getScssVariables'); + $this->cacheFactory->createDistributed('theming-')->clear('getScssVariables'); } /** diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index 36fe9d17b7..2e1ad44b7a 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -79,7 +79,7 @@ class ThemingDefaultsTest extends TestCase { $this->cacheFactory ->expects($this->any()) ->method('createDistributed') - ->with('theming') + ->with('theming-') ->willReturn($this->cache); $this->template = new ThemingDefaults( $this->config, diff --git a/tests/lib/Template/CSSResourceLocatorTest.php b/tests/lib/Template/CSSResourceLocatorTest.php index ef3ef4f42c..5e2c3705ef 100644 --- a/tests/lib/Template/CSSResourceLocatorTest.php +++ b/tests/lib/Template/CSSResourceLocatorTest.php @@ -25,11 +25,11 @@ namespace Test\Template; use OC\Files\AppData\Factory; use OCP\Files\IAppData; +use OCP\ICacheFactory; use OCP\ILogger; use OCP\IURLGenerator; use OCP\IConfig; use OCA\Theming\ThemingDefaults; -use OCP\ICache; use OC\Template\SCSSCacher; use OC\Template\CSSResourceLocator; @@ -42,8 +42,8 @@ class CSSResourceLocatorTest extends \Test\TestCase { protected $config; /** @var ThemingDefaults|\PHPUnit_Framework_MockObject_MockObject */ protected $themingDefaults; - /** @var ICache|\PHPUnit_Framework_MockObject_MockObject */ - protected $depsCache; + /** @var ICacheFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $cacheFactory; /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ protected $logger; @@ -54,7 +54,7 @@ class CSSResourceLocatorTest extends \Test\TestCase { $this->appData = $this->createMock(IAppData::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->config = $this->createMock(IConfig::class); - $this->depsCache = $this->createMock(ICache::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->themingDefaults = $this->createMock(ThemingDefaults::class); } @@ -69,7 +69,7 @@ class CSSResourceLocatorTest extends \Test\TestCase { $this->config, $this->themingDefaults, \OC::$SERVERROOT, - $this->depsCache + $this->cacheFactory ); return new CSSResourceLocator( $this->logger, diff --git a/tests/lib/Template/JSCombinerTest.php b/tests/lib/Template/JSCombinerTest.php index bec88801d6..6eaedccbe8 100644 --- a/tests/lib/Template/JSCombinerTest.php +++ b/tests/lib/Template/JSCombinerTest.php @@ -31,6 +31,7 @@ use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICache; +use OCP\ICacheFactory; use OCP\ILogger; use OCP\IURLGenerator; @@ -47,6 +48,8 @@ class JSCombinerTest extends \Test\TestCase { protected $jsCombiner; /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ protected $logger; + /** @var ICacheFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $cacheFactory; protected function setUp() { parent::setUp(); @@ -54,15 +57,20 @@ class JSCombinerTest extends \Test\TestCase { $this->appData = $this->createMock(IAppData::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->config = $this->createMock(SystemConfig::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->depsCache = $this->createMock(ICache::class); + $this->cacheFactory->expects($this->at(0)) + ->method('createDistributed') + ->willReturn($this->depsCache); $this->logger = $this->createMock(ILogger::class); $this->jsCombiner = new JSCombiner( $this->appData, $this->urlGenerator, - $this->depsCache, + $this->cacheFactory, $this->config, $this->logger ); + } public function testProcessDebugMode() { @@ -539,7 +547,11 @@ var b = \'world\'; ->method('getDirectoryListing') ->willReturn([$file]); - $this->depsCache->expects($this->once()) + $cache = $this->createMock(ICache::class); + $this->cacheFactory->expects($this->once()) + ->method('createDistributed') + ->willReturn($cache); + $cache->expects($this->once()) ->method('clear') ->with(''); $this->appData->expects($this->once()) diff --git a/tests/lib/Template/JSResourceLocatorTest.php b/tests/lib/Template/JSResourceLocatorTest.php index 6841ee2fee..f5dff62729 100644 --- a/tests/lib/Template/JSResourceLocatorTest.php +++ b/tests/lib/Template/JSResourceLocatorTest.php @@ -25,8 +25,8 @@ namespace Test\Template; use OC\Template\JSCombiner; use OCP\Files\IAppData; +use OCP\ICacheFactory; use OCP\IURLGenerator; -use OCP\ICache; use OC\SystemConfig; use OCP\ILogger; use OC\Template\JSResourceLocator; @@ -38,8 +38,8 @@ class JSResourceLocatorTest extends \Test\TestCase { protected $urlGenerator; /** @var SystemConfig|\PHPUnit_Framework_MockObject_MockObject */ protected $config; - /** @var ICache|\PHPUnit_Framework_MockObject_MockObject */ - protected $depsCache; + /** @var ICacheFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $cacheFactory; /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ protected $logger; @@ -49,7 +49,7 @@ class JSResourceLocatorTest extends \Test\TestCase { $this->appData = $this->createMock(IAppData::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->config = $this->createMock(SystemConfig::class); - $this->depsCache = $this->createMock(ICache::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->logger = $this->createMock(ILogger::class); } @@ -57,7 +57,7 @@ class JSResourceLocatorTest extends \Test\TestCase { $jsCombiner = new JSCombiner( $this->appData, $this->urlGenerator, - $this->depsCache, + $this->cacheFactory, $this->config, $this->logger ); diff --git a/tests/lib/Template/SCSSCacherTest.php b/tests/lib/Template/SCSSCacherTest.php index dfaeb80357..169ee26802 100644 --- a/tests/lib/Template/SCSSCacherTest.php +++ b/tests/lib/Template/SCSSCacherTest.php @@ -31,6 +31,7 @@ use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\ILogger; use OCP\IURLGenerator; @@ -50,6 +51,8 @@ class SCSSCacherTest extends \Test\TestCase { protected $scssCacher; /** @var ICache|\PHPUnit_Framework_MockObject_MockObject */ protected $depsCache; + /** @var ICacheFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $cacheFactory; protected function setUp() { parent::setUp(); @@ -60,7 +63,11 @@ class SCSSCacherTest extends \Test\TestCase { $factory->method('get')->with('css')->willReturn($this->appData); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->config = $this->createMock(IConfig::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->depsCache = $this->createMock(ICache::class); + $this->cacheFactory->expects($this->at(0)) + ->method('createDistributed') + ->willReturn($this->depsCache); $this->themingDefaults = $this->createMock(ThemingDefaults::class); $this->scssCacher = new SCSSCacher( $this->logger, @@ -69,7 +76,7 @@ class SCSSCacherTest extends \Test\TestCase { $this->config, $this->themingDefaults, \OC::$SERVERROOT, - $this->depsCache + $this->cacheFactory ); $this->themingDefaults->expects($this->any())->method('getScssVariables')->willReturn([]); @@ -459,7 +466,11 @@ class SCSSCacherTest extends \Test\TestCase { ->method('getDirectoryListing') ->willReturn([$file]); - $this->depsCache->expects($this->once()) + $cache = $this->createMock(ICache::class); + $this->cacheFactory->expects($this->once()) + ->method('createDistributed') + ->willReturn($cache); + $cache->expects($this->once()) ->method('clear') ->with(''); $this->appData->expects($this->once()) From 573b94076103b2f3740ee243a2089279600523d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 7 Mar 2018 15:07:03 +0100 Subject: [PATCH 5/5] Use cache prefix for theming as well 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 986de4b1e8..688878bb50 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -298,7 +298,7 @@ class ThemingDefaults extends \OC_Defaults { * @return bool */ public function shouldReplaceIcons() { - $cache = $this->cacheFactory->createDistributed('theming-'); + $cache = $this->cacheFactory->createDistributed('theming-' . $this->urlGenerator->getBaseUrl()); if($value = $cache->get('shouldReplaceIcons')) { return (bool)$value; }