From 3ee5b80355deca6035ad467175a8a60170aa75cb Mon Sep 17 00:00:00 2001 From: agis Date: Fri, 11 Sep 2026 23:01:13 +0700 Subject: [PATCH] refactor(cache): tighten TTL to interval-only, forbid bare-int minutes (task 3.6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repository/TaggedCache put/add/remember now require DateTimeInterface|DateInterval. Bare-int minutes (ambiguous minutes-vs-seconds — the silent 60x bug once swapped to illuminate/cache v13, which reads int as seconds) is rejected. App must pass Carbon::now()->addMinutes(N), unit-safe on both fork and stock v13. - TaggedCache::put stays untyped (StoreInterface::put is untyped -> narrowing = LSP fatal) but getMinutes tightened rejects bare int at runtime; add/remember typed. - Convert 4 fork-internal bare-int callers to intervals: CacheBasedSessionHandler, Repository::offsetSet, Query\Builder::getCached, CachedRouting\Router. - Deferred to task 2.3: relocate StoreInterface -> Contracts\Cache\Store + implement Contracts\Cache\Repository (contract's put($ttl=null) is looser -> can't implement while stricter). Framework-first task 2.2 (fork side). Permanent guard (Psalm bare-int rule) and app conform are follow-ons. Fork suite green (1642). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Illuminate/Cache/Repository.php | 37 ++++++++++--------- src/Illuminate/Cache/TaggedCache.php | 35 +++++++++--------- src/Illuminate/CachedRouting/Router.php | 3 +- src/Illuminate/Database/Query/Builder.php | 3 +- .../Session/CacheBasedSessionHandler.php | 3 +- tests/Cache/CacheRepositoryTest.php | 20 +++++----- tests/Cache/CacheTaggedCacheTest.php | 13 ++++--- tests/Database/DatabaseQueryBuilderTest.php | 4 +- 8 files changed, 62 insertions(+), 56 deletions(-) diff --git a/src/Illuminate/Cache/Repository.php b/src/Illuminate/Cache/Repository.php index 46b94aa69..5a0518d1b 100755 --- a/src/Illuminate/Cache/Repository.php +++ b/src/Illuminate/Cache/Repository.php @@ -1,9 +1,10 @@ getMinutes($minutes); + $minutes = $this->getMinutes($ttl); if ( ! is_null($minutes)) { @@ -100,14 +101,14 @@ public function put($key, $value, $minutes) * * @param string $key * @param mixed $value - * @param \DateTime|int $minutes + * @param \DateTimeInterface|\DateInterval $ttl * @return bool */ - public function add($key, $value, $minutes) + public function add($key, $value, DateTimeInterface|DateInterval $ttl) { if (is_null($this->get($key))) { - $this->put($key, $value, $minutes); return true; + $this->put($key, $value, $ttl); return true; } return false; @@ -117,11 +118,11 @@ public function add($key, $value, $minutes) * Get an item from the cache, or store the default value. * * @param string $key - * @param \DateTime|int $minutes + * @param \DateTimeInterface|\DateInterval $ttl * @param \Closure $callback * @return mixed */ - public function remember($key, $minutes, Closure $callback) + public function remember($key, DateTimeInterface|DateInterval $ttl, Closure $callback) { // If the item exists in the cache we will just return this immediately // otherwise we will execute the given Closure and cache the result @@ -131,7 +132,7 @@ public function remember($key, $minutes, Closure $callback) return $value; } - $this->put($key, $value = $callback(), $minutes); + $this->put($key, $value = $callback(), $ttl); return $value; } @@ -232,7 +233,7 @@ public function offsetGet($key): mixed */ public function offsetSet($key, $value): void { - $this->put($key, $value, $this->default); + $this->put($key, $value, Carbon::now()->addMinutes($this->default)); } /** @@ -247,21 +248,21 @@ public function offsetUnset($key): void } /** - * Calculate the number of minutes with the given duration. + * Calculate the number of minutes until the given TTL. * - * @param \DateTime|int $duration + * @param \DateTimeInterface|\DateInterval $duration * @return int|null */ protected function getMinutes($duration) { - if ($duration instanceof DateTime) + if ($duration instanceof DateInterval) { - $fromNow = Carbon::instance($duration)->diffInMinutes(); - - return $fromNow > 0 ? $fromNow : null; + $duration = Carbon::now()->add($duration); } - return is_string($duration) ? (int) $duration : $duration; + $fromNow = Carbon::instance($duration)->diffInMinutes(); + + return $fromNow > 0 ? $fromNow : null; } /** diff --git a/src/Illuminate/Cache/TaggedCache.php b/src/Illuminate/Cache/TaggedCache.php index 6b9eb7ae8..bddccdf57 100644 --- a/src/Illuminate/Cache/TaggedCache.php +++ b/src/Illuminate/Cache/TaggedCache.php @@ -1,8 +1,9 @@ getMinutes($minutes); + $minutes = $this->getMinutes($ttl); if ( ! is_null($minutes)) { @@ -81,14 +82,14 @@ public function put($key, $value, $minutes) * * @param string $key * @param mixed $value - * @param \DateTime|int $minutes + * @param \DateTimeInterface|\DateInterval $ttl * @return bool */ - public function add($key, $value, $minutes) + public function add($key, $value, DateTimeInterface|DateInterval $ttl) { if (is_null($this->get($key))) { - $this->put($key, $value, $minutes); return true; + $this->put($key, $value, $ttl); return true; } return false; @@ -155,18 +156,18 @@ public function flush() * Get an item from the cache, or store the default value. * * @param string $key - * @param \DateTime|int $minutes + * @param \DateTimeInterface|\DateInterval $ttl * @param \Closure $callback * @return mixed */ - public function remember($key, $minutes, Closure $callback) + public function remember($key, DateTimeInterface|DateInterval $ttl, Closure $callback) { // If the item exists in the cache we will just return this immediately // otherwise we will execute the given Closure and cache the result // of that execution for the given number of minutes in storage. if ( ! is_null($value = $this->get($key))) return $value; - $this->put($key, $value = $callback(), $minutes); + $this->put($key, $value = $callback(), $ttl); return $value; } @@ -224,21 +225,21 @@ public function getPrefix() } /** - * Calculate the number of minutes with the given duration. + * Calculate the number of minutes until the given TTL. * - * @param \DateTime|int $duration + * @param \DateTimeInterface|\DateInterval $duration * @return int|null */ protected function getMinutes($duration) { - if ($duration instanceof DateTime) + if ($duration instanceof DateInterval) { - $fromNow = Carbon::instance($duration)->diffInMinutes(); - - return $fromNow > 0 ? $fromNow : null; + $duration = Carbon::now()->add($duration); } - return is_string($duration) ? (int) $duration : $duration; + $fromNow = Carbon::instance($duration)->diffInMinutes(); + + return $fromNow > 0 ? $fromNow : null; } } diff --git a/src/Illuminate/CachedRouting/Router.php b/src/Illuminate/CachedRouting/Router.php index 01a4ad147..580e26a51 100644 --- a/src/Illuminate/CachedRouting/Router.php +++ b/src/Illuminate/CachedRouting/Router.php @@ -36,6 +36,7 @@ */ use Closure; +use Carbon\Carbon; use Illuminate\Container\Container; use Illuminate\Events\Dispatcher; use Illuminate\Routing\Router as LaravelRouter; @@ -114,7 +115,7 @@ public function cache($filename, Closure $callback, $cacheMinutes = 1440): ?stri // Persist the routes, ignoring failures so a broken cache store // never propagates out of boot (routes stay defined in memory). try { - $cacher->put($cacheKey, $this->routes->getCacheableRoutes(), $cacheMinutes); + $cacher->put($cacheKey, $this->routes->getCacheableRoutes(), Carbon::now()->addMinutes($cacheMinutes)); } catch (\Throwable $e) { // Best-effort cache; a write failure is non-fatal. } diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 2317535dd..d9deeda95 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1,6 +1,7 @@ rememberForever($key, $callback); } - return $cache->remember($key, $minutes, $callback); + return $cache->remember($key, Carbon::now()->addMinutes($minutes), $callback); } /** diff --git a/src/Illuminate/Session/CacheBasedSessionHandler.php b/src/Illuminate/Session/CacheBasedSessionHandler.php index f5c9908f7..af63d7934 100755 --- a/src/Illuminate/Session/CacheBasedSessionHandler.php +++ b/src/Illuminate/Session/CacheBasedSessionHandler.php @@ -1,5 +1,6 @@ cache->put($id, $data, $this->minutes); + $this->cache->put($id, $data, Carbon::now()->addMinutes($this->minutes)); } catch (\Throwable) { return false; } diff --git a/tests/Cache/CacheRepositoryTest.php b/tests/Cache/CacheRepositoryTest.php index d449ed5d1..67453db75 100755 --- a/tests/Cache/CacheRepositoryTest.php +++ b/tests/Cache/CacheRepositoryTest.php @@ -1,5 +1,6 @@ getRepository(); $repo->getStore()->shouldReceive('get')->andReturn(null); - $repo->getStore()->shouldReceive('put')->once()->with('foo', 'bar', 10); - $result = $repo->remember('foo', 10, function() { return 'bar'; }); + $repo->getStore()->shouldReceive('put')->once()->with('foo', 'bar', m::type('int')); + $result = $repo->remember('foo', Carbon::now()->addMinutes(10), function() { return 'bar'; }); $this->assertEquals('bar', $result); + } + - /** - * Use Carbon object... - */ - // $repo = $this->getRepository(); - // $repo->getStore()->shouldReceive('get')->andReturn(null); - // $repo->getStore()->shouldReceive('put')->once()->with('foo', 'bar', 9); - // $result = $repo->remember('foo', Carbon::now()->addMinutes(10), function() { return 'bar'; }); - // $this->assertEquals('bar', $result); + public function testPutAcceptsDateIntervalTtl() + { + $repo = $this->getRepository(); + $repo->getStore()->shouldReceive('put')->once()->with('foo', 'bar', m::type('int')); + $repo->put('foo', 'bar', new DateInterval('PT10M')); } diff --git a/tests/Cache/CacheTaggedCacheTest.php b/tests/Cache/CacheTaggedCacheTest.php index 0dd474f27..f1fb4199a 100644 --- a/tests/Cache/CacheTaggedCacheTest.php +++ b/tests/Cache/CacheTaggedCacheTest.php @@ -1,5 +1,6 @@ section('bop')->put('foo', 'bar', 10); - $store->section('zap')->put('baz', 'boom', 10); + $store->section('bop')->put('foo', 'bar', Carbon::now()->addMinutes(10)); + $store->section('zap')->put('baz', 'boom', Carbon::now()->addMinutes(10)); $store->section('bop')->flush(); $this->assertNull($store->section('bop')->get('foo')); $this->assertEquals('boom', $store->section('zap')->get('baz')); @@ -30,7 +31,7 @@ public function testCacheCanBeSavedWithMultipleTags() { $store = new ArrayStore; $tags = ['bop', 'zap']; - $store->tags($tags)->put('foo', 'bar', 10); + $store->tags($tags)->put('foo', 'bar', Carbon::now()->addMinutes(10)); $this->assertEquals('bar', $store->tags($tags)->get('foo')); } @@ -50,9 +51,9 @@ public function testCacheSavedWithMultipleTagsCanBeFlushed() { $store = new ArrayStore; $tags1 = ['bop', 'zap']; - $store->tags($tags1)->put('foo', 'bar', 10); + $store->tags($tags1)->put('foo', 'bar', Carbon::now()->addMinutes(10)); $tags2 = ['bam', 'pow']; - $store->tags($tags2)->put('foo', 'bar', 10); + $store->tags($tags2)->put('foo', 'bar', Carbon::now()->addMinutes(10)); $store->tags('zap')->flush(); $this->assertNull($store->tags($tags1)->get('foo')); $this->assertEquals('bar', $store->tags($tags2)->get('foo')); @@ -62,7 +63,7 @@ public function testCacheSavedWithMultipleTagsCanBeFlushed() public function testTagsWithStringArgument() { $store = new ArrayStore; - $store->tags('bop')->put('foo', 'bar', 10); + $store->tags('bop')->put('foo', 'bar', Carbon::now()->addMinutes(10)); $this->assertEquals('bar', $store->tags('bop')->get('foo')); } diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 513d200ed..bf0db305a 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -91,7 +91,7 @@ public function testSelectWithCaching(): void $driver->shouldReceive('remember') ->once() - ->with($query->getCacheKey(), 5, m::type('Closure')) + ->with($query->getCacheKey(), m::type(\DateTimeInterface::class), m::type('Closure')) ->andReturnUsing(function($key, $minutes, $callback) { return $callback(); }); @@ -134,7 +134,7 @@ public function testSelectWithCachingAndTags(): void $taggedCache->shouldReceive('remember') ->once() - ->with($query->getCacheKey(), 5, m::type('Closure')) + ->with($query->getCacheKey(), m::type(\DateTimeInterface::class), m::type('Closure')) ->andReturnUsing(function($key, $minutes, $callback) { return $callback(); }); $this->assertEquals($query->get(), ['results']);