refactor(cache): tighten TTL to interval-only, forbid bare-int minutes (task 2.2 / 3.6) - #42
Open
agissept wants to merge 1 commit into
Open
Conversation
…s (task 3.6) 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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on
migration/3.5-config-log-filesystem(retarget todev-13when the stack merges). Framework-first task 2.2 (fork side).Why
L13 reads a bare-int cache TTL as seconds; the fork reads it as minutes. Once we swap to
illuminate/cachev13 a bare int silently becomes 60× shorter. This makes the fork stricter than stock so the ambiguous form breaks loudly now and forces the app onto an absolute interval (Carbon::now()->addMinutes(N)), which is unit-safe on both fork and stock v13.What
Repository::put/add/rememberandTaggedCache::add/remembernow requireDateTimeInterface|DateInterval $ttl.getMinutes()tightened to an interval→minutes converter (fork stores stay minutes-based).TaggedCache::putstays untyped — it implementsStoreInterface::put(untyped), so narrowing the param is an LSP fatal. Its tightenedgetMinutes()still rejects bare int at runtime;add/remember(not inStoreInterface) are typed.CacheBasedSessionHandler,Repository::offsetSet,Query\Builder::getCached,CachedRouting\Router(this last one'sTypeErrorwas swallowed by a best-effortcatch(\Throwable)→ routes silently uncached; only the full suite caught it).Deliberately deferred (to task 2.3)
Relocate
StoreInterface→Contracts\Cache\Storeand implementContracts\Cache\Repository. The contract'sput($ttl = null)is looser, so the fork can't implement it while it's stricter (param-narrowing LSP). KeptgetDefaultCacheTime/setDefaultCacheTimefor now.Enforcement note
This fork strictness evaporates on swap (stock v13 re-admits bare int). The permanent guard is a Psalm bare-int rule (task 0.3), not this type change.
Validation
Fork suite green (1642). App conform lives in a separate
dicodingbranch (improvement/l13-migration/cache-ttl-conform): unit 8088 green, cache integration 21 green, full integration back to the pre-existing baseline with 0 cache regressions.🤖 Generated with Claude Code