Fixed cached files containing encoded characters

putyourlightson · Sep 9, 2020 · 4305794 · 4305794
1 parent abef8d3
commit 4305794
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Release Notes for Blitz
 
+## 3.6.9 - Unreleased
+### Fixed
+- Fixed a bug in which cached files containing encoded characters would not be cleared correctly ([#255](https://github.com/putyourlightson/craft-blitz/issues/255)).
+
 ## 3.6.8 - 2020-08-26
 ### Added
 - Added `outputComments` as a page specific option ([#243](https://github.com/putyourlightson/craft-blitz/issues/243)).

diff --git a/src/drivers/storage/FileStorage.php b/src/drivers/storage/FileStorage.php
@@ -83,50 +83,34 @@ public function rules(): array
      */
     public function get(SiteUriModel $siteUri): string
     {
-        $value = '';
+        $filePaths = $this->_getFilePaths($siteUri);
 
-        $filePath = $this->_getFilePath($siteUri);
-
-        if (is_file($filePath)) {
-            $value = file_get_contents($filePath);
+        foreach ($filePaths as $filePath) {
+            if (is_file($filePath)) {
+                return file_get_contents($filePath);
+            }
         }
 
-        return $value;
+        return '';
     }
 
     /**
      * @inheritdoc
      */
     public function save(string $value, SiteUriModel $siteUri)
     {
-        $filePath = $this->_getFilePath($siteUri);
+        $filePaths = $this->_getFilePaths($siteUri);
 
-        if (empty($filePath)) {
+        if (empty($filePaths)) {
             return;
         }
 
         try {
-            FileHelper::writeToFile($filePath, $value);
-
-            if ($this->createGzipFiles) {
-                FileHelper::writeToFile($filePath.'.gz', gzencode($value));
-            }
-
-            /**
-             * If the filename includes URL encoded characters, create an extra copy with the characters decoded
-             *
-             * Solves:
-             * https://github.com/putyourlightson/craft-blitz/issues/222
-             * https://github.com/putyourlightson/craft-blitz/issues/224
-             *
-             * Similar issue: https://www.drupal.org/project/boost/issues/1398578
-             * Solution: https://www.drupal.org/files/issues/boost-n1398578-19.patch
-             */
-            if (rawurldecode($filePath) != $filePath) {
-                FileHelper::writeToFile(rawurldecode($filePath), $value);
+            foreach ($filePaths as $filePath) {
+                FileHelper::writeToFile($filePath, $value);
 
                 if ($this->createGzipFiles) {
-                    FileHelper::writeToFile(rawurldecode($filePath).'.gz', gzencode($value));
+                    FileHelper::writeToFile($filePath.'.gz', gzencode($value));
                 }
             }
         }
@@ -144,15 +128,17 @@ public function save(string $value, SiteUriModel $siteUri)
     public function deleteUris(array $siteUris)
     {
         foreach ($siteUris as $siteUri) {
-            $filePath = $this->_getFilePath($siteUri);
+            $filePaths = $this->_getFilePaths($siteUri);
 
-            // Delete file if it exists
-            if (is_file($filePath)) {
-                unlink($filePath);
-            }
+            foreach ($filePaths as $filePath) {
+                // Delete file if it exists
+                if (is_file($filePath)) {
+                    unlink($filePath);
+                }
 
-            if (is_file($filePath.'.gz')) {
-                unlink($filePath.'.gz');
+                if (is_file($filePath.'.gz')) {
+                    unlink($filePath.'.gz');
+                }
             }
         }
     }
@@ -225,13 +211,13 @@ public function getSettingsHtml()
     // =========================================================================
 
     /**
-     * Returns file path from provided site ID and URI.
+     * Returns file paths for the provided site ID and URI.
      *
      * @param SiteUriModel $siteUri
      *
-     * @return string
+     * @return string[]
      */
-    private function _getFilePath(SiteUriModel $siteUri): string
+    private function _getFilePaths(SiteUriModel $siteUri): array
     {
         $sitePath = $this->_getSitePath($siteUri->siteId);
 
@@ -247,10 +233,27 @@ private function _getFilePath(SiteUriModel $siteUri): string
 
         // Ensure that file path is a sub path of the site path
         if (strpos($filePath, $sitePath) === false) {
-            return '';
+            return [];
+        }
+
+        $filePaths = [$filePath];
+
+        /**
+         * If the filename includes URL encoded characters, create a copy with the characters decoded
+         *
+         * Solves:
+         * https://github.com/putyourlightson/craft-blitz/issues/222
+         * https://github.com/putyourlightson/craft-blitz/issues/224
+         * https://github.com/putyourlightson/craft-blitz/issues/252
+         *
+         * Similar issue: https://www.drupal.org/project/boost/issues/1398578
+         * Solution: https://www.drupal.org/files/issues/boost-n1398578-19.patch
+         */
+        if (rawurldecode($filePath) != $filePath) {
+            $filePaths[] = rawurldecode($filePath);
         }
 
-        return $filePath;
+        return $filePaths;
     }
 
     /**

diff --git a/tests/unit/drivers/FileStorageTest.php b/tests/unit/drivers/FileStorageTest.php
@@ -0,0 +1,88 @@
+<?php
+/**
+ * @copyright Copyright (c) PutYourLightsOn
+ */
+
+namespace putyourlightson\blitztests\unit;
+
+use Codeception\Test\Unit;
+use putyourlightson\blitz\Blitz;
+use putyourlightson\blitz\models\SiteUriModel;
+use UnitTester;
+
+/**
+ * @author    PutYourLightsOn
+ * @package   Blitz
+ * @since     3.6.9
+ */
+
+class FileStorageTest extends Unit
+{
+    // Properties
+    // =========================================================================
+
+    /**
+     * @var UnitTester
+     */
+    protected $tester;
+
+    /**
+     * @var SiteUriModel
+     */
+    private $siteUri;
+
+    /**
+     * @var string
+     */
+    private $output = 'xyz';
+
+    // Protected methods
+    // =========================================================================
+
+    protected function _before()
+    {
+        parent::_before();
+
+        Blitz::$plugin->generateCache->options->cachingEnabled = true;
+        Blitz::$plugin->cacheStorage->deleteAll();
+        Blitz::$plugin->flushCache->flushAll();
+
+        $this->siteUri = new SiteUriModel([
+            'siteId' => 1,
+            'uri' => 'möbelträgerfüße',
+        ]);
+
+        Blitz::$plugin->cacheStorage->save($this->output, $this->siteUri);
+    }
+
+    // Public methods
+    // =========================================================================
+
+    public function testSave()
+    {
+        $value = Blitz::$plugin->cacheStorage->get($this->siteUri);
+        $this->assertStringContainsString($this->output, $value);
+    }
+
+    public function testSaveDecoded()
+    {
+        $this->siteUri->uri = rawurldecode($this->siteUri->uri);
+        $value = Blitz::$plugin->cacheStorage->get($this->siteUri);
+        $this->assertStringContainsString($this->output, $value);
+    }
+
+    public function testDelete()
+    {
+        Blitz::$plugin->cacheStorage->deleteUris([$this->siteUri]);
+        $value = Blitz::$plugin->cacheStorage->get($this->siteUri);
+        $this->assertEmpty($value);
+    }
+
+    public function testDeleteDecoded()
+    {
+        $this->siteUri->uri = rawurldecode($this->siteUri->uri);
+        Blitz::$plugin->cacheStorage->deleteUris([$this->siteUri]);
+        $value = Blitz::$plugin->cacheStorage->get($this->siteUri);
+        $this->assertEmpty($value);
+    }
+}
diff --git a/tests/unit/services/RefreshCacheTest.php b/tests/unit/services/RefreshCacheTest.php
@@ -349,4 +349,22 @@ public function testRefreshSourceTag()
         // Assert that the cached value is a blank string
         $this->assertEquals('', Blitz::$plugin->cacheStorage->get($this->siteUri));
     }
+
+    public function testRefreshCacheTags()
+    {
+        // Add tag and save
+        $tag = 'abc';
+        Blitz::$plugin->generateCache->options->tags($tag);
+        Blitz::$plugin->generateCache->save($this->output, $this->siteUri);
+
+        // Assert that the output (which may also contain a timestamp) contains the cached value
+        $this->assertStringContainsString($this->output, Blitz::$plugin->cacheStorage->get($this->siteUri));
+
+        Blitz::$plugin->refreshCache->refreshCacheTags([$tag]);
+
+        Craft::$app->runAction('queue/run');
+
+        // Assert that the cached value is a blank string
+        $this->assertEquals('', Blitz::$plugin->cacheStorage->get($this->siteUri));
+    }
 }