Skip to content

Commit f5312ee

Browse files
annaadamchukclaude
andcommitted
refactor: early-return writeResponseToCache on cache hit
Skip the parent invocation when shouldWriteResponseToCache() is false. Behavior is unchanged - the parent already short-circuits via its own guard - but the structure is clearer and avoids walking into the parent when no write will happen. Adds tests covering cache hit (refresh marker untouched), fresh fetch (marker written), and setWriteCache(false) (marker skipped even on a fresh fetch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 733d206 commit f5312ee

2 files changed

Lines changed: 60 additions & 2 deletions

File tree

app/AbstractUseStaleRequest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,10 @@ protected function responseFromCache(): ?Response
4343

4444
protected function writeResponseToCache(): void
4545
{
46-
if ($this->shouldWriteResponseToCache()) {
47-
Cache::tags($this->getCacheTags())->put($this->refreshCacheKey(), 'refresh after', $this->refreshAfter());
46+
if (!$this->shouldWriteResponseToCache()) {
47+
return;
4848
}
49+
Cache::tags($this->getCacheTags())->put($this->refreshCacheKey(), 'refresh after', $this->refreshAfter());
4950
parent::writeResponseToCache();
5051
}
5152

tests/Feature/AbstractUseStaleRequestTest.php

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,63 @@ public function testRefreshOnNextRequest(): void
121121
self::assertTrue($request->needsRefresh(), 'But we want to refresh opportunistically');
122122
}
123123

124+
public function testCacheHitDoesNotWriteRefreshMarker(): void
125+
{
126+
$request = new ConcreteUseStaleRequest('thing');
127+
128+
self::mockRequestCachedResponse($request, 'cached body');
129+
Cache::tags($request->getCacheTags())->put(
130+
$request->refreshCacheKey(),
131+
'sentinel',
132+
Carbon::now()->addMinutes(15),
133+
);
134+
135+
self::assertSame('cached body', $request->sync());
136+
137+
self::assertSame(
138+
'sentinel',
139+
Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
140+
'Refresh marker should be untouched on a cache hit — writeResponseToCache must early-return.',
141+
);
142+
}
143+
144+
public function testFreshFetchWritesRefreshMarker(): void
145+
{
146+
$this->mockGuzzleWithTapper()->addMatchBody('GET', '/test/', 'fresh');
147+
$request = new ConcreteUseStaleRequest('thing');
148+
149+
self::assertNull(
150+
Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
151+
'Marker should not exist before first fetch.',
152+
);
153+
154+
self::assertSame('fresh', $request->sync());
155+
156+
self::assertSame(
157+
'refresh after',
158+
Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
159+
'A successful live fetch must write the refresh marker.',
160+
);
161+
}
162+
163+
public function testWriteCacheDisabledSkipsRefreshMarker(): void
164+
{
165+
$this->mockGuzzleWithTapper()->addMatchBody('GET', '/test/', 'fresh');
166+
$request = new ConcreteUseStaleRequest('thing');
167+
$request->setWriteCache(false);
168+
169+
self::assertSame('fresh', $request->sync());
170+
171+
self::assertNull(
172+
Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
173+
'setWriteCache(false) must prevent the refresh marker write.',
174+
);
175+
self::assertFalse(
176+
$request->canBeFulfilledByCache(),
177+
'setWriteCache(false) must also prevent the response from being cached.',
178+
);
179+
}
180+
124181
public function testCacheBehaviorUnderHeavyLoad(): void
125182
{
126183
Queue::fake();

0 commit comments

Comments
 (0)