diff --git a/.prettierrc b/.prettierrc index c589811..c1c218b 100644 --- a/.prettierrc +++ b/.prettierrc @@ -4,5 +4,6 @@ "singleQuote": true, "bracketSpacing": true, "trailingComma": "es5", - "phpVersion": "8.0" + "plugins": ["@prettier/plugin-php"], + "phpVersion": "8.2" } diff --git a/app/AbstractRequest.php b/app/AbstractRequest.php index f8b4ff7..d641a6c 100644 --- a/app/AbstractRequest.php +++ b/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(); + } + } diff --git a/app/LogFile.php b/app/LogFile.php index a3dd204..0557ad5 100644 --- a/app/LogFile.php +++ b/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,7 +180,7 @@ 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); }) @@ -161,13 +188,13 @@ public static function files_like(string $path, string $regex): Collection } /** - * 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); }); } diff --git a/tests/Feature/LogFileTest.php b/tests/Feature/LogFileTest.php new file mode 100644 index 0000000..4718fd5 --- /dev/null +++ b/tests/Feature/LogFileTest.php @@ -0,0 +1,149 @@ + ['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()); + } +}