diff --git a/bin/console.php b/bin/console.php index f1e2e6ba..25a6504c 100755 --- a/bin/console.php +++ b/bin/console.php @@ -37,6 +37,9 @@ use Laminas\AutomaticReleases\Github\JwageGenerateChangelog; use Laminas\AutomaticReleases\Github\MergeMultipleReleaseNotes; use Laminas\AutomaticReleases\Gpg\ImportGpgKeyFromStringViaTemporaryFile; +use Laminas\AutomaticReleases\HttpClient\LoggingHttpClient; +use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings; +use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpResponsesIntoStrings; use Lcobucci\Clock\SystemClock; use Monolog\Handler\StreamHandler; use Monolog\Logger; @@ -60,14 +63,20 @@ static function (int $errorCode, string $message = '', string $file = '', int $l E_STRICT | E_NOTICE | E_WARNING, ); - $variables = EnvironmentVariables::fromEnvironment(new ImportGpgKeyFromStringViaTemporaryFile()); - $logger = new Logger('automatic-releases'); - $logger->pushHandler(new StreamHandler(STDERR, $variables->logLevel())); + $variables = EnvironmentVariables::fromEnvironment(new ImportGpgKeyFromStringViaTemporaryFile()); + $logger = new Logger( + 'automatic-releases', + [new StreamHandler(STDERR, $variables->logLevel())], + [ + new ConvertLogContextHttpRequestsIntoStrings(), + new ConvertLogContextHttpResponsesIntoStrings(), + ], + ); $loadEvent = new LoadCurrentGithubEventFromGithubActionPath($variables); $fetch = new FetchAndSetCurrentUserByReplacingCurrentOriginRemote($variables); $getCandidateBranches = new GetMergeTargetCandidateBranchesFromRemoteBranches(); $makeRequests = Psr17FactoryDiscovery::findRequestFactory(); - $httpClient = HttpClientDiscovery::find(); + $httpClient = new LoggingHttpClient(HttpClientDiscovery::find(), $logger); $githubToken = $variables->githubToken(); $getMilestone = new GetMilestoneFirst100IssuesAndPullRequests(new RunGraphQLQuery( $makeRequests, diff --git a/infection.json.dist b/infection.json.dist index 8acadb61..3bafc36e 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -12,6 +12,6 @@ "mutators": { "@default": true }, - "minMsi": 97, + "minMsi": 98, "minCoveredMsi": 100 } diff --git a/psalm.xml.dist b/psalm.xml.dist index 6cbbbd44..419701dc 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -15,19 +15,6 @@ - - - - - - - - - - - - - diff --git a/src/Changelog/ChangelogReleaseNotes.php b/src/Changelog/ChangelogReleaseNotes.php index 711df2c1..9c188d03 100644 --- a/src/Changelog/ChangelogReleaseNotes.php +++ b/src/Changelog/ChangelogReleaseNotes.php @@ -58,8 +58,7 @@ public function merge(self $next): self { if ($this->changelogEntry && $next->changelogEntry) { throw new RuntimeException( - 'Aborting: Both current release notes and next contain a ChangelogEntry;' - . ' only one CreateReleaseText implementation should resolve one.', + 'Aborting: Both current release notes and next contain a ChangelogEntry; only one CreateReleaseText implementation should resolve one.', ); } diff --git a/src/Git/Value/MergeTargetCandidateBranches.php b/src/Git/Value/MergeTargetCandidateBranches.php index 1df65e22..cec01cd0 100644 --- a/src/Git/Value/MergeTargetCandidateBranches.php +++ b/src/Git/Value/MergeTargetCandidateBranches.php @@ -26,9 +26,7 @@ public static function fromAllBranches(BranchName ...$branches): self return $branch->isReleaseBranch(); }); - $mergeTargetBranches = Vec\sort($mergeTargetBranches, static function (BranchName $a, BranchName $b): int { - return $a->majorAndMinor() <=> $b->majorAndMinor(); - }); + $mergeTargetBranches = Vec\sort($mergeTargetBranches, self::branchOrder(...)); return new self($mergeTargetBranches); } @@ -98,4 +96,10 @@ public function contains(BranchName $needle): bool static fn (BranchName $branch): bool => $needle->equals($branch) ); } + + /** @return -1|0|1 */ + private static function branchOrder(BranchName $a, BranchName $b): int + { + return $a->majorAndMinor() <=> $b->majorAndMinor(); + } } diff --git a/src/Github/CreateReleaseTextViaKeepAChangelog.php b/src/Github/CreateReleaseTextViaKeepAChangelog.php index d19ce5e9..a34380f8 100644 --- a/src/Github/CreateReleaseTextViaKeepAChangelog.php +++ b/src/Github/CreateReleaseTextViaKeepAChangelog.php @@ -130,17 +130,20 @@ private function updateReleaseDate(string $changelog, string $version): string */ private function removeDefaultContents(string $changelog): string { - $contents = Iter\reduce( + return Type\non_empty_string()->assert(Iter\reduce( self::DEFAULT_SECTIONS, - static fn (string $changelog, string $section): string => Regex\replace( - $changelog, - "/\n\#{3} " . $section . "\n\n- Nothing.\n/s", - '', - ), + self::removeEmptyDefaultChangelogSection(...), $changelog, - ); + )); + } - return Type\non_empty_string()->assert($contents); + private static function removeEmptyDefaultChangelogSection(string $changelog, string $section): string + { + return Regex\replace( + $changelog, + "/\n\#{3} " . $section . "\n\n- Nothing.\n/s", + '', + ); } /** diff --git a/src/HttpClient/LoggingHttpClient.php b/src/HttpClient/LoggingHttpClient.php new file mode 100644 index 00000000..017cdb1d --- /dev/null +++ b/src/HttpClient/LoggingHttpClient.php @@ -0,0 +1,35 @@ +logger->debug('Sending request {request}', ['request' => $request]); + + $response = $this->next->sendRequest($request); + + $this->logger->debug( + 'Received response {response} to request {request}', + [ + 'request' => $request, + 'response' => $response, + ], + ); + + return $response; + } +} diff --git a/src/Monolog/ConvertLogContextHttpRequestsIntoStrings.php b/src/Monolog/ConvertLogContextHttpRequestsIntoStrings.php new file mode 100644 index 00000000..329d307f --- /dev/null +++ b/src/Monolog/ConvertLogContextHttpRequestsIntoStrings.php @@ -0,0 +1,42 @@ +datetime, + $record->channel, + $record->level, + $record->message, + array_map(self::contextItemToMessage(...), $record->context), + $record->extra, + $record->formatted, + ); + } + + private static function contextItemToMessage(mixed $item): mixed + { + if (! $item instanceof RequestInterface) { + return $item; + } + + return $item->getMethod() + . ' ' + . $item + ->getUri() + ->withUserInfo('') + ->__toString(); + } +} diff --git a/src/Monolog/ConvertLogContextHttpResponsesIntoStrings.php b/src/Monolog/ConvertLogContextHttpResponsesIntoStrings.php new file mode 100644 index 00000000..9987a16e --- /dev/null +++ b/src/Monolog/ConvertLogContextHttpResponsesIntoStrings.php @@ -0,0 +1,42 @@ +datetime, + $record->channel, + $record->level, + $record->message, + array_map(self::contextItemToMessage(...), $record->context), + $record->extra, + $record->formatted, + ); + } + + private static function contextItemToMessage(mixed $item): mixed + { + if (! $item instanceof ResponseInterface) { + return $item; + } + + return $item->getStatusCode() + . ' "' + . $item + ->getBody() + ->__toString() + . '"'; + } +} diff --git a/test/unit/HttpClient/LoggingHttpClientTest.php b/test/unit/HttpClient/LoggingHttpClientTest.php new file mode 100644 index 00000000..801152c7 --- /dev/null +++ b/test/unit/HttpClient/LoggingHttpClientTest.php @@ -0,0 +1,51 @@ +createRequest('get', 'http://example.com/foo/bar'); + $response = Psr17FactoryDiscovery::findResponseFactory()->createResponse(204); + + $response->getBody() + ->write('hello world'); + + $logger = $this->createMock(LoggerInterface::class); + $next = $this->createMock(ClientInterface::class); + + $next->expects(self::once()) + ->method('sendRequest') + ->with($request) + ->willReturn($response); + + $logger->expects(self::exactly(2)) + ->method('debug') + ->withConsecutive( + ['Sending request {request}', ['request' => $request]], + [ + 'Received response {response} to request {request}', + [ + 'request' => $request, + 'response' => $response, + ], + ], + ); + + self::assertSame( + $response, + (new LoggingHttpClient($next, $logger)) + ->sendRequest($request), + ); + } +} diff --git a/test/unit/Monolog/ConvertLogContextHttpRequestsIntoStringsTest.php b/test/unit/Monolog/ConvertLogContextHttpRequestsIntoStringsTest.php new file mode 100644 index 00000000..039cdbd2 --- /dev/null +++ b/test/unit/Monolog/ConvertLogContextHttpRequestsIntoStringsTest.php @@ -0,0 +1,56 @@ +createRequest('GET', 'http://example.com/foo'); + + $sensitiveRequest = $requestFactory->createRequest('POST', 'https://user:pass@example.com/foo?bar=baz') + ->withAddedHeader('Authentication', ['also secret']); + + $sensitiveRequest->getBody() + ->write('super: secret'); + + self::assertEquals( + new LogRecord( + $date, + 'a-channel', + Level::Critical, + 'a message', + [ + 'foo' => 'bar', + 'plain request' => 'GET http://example.com/foo', + 'sensitive request' => 'POST https://example.com/foo?bar=baz', + ], + ), + (new ConvertLogContextHttpRequestsIntoStrings())(new LogRecord( + $date, + 'a-channel', + Level::Critical, + 'a message', + [ + 'foo' => 'bar', + 'plain request' => $plainRequest, + 'sensitive request' => $sensitiveRequest, + ], + )), + ); + } +} diff --git a/test/unit/Monolog/ConvertLogContextHttpResponsesIntoStringsTest.php b/test/unit/Monolog/ConvertLogContextHttpResponsesIntoStringsTest.php new file mode 100644 index 00000000..157eaa84 --- /dev/null +++ b/test/unit/Monolog/ConvertLogContextHttpResponsesIntoStringsTest.php @@ -0,0 +1,56 @@ +createResponse(401); + + $sensitiveResponse = $requestFactory->createResponse(403) + ->withAddedHeader('Super', 'secret'); + + $sensitiveResponse->getBody() + ->write('this should be printed'); + + self::assertEquals( + new LogRecord( + $date, + 'a-channel', + Level::Critical, + 'a message', + [ + 'foo' => 'bar', + 'plain response' => '401 ""', + 'sensitive response' => '403 "this should be printed"', + ], + ), + (new ConvertLogContextHttpResponsesIntoStrings())(new LogRecord( + $date, + 'a-channel', + Level::Critical, + 'a message', + [ + 'foo' => 'bar', + 'plain response' => $plainResponse, + 'sensitive response' => $sensitiveResponse, + ], + )), + ); + } +} diff --git a/test/unit/Monolog/VerifyLoggingIntegrationTest.php b/test/unit/Monolog/VerifyLoggingIntegrationTest.php new file mode 100644 index 00000000..bd65f6d3 --- /dev/null +++ b/test/unit/Monolog/VerifyLoggingIntegrationTest.php @@ -0,0 +1,56 @@ +createRequest('GET', 'http://example.com/foo/bar'); + $response = Psr17FactoryDiscovery::findResponseFactory()->createResponse(204); + + $response->getBody() + ->write('hello world'); + + $stream = fopen('php://memory', 'rwb+'); + + $bufferHandler = new StreamHandler($stream); + + $logger = new Logger( + 'test-logger', + [$bufferHandler], + [ + new ConvertLogContextHttpRequestsIntoStrings(), + new ConvertLogContextHttpResponsesIntoStrings(), + ], + ); + + $logger->debug('message', ['request' => $request, 'response' => $response]); + + rewind($stream); + + self::assertStringContainsString( + ': message {"request":"GET http://example.com/foo/bar","response":"204 \"hello world\""} []', + stream_get_contents($stream), + 'Request and response contents have been serialized into the final log message', + ); + } +}