diff --git a/CHANGELOG.md b/CHANGELOG.md index 81b3ac1..c11636b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] ### Changed +- Cache files are now written to a temporary file and atomically published, + preventing readers from observing partially written cache entries. - Added scalar, nullable, and `mixed` type declarations to the public cache API and its configuration properties. Calls that already use the documented value types remain compatible. diff --git a/TODO.md b/TODO.md index ad11245..7172fb1 100644 --- a/TODO.md +++ b/TODO.md @@ -4,10 +4,6 @@ Deferred improvements — implement when explicitly requested or when capacity a ## v1.x -- **[MEDIUM] Make cache writes atomic and protect readers from partial files** - `LOCK_EX` does not prevent readers from observing a file while it is being - written. Add an atomic-write strategy and concurrent-read/write tests. - - **[MEDIUM] Publish to Packagist** Run `composer validate --strict`, submit to packagist.org. Prerequisite: stable tagged 1.0.0 release with namespace. @@ -25,6 +21,14 @@ Deferred improvements — implement when explicitly requested or when capacity a Edge cases: empty string IDs, very long IDs, unreadable cache directory, Unicode in cache IDs. +- **[OPTIONAL] Prevent cache stampedes for concurrent cache misses** + Add per-key coordination so concurrent requests do not regenerate the same + missing or expired entry. Relevant for expensive cache generation. + +- **[OPTIONAL] Define `clearCache()` behaviour during concurrent writes** + Decide whether an active writer may republish an entry after it was cleared; + implement synchronization only if strict clearing semantics are required. + ## v2.x - **[MEDIUM] Implement PSR-16 SimpleCache interface** diff --git a/demo/index.php b/demo/index.php index c84d201..74e7c8a 100644 --- a/demo/index.php +++ b/demo/index.php @@ -7,8 +7,6 @@ // Set the cacheBaseDir (default is the system temp directory). SimplePhpCache::$cacheBaseDir = __DIR__; -echo SimplePhpCache::$cacheBaseDir; - // Set the max cache time (if needed). Default is 86400; //SimplePhpCache::$maxCacheTime = 86400; diff --git a/src/SimplePhpCache.php b/src/SimplePhpCache.php index 9d92cad..43cc346 100644 --- a/src/SimplePhpCache.php +++ b/src/SimplePhpCache.php @@ -100,8 +100,7 @@ public static function finishHTMLCaching(string $id): string throw new RuntimeException("Error reading output buffer"); } - if (file_put_contents($cacheFile, $content, LOCK_EX) === false) - throw new RuntimeException("Error writing cache: '$cacheFile'"); + self::writeCacheFile($cacheFile, $content); } self::$startedCache = null; @@ -311,8 +310,47 @@ private static function writeVarCachePayload(string $cacheFile, mixed $data): vo throw new RuntimeException("Error encoding variable cache payload: " . $e->getMessage(), 0, $e); } - if (file_put_contents($cacheFile, $payload, LOCK_EX) === false) { - throw new RuntimeException("Error writing cache: '$cacheFile'"); + self::writeCacheFile($cacheFile, $payload); + } + + /** + * Write a cache file through a temporary file and atomically publish it. + * + * Keeping the temporary file in the cache directory makes rename() an + * atomic replacement on supported filesystems, so readers see either the + * complete old file or the complete new file. + * + * @param string $cacheFile + * @param string $content + * @throws RuntimeException + */ + private static function writeCacheFile(string $cacheFile, string $content): void + { + $cacheDir = realpath(dirname($cacheFile)); + if ($cacheDir === false) { + throw new RuntimeException("Cache directory does not exist for: '$cacheFile'"); + } + + $temporaryFile = tempnam($cacheDir, '.simplephpcache-'); + if ($temporaryFile === false) { + throw new RuntimeException("Error creating temporary cache file for: '$cacheFile'"); + } + + try { + $writtenBytes = file_put_contents($temporaryFile, $content, LOCK_EX); + if ($writtenBytes === false || $writtenBytes !== strlen($content)) { + throw new RuntimeException("Error writing temporary cache: '$temporaryFile'"); + } + + if (!rename($temporaryFile, $cacheFile)) { + throw new RuntimeException("Error publishing cache: '$cacheFile'"); + } + + $temporaryFile = null; + } finally { + if ($temporaryFile !== null && file_exists($temporaryFile)) { + @unlink($temporaryFile); + } } } diff --git a/tests/SimplePhpCacheTest.php b/tests/SimplePhpCacheTest.php index 4f4a043..6ff9fbf 100644 --- a/tests/SimplePhpCacheTest.php +++ b/tests/SimplePhpCacheTest.php @@ -206,6 +206,29 @@ public function testVarCachingForceRefresh(): void $this->assertEquals('updated', $result); } + public function testCacheRefreshAtomicallyReplacesExistingFile(): void + { + $id = 'atomic_refresh'; + + SimplePhpCache::initVarCaching($id); + SimplePhpCache::setVarCaching($id, 'original'); + SimplePhpCache::finishVarCaching($id); + $this->resetStaticState(); + + SimplePhpCache::initVarCaching($id, true); + SimplePhpCache::setVarCaching($id, 'replacement'); + SimplePhpCache::finishVarCaching($id); + $this->resetStaticState(); + + $this->assertFalse(SimplePhpCache::initVarCaching($id)); + $this->assertSame('replacement', SimplePhpCache::finishVarCaching($id)); + $this->assertSame( + [], + glob($this->testCacheDir . '/.simplePhpCache/.simplephpcache-*') ?: [], + 'Successful cache writes must not leave temporary files behind.' + ); + } + public function testInitVarThrowsWhenAlreadyStarted(): void { $this->expectException(RuntimeException::class);