Skip to content

Commit

Permalink
Move connectionAcquired call into connections
Browse files Browse the repository at this point in the history
  • Loading branch information
@kelunik
kelunik committed Aug 30, 2023
1 parent ac99861 commit 482c5f6
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 7 deletions.
6 changes: 0 additions & 6 deletions src/Connection/ConnectionLimitingPool.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand All @@ -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);

Expand Down Expand Up @@ -289,8 +285,6 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance
}
}

events()->connectionAcquired($request, $connection);

return [$connection, $stream];
}

Expand Down
2 changes: 2 additions & 0 deletions src/Connection/Http1Connection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(...));
}

Expand Down
3 changes: 3 additions & 0 deletions src/Connection/Http2Connection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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(...));
}

Expand Down
17 changes: 16 additions & 1 deletion test/Connection/Http1ConnectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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

Expand All @@ -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);

Expand All @@ -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
Expand All @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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));
Expand Down
18 changes: 18 additions & 0 deletions test/Connection/Http2ConnectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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([
Expand Down Expand Up @@ -79,6 +81,8 @@ public function testSwitchingProtocols(): void

$request = new Request('http://localhost/');

events()->requestStart($request);

/** @var Stream $stream */
$stream = $connection->getStream($request);

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -301,6 +313,8 @@ public function testCancellingPushPromiseBody(): void
$pushPromise = $future;
});

events()->requestStart($request);

$stream = $connection->getStream($request);

EventLoop::queue(static function () use ($server, $hpack) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
3 changes: 3 additions & 0 deletions test/ParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 482c5f6

Please sign in to comment.