Skip to content

Commit c4fcb8f

Browse files
vjiksamdark
andauthored
Don't create cache directory in constructor + Fix nested directory permissions (#88)
Co-authored-by: Alexander Makarov <[email protected]>
1 parent 71ddc00 commit c4fcb8f

File tree

3 files changed

+29
-38
lines changed

3 files changed

+29
-38
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## 3.1.1 under development
44

5+
- Enh #88: Don't create cache directory on `FileCache` initialization (@vjik)
6+
- Bug #88: Set correct permissions for nested directories (@vjik)
57
- Bug #85: Clear stat cache in `FileCache::set()` (@samdark)
68

79
## 3.1.0 October 09, 2023

src/FileCache.php

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use function array_keys;
1313
use function array_map;
1414
use function closedir;
15-
use function dirname;
1615
use function error_get_last;
1716
use function filemtime;
1817
use function fileowner;
@@ -91,9 +90,6 @@ public function __construct(
9190
private string $cachePath,
9291
private int $directoryMode = 0775,
9392
) {
94-
if (!$this->createDirectoryIfNotExists($cachePath)) {
95-
throw new CacheException("Failed to create cache directory \"$cachePath\".");
96-
}
9793
}
9894

9995
public function get(string $key, mixed $default = null): mixed
@@ -123,14 +119,7 @@ public function set(string $key, mixed $value, null|int|DateInterval $ttl = null
123119
return $this->delete($key);
124120
}
125121

126-
$file = $this->getCacheFile($key);
127-
$cacheDirectory = dirname($file);
128-
129-
if (!is_dir($this->cachePath)
130-
|| $this->directoryLevel > 0 && !$this->createDirectoryIfNotExists($cacheDirectory)
131-
) {
132-
throw new CacheException("Failed to create cache directory \"$cacheDirectory\".");
133-
}
122+
$file = $this->getCacheFile($key, ensureDirectory: true);
134123

135124
// If ownership differs, the touch call will fail, so we try to
136125
// rebuild the file from scratch by deleting it first
@@ -325,25 +314,27 @@ private function normalizeTtl(null|int|string|DateInterval $ttl = null): ?int
325314
}
326315

327316
/**
328-
* Ensures that the directory is created.
317+
* Ensures that the directory exists.
329318
*
330319
* @param string $path The path to the directory.
331-
*
332-
* @return bool Whether the directory was created.
333320
*/
334-
private function createDirectoryIfNotExists(string $path): bool
321+
private function ensureDirectory(string $path): void
335322
{
336323
if (is_dir($path)) {
337-
return true;
324+
return;
325+
}
326+
327+
if (is_file($path)) {
328+
throw new CacheException("Failed to create cache directory, file with the same name exists: \"$path\".");
338329
}
339330

340-
$result = !is_file($path) && mkdir(directory: $path, recursive: true) && is_dir($path);
331+
mkdir($path, recursive: true);
341332

342-
if ($result) {
343-
chmod($path, $this->directoryMode);
333+
if (!is_dir($path)) {
334+
throw new CacheException("Failed to create cache directory \"$path\".");
344335
}
345336

346-
return $result;
337+
chmod($path, $this->directoryMode);
347338
}
348339

349340
/**
@@ -353,8 +344,12 @@ private function createDirectoryIfNotExists(string $path): bool
353344
*
354345
* @return string The cache file path.
355346
*/
356-
private function getCacheFile(string $key): string
347+
private function getCacheFile(string $key, bool $ensureDirectory = false): string
357348
{
349+
if ($ensureDirectory) {
350+
$this->ensureDirectory($this->cachePath);
351+
}
352+
358353
if ($this->directoryLevel < 1) {
359354
return $this->cachePath . DIRECTORY_SEPARATOR . $key . $this->fileSuffix;
360355
}
@@ -364,6 +359,9 @@ private function getCacheFile(string $key): string
364359
for ($i = 0; $i < $this->directoryLevel; ++$i) {
365360
if (($prefix = substr($key, $i + $i, 2)) !== '') {
366361
$base .= DIRECTORY_SEPARATOR . $prefix;
362+
if ($ensureDirectory) {
363+
$this->ensureDirectory($base);
364+
}
367365
}
368366
}
369367

tests/FileCacheTest.php

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -449,17 +449,16 @@ public function testDirectoryMode(): void
449449
$this->markTestSkipped('Can not test permissions on Windows');
450450
}
451451

452-
$cache = new FileCache($this->tmpDir, 0777);
452+
$cache = (new FileCache($this->tmpDir, 0777))->withDirectoryLevel(2);
453453

454454
$this->assertInstanceOf(FileCache::class, $cache);
455455

456-
$cache->set('a', 1);
457-
$this->assertSameExceptObject(1, $cache->get('a'));
458-
459-
$cacheFile = $this->invokeMethod($cache, 'getCacheFile', ['a']);
460-
$permissions = substr(sprintf('%o', fileperms(dirname($cacheFile))), -4);
456+
$cache->set('test', 1);
457+
$this->assertSameExceptObject(1, $cache->get('test'));
461458

462-
$this->assertEquals('0777', $permissions);
459+
$cacheFile = $this->invokeMethod($cache, 'getCacheFile', ['test']);
460+
$this->assertEquals('0777', substr(sprintf('%o', fileperms(dirname($cacheFile))), -4));
461+
$this->assertEquals('0777', substr(sprintf('%o', fileperms(dirname($cacheFile, 2))), -4));
463462

464463
// also check top level cache dir permissions
465464
$permissions = substr(sprintf('%o', fileperms($this->tmpDir)), -4);
@@ -539,21 +538,13 @@ public function testSetThrowExceptionForInvalidCacheDirectory(): void
539538
$directory = self::RUNTIME_DIRECTORY . '/cache/fail';
540539
$cache = new FileCache($directory);
541540

542-
$this->removeDirectory($directory);
541+
mkdir(dirname($directory), recursive: true);
543542
file_put_contents($directory, 'fail');
544543

545544
$this->expectException(CacheException::class);
546545
$cache->set('key', 'value');
547546
}
548547

549-
public function testConstructorThrowExceptionForInvalidCacheDirectory(): void
550-
{
551-
$file = self::RUNTIME_DIRECTORY . '/fail';
552-
file_put_contents($file, 'fail');
553-
$this->expectException(CacheException::class);
554-
new FileCache($file);
555-
}
556-
557548
public function invalidKeyProvider(): array
558549
{
559550
return [

0 commit comments

Comments
 (0)