feat: Request classes can customize the LogFile class

.prettierrc

@@ -4,5 +4,6 @@
     "singleQuote": true,
     "bracketSpacing": true,
     "trailingComma": "es5",
-    "phpVersion": "8.0"
+    "plugins": ["@prettier/plugin-php"],
+    "phpVersion": "8.2"
 }

app/AbstractRequest.php

@@ -13,6 +13,8 @@
  * Children MAY implement getLogFolder() to indicate where logs should be stored (if omitted, logging is disabled)
  * Children MAY implement postProcess() to turn the parsed response body into a more useful format for our application
  */
+declare(strict_types=1);
+
 namespace Carsdotcom\ApiRequest;
 
 use Carbon\CarbonInterval;
@@ -430,7 +432,7 @@ public function log($outcome): void
         if (!$this->shouldLog) {
             return;
         }
-        $logged = LogFile::put($this->getLogFolder(), [$this->toGuzzle(), $outcome, $this->requestStats]);
+        $logged = $this->getLogFileHelper()::put($this->getLogFolder(), [$this->toGuzzle(), $outcome, $this->requestStats]);
         if ($logged) {
             $this->sentLogs[] = $logged;
         }
@@ -442,7 +444,7 @@ public function log($outcome): void
      */
     public function getLastLogContents(): string
     {
-        return LogFile::disk()->get($this->getLastLogFile());
+        return $this->getLogFileHelper()::disk()->get($this->getLastLogFile());
     }
 
     /**
@@ -512,4 +514,12 @@ public function setTimeout(CarbonInterval $interval): void
         $this->guzzleOptions[RequestOptions::TIMEOUT] = $interval->totalSeconds;
     }
 
+    /**
+     * Implementers can override this method to return a customized LogFile class, e.g. with improved formatting, or including different headers in the log
+     */
+    public function getLogFileHelper(): LogFile
+    {
+        return new LogFile();
+    }
+
 }

app/LogFile.php

@@ -7,6 +7,7 @@
  *
  * If you're thoughtful about building metadata into the log folder structure, it's easy to build a GUI to list and retrieve those logs
  */
+declare(strict_types=1);
 
 namespace Carsdotcom\ApiRequest;
 
@@ -49,14 +50,14 @@ public static function put(string $folder, array $contents): ?string
         // Make sure folder ends with a /
         $folder = Str::finish($folder, '/');
 
-        $filename = Carbon::now()->format(self::NAME_FORMAT);
+        $filename = Carbon::now()->format(static::NAME_FORMAT);
 
         // You can pass in objects and we'll stringify them in the most awesome way we know how (or fall back to JSON)
