From 2e8dd218157123cdb7f1741980e12dc22b95f320 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 14 Oct 2016 14:57:58 +0200 Subject: [PATCH] Improve caching Signed-off-by: Julius Haertl --- .../theming/lib/Controller/IconController.php | 47 ++++++++++++------- apps/theming/lib/IconBuilder.php | 4 +- apps/theming/lib/ThemingDefaults.php | 17 +++++-- .../tests/Controller/IconControllerTest.php | 25 ++++++---- apps/theming/tests/ThemingDefaultsTest.php | 7 ++- lib/private/Server.php | 3 +- 6 files changed, 71 insertions(+), 32 deletions(-) diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index eb65ae54f1..887dba8a68 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -96,7 +96,10 @@ class IconController extends Controller { } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $response->addHeader('Pragma', 'cache'); return $response; } @@ -111,19 +114,24 @@ class IconController extends Controller { * @return FileDisplayResponse|DataDisplayResponse */ public function getFavicon($app = "core") { - $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); - if($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { - $icon = $this->iconBuilder->getFavicon($app); - $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); - } if ($this->themingDefaults->shouldReplaceIcons()) { + $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app); + if($iconFile === null) { + $icon = $this->iconBuilder->getFavicon($app); + $iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon); + } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(0); + $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); return $response; } @@ -137,19 +145,24 @@ class IconController extends Controller { * @return FileDisplayResponse|DataDisplayResponse */ public function getTouchIcon($app = "core") { - $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); - if ($iconFile === null && $this->themingDefaults->shouldReplaceIcons()) { - $icon = $this->iconBuilder->getTouchIcon($app); - $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); - } if ($this->themingDefaults->shouldReplaceIcons()) { + $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app); + if ($iconFile === null) { + $icon = $this->iconBuilder->getTouchIcon($app); + $iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon); + } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC2822)); + $response->addHeader('Pragma', 'cache'); } else { $response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); + $response->cacheFor(0); + $response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); } - $response->cacheFor(86400); - $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $response->addHeader('Pragma', 'cache'); return $response; } } \ No newline at end of file diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 10ba3cacc4..fac8cad430 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -94,6 +94,8 @@ class IconBuilder { if($mime === "image/svg+xml" || substr($appIconContent, 0, 4) === "".$appIconContent; + } else { + $svg = $appIconContent; } $tmp = new Imagick(); $tmp->readImageBlob($svg); @@ -147,4 +149,4 @@ class IconBuilder { return $svg; } -} \ No newline at end of file +} diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index b7968d0073..e10870685a 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -23,6 +23,7 @@ namespace OCA\Theming; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -38,6 +39,8 @@ class ThemingDefaults extends \OC_Defaults { private $urlGenerator; /** @var IRootFolder */ private $rootFolder; + /** @var ICacheFactory */ + private $cacheFactory; /** @var string */ private $name; /** @var string */ @@ -60,13 +63,15 @@ class ThemingDefaults extends \OC_Defaults { IL10N $l, IURLGenerator $urlGenerator, \OC_Defaults $defaults, - IRootFolder $rootFolder + IRootFolder $rootFolder, + ICacheFactory $cacheFactory ) { parent::__construct(); $this->config = $config; $this->l = $l; $this->urlGenerator = $urlGenerator; $this->rootFolder = $rootFolder; + $this->cacheFactory = $cacheFactory; $this->name = $defaults->getName(); $this->url = $defaults->getBaseUrl(); @@ -151,14 +156,20 @@ class ThemingDefaults extends \OC_Defaults { * @return bool */ public function shouldReplaceIcons() { + $cache = $this->cacheFactory->create('theming'); + if($cache->hasKey('shouldReplaceIcons')) { + return (bool)$cache->get('shouldReplaceIcons'); + } + $value = false; if(extension_loaded('imagick')) { $checkImagick = new \Imagick(); if (count($checkImagick->queryFormats('SVG')) >= 1) { - return true; + $value = true; } $checkImagick->clear(); } - return false; + $cache->set('shouldReplaceIcons', $value); + return $value; } /** diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 82a937c3b9..98fa4f1a24 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -103,7 +103,10 @@ class IconControllerTest extends TestCase { ->willReturn($file); $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->iconController->getThemedIcon('core', 'filetypes/folder.svg')); @@ -133,7 +136,10 @@ class IconControllerTest extends TestCase { $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -143,9 +149,8 @@ class IconControllerTest extends TestCase { ->method('shouldReplaceIcons') ->willReturn(false); $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $expected->addHeader('Pragma', 'cache'); + $expected->cacheFor(0); + $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -172,7 +177,10 @@ class IconControllerTest extends TestCase { $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $expected->addHeader('Expires', $expires->format(\DateTime::RFC2822)); $expected->addHeader('Pragma', 'cache'); $this->assertEquals($expected, $this->iconController->getTouchIcon()); } @@ -182,9 +190,8 @@ class IconControllerTest extends TestCase { ->method('shouldReplaceIcons') ->willReturn(false); $expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND); - $expected->cacheFor(86400); - $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); - $expected->addHeader('Pragma', 'cache'); + $expected->cacheFor(0); + $expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT'))); $this->assertEquals($expected, $this->iconController->getTouchIcon()); } diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index 204c96d86d..cd3a90e760 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\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -43,6 +44,8 @@ class ThemingDefaultsTest extends TestCase { private $template; /** @var IRootFolder */ private $rootFolder; + /** @var ICacheFactory */ + private $cacheFactory; public function setUp() { parent::setUp(); @@ -52,6 +55,7 @@ class ThemingDefaultsTest extends TestCase { $this->rootFolder = $this->getMockBuilder(IRootFolder::class) ->disableOriginalConstructor() ->getMock(); + $this->cacheFactory = $this->getMockBuilder(ICacheFactory::class)->getMock(); $this->defaults = $this->getMockBuilder(\OC_Defaults::class) ->disableOriginalConstructor() ->getMock(); @@ -76,7 +80,8 @@ class ThemingDefaultsTest extends TestCase { $this->l10n, $this->urlGenerator, $this->defaults, - $this->rootFolder + $this->rootFolder, + $this->cacheFactory ); } diff --git a/lib/private/Server.php b/lib/private/Server.php index c6755357a1..c6cfa018be 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -703,7 +703,8 @@ class Server extends ServerContainer implements IServerContainer { $c->getL10N('theming'), $c->getURLGenerator(), new \OC_Defaults(), - $c->getLazyRootFolder() + $c->getLazyRootFolder(), + $c->getMemCacheFactory() ); } return new \OC_Defaults();