Don't create cache directory in constructor + Fix nested directory permissions

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## 3.1.1 under development
 
+- Enh #88: Don't create cache directory on `FileCache` initialization (@vjik)
+- Bug #88: Set correct permissions for nested directories (@vjik)
 - Bug #85: Clear stat cache in `FileCache::set()` (@samdark)
 
 ## 3.1.0 October 09, 2023

src/FileCache.php

@@ -12,7 +12,6 @@
 use function array_keys;
 use function array_map;
 use function closedir;
-use function dirname;
 use function error_get_last;
 use function filemtime;
 use function fileowner;
@@ -89,11 +88,8 @@
      */
     public function __construct(
         private string $cachePath,
         private int $directoryMode = 0775,
     ) {
-        if (!$this->createDirectoryIfNotExists($cachePath)) {
-            throw new CacheException("Failed to create cache directory \"$cachePath\".");
-        }
     }
 
     public function get(string $key, mixed $default = null): mixed
@@ -107,7 +103,7 @@
 
         flock($filePointer, LOCK_SH);
         $value = stream_get_contents($filePointer);
         flock($filePointer, LOCK_UN);
         fclose($filePointer);
 
         return unserialize($value);
@@ -119,23 +115,16 @@
         $this->gc();
         $expiration = $this->ttlToExpiration($ttl);
 
         if ($expiration <= self::EXPIRATION_EXPIRED) {
             return $this->delete($key);
         }
 
-        $file = $this->getCacheFile($key);
-        $cacheDirectory = dirname($file);
-
-        if (!is_dir($this->cachePath)
-            || $this->directoryLevel > 0 && !$this->createDirectoryIfNotExists($cacheDirectory)
-        ) {
-            throw new CacheException("Failed to create cache directory \"$cacheDirectory\".");
-        }
+        $file = $this->getCacheFile($key, ensureDirectory: true);
 
         // If ownership differs, the touch call will fail, so we try to
         // rebuild the file from scratch by deleting it first
         // https://github.com/yiisoft/yii2/pull/16120
         if (function_exists('posix_geteuid') && is_file($file) && fileowner($file) !== posix_geteuid()) {
             @unlink($file);
         }
 
@@ -325,25 +314,27 @@
     }
 
     /**
-     * Ensures that the directory is created.
+     * Ensures that the directory is existing.
      *
      * @param string $path The path to the directory.
-     *
-     * @return bool Whether the directory was created.
      */
-    private function createDirectoryIfNotExists(string $path): bool
+    private function ensureDirectory(string $path): void
     {
         if (is_dir($path)) {
-            return true;
+            return;
+        }
+
+        if (is_file($path)) {
+            throw new CacheException("Failed to create cache directory \"$path\".");
         }
 
-        $result = !is_file($path) && mkdir(directory: $path, recursive: true) && is_dir($path);
+        mkdir($path, recursive: true);
 
-        if ($result) {
-            chmod($path, $this->directoryMode);
+        if (!is_dir($path)) {
+            throw new CacheException("Failed to create cache directory \"$path\".");
         }
 
-        return $result;
+        chmod($path, $this->directoryMode);
     }
 
     /**
@@ -353,8 +344,12 @@
      *
      * @return string The cache file path.
      */
-    private function getCacheFile(string $key): string
+    private function getCacheFile(string $key, bool $ensureDirectory = false): string
     {
+        if ($ensureDirectory) {
+            $this->ensureDirectory($this->cachePath);
+        }
+
         if ($this->directoryLevel < 1) {
             return $this->cachePath . DIRECTORY_SEPARATOR . $key . $this->fileSuffix;
         }
@@ -364,6 +359,9 @@
         for ($i = 0; $i < $this->directoryLevel; ++$i) {
             if (($prefix = substr($key, $i + $i, 2)) !== '') {
                 $base .= DIRECTORY_SEPARATOR . $prefix;
+                if ($ensureDirectory) {
+                    $this->ensureDirectory($base);
+                }
             }
         }
 

tests/FileCacheTest.php

@@ -449,17 +449,16 @@ public function testDirectoryMode(): void
             $this->markTestSkipped('Can not test permissions on Windows');
         }
 
-        $cache = new FileCache($this->tmpDir, 0777);
+        $cache = (new FileCache($this->tmpDir, 0777))->withDirectoryLevel(2);
 
         $this->assertInstanceOf(FileCache::class, $cache);
 
-        $cache->set('a', 1);
-        $this->assertSameExceptObject(1, $cache->get('a'));
-
-        $cacheFile = $this->invokeMethod($cache, 'getCacheFile', ['a']);
-        $permissions = substr(sprintf('%o', fileperms(dirname($cacheFile))), -4);
+        $cache->set('test', 1);
+        $this->assertSameExceptObject(1, $cache->get('test'));
 
-        $this->assertEquals('0777', $permissions);
+        $cacheFile = $this->invokeMethod($cache, 'getCacheFile', ['test']);
+        $this->assertEquals('0777', substr(sprintf('%o', fileperms(dirname($cacheFile))), -4));
+        $this->assertEquals('0777', substr(sprintf('%o', fileperms(dirname($cacheFile, 2))), -4));
 
         // also check top level cache dir permissions
         $permissions = substr(sprintf('%o', fileperms($this->tmpDir)), -4);
@@ -539,21 +538,13 @@ public function testSetThrowExceptionForInvalidCacheDirectory(): void
         $directory = self::RUNTIME_DIRECTORY . '/cache/fail';
         $cache = new FileCache($directory);
 
-        $this->removeDirectory($directory);
+        mkdir(dirname($directory), recursive: true);
         file_put_contents($directory, 'fail');
 
         $this->expectException(CacheException::class);
         $cache->set('key', 'value');
     }
 
-    public function testConstructorThrowExceptionForInvalidCacheDirectory(): void
-    {
-        $file = self::RUNTIME_DIRECTORY . '/fail';
-        file_put_contents($file, 'fail');
-        $this->expectException(CacheException::class);
-        new FileCache($file);
-    }
-
     public function invalidKeyProvider(): array
     {
         return [