fix: file cache tweaks (#3470)

* fix: improve file cache

* fix(filecache): log when unserialize fails

caches/FileCache.php

@@ -8,23 +8,36 @@ class FileCache implements CacheInterface
 
     public function __construct(array $config = [])
     {
-        $this->config = $config;
+        $default = [
+            'path' => null,
+            'enable_purge' => true,
+        ];
+        $this->config = array_merge($default, $config);
+        if (!$this->config['path']) {
+            throw new \Exception('The FileCache needs a path value');
+        }
+        // Normalize with a single trailing slash
+        $this->config['path'] = rtrim($this->config['path'], '/') . '/';
+    }
 
-        if (!is_dir($this->config['path'])) {
+    public function getConfig()
-            throw new \Exception('The cache path does not exists. You probably want: mkdir cache && chown www-data:www-data cache');
+    {
-        }
+        return $this->config;
-        if (!is_writable($this->config['path'])) {
-            throw new \Exception('The cache path is not writeable. You probably want: chown www-data:www-data cache');
-        }
     }
 
     public function loadData()
     {
-        if (file_exists($this->getCacheFile())) {
+        if (!file_exists($this->getCacheFile())) {
-            return unserialize(file_get_contents($this->getCacheFile()));
-        }
             return null;
         }
+        $data = unserialize(file_get_contents($this->getCacheFile()));
+        if ($data === false) {
+            // Intentionally not throwing an exception
+            Logger::warning(sprintf('Failed to unserialize: %s', $this->getCacheFile()));
+            return null;
+        }
+        return $data;
+    }
 
     public function saveData($data)
     {

lib/CacheFactory.php

@@ -38,11 +38,18 @@ class CacheFactory
             case NullCache::class:
                 return new NullCache();
             case FileCache::class:
-                return new FileCache([
+                $fileCacheConfig = [
-                    // Intentionally checking for "truthy" value
+                    // Intentionally checking for truthy value because the historic default value is the empty string
                     'path' => Configuration::getConfig('FileCache', 'path') ?: PATH_CACHE,
                     'enable_purge' => Configuration::getConfig('FileCache', 'enable_purge'),
-                ]);
+                ];
+                if (!is_dir($fileCacheConfig['path'])) {
+                    throw new \Exception(sprintf('The FileCache path does not exists: %s', $fileCacheConfig['path']));
+                }
+                if (!is_writable($fileCacheConfig['path'])) {
+                    throw new \Exception(sprintf('The FileCache path is not writable: %s', $fileCacheConfig['path']));
+                }
+                return new FileCache($fileCacheConfig);
             case SQLiteCache::class:
                 return new SQLiteCache();
             case MemcachedCache::class:

tests/CacheTest.php

@@ -6,6 +6,18 @@ use PHPUnit\Framework\TestCase;
 
 class CacheTest extends TestCase
 {
+    public function testConfig()
+    {
+        $sut = new \FileCache(['path' => '/tmp/']);
+        $this->assertSame(['path' => '/tmp/', 'enable_purge' => true], $sut->getConfig());
+
+        $sut = new \FileCache(['path' => '/', 'enable_purge' => false]);
+        $this->assertSame(['path' => '/', 'enable_purge' => false], $sut->getConfig());
+
+        $sut = new \FileCache(['path' => '/tmp', 'enable_purge' => true]);
+        $this->assertSame(['path' => '/tmp/', 'enable_purge' => true], $sut->getConfig());
+    }
+
     public function testFileCache()
     {
         $temporaryFolder = sprintf('%s/rss_bridge_%s/', sys_get_temp_dir(), create_random_string());