Merge pull request #211 from laminas/feature/log-http-responses

Log HTTP requests/responses (only non-sensitive data)
laminas · Sep 19, 2022 · 1b61a5f · 1b61a5f
2 parents e5e024e + d458075
commit 1b61a5f
Show file tree

Hide file tree

Showing 13 changed files with 371 additions and 31 deletions.
diff --git 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
@@ -12,6 +12,6 @@
     "mutators": {
         "@default": true
     },
-    "minMsi": 97,
+    "minMsi": 98,
     "minCoveredMsi": 100
 }
diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -15,19 +15,6 @@
         </ignoreFiles>
     </projectFiles>
 
-    <issueHandlers>
-        <InternalMethod>
-            <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::method"/>
-            </errorLevel>
-            <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn"/>
-            </errorLevel>
-            <errorLevel type="suppress">
-                <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::with"/>
-            </errorLevel>
-        </InternalMethod>
-    </issueHandlers>
     <plugins>
         <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
         <pluginClass class="Psl\Psalm\Plugin"/>

diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\HttpClient;
+
+use Psr\Http\Client\ClientInterface;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Log\LoggerInterface;
+
+/** @internal */
+final class LoggingHttpClient implements ClientInterface
+{
+    public function __construct(private readonly ClientInterface $next, private readonly LoggerInterface $logger)
+    {
+    }
+
+    public function sendRequest(RequestInterface $request): ResponseInterface
+    {
+        $this->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
@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Monolog;
+
+use Monolog\LogRecord;
+use Monolog\Processor\ProcessorInterface;
+use Psr\Http\Message\RequestInterface;
+
+use function array_map;
+
+/** @internal */
+final class ConvertLogContextHttpRequestsIntoStrings implements ProcessorInterface
+{
+    public function __invoke(LogRecord $record): LogRecord
+    {
+        return new LogRecord(
+            $record->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
@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Monolog;
+
+use Monolog\LogRecord;
+use Monolog\Processor\ProcessorInterface;
+use Psr\Http\Message\ResponseInterface;
+
+use function array_map;
+
+/** @internal */
+final class ConvertLogContextHttpResponsesIntoStrings implements ProcessorInterface
+{
+    public function __invoke(LogRecord $record): LogRecord
+    {
+        return new LogRecord(
+            $record->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
@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Test\Unit\HttpClient;
+
+use Http\Discovery\Psr17FactoryDiscovery;
+use Laminas\AutomaticReleases\HttpClient\LoggingHttpClient;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Client\ClientInterface;
+use Psr\Log\LoggerInterface;
+
+/** @covers \Laminas\AutomaticReleases\HttpClient\LoggingHttpClient */
+final class LoggingHttpClientTest extends TestCase
+{
+    public function testWillLogRequestAndResponse(): void
+    {
+        $request  = Psr17FactoryDiscovery::findRequestFactory()->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
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\AutomaticReleases\Test\Unit\Monolog;
+
+use DateTimeImmutable;
+use Http\Discovery\Psr17FactoryDiscovery;
+use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings;
+use Monolog\Level;
+use Monolog\LogRecord;
+use PHPUnit\Framework\TestCase;
+
+/** @covers \Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings */
+final class ConvertLogContextHttpRequestsIntoStringsTest extends TestCase
+{
+    public function testWillScrubSensitiveRequestInformation(): void
+    {
+        $date = new DateTimeImmutable();
+
+        $requestFactory = Psr17FactoryDiscovery::findRequestFactory();
+
+        $plainRequest = $requestFactory->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,
+                ],
+            )),
+        );
+    }
+}