Skip to content

Commit a085746

Browse files
bbenebbene
authored
Merge pull request #16 from carsdotcom/overridable-log-file
feat: Request classes can customize the LogFile class
2 parents 926581a + cf35b89 commit a085746

4 files changed

Lines changed: 206 additions & 19 deletions

File tree

‎.prettierrc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44
"singleQuote": true,
55
"bracketSpacing": true,
66
"trailingComma": "es5",
7-
"phpVersion": "8.0"
7+
"plugins": ["@prettier/plugin-php"],
8+
"phpVersion": "8.2"
89
}

‎app/AbstractRequest.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* Children MAY implement getLogFolder() to indicate where logs should be stored (if omitted, logging is disabled)
1414
* Children MAY implement postProcess() to turn the parsed response body into a more useful format for our application
1515
*/
16+
declare(strict_types=1);
17+
1618
namespace Carsdotcom\ApiRequest;
1719

1820
use Carbon\CarbonInterval;
@@ -430,7 +432,7 @@ public function log($outcome): void
430432
if (!$this->shouldLog) {
431433
return;
432434
}
433-
$logged = LogFile::put($this->getLogFolder(), [$this->toGuzzle(), $outcome, $this->requestStats]);
435+
$logged = $this->getLogFileHelper()::put($this->getLogFolder(), [$this->toGuzzle(), $outcome, $this->requestStats]);
434436
if ($logged) {
435437
$this->sentLogs[] = $logged;
436438
}
@@ -442,7 +444,7 @@ public function log($outcome): void
442444
*/
443445
public function getLastLogContents(): string
444446
{
445-
return LogFile::disk()->get($this->getLastLogFile());
447+
return $this->getLogFileHelper()::disk()->get($this->getLastLogFile());
446448
}
447449

448450
/**
@@ -512,4 +514,12 @@ public function setTimeout(CarbonInterval $interval): void
512514
$this->guzzleOptions[RequestOptions::TIMEOUT] = $interval->totalSeconds;
513515
}
514516

517+
/**
518+
* Implementers can override this method to return a customized LogFile class, e.g. with improved formatting, or including different headers in the log
519+
*/
520+
public function getLogFileHelper(): LogFile
521+
{
522+
return new LogFile();
523+
}
524+
515525
}

‎app/LogFile.php‎

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*
88
* 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
99
*/
10+
declare(strict_types=1);
1011

1112
namespace Carsdotcom\ApiRequest;
1213

@@ -49,14 +50,14 @@ public static function put(string $folder, array $contents): ?string
4950
// Make sure folder ends with a /
5051
$folder = Str::finish($folder, '/');
5152

52-
$filename = Carbon::now()->format(self::NAME_FORMAT);
53+
$filename = Carbon::now()->format(static::NAME_FORMAT);
5354

5455
// You can pass in objects and we'll stringify them in the most awesome way we know how (or fall back to JSON)
55-
$lines = array_map(self::stringify_body(...), $contents);
56+
$lines = array_map(static::stringify_body(...), $contents);
5657
$file_body = implode("\n\n", $lines) . "\n";
5758

