From 8e69f86b406bcb17f3fbab93c0206dfe183c641d Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Thu, 17 Dec 2020 09:32:39 +0100 Subject: [PATCH 1/3] Refactor cache path computation into single method --- src/Cache/FileCache.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Cache/FileCache.php b/src/Cache/FileCache.php index b65da8f..920c4f0 100644 --- a/src/Cache/FileCache.php +++ b/src/Cache/FileCache.php @@ -24,7 +24,7 @@ public function __construct(string $dir) public function load(string $class): ?ClassMetadata { - $path = $this->dir . '/' . $this->sanitizeCacheKey($class) . '.cache.php'; + $path = $this->getCachePath($class); if (!file_exists($path)) { return null; } @@ -49,7 +49,7 @@ public function put(ClassMetadata $metadata): void throw new \InvalidArgumentException(sprintf('The directory "%s" is not writable.', $this->dir)); } - $path = $this->dir . '/' . $this->sanitizeCacheKey($metadata->name) . '.cache.php'; + $path = $this->getCachePath($metadata->name); $tmpFile = tempnam($this->dir, 'metadata-cache'); if (false === $tmpFile) { @@ -97,18 +97,22 @@ private function renameFile(string $source, string $target): void public function evict(string $class): void { - $path = $this->dir . '/' . $this->sanitizeCacheKey($class) . '.cache.php'; + $path = $this->getCachePath($class); if (file_exists($path)) { @unlink($path); } } /** + * This function computes the cache file path. + * * If anonymous class is to be cached, it contains invalid path characters that need to be removed/replaced * Example of anonymous class name: class@anonymous\x00/app/src/Controller/DefaultController.php0x7f82a7e026ec */ - private function sanitizeCacheKey(string $key): string + private function getCachePath(string $key): string { - return str_replace(['\\', "\0", '@', '/', '$', '{', '}', ':'], '-', $key); + $fileName = str_replace(['\\', "\0", '@', '/', '$', '{', '}', ':'], '-', $key); + + return $this->dir . '/' . $fileName . '.cache.php'; } } From 66a164ea17f3c910063dd6231d2d5a7ff0dcf7d0 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Thu, 17 Dec 2020 09:42:37 +0100 Subject: [PATCH 2/3] Always create cache directory if it doesn't exist --- src/Cache/FileCache.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Cache/FileCache.php b/src/Cache/FileCache.php index 920c4f0..3ea4af0 100644 --- a/src/Cache/FileCache.php +++ b/src/Cache/FileCache.php @@ -15,8 +15,8 @@ class FileCache implements CacheInterface public function __construct(string $dir) { - if (!is_dir($dir)) { - throw new \InvalidArgumentException(sprintf('The directory "%s" does not exist.', $dir)); + if (!is_dir($dir) && false === @mkdir($dir, 0777, true)) { + throw new \InvalidArgumentException(sprintf('Can\'t create directory for cache at "%s"', $dir)); } $this->dir = rtrim($dir, '\\/'); @@ -50,6 +50,9 @@ public function put(ClassMetadata $metadata): void } $path = $this->getCachePath($metadata->name); + if (!is_writable(dirname($path))) { + throw new \RuntimeException(sprintf('Cache file "%s" is not writable.', $path)); + } $tmpFile = tempnam($this->dir, 'metadata-cache'); if (false === $tmpFile) { From 6af9152795e8d99bc928a4b24fd7ef90727bd1b2 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Thu, 17 Dec 2020 09:56:53 +0100 Subject: [PATCH 3/3] Add tests for non-writable error using vfsStream --- composer.json | 3 ++- tests/Cache/FileCacheTest.php | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f5e6e08..ff86b64 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,8 @@ "phpunit/phpunit": "^8.5|^9.0", "psr/container": "^1.0", "symfony/cache" : "^3.1|^4.0|^5.0", - "symfony/dependency-injection" : "^3.1|^4.0|^5.0" + "symfony/dependency-injection" : "^3.1|^4.0|^5.0", + "mikey179/vfsstream": "^1.6.7" }, "autoload": { "psr-4": { "Metadata\\": "src/" } diff --git a/tests/Cache/FileCacheTest.php b/tests/Cache/FileCacheTest.php index a454d60..aab6c5f 100644 --- a/tests/Cache/FileCacheTest.php +++ b/tests/Cache/FileCacheTest.php @@ -7,16 +7,24 @@ use Metadata\Cache\FileCache; use Metadata\ClassMetadata; use Metadata\Tests\Fixtures\TestObject; +use org\bovigo\vfs\vfsStream; use PHPUnit\Framework\TestCase; class FileCacheTest extends TestCase { private $dir; + private $nestedDir; protected function setUp(): void { $this->dir = sys_get_temp_dir() . '/jms-' . md5(__CLASS__); + $this->nestedDir = $this->dir . '/some-dir'; if (is_dir($this->dir)) { + if (is_dir($this->nestedDir)) { + array_map('unlink', glob($this->nestedDir . '/*')); + rmdir($this->nestedDir); + } + array_map('unlink', glob($this->dir . '/*')); } else { mkdir($this->dir); @@ -69,4 +77,33 @@ public function testNonReturningCache(string $fileContents) $this->assertNull($cache->load(TestObject::class)); } + + public function testPutCacheFileInNotExistingDirectory() + { + $cache = new FileCache($this->nestedDir); + + $reflectionClass = new \ReflectionClass('Metadata\Tests\Fixtures\TestObject'); + $cache->put($metadata = new ClassMetadata('Metadata\Tests\Fixtures\TestObject')); + $this->assertEquals($metadata, $cache->load($reflectionClass->name)); + } + + public function testThrowExceptionIfCacheDirNotWritable() + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Can\'t create directory for cache at "vfs://root/JMS"'); + + $root = vfsStream::setup('root', 0555); + $cache = new FileCache($root->url() . '/JMS'); + } + + public function testThrowExceptionIfCacheFilePathNotWritable() + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('The directory "vfs://root" is not writable.'); + + $root = vfsStream::setup('root', 0555); + $cache = new FileCache($root->url()); + + $cache->put($metadata = new ClassMetadata('Metadata\Tests\Fixtures\TestParent')); + } }