From f5312ee183eb29086084d0e5b7e960cbf8256b6b Mon Sep 17 00:00:00 2001
From: annaadamchuk <aadamchuk@carscommerce.inc>
Date: Fri, 1 May 2026 15:58:24 -0400
Subject: [PATCH] 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>
---
 app/AbstractUseStaleRequest.php               |  5 +-
 tests/Feature/AbstractUseStaleRequestTest.php | 57 +++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/app/AbstractUseStaleRequest.php b/app/AbstractUseStaleRequest.php
index 82697c3..3d6b9d1 100644
--- a/app/AbstractUseStaleRequest.php
+++ b/app/AbstractUseStaleRequest.php
@@ -43,9 +43,10 @@ protected function responseFromCache(): ?Response
 
     protected function writeResponseToCache(): void
     {
-        if ($this->shouldWriteResponseToCache()) {
-            Cache::tags($this->getCacheTags())->put($this->refreshCacheKey(), 'refresh after', $this->refreshAfter());
+        if (!$this->shouldWriteResponseToCache()) {
+            return;
         }
+        Cache::tags($this->getCacheTags())->put($this->refreshCacheKey(), 'refresh after', $this->refreshAfter());
         parent::writeResponseToCache();
     }
 
diff --git a/tests/Feature/AbstractUseStaleRequestTest.php b/tests/Feature/AbstractUseStaleRequestTest.php
index 71b25cf..5ab15d3 100644
--- a/tests/Feature/AbstractUseStaleRequestTest.php
+++ b/tests/Feature/AbstractUseStaleRequestTest.php
@@ -121,6 +121,63 @@ public function testRefreshOnNextRequest(): void
         self::assertTrue($request->needsRefresh(), 'But we want to refresh opportunistically');
     }
 
+    public function testCacheHitDoesNotWriteRefreshMarker(): void
+    {
+        $request = new ConcreteUseStaleRequest('thing');
+
+        self::mockRequestCachedResponse($request, 'cached body');
+        Cache::tags($request->getCacheTags())->put(
+            $request->refreshCacheKey(),
+            'sentinel',
+            Carbon::now()->addMinutes(15),
+        );
+
+        self::assertSame('cached body', $request->sync());
+
+        self::assertSame(
+            'sentinel',
+            Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
+            'Refresh marker should be untouched on a cache hit — writeResponseToCache must early-return.',
+        );
+    }
+
+    public function testFreshFetchWritesRefreshMarker(): void
+    {
+        $this->mockGuzzleWithTapper()->addMatchBody('GET', '/test/', 'fresh');
+        $request = new ConcreteUseStaleRequest('thing');
+
+        self::assertNull(
+            Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
+            'Marker should not exist before first fetch.',
+        );
+
+        self::assertSame('fresh', $request->sync());
+
+        self::assertSame(
+            'refresh after',
+            Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
+            'A successful live fetch must write the refresh marker.',
+        );
+    }
+
+    public function testWriteCacheDisabledSkipsRefreshMarker(): void
+    {
+        $this->mockGuzzleWithTapper()->addMatchBody('GET', '/test/', 'fresh');
+        $request = new ConcreteUseStaleRequest('thing');
+        $request->setWriteCache(false);
+
+        self::assertSame('fresh', $request->sync());
+
+        self::assertNull(
+            Cache::tags($request->getCacheTags())->get($request->refreshCacheKey()),
+            'setWriteCache(false) must prevent the refresh marker write.',
+        );
+        self::assertFalse(
+            $request->canBeFulfilledByCache(),
+            'setWriteCache(false) must also prevent the response from being cached.',
+        );
+    }
+
     public function testCacheBehaviorUnderHeavyLoad(): void
     {
         Queue::fake();