5859
try {
59-
self::disk()->put($folder . $filename, $file_body);
60+
static::disk()->put($folder . $filename, $file_body);
6061
return $filename;
6162
} catch (\Exception $e) {
6263
// An error in the logging server ABSOLUTELY MAY NOT halt normal operations
@@ -80,35 +81,45 @@ public static function stringify_body($body): string
8081
if (is_string($body)) {
8182
return $body;
8283
} elseif ($body instanceof Request) {
83-
$string = $body->getMethod() . ' ' . $body->getUri();
84+
$string = $body->getMethod() . ' ' . $body->getUri() . "\n\n";
85+
if (!empty($requestHeaders = static::interestingRequestHeaders($body))) {
86+
$string .= "Request headers include:\n";
87+
foreach ($requestHeaders as $key => $values) {
88+
foreach ($values as $value) {
89+
$string .= "$key: $value\n";
90+
}
91+
}
92+
$string .= "\n";
93+
}
94+
8495
$request_body = (string) $body->getBody();
8596
if ($request_body) {
86-
$string .= "\n\n" . self::beautifyIfJson($request_body);
97+
$string .= static::beautifyIfJson($request_body);
8798
}
88-
return $string;
99+
return trim($string);
89100
} elseif ($body instanceof Response) {
90101
$string = 'Response Status Code ' . $body->getStatusCode() . "\n\n";
91-
if (self::interestingResponseHeaders($body)) {
102+
if (!empty($responseHeaders = static::interestingResponseHeaders($body))) {
92103
$string .= "Response headers include:\n";
93-
foreach (self::interestingResponseHeaders($body) as $key => $values) {
104+
foreach ($responseHeaders as $key => $values) {
94105
foreach ($values as $value) {
95106
$string .= "$key: $value\n";
96107
}
97108
}
98109
$string .= "\n";
99110
}
100-
$response_body = $body->getBody(true);
111+
$response_body = (string) $body->getBody();
101112
if (empty($response_body)) {
102113
$string .= 'Empty Response Body';
103114
} else {
104-
$string .= self::beautifyIfJson($response_body);
115+
$string .= static::beautifyIfJson($response_body);
105116
}
106117
return $string;
107118
} elseif ($body instanceof RequestException) {
108119
$string = 'Request Exception: ' . $body->getMessage();
109120

110121
if ($body->hasResponse()) {
111-
$string .= "\n\n" . self::stringify_body($body->getResponse());
122+
$string .= "\n\n" . static::stringify_body($body->getResponse());
112123
}
113124
return $string;
114125
} elseif ($body instanceof \Throwable) {
@@ -126,10 +137,26 @@ public static function stringify_body($body): string
126137
return json_encode($body, flags: JSON_THROW_ON_ERROR);
127138
}
128139

140+
/**
141+
* Any AbstractRequest can implement a descendent of this class and override this method to start logging interesting Response headers
142+
* We log no headers by default because in most requests it's a combination of staggeringly boring (Accept, Content-Type)
143+
* and must-not-be-logged (Authentication)
144+
*
145+
* But we do have partners who put vital information in headers (trace-id, x-ciq-request-id) so implementers
146+
* can make a thoughtful choice to log headers appropriate to be logged.
147+
*/
129148
public static function interestingResponseHeaders(Response $response): array
130149
{
131-
$allHeaders = $response->getHeaders();
132-
return Arr::only($allHeaders, ['x-ciq-request-id']);
150+
return []; // Log no headers by default
151+
// A typical implementation may look like:
152+
// return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
153+
}
154+
155+
public static function interestingRequestHeaders(Request $request): array
156+
{
157+
return []; // Log no headers by default
158+
// A typical implementation may look like:
159+
// return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
133160
}
134161

135162
/**
@@ -153,21 +180,21 @@ public static function beautifyIfJson(string $string): string
153180
*/
154181
public static function files_like(string $path, string $regex): Collection
155182
{
156-
return collect(self::disk()->allFiles($path))
183+
return collect(static::disk()->allFiles($path))
157184
->filter(function ($filename) use ($regex) {
158185
return preg_match($regex, $filename);
159186
})
160187
->values(); //Re-index, helps JSON stringifying
161188
}
162189

163190
/**
164-
* Given a path in self::disk(), return all files' basename
191+
* Given a path in static::disk(), return all files' basename
165192
* @param string $folder
166193
* @return Collection
167194
*/
168195
public static function filesInFolder(string $folder): Collection
169196
{
170-
return collect(LogFile::disk()->files($folder))->map(function ($full_name) {
197+
return collect(static::disk()->files($folder))->map(function ($full_name) {
171198
return basename($full_name);
172199
});
173200
}

‎tests/Feature/LogFileTest.php‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
<?php
2+
/**
3+
* Unit test the LogFile class and its interaction with AbstractRequest
4+
*/
5+
declare(strict_types=1);
6+
7+
namespace Tests\Feature;
8+
9+
use Carbon\Carbon;
10+
use Carsdotcom\ApiRequest\LogFile;
11+
use Carsdotcom\ApiRequest\Testing\MocksGuzzleInstance;
12+
use Carsdotcom\ApiRequest\Testing\RequestClassAssertions;
13+
use GuzzleHttp\Psr7\Request;
14+
use GuzzleHttp\Psr7\Response;
15+
use Illuminate\Support\Arr;
16+
use Illuminate\Support\Facades\Storage;
17+
use Tests\BaseTestCase;
18+
use Tests\MockClasses\ConcreteRequest;
19+
20+
class LogFileTest extends BaseTestCase
21+
{
22+
use MocksGuzzleInstance;
23+
use RequestClassAssertions;
24+
25+
protected function setUp(): void
26+
{
27+
parent::setUp();
28+
Storage::fake('api-logs');
29+
}
30+
31+
public function testLogsNoRequestHeadersByDefault(): void
32+
{
33+
$request = new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
34+
$interesting = LogFile::interestingRequestHeaders($request);
35+
self::assertSame([], $interesting);
36+
$string = LogFile::stringify_body($request);
37+
self::assertStringNotContainsString('SuperSecret', $string);
38+
self::assertStringNotContainsString('0098b8f2', LogFile::stringify_body($request));
39+
}
40+
41+
public function testLogsRequestedRequestHeaders(): void
42+
{
43+
$logFile = new class extends LogFile {
44+
public static function interestingRequestHeaders(Request $request): array
45+
{
46+
return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
47+
}
48+
};
49+
$request = new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
50+
$interesting = $logFile::interestingRequestHeaders($request);
51+
self::assertSame(['x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], $interesting);
52+
$string = $logFile::stringify_body($request);
53+
self::assertStringNotContainsString('SuperSecret', $string);
54+
self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $string);
55+
}
56+
57+
public function testRequestHeadersInRequestClass(): void
58+
{
59+
$this->mockGuzzleWithTapper();
60+
$this->tapper->addMatchBody('GET', '/.*?/', '{"awesome":"sauce"}');
61+
62+
$request = new class extends ConcreteRequest{
63+
protected bool $shouldLog = true;
64+
65+
public function getLogFolder(): string
66+
{
67+
return 'loggy';
68+
}
69+
70+
public function toGuzzle(): Request
71+
{
72+
return new Request('GET', 'https://example.com', ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
73+
}
74+
75+
public function getLogFileHelper(): LogFile
76+
{
77+
return new class extends LogFile {
78+
public static function interestingRequestHeaders(Request $request): array
79+
{
80+
return Arr::only($request->getHeaders(), ['x-ciq-request-id']);
81+
}
82+
};
83+
}
84+
};
85+
$firstLogTime = '2018-01-01T00:00:00.000000+00:00';
86+
Carbon::setTestNow($firstLogTime);
87+
$request->sync();
88+
89+
self::assertStringNotContainsString('SuperSecret', $request->getLastLogContents());
90+
self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $request->getLastLogContents());
91+
}
92+
93+
public function testLogsNoResponseHeadersByDefault(): void
94+
{
95+
$response = new Response(200, headers: ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
96+
self::assertSame([], LogFile::interestingResponseHeaders($response));
97+
self::assertStringNotContainsString('SuperSecret', LogFile::stringify_body($response));
98+
self::assertStringNotContainsString('0098b8f2', LogFile::stringify_body($response));
99+
}
100+
101+
public function testLogsRequestedResponseHeaders(): void
102+
{
103+
$logFile = new class extends LogFile {
104+
public static function interestingResponseHeaders(Response $response): array
105+
{
106+
return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
107+
}
108+
};
109+
$response = new Response(200, headers: ['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']]);
110+
$interesting = $logFile::interestingResponseHeaders($response);
111+
self::assertSame(['x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], $interesting);
112+
$string = $logFile::stringify_body($response);
113+
self::assertStringNotContainsString('SuperSecret', $string);
114+
self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $string);
115+
}
116+
117+
public function testResponseHeadersInRequestClass(): void
118+
{
119+
$this->mockGuzzleWithTapper();
120+
$this->tapper->addMatch(
121+
'POST',
122+
'/.*?/',
123+
new Response(200,['Authentication' => ['Bearer SuperSecret'], 'x-ciq-request-id' => ['0098b8f2-fd78-4e13-afb8-ce655b29bc34']], '{"awesome":"sauce"}')
124+
);
125+
126+
$request = new class extends ConcreteRequest{
127+
protected bool $shouldLog = true;
128+
129+
public function getLogFolder(): string
130+
{
131+
return 'loggy';
132+
}
133+
134+
public function getLogFileHelper(): LogFile
135+
{
136+
return new class extends LogFile {
137+
public static function interestingResponseHeaders(Response $response): array
138+
{
139+
return Arr::only($response->getHeaders(), ['x-ciq-request-id']);
140+
}
141+
};
142+
}
143+
};
144+
$request->sync();
145+
146+
self::assertStringNotContainsString('SuperSecret', $request->getLastLogContents());
147+
self::assertStringContainsString("x-ciq-request-id: 0098b8f2-fd78-4e13-afb8-ce655b29bc34", $request->getLastLogContents());
148+
}
149+
}

0 commit comments

Comments
 (0)