-        $lines = array_map(self::stringify_body(...), $contents);
+        $lines = array_map(static::stringify_body(...), $contents);
         $file_body = implode("\n\n", $lines) . "\n";
 
         try {
-            self::disk()->put($folder . $filename, $file_body);
+            static::disk()->put($folder . $filename, $file_body);
             return $filename;
         } catch (\Exception $e) {
             // An error in the logging server ABSOLUTELY MAY NOT halt normal operations
@@ -80,35 +81,45 @@ public static function stringify_body($body): string
         if (is_string($body)) {
             return $body;
         } elseif ($body instanceof Request) {
-            $string = $body->getMethod() . ' ' . $body->getUri();
+            $string = $body->getMethod() . ' ' . $body->getUri() . "\n\n";
+            if (!empty($requestHeaders = static::interestingRequestHeaders($body))) {
+                $string .= "Request headers include:\n";
+                foreach ($requestHeaders as $key => $values) {
+                    foreach ($values as $value) {
+                        $string .= "$key: $value\n";
+                    }
+                }
+                $string .= "\n";
+            }
+
             $request_body = (string) $body->getBody();
             if ($request_body) {
-                $string .= "\n\n" . self::beautifyIfJson($request_body);
+                $string .= static::beautifyIfJson($request_body);
             }
-            return $string;
+            return trim($string);
         } elseif ($body instanceof Response) {
             $string = 'Response Status Code ' . $body->getStatusCode() . "\n\n";
-            if (self::interestingResponseHeaders($body)) {
+            if (!empty($responseHeaders = static::interestingResponseHeaders($body))) {
                 $string .= "Response headers include:\n";
-                foreach (self::interestingResponseHeaders($body) as $key => $values) {
+                foreach ($responseHeaders as $key => $values) {
                     foreach ($values as $value) {
                         $string .= "$key: $value\n";
                     }
                 }
                 $string .= "\n";
             }
-            $response_body = $body->getBody(true);
+            $response_body = (string) $body->getBody();
             if (empty($response_body)) {
                 $string .= 'Empty Response Body';
             } else {
-                $string .= self::beautifyIfJson($response_body);
+                $string .= static::beautifyIfJson($response_body);
             }
             return $string;
         } elseif ($body instanceof RequestException) {
             $string = 'Request Exception: ' . $body->getMessage();
 
             if ($body->hasResponse()) {
-                $string .= "\n\n" . self::stringify_body($body->getResponse());
+                $string .= "\n\n" . static::stringify_body($body->getResponse());
             }
             return $string;
         } elseif ($body instanceof \Throwable) {
@@ -126,10 +137,26 @@ public static function stringify_body($body): string
         return json_encode($body, flags: JSON_THROW_ON_ERROR);
     }
 
+    /**
+     * Any AbstractRequest can implement a descendent of this class and override this method to start logging interesting Response headers
+     * We log no headers by default because in most requests it's a combination of staggeringly boring (Accept, Content-Type)
+     * and must-not-be-logged (Authentication)
+     *
+     * But we do have partners who put vital information in headers (trace-id, x-ciq-request-id) so implementers
+     * can make a thoughtful choice to log headers appropriate to be logged.
+     */
     public static function interestingResponseHeaders(Response $response): array
     {
-        $allHeaders = $response->getHeaders();
-        return Arr::only($allHeaders, ['x-ciq-request-id']);
+        return []; // Log no headers by default
+        // A typical implementation may look like:
+        // return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
+    }
+
+    public static function interestingRequestHeaders(Request $request): array
+    {
+        return []; // Log no headers by default
+        // A typical implementation may look like:
+        // return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
     }
 
     /**
@@ -153,21 +180,21 @@ public static function beautifyIfJson(string $string): string
      */
     public static function files_like(string $path, string $regex): Collection
     {
-        return collect(self::disk()->allFiles($path))
+        return collect(static::disk()->allFiles($path))
             ->filter(function ($filename) use ($regex) {
                 return preg_match($regex, $filename);
             })
             ->values(); //Re-index, helps JSON stringifying
     }
 
     /**
-     * Given a path in self::disk(), return all files' basename
+     * Given a path in static::disk(), return all files' basename
      * @param string $folder
      * @return Collection
      */
     public static function filesInFolder(string $folder): Collection
     {
-        return collect(LogFile::disk()->files($folder))->map(function ($full_name) {
+        return collect(static::disk()->files($folder))->map(function ($full_name) {
             return basename($full_name);
         });
     }

tests/Feature/LogFileTest.php

@@ -0,0 +1,149 @@
+<?php
+/**
+ * Unit test the LogFile class and its interaction with AbstractRequest
+ */
+declare(strict_types=1);
+
+namespace Tests\Feature;
+
+use Carbon\Carbon;
+use Carsdotcom\ApiRequest\LogFile;
+use Carsdotcom\ApiRequest\Testing\MocksGuzzleInstance;
+use Carsdotcom\ApiRequest\Testing\RequestClassAssertions;
+use GuzzleHttp\Psr7\Request;
+use GuzzleHttp\Psr7\Response;
+use Illuminate\Support\Arr;
+use Illuminate\Support\Facades\Storage;
+use Tests\BaseTestCase;
+use Tests\MockClasses\ConcreteRequest;
+
+class LogFileTest extends BaseTestCase
+{
+    use MocksGuzzleInstance;
+    use RequestClassAssertions;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        Storage::fake('api-logs');
+    }
+
+    public function testLogsNoRequestHeadersByDefault(): void
+    {
+        $request = new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
+        $interesting = LogFile::interestingRequestHeaders($request);
+        self::assertSame([], $interesting);
+        $string = LogFile::stringify_body($request);
+        self::assertStringNotContainsString('SuperSecret', $string);
+        self::assertStringNotContainsString('0098b8f2', LogFile::stringify_body($request));
+    }
+
+    public function testLogsRequestedRequestHeaders(): void
+    {
+        $logFile = new class extends LogFile {
+            public static function interestingRequestHeaders(Request $request): array
+            {
+                return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
+            }
+        };
+        $request = new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
+        $interesting = $logFile::interestingRequestHeaders($request);
+        self::assertSame(['x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], $interesting);
+        $string = $logFile::stringify_body($request);
+        self::assertStringNotContainsString('SuperSecret', $string);
+        self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $string);
+    }
+
+    public function testRequestHeadersInRequestClass(): void
+    {
+        $this->mockGuzzleWithTapper();
+        $this->tapper->addMatchBody('GET', '/.*?/', '{"awesome":"sauce"}');
+
+        $request = new class extends ConcreteRequest{
+            protected bool $shouldLog = true;
+
+            public function getLogFolder(): string
+            {
+                return 'loggy';
+            }
+
+            public function toGuzzle(): Request
+            {
+                return new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
+            }
+
+            public function getLogFileHelper(): LogFile
+            {
+                return new class extends LogFile {
+                    public static function interestingRequestHeaders(Request $request): array
+                    {
+                        return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
+                    }
+                };
+            }
+        };
+        $firstLogTime = '2018-01-01T00:00:00.000000+00:00';
+        Carbon::setTestNow($firstLogTime);
+        $request->sync();
+
+        self::assertStringNotContainsString('SuperSecret', $request->getLastLogContents());
+        self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $request->getLastLogContents());
+    }
+
+    public function testLogsNoResponseHeadersByDefault(): void
+    {
+        $response = new Response(200, headers: ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
+        self::assertSame([], LogFile::interestingResponseHeaders($response));
+        self::assertStringNotContainsString('SuperSecret', LogFile::stringify_body($response));
+        self::assertStringNotContainsString('0098b8f2', LogFile::stringify_body($response));
+    }
+
+    public function testLogsRequestedResponseHeaders(): void
+    {
+        $logFile = new class extends LogFile {
+            public static function interestingResponseHeaders(Response $response): array
+            {
+                return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
+            }
+        };
+        $response = new Response(200, headers: ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
+        $interesting = $logFile::interestingResponseHeaders($response);
+        self::assertSame(['x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], $interesting);
+        $string = $logFile::stringify_body($response);
+        self::assertStringNotContainsString('SuperSecret', $string);
+        self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $string);
+    }
+
+    public function testResponseHeadersInRequestClass(): void
+    {
+        $this->mockGuzzleWithTapper();
+        $this->tapper->addMatch(
+            'POST',
+            '/.*?/',
+            new Response(200,['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], '{"awesome":"sauce"}')
+        );
+
+        $request = new class extends ConcreteRequest{
+            protected bool $shouldLog = true;
+
+            public function getLogFolder(): string
+            {
+                return 'loggy';
+            }
+
+            public function getLogFileHelper(): LogFile
+            {
+                return new class extends LogFile {
+                    public static function interestingResponseHeaders(Response $response): array
+                    {
+                        return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
+                    }
+                };
+            }
+        };
+        $request->sync();
+
+        self::assertStringNotContainsString('SuperSecret', $request->getLastLogContents());
+        self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $request->getLastLogContents());
+    }
+}