Move connectionAcquired call into connections

amphp · Aug 30, 2023 · 482c5f6 · 482c5f6
1 parent ac99861
commit 482c5f6
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 7 deletions.
diff --git a/src/Connection/ConnectionLimitingPool.php b/src/Connection/ConnectionLimitingPool.php
@@ -195,8 +195,6 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance
                     continue; // No stream available for the given request.
                 }
 
-                events()->connectionAcquired($request, $connection);
-
                 return [$connection, $stream];
             }
 
@@ -219,8 +217,6 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance
                 continue; // Wait for a different connection to become available.
             }
 
-            events()->connectionAcquired($request, $connection);
-
             return [$connection, $stream];
         } while (true);
 
@@ -289,8 +285,6 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance
             }
         }
 
-        events()->connectionAcquired($request, $connection);
-
         return [$connection, $stream];
     }
 

diff --git a/src/Connection/Http1Connection.php b/src/Connection/Http1Connection.php
@@ -159,6 +159,8 @@ public function getStream(Request $request): ?Stream
 
         $this->busy = true;
 
+        events()->connectionAcquired($request, $this);
+
         return HttpStream::fromConnection($this, $this->request(...), $this->release(...));
     }
 

diff --git a/src/Connection/Http2Connection.php b/src/Connection/Http2Connection.php
@@ -13,6 +13,7 @@
 use Amp\Socket\SocketAddress;
 use Amp\Socket\TlsInfo;
 use Amp\TimeoutCancellation;
+use function Amp\Http\Client\events;
 
 final class Http2Connection implements Connection
 {
@@ -60,6 +61,8 @@ public function getStream(Request $request): ?Stream
 
         $this->processor->reserveStream();
 
+        events()->connectionAcquired($request, $this);
+
         return HttpStream::fromConnection($this, $this->request(...), $this->processor->unreserveStream(...));
     }
 

diff --git a/test/Connection/Http1ConnectionTest.php b/test/Connection/Http1ConnectionTest.php
@@ -21,6 +21,7 @@
 use Revolt\EventLoop;
 use function Amp\async;
 use function Amp\delay;
+use function Amp\Http\Client\events;
 
 class Http1ConnectionTest extends AsyncTestCase
 {
@@ -33,7 +34,9 @@ public function testConnectionBusyAfterRequestIsIssued(): void
         $request = new Request('http://localhost');
         $request->setBody($this->createSlowBody());
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
+
         async(fn () => $stream->request($request, new NullCancellation))->ignore();
         $stream = null; // gc instance
 
@@ -49,6 +52,8 @@ public function testConnectionBusyWithoutRequestButNotGarbageCollected(): void
         $request = new Request('http://localhost');
         $request->setBody($this->createSlowBody());
 
+        events()->requestStart($request);
+
         /** @noinspection PhpUnusedLocalVariableInspection */
         $stream = $connection->getStream($request);
 
@@ -64,13 +69,17 @@ public function testConnectionNotBusyWithoutRequestGarbageCollected(): void
         $request = new Request('http://localhost');
         $request->setBody($this->createSlowBody());
 
+        events()->requestStart($request);
+
         /** @noinspection PhpUnusedLocalVariableInspection */
         $stream = $connection->getStream($request);
         unset($stream); // gc instance
 
         delay(0); // required to clear instance in async :-(
 
-        self::assertNotNull($connection->getStream($request));
+        $secondRequest = new Request('http://localhost');
+        events()->requestStart($secondRequest);
+        self::assertNotNull($connection->getStream($secondRequest));
     }
 
     public function test100Continue(): void
@@ -82,6 +91,7 @@ public function test100Continue(): void
         $request = new Request('http://httpbin.org/post', 'POST');
         $request->setHeader('expect', '100-continue');
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $server->write("HTTP/1.1 100 Continue\r\nFoo: Bar\r\n\r\nHTTP/1.1 204 Nothing to send\r\n\r\n");
@@ -118,6 +128,7 @@ public function testUpgrade(): void
         $request->setHeader('connection', 'upgrade');
         $request->setUpgradeHandler($callback);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $server->write("HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: test\r\n\r\n" . $socketData);
@@ -144,6 +155,7 @@ public function testTransferTimeout(): void
         $request = new Request('http://localhost');
         $request->setTransferTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $server->write("HTTP/1.1 200 Continue\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\ntest");
@@ -172,6 +184,7 @@ public function testInactivityTimeout(): void
         $request = new Request('http://localhost');
         $request->setInactivityTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $server->write("HTTP/1.1 200 Continue\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\n");
