Improve caching

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 eb65ae54f1ab3964297c73c3ade20a7d12293998..887dba8a68a8af74fe6113b223f90139b4822115 100644 (file)
--- 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 10ba3cacc4363e9d980d204d6f05612e13f64c15..fac8cad430be03595f0303ceec6c64d731e0ba89 100644 (file)
--- 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) === "<svg") {
                        if(substr($appIconContent, 0, 5) !== "<?xml") {
                                $svg = "<?xml version=\"1.0\"?>".$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 b7968d0073fb99e2b5d576f06794a838ac9212ac..e10870685a5e4090f0f88d6df481c1332c580f19 100644 (file)
--- 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 82a937c3b9175075b4b56ea2c02b50f02a1bbf3c..98fa4f1a243715125cf641b2de8d04a13b61b7ce 100644 (file)
--- 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 204c96d86d5a92a5e1e31d5031a8692be8c95254..cd3a90e760aa7963d5531232805423f3994c5fc0 100644 (file)
--- 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 c6755357a1d53d7a9cc2906760671c3ac4361ca5..c6cfa018be54d98d2cb0962be38e042ed6b22565 100644 (file)
--- 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();