From 23e1958b519bac1aa46088dbf678ff97dcfc7713 Mon Sep 17 00:00:00 2001
From: Niklas Keller <me@kelunik.com>
Date: Sun, 2 Jul 2023 16:59:16 +0200
Subject: [PATCH] Turn LogHttpArchive into an event listener instead of
 separate listener and interceptor

---
 README.md                                     |   6 +-
 examples/basic/6-customization.php            |   4 +-
 .../LogHttpArchive.php                        | 180 ++++++++++++++----
 src/EventListener/RecordHarAttributes.php     | 148 --------------
 test/Interceptor/InterceptorTest.php          |   7 +
 ...ArchiveTest.php => LogHttpArchiveTest.php} |   5 +-
 6 files changed, 157 insertions(+), 193 deletions(-)
 rename src/{Interceptor => EventListener}/LogHttpArchive.php (59%)
 delete mode 100644 src/EventListener/RecordHarAttributes.php
 rename test/Interceptor/{LogIntoHttpArchiveTest.php => LogHttpArchiveTest.php} (78%)

diff --git a/README.md b/README.md
index 7f4894de..c9935ed8 100644
--- a/README.md
+++ b/README.md
@@ -262,7 +262,7 @@ See [`amphp/http-client-cookies`](https://github.com/amphp/http-client-cookies).
 
 ### Logging
 
-The `LogHttpArchive` interceptor allows logging all requests / responses including detailed timing information to an [HTTP archive (HAR)](https://en.wikipedia.org/wiki/HAR_%28file_format%29).
+The `LogHttpArchive` event listener allows logging all requests / responses including detailed timing information to an [HTTP archive (HAR)](https://en.wikipedia.org/wiki/HAR_%28file_format%29).
 
 These log files can then be imported into the browsers developer tools or online tools like [HTTP Archive Viewer](http://www.softwareishard.com/har/viewer/) or [Google's HAR Analyzer](https://toolbox.googleapps.com/apps/har_analyzer/).
 
@@ -271,10 +271,10 @@ These log files can then be imported into the browsers developer tools or online
 
 ```php
 use Amp\Http\Client\HttpClientBuilder;
-use Amp\Http\Client\Interceptor\LogHttpArchive;
+use Amp\Http\Client\EventListener\LogHttpArchive;
 
 $httpClient = (new HttpClientBuilder)
-    ->intercept(new LogHttpArchive('/tmp/http-client.har'))
+    ->listen(new LogHttpArchive('/tmp/http-client.har'))
     ->build();
 
 $httpClient->request(...);
diff --git a/examples/basic/6-customization.php b/examples/basic/6-customization.php
index c0cbdb32..5542b897 100644
--- a/examples/basic/6-customization.php
+++ b/examples/basic/6-customization.php
@@ -1,8 +1,8 @@
 <?php declare(strict_types=1);
 
+use Amp\Http\Client\EventListener\LogHttpArchive;
 use Amp\Http\Client\HttpClientBuilder;
 use Amp\Http\Client\HttpException;
-use Amp\Http\Client\Interceptor\LogHttpArchive;
 use Amp\Http\Client\Interceptor\MatchOrigin;
 use Amp\Http\Client\Interceptor\SetRequestHeader;
 use Amp\Http\Client\Request;
@@ -11,7 +11,7 @@
 
 try {
     $client = (new HttpClientBuilder)
-        ->intercept(new LogHttpArchive(__DIR__ . '/log.har'))
+        ->listen(new LogHttpArchive(__DIR__ . '/log.har'))
         ->intercept(new MatchOrigin(['https://amphp.org' => new SetRequestHeader('x-amphp', 'true')]))
         ->followRedirects(0)
         ->retry(3)
diff --git a/src/Interceptor/LogHttpArchive.php b/src/EventListener/LogHttpArchive.php
similarity index 59%
rename from src/Interceptor/LogHttpArchive.php
rename to src/EventListener/LogHttpArchive.php
index 1b1beb3e..ab2ca2e2 100644
--- a/src/Interceptor/LogHttpArchive.php
+++ b/src/EventListener/LogHttpArchive.php
@@ -1,26 +1,31 @@
 <?php declare(strict_types=1);
-/** @noinspection ALL */
 
-namespace Amp\Http\Client\Interceptor;
+namespace Amp\Http\Client\EventListener;
 
-use Amp\Cancellation;
-use Amp\File;
+use Amp\File\File;
 use Amp\File\Filesystem;
+use Amp\File\Whence;
 use Amp\ForbidCloning;
 use Amp\ForbidSerialization;
 use Amp\Http\Client\ApplicationInterceptor;
-use Amp\Http\Client\DelegateHttpClient;
+use Amp\Http\Client\Connection\Connection;
+use Amp\Http\Client\Connection\Stream;
 use Amp\Http\Client\EventListener;
-use Amp\Http\Client\EventListener\RecordHarAttributes;
 use Amp\Http\Client\HttpException;
 use Amp\Http\Client\Internal\HarAttributes;
+use Amp\Http\Client\NetworkInterceptor;
 use Amp\Http\Client\Request;
 use Amp\Http\Client\Response;
 use Amp\Http\HttpMessage;
+use Amp\Socket\InternetAddress;
+use Amp\Socket\TlsInfo;
 use Amp\Sync\LocalMutex;
 use Revolt\EventLoop;
+use function Amp\File\filesystem;
+use function Amp\File\openFile;
+use function Amp\now;
 
-final class LogHttpArchive implements ApplicationInterceptor
+final class LogHttpArchive implements EventListener
 {
     use ForbidCloning;
     use ForbidSerialization;
@@ -139,14 +144,12 @@ private static function formatEntry(Response $response): array
 
     private Filesystem $filesystem;
 
-    private ?File\File $fileHandle = null;
+    private ?File $fileHandle = null;
 
     private string $filePath;
 
     private ?\Throwable $error = null;
 
-    private EventListener $eventListener;
-
     public function __construct(string $filePath, ?Filesystem $filesystem = null)
     {
         if (!\class_exists(Filesystem::class)) {
@@ -155,26 +158,7 @@ public function __construct(string $filePath, ?Filesystem $filesystem = null)
 
         $this->filePath = $filePath;
         $this->fileMutex = new LocalMutex;
-        $this->eventListener = new RecordHarAttributes;
-        $this->filesystem = $filesystem ?? File\filesystem();
-    }
-
-    public function request(
-        Request $request,
-        Cancellation $cancellation,
-        DelegateHttpClient $httpClient
-    ): Response {
-        if ($this->error) {
-            throw $this->error;
-        }
-
-        $this->ensureEventListenerIsRegistered($request);
-
-        $response = $httpClient->request($request, $cancellation);
-
-        EventLoop::queue(fn () => $this->writeLog($response));
-
-        return $response;
+        $this->filesystem = $filesystem ?? filesystem();
     }
 
     public function reset(): void
@@ -208,7 +192,7 @@ private function writeLog(Response $response): void
             $firstEntry = $this->fileHandle === null;
 
             if ($firstEntry) {
-                $this->fileHandle = $fileHandle = File\openFile($this->filePath, 'w');
+                $this->fileHandle = $fileHandle = openFile($this->filePath, 'w');
 
                 $header = '{"log":{"version":"1.2","creator":{"name":"amphp/http-client","version":"4.x"},"pages":[],"entries":[';
 
@@ -218,7 +202,7 @@ private function writeLog(Response $response): void
 
                 \assert($fileHandle !== null);
 
-                $fileHandle->seek(-3, File\Whence::Current);
+                $fileHandle->seek(-3, Whence::Current);
             }
 
             $json = \json_encode(self::formatEntry($response));
@@ -233,14 +217,134 @@ private function writeLog(Response $response): void
         }
     }
 
-    private function ensureEventListenerIsRegistered(Request $request): void
+    public function requestStart(Request $request): void
     {
-        foreach ($request->getEventListeners() as $eventListener) {
-            if ($eventListener instanceof RecordHarAttributes) {
-                return; // user added it manually
-            }
+        if (!$request->hasAttribute(HarAttributes::STARTED_DATE_TIME)) {
+            $request->setAttribute(HarAttributes::STARTED_DATE_TIME, new \DateTimeImmutable);
+        }
+
+        $this->addTiming(HarAttributes::TIME_START, $request);
+    }
+
+    public function connectStart(Request $request): void
+    {
+        $this->addTiming(HarAttributes::TIME_CONNECT, $request);
+    }
+
+    public function requestHeaderStart(Request $request, Stream $stream): void
+    {
+        $address = $stream->getRemoteAddress();
+        $host = match (true) {
+            $address instanceof InternetAddress => $address->getAddress(),
+            default => $address->toString(),
+        };
+        if (\strrpos($host, ':')) {
+            $host = '[' . $host . ']';
+        }
+
+        $request->setAttribute(HarAttributes::SERVER_IP_ADDRESS, $host);
+        $this->addTiming(HarAttributes::TIME_SEND, $request);
+    }
+
+    public function requestBodyEnd(Request $request, Stream $stream): void
+    {
+        $this->addTiming(HarAttributes::TIME_WAIT, $request);
+    }
+
+    public function responseHeaderStart(Request $request, Stream $stream): void
+    {
+        $this->addTiming(HarAttributes::TIME_RECEIVE, $request);
+    }
+
+    public function requestEnd(Request $request, Response $response): void
+    {
+        $this->addTiming(HarAttributes::TIME_COMPLETE, $request);
+
+        EventLoop::queue(fn () => $this->writeLog($response));
+    }
+
+    /**
+     * @param non-empty-string $key
+     */
+    private function addTiming(string $key, Request $request): void
+    {
+        if (!$request->hasAttribute($key)) {
+            $request->setAttribute($key, now());
         }
+    }
+
+    public function requestFailed(Request $request, HttpException $exception): void
+    {
+        // TODO: Log error to archive
+    }
 
-        $request->addEventListener($this->eventListener);
+    public function connectEnd(Request $request, Connection $connection): void
+    {
+        // nothing to do
+    }
+
+    public function tlsHandshakeStart(Request $request): void
+    {
+        $this->addTiming(HarAttributes::TIME_SSL, $request);
+    }
+
+    public function tlsHandshakeEnd(Request $request, TlsInfo $tlsInfo): void
+    {
+        // nothing to do
+    }
+
+    public function requestHeaderEnd(Request $request, Stream $stream): void
+    {
+        // nothing to do
+    }
+
+    public function requestBodyStart(Request $request, Stream $stream): void
+    {
+        // nothing to do
+    }
+
+    public function requestBodyProgress(Request $request, Stream $stream): void
+    {
+        // nothing to do
+    }
+
+    public function responseHeaderEnd(Request $request, Stream $stream, Response $response): void
+    {
+        // nothing to do
+    }
+
+    public function responseBodyStart(Request $request, Stream $stream, Response $response): void
+    {
+        // nothing to do
+    }
+
+    public function responseBodyProgress(Request $request, Stream $stream, Response $response): void
+    {
+        // nothing to do
+    }
+
+    public function responseBodyEnd(Request $request, Stream $stream, Response $response): void
+    {
+        // nothing to do
+    }
+
+    public function applicationInterceptorStart(Request $request, ApplicationInterceptor $interceptor): void
+    {
+        // nothing to do
+    }
+
+    public function applicationInterceptorEnd(Request $request, ApplicationInterceptor $interceptor, Response $response): void
+    {
+        // nothing to do
+    }
+
+    public function networkInterceptorStart(Request $request, NetworkInterceptor $interceptor): void
+    {
+        // nothing to do
+    }
+
+    public function networkInterceptorEnd(Request $request, NetworkInterceptor $interceptor, Response $response): void
+    {
+        // nothing to do
     }
 }
diff --git a/src/EventListener/RecordHarAttributes.php b/src/EventListener/RecordHarAttributes.php
deleted file mode 100644
index b8fda1bf..00000000
--- a/src/EventListener/RecordHarAttributes.php
+++ /dev/null
@@ -1,148 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Amp\Http\Client\EventListener;
-
-use Amp\Http\Client\ApplicationInterceptor;
-use Amp\Http\Client\Connection\Connection;
-use Amp\Http\Client\Connection\Stream;
-use Amp\Http\Client\EventListener;
-use Amp\Http\Client\HttpException;
-use Amp\Http\Client\Internal\HarAttributes;
-use Amp\Http\Client\NetworkInterceptor;
-use Amp\Http\Client\Request;
-use Amp\Http\Client\Response;
-use Amp\Socket\InternetAddress;
-use Amp\Socket\TlsInfo;
-use function Amp\now;
-
-final class RecordHarAttributes implements EventListener
-{
-    public function requestStart(Request $request): void
-    {
-        if (!$request->hasAttribute(HarAttributes::STARTED_DATE_TIME)) {
-            $request->setAttribute(HarAttributes::STARTED_DATE_TIME, new \DateTimeImmutable);
-        }
-
-        $this->addTiming(HarAttributes::TIME_START, $request);
-    }
-
-    public function connectStart(Request $request): void
-    {
-        $this->addTiming(HarAttributes::TIME_CONNECT, $request);
-    }
-
-    public function requestHeaderStart(Request $request, Stream $stream): void
-    {
-        $address = $stream->getRemoteAddress();
-        $host = match (true) {
-            $address instanceof InternetAddress => $address->getAddress(),
-            default => $address->toString(),
-        };
-        if (\strrpos($host, ':')) {
-            $host = '[' . $host . ']';
-        }
-
-        $request->setAttribute(HarAttributes::SERVER_IP_ADDRESS, $host);
-        $this->addTiming(HarAttributes::TIME_SEND, $request);
-    }
-
-    public function requestBodyEnd(Request $request, Stream $stream): void
-    {
-        $this->addTiming(HarAttributes::TIME_WAIT, $request);
-    }
-
-    public function responseHeaderStart(Request $request, Stream $stream): void
-    {
-        $this->addTiming(HarAttributes::TIME_RECEIVE, $request);
-    }
-
-    public function requestEnd(Request $request, Response $response): void
-    {
-        $this->addTiming(HarAttributes::TIME_COMPLETE, $request);
-    }
-
-    /**
-     * @param non-empty-string $key
-     */
-    private function addTiming(string $key, Request $request): void
-    {
-        if (!$request->hasAttribute($key)) {
-            $request->setAttribute($key, now());
-        }
-    }
-
-    public function requestFailed(Request $request, HttpException $exception): void
-    {
-        // nothing to do
-    }
-
-    public function connectEnd(Request $request, Connection $connection): void
-    {
-        // nothing to do
-    }
-
-    public function tlsHandshakeStart(Request $request): void
-    {
-        $this->addTiming(HarAttributes::TIME_SSL, $request);
-    }
-
-    public function tlsHandshakeEnd(Request $request, TlsInfo $tlsInfo): void
-    {
-        // nothing to do
-    }
-
-    public function requestHeaderEnd(Request $request, Stream $stream): void
-    {
-        // nothing to do
-    }
-
-    public function requestBodyStart(Request $request, Stream $stream): void
-    {
-        // nothing to do
-    }
-
-    public function requestBodyProgress(Request $request, Stream $stream): void
-    {
-        // nothing to do
-    }
-
-    public function responseHeaderEnd(Request $request, Stream $stream, Response $response): void
-    {
-        // nothing to do
-    }
-
-    public function responseBodyStart(Request $request, Stream $stream, Response $response): void
-    {
-        // nothing to do
-    }
-
-    public function responseBodyProgress(Request $request, Stream $stream, Response $response): void
-    {
-        // nothing to do
-    }
-
-    public function responseBodyEnd(Request $request, Stream $stream, Response $response): void
-    {
-        // nothing to do
-    }
-
-    public function applicationInterceptorStart(Request $request, ApplicationInterceptor $interceptor): void
-    {
-        // nothing to do
-    }
-
-    public function applicationInterceptorEnd(Request $request, ApplicationInterceptor $interceptor, Response $response): void
-    {
-        // nothing to do
-    }
-
-    public function networkInterceptorStart(Request $request, NetworkInterceptor $interceptor): void
-    {
-        // nothing to do
-    }
-
-    public function networkInterceptorEnd(Request $request, NetworkInterceptor $interceptor, Response $response): void
-    {
-        // nothing to do
-    }
-}
diff --git a/test/Interceptor/InterceptorTest.php b/test/Interceptor/InterceptorTest.php
index 319a4366..d805efb0 100644
--- a/test/Interceptor/InterceptorTest.php
+++ b/test/Interceptor/InterceptorTest.php
@@ -6,6 +6,7 @@
 use Amp\Http\Client\ApplicationInterceptor;
 use Amp\Http\Client\Connection\DefaultConnectionFactory;
 use Amp\Http\Client\Connection\UnlimitedConnectionPool;
+use Amp\Http\Client\EventListener;
 use Amp\Http\Client\HttpClient;
 use Amp\Http\Client\HttpClientBuilder;
 use Amp\Http\Client\NetworkInterceptor;
@@ -46,6 +47,12 @@ final public function getServerAddress(): SocketAddress
         return $this->serverSocket->getAddress();
     }
 
+    final protected function givenEventListener(EventListener $eventListener): void
+    {
+        $this->builder = $this->builder->listen($eventListener);
+        $this->client = $this->builder->build();
+    }
+
     final protected function givenApplicationInterceptor(ApplicationInterceptor $interceptor): void
     {
         $this->builder = $this->builder->intercept($interceptor);
diff --git a/test/Interceptor/LogIntoHttpArchiveTest.php b/test/Interceptor/LogHttpArchiveTest.php
similarity index 78%
rename from test/Interceptor/LogIntoHttpArchiveTest.php
rename to test/Interceptor/LogHttpArchiveTest.php
index 0d390eb0..b5320344 100644
--- a/test/Interceptor/LogIntoHttpArchiveTest.php
+++ b/test/Interceptor/LogHttpArchiveTest.php
@@ -2,16 +2,17 @@
 
 namespace Amp\Http\Client\Interceptor;
 
+use Amp\Http\Client\EventListener\LogHttpArchive;
 use Amp\Http\Client\Request;
 
-class LogIntoHttpArchiveTest extends InterceptorTest
+class LogHttpArchiveTest extends InterceptorTest
 {
     public function testProducesValidJson(): void
     {
         $filePath = \tempnam(\sys_get_temp_dir(), 'amphp-http-client-test-');
         $logger = new LogHttpArchive($filePath);
 
-        $this->givenApplicationInterceptor($logger);
+        $this->givenEventListener($logger);
 
         $this->whenRequestIsExecuted(new Request('http://example.com/foo/bar?test=1'));
         $logger->reset(); // awaits write because of the mutex