From f185ea034d97db5ca9ea6f670abcf689dbf5b406 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 30 Oct 2023 21:14:43 +0400 Subject: [PATCH 01/10] Add proto to request converter; add the `roadrunner-php/roadrunner-api-dto` package --- composer.json | 6 ++-- src/HttpWorker.php | 76 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index e5e9eb5..e170999 100644 --- a/composer.json +++ b/composer.json @@ -42,13 +42,14 @@ "psr/http-factory": "^1.0.1", "psr/http-message": "^1.0.1 || ^2.0", "spiral/roadrunner": "^2023.3", - "spiral/roadrunner-worker": "^3.1.0" + "spiral/roadrunner-worker": "^3.3.0" }, "require-dev": { "buggregator/trap": "^1.0", "jetbrains/phpstorm-attributes": "^1.0", "nyholm/psr7": "^1.3", "phpunit/phpunit": "^10.0", + "roadrunner-php/roadrunner-api-dto": "^1.4", "symfony/process": "^6.2", "symfony/var-dumper": "^6.3", "vimeo/psalm": "^5.9" @@ -73,7 +74,8 @@ "analyze": "psalm" }, "suggest": { - "spiral/roadrunner-cli": "Provides RoadRunner installation and management CLI tools" + "spiral/roadrunner-cli": "Provides RoadRunner installation and management CLI tools", + "ext-protobuf": "Provides Protocol Buffers support" }, "config": { "sort-packages": true diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 92887f7..6626f6f 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -5,6 +5,10 @@ namespace Spiral\RoadRunner\Http; use Generator; +use RoadRunner\HTTP\DTO\V1BETA1\FileUpload; +use RoadRunner\HTTP\DTO\V1BETA1\HeaderValue; +use RoadRunner\HTTP\DTO\V1BETA1\Request as RequestProto; +use Spiral\RoadRunner\Encoding; use Spiral\RoadRunner\Http\Exception\StreamStoppedException; use Spiral\RoadRunner\Message\Command\StreamStop; use Spiral\RoadRunner\Payload; @@ -56,10 +60,17 @@ public function waitRequest(): ?Request return null; } + if ($payload->encoding === Encoding::Protobuf) { + $message = new RequestProto(); + $message->mergeFromString($payload->body); + + return $this->requestFromProto($message); + } + /** @var RequestContext $context */ $context = \json_decode($payload->header, true, 512, \JSON_THROW_ON_ERROR); - return $this->createRequest($payload->body, $context); + return $this->arrayToRequest($payload->body, $context); } /** @@ -134,7 +145,7 @@ private function respondStream(int $status, Generator $body, array $headers = [] /** * @param RequestContext $context */ - private function createRequest(string $body, array $context): Request + private function arrayToRequest(string $body, array $context): Request { \parse_str($context['rawQuery'], $query); return new Request( @@ -154,6 +165,47 @@ private function createRequest(string $body, array $context): Request ); } + private function requestFromProto(RequestProto $message): Request + { + $headers = $this->headerValueToArray($message->getHeader()); + $uploadedFiles = []; + + /** + * @var non-empty-string $name + * @var FileUpload $uploads + */ + foreach ($message->getUploads() as $name => $uploads) { + $uploadedFiles[$name] = [ + 'name' => $uploads->getName(), + 'mime' => $uploads->getMime(), + 'size' => $uploads->getSize(), + 'error' => $uploads->getError(), + 'tmpName' => $uploads->getTempFilename(), + ]; + } + + \parse_str($message->getRawQuery(), $query); + return new Request( + remoteAddr: $message->getRemoteAddr(), + protocol: $message->getProtocol(), + method: $message->getMethod(), + uri: $message->getUri(), + headers: $this->filterHeaders($headers), + cookies: \array_map( + static fn(array $values) => \implode(',', $values), + $this->headerValueToArray($message->getCookies()), + ), + uploads: $uploadedFiles, + attributes: [ + Request::PARSED_BODY_ATTRIBUTE_NAME => $message->getParsed(), + ] + \iterator_to_array($message->getAttributes()), + query: $query, + // todo rawBody? + body: $message->getBody(), + parsed: $message->getParsed(), + ); + } + /** * Remove all non-string and empty-string keys * @@ -164,7 +216,7 @@ private function filterHeaders(array $headers): array { foreach ($headers as $key => $_) { if (!\is_string($key) || $key === '') { - // ignore invalid header names or values (otherwise, the worker will be crashed) + // ignore invalid header names or values (otherwise, the worker might be crashed) // @see: unset($headers[$key]); } @@ -173,4 +225,22 @@ private function filterHeaders(array $headers): array /** @var HeadersList $headers */ return $headers; } + + /** + * @param \Traversable $message + * @return HeadersList + */ + private function headerValueToArray(\Traversable $message): array + { + $result = []; + /** + * @var non-empty-string $key + * @var HeaderValue $value + */ + foreach ($message as $key => $value) { + $result[$key] = \iterator_to_array($value->getValue()); + } + + return $result; + } } From 7ea1cff9a15c6020d010194f6664b94061776f2e Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Mon, 11 Mar 2024 18:27:39 +0200 Subject: [PATCH 02/10] Remove Encoding enum, fixes after testing --- composer.json | 6 +-- src/HttpWorker.php | 87 +++++++++++++++++++++++++------------ src/HttpWorkerInterface.php | 5 ++- 3 files changed, 66 insertions(+), 32 deletions(-) diff --git a/composer.json b/composer.json index 1c32aa9..faa3f1d 100644 --- a/composer.json +++ b/composer.json @@ -42,14 +42,14 @@ "psr/http-factory": "^1.0.1", "psr/http-message": "^1.0.1 || ^2.0", "spiral/roadrunner": "^2023.3", - "spiral/roadrunner-worker": "^3.1.0" + "spiral/roadrunner-worker": "^3.1", + "roadrunner-php/roadrunner-api-dto": "^1.4", + "symfony/polyfill-php83": "^1.29" }, "require-dev": { - "buggregator/trap": "^1.0", "jetbrains/phpstorm-attributes": "^1.0", "nyholm/psr7": "^1.3", "phpunit/phpunit": "^10.0", - "roadrunner-php/roadrunner-api-dto": "^1.4", "symfony/process": "^6.2 || ^7.0", "vimeo/psalm": "^5.9" }, diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 6626f6f..4a4d276 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -8,7 +8,7 @@ use RoadRunner\HTTP\DTO\V1BETA1\FileUpload; use RoadRunner\HTTP\DTO\V1BETA1\HeaderValue; use RoadRunner\HTTP\DTO\V1BETA1\Request as RequestProto; -use Spiral\RoadRunner\Encoding; +use RoadRunner\HTTP\DTO\V1BETA1\Response; use Spiral\RoadRunner\Http\Exception\StreamStoppedException; use Spiral\RoadRunner\Message\Command\StreamStop; use Spiral\RoadRunner\Payload; @@ -38,6 +38,8 @@ */ class HttpWorker implements HttpWorkerInterface { + private static ?bool $isProto = null; + public function __construct( private readonly WorkerInterface $worker, ) { @@ -60,11 +62,11 @@ public function waitRequest(): ?Request return null; } - if ($payload->encoding === Encoding::Protobuf) { + if ($this->isProtoPayload($payload)) { $message = new RequestProto(); - $message->mergeFromString($payload->body); + $message->mergeFromString($payload->header); - return $this->requestFromProto($message); + return $this->requestFromProto($payload->body, $message); } /** @var RequestContext $context */ @@ -74,6 +76,7 @@ public function waitRequest(): ?Request } /** + * @param array> $headers * @throws \JsonException */ public function respond(int $status, string|Generator $body = '', array $headers = [], bool $endOfStream = true): void @@ -87,21 +90,14 @@ public function respond(int $status, string|Generator $body = '', array $headers return; } - $head = \json_encode([ - 'status' => $status, - 'headers' => $headers ?: (object)[], - ], \JSON_THROW_ON_ERROR); - - $this->worker->respond(new Payload($body, $head, $endOfStream)); + $this->worker->respond($this->createRespondPayload($status, $body, $headers, $endOfStream)); } + /** + * @param array> $headers + */ private function respondStream(int $status, Generator $body, array $headers = [], bool $endOfStream = true): void { - $head = \json_encode([ - 'status' => $status, - 'headers' => $headers ?: (object)[], - ], \JSON_THROW_ON_ERROR); - $worker = $this->worker instanceof StreamWorkerInterface ? $this->worker->withStreamMode() : $this->worker; @@ -114,7 +110,7 @@ private function respondStream(int $status, Generator $body, array $headers = [] // We don't need to send an empty frame if the stream is not ended return; } - $worker->respond(new Payload($content, $head, $endOfStream)); + $worker->respond($this->createRespondPayload($status, $content, $headers, $endOfStream)); break; } @@ -129,8 +125,7 @@ private function respondStream(int $status, Generator $body, array $headers = [] } // Send a chunk of data - $worker->respond(new Payload($content, $head, false)); - $head = null; + $worker->respond($this->createRespondPayload($status, $content, $headers, false)); try { $body->next(); @@ -165,26 +160,26 @@ private function arrayToRequest(string $body, array $context): Request ); } - private function requestFromProto(RequestProto $message): Request + private function requestFromProto(string $body, RequestProto $message): Request { $headers = $this->headerValueToArray($message->getHeader()); $uploadedFiles = []; /** - * @var non-empty-string $name * @var FileUpload $uploads */ - foreach ($message->getUploads() as $name => $uploads) { - $uploadedFiles[$name] = [ + foreach ($message->getUploads()?->getList() ?? [] as $uploads) { + $uploadedFiles[$uploads->getName()] = [ 'name' => $uploads->getName(), 'mime' => $uploads->getMime(), - 'size' => $uploads->getSize(), - 'error' => $uploads->getError(), + 'size' => (int) $uploads->getSize(), + 'error' => (int) $uploads->getError(), 'tmpName' => $uploads->getTempFilename(), ]; } \parse_str($message->getRawQuery(), $query); + /** @psalm-suppress ArgumentTypeCoercion, MixedArgumentTypeCoercion */ return new Request( remoteAddr: $message->getRemoteAddr(), protocol: $message->getProtocol(), @@ -200,8 +195,7 @@ private function requestFromProto(RequestProto $message): Request Request::PARSED_BODY_ATTRIBUTE_NAME => $message->getParsed(), ] + \iterator_to_array($message->getAttributes()), query: $query, - // todo rawBody? - body: $message->getBody(), + body: $body, parsed: $message->getParsed(), ); } @@ -228,7 +222,6 @@ private function filterHeaders(array $headers): array /** * @param \Traversable $message - * @return HeadersList */ private function headerValueToArray(\Traversable $message): array { @@ -243,4 +236,44 @@ private function headerValueToArray(\Traversable $message): array return $result; } + + /** + * @param array> $headers + * @return array + */ + private function arrayToHeaderValue(array $headers = []): array + { + $result = []; + /** + * @var non-empty-string $key + * @var array $value + */ + foreach ($headers as $key => $value) { + $result[$key] = new HeaderValue(['value' => $value]); + } + + return $result; + } + + /** + * @param array> $headers + */ + private function createRespondPayload(int $status, string $body, array $headers = [], bool $eos = true): Payload + { + $head = static::$isProto + ? (new Response(['status' => $status, 'headers' => $this->arrayToHeaderValue($headers)])) + ->serializeToString() + : \json_encode(['status' => $status, 'headers' => $headers ?: (object)[]], \JSON_THROW_ON_ERROR); + + return new Payload(body: $body, header: $head, eos: $eos); + } + + private function isProtoPayload(Payload $payload): bool + { + if (static::$isProto === null) { + static::$isProto = !json_validate($payload->header); + } + + return static::$isProto; + } } diff --git a/src/HttpWorkerInterface.php b/src/HttpWorkerInterface.php index c163193..8643e77 100644 --- a/src/HttpWorkerInterface.php +++ b/src/HttpWorkerInterface.php @@ -26,8 +26,9 @@ public function waitRequest(): ?Request; * If the body is a generator, then each yielded value will be sent as a separated stream chunk. * Returned value will be sent as a last stream package. * Note: Stream response is supported by RoadRunner since version 2023.3 - * @param HeadersList|array $headers An associative array of the message's headers. Each key MUST be a header name, - * and each value MUST be an array of strings for that header. + * @param HeadersList|array> $headers $headers An associative array of the + * message's headers. Each key MUST be a header name, and each value MUST be an array of strings for + * that header. */ public function respond(int $status, string|Generator $body, array $headers = []): void; } From cfd3dcb41e079196ffb085dce4044349d0abae99 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Mon, 11 Mar 2024 19:32:02 +0200 Subject: [PATCH 03/10] Pass codec to worker, allow spiral/roadrunner 2024 --- composer.json | 2 +- src/HttpWorker.php | 36 ++++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/composer.json b/composer.json index faa3f1d..10cf2c3 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,7 @@ "ext-json": "*", "psr/http-factory": "^1.0.1", "psr/http-message": "^1.0.1 || ^2.0", - "spiral/roadrunner": "^2023.3", + "spiral/roadrunner": "^2023.3 || ^2024.1", "spiral/roadrunner-worker": "^3.1", "roadrunner-php/roadrunner-api-dto": "^1.4", "symfony/polyfill-php83": "^1.29" diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 4a4d276..f8edff5 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -9,6 +9,7 @@ use RoadRunner\HTTP\DTO\V1BETA1\HeaderValue; use RoadRunner\HTTP\DTO\V1BETA1\Request as RequestProto; use RoadRunner\HTTP\DTO\V1BETA1\Response; +use Spiral\Goridge\Frame; use Spiral\RoadRunner\Http\Exception\StreamStoppedException; use Spiral\RoadRunner\Message\Command\StreamStop; use Spiral\RoadRunner\Payload; @@ -38,7 +39,7 @@ */ class HttpWorker implements HttpWorkerInterface { - private static ?bool $isProto = null; + private static ?int $codec = null; public function __construct( private readonly WorkerInterface $worker, @@ -62,7 +63,11 @@ public function waitRequest(): ?Request return null; } - if ($this->isProtoPayload($payload)) { + if (static::$codec === null) { + static::$codec = json_validate($payload->header) ? Frame::CODEC_JSON : Frame::CODEC_PROTO; + } + + if (static::$codec === Frame::CODEC_PROTO) { $message = new RequestProto(); $message->mergeFromString($payload->header); @@ -90,7 +95,8 @@ public function respond(int $status, string|Generator $body = '', array $headers return; } - $this->worker->respond($this->createRespondPayload($status, $body, $headers, $endOfStream)); + /** @psalm-suppress TooManyArguments */ + $this->worker->respond($this->createRespondPayload($status, $body, $headers, $endOfStream), static::$codec); } /** @@ -110,7 +116,11 @@ private function respondStream(int $status, Generator $body, array $headers = [] // We don't need to send an empty frame if the stream is not ended return; } - $worker->respond($this->createRespondPayload($status, $content, $headers, $endOfStream)); + /** @psalm-suppress TooManyArguments */ + $worker->respond( + $this->createRespondPayload($status, $content, $headers, $endOfStream), + static::$codec + ); break; } @@ -124,8 +134,11 @@ private function respondStream(int $status, Generator $body, array $headers = [] return; } - // Send a chunk of data - $worker->respond($this->createRespondPayload($status, $content, $headers, false)); + /** + * Send a chunk of data + * @psalm-suppress TooManyArguments + */ + $worker->respond($this->createRespondPayload($status, $content, $headers, false), static::$codec); try { $body->next(); @@ -260,20 +273,11 @@ private function arrayToHeaderValue(array $headers = []): array */ private function createRespondPayload(int $status, string $body, array $headers = [], bool $eos = true): Payload { - $head = static::$isProto + $head = static::$codec === Frame::CODEC_PROTO ? (new Response(['status' => $status, 'headers' => $this->arrayToHeaderValue($headers)])) ->serializeToString() : \json_encode(['status' => $status, 'headers' => $headers ?: (object)[]], \JSON_THROW_ON_ERROR); return new Payload(body: $body, header: $head, eos: $eos); } - - private function isProtoPayload(Payload $payload): bool - { - if (static::$isProto === null) { - static::$isProto = !json_validate($payload->header); - } - - return static::$isProto; - } } From fadfc2361d114df9aa6d873b733c93d8e990c33e Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Wed, 27 Mar 2024 17:05:40 +0200 Subject: [PATCH 04/10] Update code for new implementation in RR --- composer.json | 2 +- src/HttpWorker.php | 25 ++++++------------------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/composer.json b/composer.json index 10cf2c3..007eec1 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "psr/http-message": "^1.0.1 || ^2.0", "spiral/roadrunner": "^2023.3 || ^2024.1", "spiral/roadrunner-worker": "^3.1", - "roadrunner-php/roadrunner-api-dto": "^1.4", + "roadrunner-php/roadrunner-api-dto": "dev-http-beta as 1.6.0", "symfony/polyfill-php83": "^1.29" }, "require-dev": { diff --git a/src/HttpWorker.php b/src/HttpWorker.php index f8edff5..701eb24 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -5,10 +5,9 @@ namespace Spiral\RoadRunner\Http; use Generator; -use RoadRunner\HTTP\DTO\V1BETA1\FileUpload; -use RoadRunner\HTTP\DTO\V1BETA1\HeaderValue; -use RoadRunner\HTTP\DTO\V1BETA1\Request as RequestProto; -use RoadRunner\HTTP\DTO\V1BETA1\Response; +use RoadRunner\HTTP\DTO\V1\HeaderValue; +use RoadRunner\HTTP\DTO\V1\Request as RequestProto; +use RoadRunner\HTTP\DTO\V1\Response; use Spiral\Goridge\Frame; use Spiral\RoadRunner\Http\Exception\StreamStoppedException; use Spiral\RoadRunner\Message\Command\StreamStop; @@ -175,21 +174,9 @@ private function arrayToRequest(string $body, array $context): Request private function requestFromProto(string $body, RequestProto $message): Request { + /** @var UploadedFilesList $uploads */ + $uploads = \json_decode($message->getUploads(), true) ?? []; $headers = $this->headerValueToArray($message->getHeader()); - $uploadedFiles = []; - - /** - * @var FileUpload $uploads - */ - foreach ($message->getUploads()?->getList() ?? [] as $uploads) { - $uploadedFiles[$uploads->getName()] = [ - 'name' => $uploads->getName(), - 'mime' => $uploads->getMime(), - 'size' => (int) $uploads->getSize(), - 'error' => (int) $uploads->getError(), - 'tmpName' => $uploads->getTempFilename(), - ]; - } \parse_str($message->getRawQuery(), $query); /** @psalm-suppress ArgumentTypeCoercion, MixedArgumentTypeCoercion */ @@ -203,7 +190,7 @@ private function requestFromProto(string $body, RequestProto $message): Request static fn(array $values) => \implode(',', $values), $this->headerValueToArray($message->getCookies()), ), - uploads: $uploadedFiles, + uploads: $uploads, attributes: [ Request::PARSED_BODY_ATTRIBUTE_NAME => $message->getParsed(), ] + \iterator_to_array($message->getAttributes()), From 434446665343dd7f904caf992b8fa8658b66fe49 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Wed, 27 Mar 2024 19:20:19 +0200 Subject: [PATCH 05/10] Add unit tests for waitRequest method --- src/HttpWorker.php | 5 +- tests/Unit/HttpWorkerTest.php | 236 ++++++++++++++++++++++++++++++ tests/Unit/PSR7WorkerTest.php | 6 +- tests/Unit/StreamResponseTest.php | 3 +- 4 files changed, 244 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/HttpWorkerTest.php diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 701eb24..320212e 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -193,7 +193,10 @@ private function requestFromProto(string $body, RequestProto $message): Request uploads: $uploads, attributes: [ Request::PARSED_BODY_ATTRIBUTE_NAME => $message->getParsed(), - ] + \iterator_to_array($message->getAttributes()), + ] + \array_map( + static fn(array $values) => \array_shift($values), + $this->headerValueToArray($message->getAttributes()), + ), query: $query, body: $body, parsed: $message->getParsed(), diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php new file mode 100644 index 0000000..cebbf6c --- /dev/null +++ b/tests/Unit/HttpWorkerTest.php @@ -0,0 +1,236 @@ + 'first=value&arr[]=foo+bar&arr[]=baz', + 'remoteAddr' => '127.0.0.1', + 'protocol' => 'HTTP/1.1', + 'method' => 'GET', + 'uri' => 'http://localhost', + 'parsed' => false, + ]; + + private const REQUIRED_REQUEST_DATA = [ + 'remoteAddr' => '127.0.0.1', + 'protocol' => 'HTTP/1.1', + 'method' => 'GET', + 'uri' => 'http://localhost', + 'attributes' => [Request::PARSED_BODY_ATTRIBUTE_NAME => false], + 'query' => ['first' => 'value', 'arr' => ['foo bar', 'baz']], + 'parsed' => false + ]; + + #[DataProvider('requestDataProvider')] + public function testWaitRequestFromArray(array $header, array $expected): void + { + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('waitPayload') + ->willReturn(new Payload(null, \json_encode($header))); + + $worker = new HttpWorker($worker); + + $this->assertEquals(new Request(...$expected), $worker->waitRequest()); + } + + #[DataProvider('requestDataProvider')] + public function testWaitRequestFromProto(array $header, array $expected): void + { + $request = self::createProtoRequest($header); + + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('waitPayload') + ->willReturn(new Payload(null, $request->serializeToString())); + + $worker = new HttpWorker($worker); + + $this->assertEquals(new Request(...$expected), $worker->waitRequest()); + } + + #[DataProvider('emptyRequestDataProvider')] + public function testWaitRequestWithEmptyData(?Payload $payload): void + { + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('waitPayload') + ->willReturn($payload); + + $worker = new HttpWorker($worker); + + $this->assertEquals(null, $worker->waitRequest()); + } + + public static function requestDataProvider(): \Traversable + { + yield [self::REQUIRED_PAYLOAD_DATA, self::REQUIRED_REQUEST_DATA]; + yield [ + \array_merge(self::REQUIRED_PAYLOAD_DATA, ['parsed' => true]), + \array_merge( + self::REQUIRED_REQUEST_DATA, + ['parsed' => true, 'attributes' => [Request::PARSED_BODY_ATTRIBUTE_NAME => true]] + ) + ]; + yield [ + \array_merge(self::REQUIRED_PAYLOAD_DATA, [ + 'headers' => [ + 'Content-Type' => ['application/x-www-form-urlencoded'], + 111 => ['invalid-non-string-key'], + '' => ['invalid-empty-string-key'], + ], + ]), + \array_merge(self::REQUIRED_REQUEST_DATA, [ + 'headers' => ['Content-Type' => ['application/x-www-form-urlencoded']], + ]) + ]; + yield [ + \array_merge(self::REQUIRED_PAYLOAD_DATA, [ + 'cookies' => [ + 'theme' => 'light', + ], + ]), + \array_merge(self::REQUIRED_REQUEST_DATA, [ + 'cookies' => ['theme' => 'light'], + ]) + ]; + yield [ + \array_merge(self::REQUIRED_PAYLOAD_DATA, [ + 'uploads' => [ + 'single-file' => [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + 'multiple' => [ + [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + [ + 'name' => 'test2.jpg', + 'mime' => 'image/jpeg', + 'size' => 1235, + 'error' => 0, + 'tmpName' => '/tmp/php/php2h4j1o', + ] + ], + 'nested' => [ + 'some-key' => [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + ] + ], + ]), + \array_merge(self::REQUIRED_REQUEST_DATA, [ + 'uploads' => [ + 'single-file' => [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + 'multiple' => [ + [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + [ + 'name' => 'test2.jpg', + 'mime' => 'image/jpeg', + 'size' => 1235, + 'error' => 0, + 'tmpName' => '/tmp/php/php2h4j1o', + ] + ], + 'nested' => [ + 'some-key' => [ + 'name' => 'test.png', + 'mime' => 'image/png', + 'size' => 123, + 'error' => 0, + 'tmpName' => '/tmp/php/php1h4j1o', + ], + ] + ], + ]) + ]; + yield [ + \array_merge(self::REQUIRED_PAYLOAD_DATA, [ + 'attributes' => [ + 'foo' => 'bar', + ], + ]), + \array_merge(self::REQUIRED_REQUEST_DATA, [ + 'attributes' => [ + Request::PARSED_BODY_ATTRIBUTE_NAME => false, + 'foo' => 'bar' + ], + ]) + ]; + } + + public static function emptyRequestDataProvider(): \Traversable + { + yield [null]; + yield [new Payload(null, null)]; + } + + private static function createProtoRequest(array $values): \RoadRunner\HTTP\DTO\V1\Request + { + $toHeaderValue = static function (string $key, bool $wrap = true) use (&$values): void { + if (isset($values[$key])) { + foreach ($values[$key] as $valueKey => $value) { + $values[$key][$valueKey] = new HeaderValue(['value' => $wrap ? [$value] : $value]); + } + } + }; + + $toHeaderValue('headers', false); + $toHeaderValue('attributes'); + $toHeaderValue('cookies'); + + return new \RoadRunner\HTTP\DTO\V1\Request([ + 'remote_addr' => $values['remoteAddr'], + 'protocol' => $values['protocol'], + 'method' => $values['method'], + 'uri' => $values['uri'], + 'header' => $values['headers'] ?? [], + 'cookies' => $values['cookies'] ?? [], + 'raw_query' => $values['rawQuery'], + 'parsed' => $values['parsed'], + 'uploads' => \json_encode($values['uploads'] ?? []), + 'attributes' => $values['attributes'] ?? [], + ]); + } + + protected function tearDown(): void + { + (new \ReflectionProperty(HttpWorker::class, 'codec'))->setValue(null); + } +} diff --git a/tests/Unit/PSR7WorkerTest.php b/tests/Unit/PSR7WorkerTest.php index f0e7cb2..c79d52b 100644 --- a/tests/Unit/PSR7WorkerTest.php +++ b/tests/Unit/PSR7WorkerTest.php @@ -1,5 +1,7 @@ 0) { \ob_end_clean(); } - - parent::tearDown(); // TODO: Change the autogenerated stub } } diff --git a/tests/Unit/StreamResponseTest.php b/tests/Unit/StreamResponseTest.php index e491129..f785742 100644 --- a/tests/Unit/StreamResponseTest.php +++ b/tests/Unit/StreamResponseTest.php @@ -10,7 +10,7 @@ use Spiral\RoadRunner\Tests\Http\Unit\Stub\TestRelay; use Spiral\RoadRunner\Worker; -class StreamResponseTest extends TestCase +final class StreamResponseTest extends TestCase { private TestRelay $relay; private Worker $worker; @@ -18,7 +18,6 @@ class StreamResponseTest extends TestCase protected function tearDown(): void { unset($this->relay, $this->worker); - parent::tearDown(); } /** From e759b9dafb16388ed5cb449d2577753eb76d347f Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Thu, 28 Mar 2024 15:37:56 +0200 Subject: [PATCH 06/10] Add checking empty body with parsed flag --- src/HttpWorker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 320212e..e42ee50 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -198,7 +198,7 @@ private function requestFromProto(string $body, RequestProto $message): Request $this->headerValueToArray($message->getAttributes()), ), query: $query, - body: $body, + body: $message->getParsed() && empty($body) ? \json_encode([]) : $body, parsed: $message->getParsed(), ); } From 29b83ab74ba54bc434a9b63942f7b02ac9e68e86 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Thu, 28 Mar 2024 15:42:55 +0200 Subject: [PATCH 07/10] Fix tests --- tests/Unit/HttpWorkerTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php index cebbf6c..4f020d2 100644 --- a/tests/Unit/HttpWorkerTest.php +++ b/tests/Unit/HttpWorkerTest.php @@ -30,7 +30,8 @@ final class HttpWorkerTest extends TestCase 'uri' => 'http://localhost', 'attributes' => [Request::PARSED_BODY_ATTRIBUTE_NAME => false], 'query' => ['first' => 'value', 'arr' => ['foo bar', 'baz']], - 'parsed' => false + 'parsed' => false, + 'body' => 'foo' ]; #[DataProvider('requestDataProvider')] @@ -39,7 +40,7 @@ public function testWaitRequestFromArray(array $header, array $expected): void $worker = $this->createMock(WorkerInterface::class); $worker->expects($this->once()) ->method('waitPayload') - ->willReturn(new Payload(null, \json_encode($header))); + ->willReturn(new Payload('foo', \json_encode($header))); $worker = new HttpWorker($worker); @@ -54,7 +55,7 @@ public function testWaitRequestFromProto(array $header, array $expected): void $worker = $this->createMock(WorkerInterface::class); $worker->expects($this->once()) ->method('waitPayload') - ->willReturn(new Payload(null, $request->serializeToString())); + ->willReturn(new Payload('foo', $request->serializeToString())); $worker = new HttpWorker($worker); From 8cb1887969466e51f918a4deb589d1628cb76a24 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Thu, 28 Mar 2024 17:10:59 +0200 Subject: [PATCH 08/10] Add unit tests --- tests/Unit/HttpWorkerTest.php | 60 +++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php index 4f020d2..90b63f2 100644 --- a/tests/Unit/HttpWorkerTest.php +++ b/tests/Unit/HttpWorkerTest.php @@ -7,6 +7,8 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RoadRunner\HTTP\DTO\V1\HeaderValue; +use RoadRunner\HTTP\DTO\V1\Response; +use Spiral\Goridge\Frame; use Spiral\RoadRunner\Http\HttpWorker; use Spiral\RoadRunner\Http\Request; use Spiral\RoadRunner\Payload; @@ -75,6 +77,64 @@ public function testWaitRequestWithEmptyData(?Payload $payload): void $this->assertEquals(null, $worker->waitRequest()); } + public function testEmptyBodyShouldBeConvertedIntoEmptyArrayWithParsedTrue(): void + { + $request = self::createProtoRequest(\array_merge(self::REQUIRED_REQUEST_DATA, ['parsed' => true])); + + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('waitPayload') + ->willReturn(new Payload('', $request->serializeToString())); + + $worker = new HttpWorker($worker); + + $request = $worker->waitRequest(); + $this->assertSame([], $request->getParsedBody()); + } + + public function testRespondUnableToSendBodyWithInfoStatusException(): void + { + $worker = new HttpWorker($this->createMock(WorkerInterface::class)); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Unable to send a body with informational status code.'); + $worker->respond(100, 'foo'); + } + + public function testRespondWithProtoCodec(): void + { + $expectedHeader = new Response([ + 'status' => 200, + 'headers' => ['Content-Type' => new HeaderValue(['value' => ['application/x-www-form-urlencoded']])], + ]); + + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('respond') + ->with(new Payload('foo', $expectedHeader->serializeToString()), Frame::CODEC_PROTO); + + (new \ReflectionProperty(HttpWorker::class, 'codec'))->setValue(Frame::CODEC_PROTO); + $worker = new HttpWorker($worker); + + $worker->respond(200, 'foo', ['Content-Type' => ['application/x-www-form-urlencoded']]); + } + + public function testRespondWithJsonCodec(): void + { + $worker = $this->createMock(WorkerInterface::class); + $worker->expects($this->once()) + ->method('respond') + ->with(new Payload('foo', \json_encode([ + 'status' => 200, + 'headers' => ['Content-Type' => ['application/x-www-form-urlencoded']] + ])), Frame::CODEC_JSON); + + (new \ReflectionProperty(HttpWorker::class, 'codec'))->setValue(Frame::CODEC_JSON); + $worker = new HttpWorker($worker); + + $worker->respond(200, 'foo', ['Content-Type' => ['application/x-www-form-urlencoded']]); + } + public static function requestDataProvider(): \Traversable { yield [self::REQUIRED_PAYLOAD_DATA, self::REQUIRED_REQUEST_DATA]; From 7f7fa2ad2441cbce63be00e17e410b52f0837118 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Fri, 29 Mar 2024 10:24:37 +0200 Subject: [PATCH 09/10] Fix dependencies --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 007eec1..b75d83d 100644 --- a/composer.json +++ b/composer.json @@ -42,8 +42,8 @@ "psr/http-factory": "^1.0.1", "psr/http-message": "^1.0.1 || ^2.0", "spiral/roadrunner": "^2023.3 || ^2024.1", - "spiral/roadrunner-worker": "^3.1", - "roadrunner-php/roadrunner-api-dto": "dev-http-beta as 1.6.0", + "spiral/roadrunner-worker": "^3.5", + "roadrunner-php/roadrunner-api-dto": "^1.6", "symfony/polyfill-php83": "^1.29" }, "require-dev": { From 8b35fd9670ac6e71f01785b835fdc3070b2523f1 Mon Sep 17 00:00:00 2001 From: Maxim Smakouz Date: Mon, 1 Apr 2024 10:36:49 +0300 Subject: [PATCH 10/10] Improve code --- composer.json | 2 +- src/HttpWorker.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index b75d83d..a77e46a 100644 --- a/composer.json +++ b/composer.json @@ -74,7 +74,7 @@ }, "suggest": { "spiral/roadrunner-cli": "Provides RoadRunner installation and management CLI tools", - "ext-protobuf": "Provides Protocol Buffers support" + "ext-protobuf": "Provides Protocol Buffers support. Without it, performance will be lower." }, "config": { "sort-packages": true diff --git a/src/HttpWorker.php b/src/HttpWorker.php index e42ee50..fa7548d 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -63,7 +63,7 @@ public function waitRequest(): ?Request } if (static::$codec === null) { - static::$codec = json_validate($payload->header) ? Frame::CODEC_JSON : Frame::CODEC_PROTO; + static::$codec = \json_validate($payload->header) ? Frame::CODEC_JSON : Frame::CODEC_PROTO; } if (static::$codec === Frame::CODEC_PROTO) { @@ -198,7 +198,7 @@ private function requestFromProto(string $body, RequestProto $message): Request $this->headerValueToArray($message->getAttributes()), ), query: $query, - body: $message->getParsed() && empty($body) ? \json_encode([]) : $body, + body: $message->getParsed() && empty($body) ? '{}' : $body, parsed: $message->getParsed(), ); }