@@ -208,6 +221,7 @@ public function testWritingRequestWithRelativeUriPathFails(): void
 
         $request = new Request(new LaminasUri('foo'));
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $this->expectException(InvalidRequestException::class);
@@ -231,6 +245,7 @@ public function testWritingRequestWithValidUriPathProceedsWithMatchingUriPath(
         $request = new Request($uri);
         $request->setInactivityTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $future = async(fn () => $stream->request($request, new NullCancellation));

diff --git a/test/Connection/Http2ConnectionTest.php b/test/Connection/Http2ConnectionTest.php
@@ -25,6 +25,7 @@
 use Revolt\EventLoop;
 use function Amp\async;
 use function Amp\delay;
+use function Amp\Http\Client\events;
 use function Amp\Http\formatDateHeader;
 
 class Http2ConnectionTest extends AsyncTestCase
@@ -48,6 +49,7 @@ public function test100Continue(): void
 
         $request = new Request('http://localhost/');
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $server->write(self::packFrame($hpack->encode([
@@ -79,6 +81,8 @@ public function testSwitchingProtocols(): void
 
         $request = new Request('http://localhost/');
 
+        events()->requestStart($request);
+
         /** @var Stream $stream */
         $stream = $connection->getStream($request);
 
@@ -107,6 +111,8 @@ public function testTrailers(): void
 
         $request = new Request('http://localhost/');
 
+        events()->requestStart($request);
+
         $stream = $connection->getStream($request);
 
         EventLoop::queue(static function () use ($server, $hpack): void {
@@ -154,6 +160,8 @@ public function testTrailersWithoutTrailers(): void
 
         $request = new Request('http://localhost/');
 
+        events()->requestStart($request);
+
         $stream = $connection->getStream($request);
 
         $server->write(self::packFrame($hpack->encode([
@@ -187,6 +195,8 @@ public function testCancellingWhileStreamingBody(): void
 
         $request = new Request('http://localhost/');
 
+        events()->requestStart($request);
+
         $stream = $connection->getStream($request);
 
         EventLoop::queue(static function () use ($server, $hpack) {
@@ -242,6 +252,8 @@ public function testTimeoutWhileStreamingBody(): void
         $request = new Request('http://localhost/');
         $request->setTransferTimeout(0.5);
 
+        events()->requestStart($request);
+
         $stream = $connection->getStream($request);
 
         EventLoop::queue(static function () use ($server, $hpack) {
@@ -301,6 +313,8 @@ public function testCancellingPushPromiseBody(): void
             $pushPromise = $future;
         });
 
+        events()->requestStart($request);
+
         $stream = $connection->getStream($request);
 
         EventLoop::queue(static function () use ($server, $hpack) {
@@ -377,6 +391,7 @@ public function testInactivityWhileStreamingBody(): void
         $request = new Request('http://localhost/');
         $request->setInactivityTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         EventLoop::queue(static function () use ($server, $hpack) {
@@ -428,6 +443,7 @@ public function testWritingRequestWithRelativeUriPathFails(): void
         $request = new Request(new LaminasUri('foo'));
         $request->setInactivityTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $this->expectException(InvalidRequestException::class);
@@ -450,6 +466,7 @@ public function testServerPushingOddStream(): void
         $request->setInactivityTimeout(0.5);
         $request->setPushHandler($this->createCallback(0));
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $future = async(fn () => $stream->request($request, new NullCancellation));
@@ -496,6 +513,7 @@ public function testWritingRequestWithValidUriPathProceedsWithMatchingUriPath(
         $request = new Request($uri);
         $request->setInactivityTimeout(0.5);
 
+        events()->requestStart($request);
         $stream = $connection->getStream($request);
 
         $future = async(fn () => $stream->request($request, new NullCancellation));

diff --git a/test/ParserTest.php b/test/ParserTest.php
@@ -3,6 +3,7 @@
 
 namespace Amp\Http\Client;
 
+use Amp\Http\Client\Connection\Connection;
 use Amp\Http\Client\Connection\Internal\Http1Parser;
 use Amp\Http\Client\Connection\Stream;
 use Amp\PHPUnit\AsyncTestCase;
@@ -16,6 +17,7 @@ public function testKeepAliveHeadResponseParse(): void
         $request = new Request('/', 'HEAD');
 
         events()->requestStart($request);
+        events()->connectionAcquired($request, $this->createMock(Connection::class));
         events()->requestHeaderStart($request, $this->createMock(Stream::class));
         events()->requestHeaderEnd($request, $this->createMock(Stream::class));
         events()->requestBodyStart($request, $this->createMock(Stream::class));
@@ -43,6 +45,7 @@ public function testResponseWithTrailers(): void
         $request = new Request('/', 'GET');
 
         events()->requestStart($request);
+        events()->connectionAcquired($request, $this->createMock(Connection::class));
         events()->requestHeaderStart($request, $this->createMock(Stream::class));
         events()->requestHeaderEnd($request, $this->createMock(Stream::class));
         events()->requestBodyStart($request, $this->createMock(Stream::class));