fix: issue-19 sentLogs should store (and cache) the filename and the log folder

app/AbstractRequest.php

@@ -365,7 +365,11 @@ protected function responseFromCache(): ?Response
         // So that previous cache entries with incompatible cached data are not read by responseFromCache
         $fromCache = Cache::tags($this->cacheTags)->get($this->cacheKey());
         if ($fromCache) {
-            $this->sentLogs = $fromCache['logs'];
+            $this->sentLogs = array_map(
+                fn ($filename) => Str::contains($filename, '/')
+                    ? $filename
+                    : Str::finish($this->getLogFolder(), '/') . $filename,
+                $fromCache['logs']);
             return new Response(...$fromCache['response']);
         }
         return null;
@@ -434,7 +438,7 @@ public function log($outcome): void
         }
         $logged = $this->getLogFileHelper()::put($this->getLogFolder(), [$this->toGuzzle(), $outcome, $this->requestStats]);
         if ($logged) {
-            $this->sentLogs[] = $logged;
+            $this->sentLogs[] = Str::finish($this->getLogFolder(), '/') . $logged;
         }
     }
 
@@ -454,10 +458,7 @@ public function getLastLogContents(): string
      */
     public function getLastLogFile(): string
     {
-        if (!$this->sentLogs) {
-            throw new DomainException('No log files have been saved by this instance.');
-        }
-        return Str::finish($this->getLogFolder(), '/') . Arr::last($this->sentLogs);
+        return Arr::last($this->sentLogs) ?? throw new DomainException('No log files have been saved by this instance.');
     }
 
     /**

tests/Feature/AbstractRequestTest.php

@@ -141,6 +141,49 @@ public function testCacheHitHasAccessToOriginalLog(): void
         self::assertSame(["one/two/{$firstLogTime}"], Storage::disk('api-logs')->files('one/two'));
     }
 
+    public function testCachedLogFileContainsFolder(): void
+    {
+        Storage::fake('api-logs');
+        $tapper = $this->mockGuzzleWithTapper();
+        $tapper->addMatchBody('POST', '/awesome/', '{"awesome":"sauce"}');
+
+        $firstRequestTime = '2018-01-01T00:00:00.000000+00:00';
+        Carbon::setTestNow($firstRequestTime);
+        $firstRequest = $this->mockRequestWithLogMutatingFolder('first');
+        self::assertSame('root/first', $firstRequest->getLogFolder());
+        $firstRequest->sync();
+        self::assertTrue($firstRequest->canBeFulfilledByCache());
+        self::assertSame("root/first/{$firstRequestTime}", $firstRequest->getLastLogFile());
+
+        $secondRequestTime = '2018-01-01T00:00:00.000000+00:00';
+        Carbon::setTestNow($secondRequestTime);
+        $secondRequest = $this->mockRequestWithLogMutatingFolder('second');
+        self::assertSame('root/second', $secondRequest->getLogFolder());
+        $secondRequest->sync();
+        self::assertTrue(getProperty($secondRequest, 'responseIsFromCache'));
+        self::assertSame("root/first/{$firstRequestTime}", $secondRequest->getLastLogFile(), "The log should be from the first request's folder and time");
+    }
+
+    /**
+     * In this request class, the log folder uses a value that is not present in the cache key
+     */
+    protected function mockRequestWithLogMutatingFolder(string $subfolder): ConcreteRequest
+    {
+        return new class($subfolder) extends ConcreteRequest {
+            use ParseResponseJSON;
+            public function __construct(public string $subfolder)
+            {
+            }
+
+            protected bool $shouldLog = true;
+
+            public function getLogFolder(): string
+            {
+                return 'root/' . $this->subfolder;
+            }
+        };
+    }
+
     public function testDontCacheErrorStatus(): void
     {
         $firstRequest = $this->mockRequestWithPostProcessor();
@@ -792,9 +835,44 @@ public function testCacheStructureCanary(): void
             3 => '1.1',
             4 => 'OK',
         ], $cached['response']);
-        self::assertSame([ "2018-01-01T00:00:00.000000+00:00" ], $cached['logs']);
+        self::assertSame([ "one/two/2018-01-01T00:00:00.000000+00:00" ], $cached['logs']);
 
         // If you modified this test in any way, you need to change the CACHE_KEY_SEED
         self::assertSame('v2024.8.6', AbstractRequest::CACHE_KEY_SEED);
     }
+
+    /**
+     * https://github.com/carsdotcom/php-request-class/issues/19
+     * Shim for cached results <=1.4.0 that have a filename but no folder
+     * This won't work as well as modern folder-inclusive storage,
+     * but it's consistent with the old behavior, so better than nothing
+     */
+    public function testCacheLogFilesNotFolders(): void
+    {
+        $firstLogTime = '2018-01-01T00:00:00.000000+00:00';
+        Carbon::setTestNow($firstLogTime);
+
+        $this->mockGuzzleWithTapper()->addMatchBody('POST', '/awesome/', '{"awesome":"sauce"}');
+        $request = $this->mockRequestWithLog();
+        Cache::tags([])->put($request->cacheKey(), [
+            'logs' => [ "2018-01-01T00:00:00.000000+00:00" ],
+            'response' => [
+                0 => 200,
+                1 => [],
+                2 => '{"awesome":"sauce"}',
+                3 => '1.1',
+                4 => 'OK',
+            ]
+        ]);
+
+        $request->setReadCache(true)->sync();
+        self::assertTrue($request->isFromCache());
+
+        self::assertSame(
+            [ "one/two/2018-01-01T00:00:00.000000+00:00" ],
+            getProperty($request, 'sentLogs'),
+            "Shim is compartmentalized to cache rehydration, private prop is set in the standard way"
+        );
+        self::assertSame("one/two/2018-01-01T00:00:00.000000+00:00" , $request->getLastLogFile());
+    }
 }