From 7ade042b4bb87fef9972bf352ab53017c0914290 Mon Sep 17 00:00:00 2001 From: PJ Dietz Date: Sun, 9 Aug 2020 13:10:14 -0400 Subject: [PATCH] Change Request constructor signature Various updates to Message classes --- psalm.xml | 12 +++++++ src/Message/Message.php | 38 +++++++++++++------- src/Message/NullStream.php | 2 ++ src/Message/Request.php | 30 ++++++++-------- src/Message/Response.php | 9 +++-- src/Message/ServerRequest.php | 46 ++++++++++++++----------- test/tests/unit/Message/RequestTest.php | 25 +++++++++----- 7 files changed, 101 insertions(+), 61 deletions(-) diff --git a/psalm.xml b/psalm.xml index 030ec2f..9b0f03a 100644 --- a/psalm.xml +++ b/psalm.xml @@ -13,4 +13,16 @@ + + + + + + + + + + + + diff --git a/src/Message/Message.php b/src/Message/Message.php index 0bdd913..b5e1a5e 100644 --- a/src/Message/Message.php +++ b/src/Message/Message.php @@ -28,11 +28,13 @@ abstract class Message implements MessageInterface * * @param array $headers Associative array with header field names as * (string) keys and lists of header field values (string[]) as values. - * @param StreamInterface $body A stream representation of the message + * @param StreamInterface|null $body A stream representation of the message * entity body */ - public function __construct(array $headers = null, StreamInterface $body = null) - { + public function __construct( + array $headers = [], + ?StreamInterface $body = null + ) { $this->headers = new HeaderCollection(); if ($headers) { foreach ($headers as $name => $values) { @@ -265,22 +267,32 @@ abstract class Message implements MessageInterface // ------------------------------------------------------------------------ - private function getValidatedHeaders($name, $value) + /** + * @param mixed $name + * @param mixed|mixed[] $values + * @return string[] + * @throws \InvalidArgumentException Name is not a string or value is not + * a string or array of strings + */ + private function getValidatedHeaders($name, $values) { - $is_allowed = function ($item) { - return is_string($item) || is_numeric($item); - }; - if (!is_string($name)) { throw new \InvalidArgumentException('Header name must be a string'); } - if ($is_allowed($value)) { - return [$value]; - } elseif (is_array($value) && count($value) === count(array_filter($value, $is_allowed))) { - return $value; - } else { + if (!is_array($values)) { + $values = [$values]; + } + + $isNotStringOrNumber = function ($item): bool { + return !(is_string($item) || is_numeric($item)); + }; + + $invalid = array_filter($values, $isNotStringOrNumber); + if ($invalid) { throw new \InvalidArgumentException('Header values must be a string or string[]'); } + + return array_map('strval', $values); } } diff --git a/src/Message/NullStream.php b/src/Message/NullStream.php index 468ad36..bf3a34d 100644 --- a/src/Message/NullStream.php +++ b/src/Message/NullStream.php @@ -91,6 +91,7 @@ class NullStream implements StreamInterface * PHP $whence values for `fseek()`. SEEK_SET: Set position equal to * offset bytes SEEK_CUR: Set position to current location plus offset * SEEK_END: Set position to end-of-stream plus offset. + * @return void * @throws \RuntimeException on failure. */ public function seek($offset, $whence = SEEK_SET) @@ -103,6 +104,7 @@ class NullStream implements StreamInterface * * @see seek() * @link http://www.php.net/manual/en/function.fseek.php + * @return void * @throws \RuntimeException on failure. */ public function rewind() diff --git a/src/Message/Request.php b/src/Message/Request.php index 2a95ae6..412bc98 100644 --- a/src/Message/Request.php +++ b/src/Message/Request.php @@ -22,7 +22,7 @@ class Request extends Message implements RequestInterface { /** @var string */ protected $method; - /** @var string */ + /** @var string|null */ protected $requestTarget; /** @var UriInterface */ protected $uri; @@ -33,26 +33,24 @@ class Request extends Message implements RequestInterface * Create a new Request. * * @see \WellRESTed\Message\Message - * @param UriInterface $uri + * @param string|UriInterface $uri * @param string $method * @param array $headers - * @param StreamInterface $body + * @param StreamInterface|null $body */ public function __construct( - UriInterface $uri = null, - $method = "GET", - array $headers = null, - StreamInterface $body = null + string $method = 'GET', + $uri = '', + array $headers = [], + ?StreamInterface $body = null ) { parent::__construct($headers, $body); - - if ($uri !== null) { - $this->uri = $uri; - } else { - $this->uri = new Uri(); - } - $this->method = $method; + if (!($uri instanceof UriInterface)) { + $uri = new Uri($uri); + } + $this->uri = $uri; + $this->requestTarget = null; } public function __clone() @@ -83,7 +81,7 @@ class Request extends Message implements RequestInterface public function getRequestTarget() { // Use the explicitly set request target first. - if (isset($this->requestTarget)) { + if ($this->requestTarget !== null) { return $this->requestTarget; } @@ -211,7 +209,7 @@ class Request extends Message implements RequestInterface // ------------------------------------------------------------------------ /** - * @param string $method + * @param mixed $method * @return string * @throws \InvalidArgumentException */ diff --git a/src/Message/Response.php b/src/Message/Response.php index 7f5d586..aca9c0c 100644 --- a/src/Message/Response.php +++ b/src/Message/Response.php @@ -36,10 +36,13 @@ class Response extends Message implements ResponseInterface * @see \WellRESTed\Message\Message * @param int $statusCode * @param array $headers - * @param StreamInterface $body + * @param StreamInterface|null $body */ - public function __construct($statusCode = 500, array $headers = null, StreamInterface $body = null) - { + public function __construct( + int $statusCode = 500, + array $headers = [], + ?StreamInterface $body = null + ) { parent::__construct($headers, $body); $this->statusCode = $statusCode; $this->reasonPhrase = $this->getDefaultReasonPhraseForStatusCode($statusCode); diff --git a/src/Message/ServerRequest.php b/src/Message/ServerRequest.php index 49cc798..8e8e458 100644 --- a/src/Message/ServerRequest.php +++ b/src/Message/ServerRequest.php @@ -5,6 +5,7 @@ namespace WellRESTed\Message; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\StreamInterface; use Psr\Http\Message\UploadedFileInterface; +use Psr\Http\Message\UriInterface; /** * Representation of an incoming, server-side HTTP request. @@ -336,9 +337,13 @@ class ServerRequest extends Request implements ServerRequestInterface // ------------------------------------------------------------------------ - protected function readFromServerRequest(array $attributes = null) + /** + * @param array $attributes + * @return void + */ + protected function readFromServerRequest(array $attributes = []) { - $this->attributes = $attributes ?: []; + $this->attributes = $attributes; $this->serverParams = $_SERVER; $this->cookieParams = $_COOKIE; $this->readUploadedFiles($_FILES); @@ -367,7 +372,7 @@ class ServerRequest extends Request implements ServerRequestInterface } } - protected function readUploadedFiles($input) + protected function readUploadedFiles(array $input): void { $uploadedFiles = []; foreach ($input as $name => $value) { @@ -376,8 +381,11 @@ class ServerRequest extends Request implements ServerRequestInterface $this->uploadedFiles = $uploadedFiles; } - protected function addUploadedFilesToBranch(&$branch, $name, $value) - { + protected function addUploadedFilesToBranch( + array &$branch, + string $name, + array $value + ): void { // Check for each of the expected keys. if (isset($value["name"], $value["type"], $value["tmp_name"], $value["error"], $value["size"])) { // This is a file. It may be a single file, or a list of files. @@ -419,7 +427,7 @@ class ServerRequest extends Request implements ServerRequestInterface } } - protected function readUri() + protected function readUri(): UriInterface { $uri = ""; @@ -449,7 +457,7 @@ class ServerRequest extends Request implements ServerRequestInterface * @return static * @static */ - public static function getServerRequest(array $attributes = null) + public static function getServerRequest(array $attributes = []) { $request = new static(); $request->readFromServerRequest($attributes); @@ -480,28 +488,22 @@ class ServerRequest extends Request implements ServerRequestInterface protected function getServerRequestHeaders() { // http://www.php.net/manual/en/function.getallheaders.php#84262 - $headers = array(); + $headers = []; foreach ($_SERVER as $name => $value) { if (substr($name, 0, 5) === "HTTP_") { $name = $this->normalizeHeaderName(substr($name, 5)); - $headers[$name] = $value; - } elseif ($this->isContentHeader($name, $value)) { + $headers[$name] = trim($value); + } elseif ($this->isContentHeader($name) && !empty(trim($value))) { $name = $this->normalizeHeaderName($name); - $headers[$name] = $value; + $headers[$name] = trim($value); } } return $headers; } - /** - * @param $name - * @param $value - * @return bool - */ - private function isContentHeader($name, $value) + private function isContentHeader(string $name): bool { - return ($name === "CONTENT_LENGTH" || $name === "CONTENT_TYPE") - && trim($value); + return ($name === 'CONTENT_LENGTH' || $name === 'CONTENT_TYPE'); } /** @@ -540,7 +542,11 @@ class ServerRequest extends Request implements ServerRequestInterface return true; } - private function isValidUploadedFilesBranch($branch) + /** + * @param UploadedFileInterface|array $branch + * @return bool + */ + private function isValidUploadedFilesBranch($branch): bool { if (is_array($branch)) { // Branch. diff --git a/test/tests/unit/Message/RequestTest.php b/test/tests/unit/Message/RequestTest.php index 4d3223d..8b8cb9f 100644 --- a/test/tests/unit/Message/RequestTest.php +++ b/test/tests/unit/Message/RequestTest.php @@ -19,32 +19,39 @@ class RequestTest extends TestCase $this->assertNotNull($request); } + public function testCreatesInstanceWithMethod() + { + $method = 'POST'; + $request = new Request($method); + $this->assertSame($method, $request->getMethod()); + } + public function testCreatesInstanceWithUri() { $uri = new Uri(); - $request = new Request($uri); + $request = new Request('GET', $uri); $this->assertSame($uri, $request->getUri()); } - public function testCreatesInstanceWithMethod() + public function testCreatesInstanceWithStringUri() { - $method = "POST"; - $request = new Request(null, $method); - $this->assertSame($method, $request->getMethod()); + $uri = 'http://localhost:8080'; + $request = new Request('GET', $uri); + $this->assertSame($uri, (string) $request->getUri()); } public function testSetsHeadersOnConstruction() { - $request = new Request(null, null, [ - "X-foo" => ["bar","baz"] + $request = new Request('GET', '/', [ + 'X-foo' => ['bar', 'baz'] ]); - $this->assertEquals(["bar","baz"], $request->getHeader("X-foo")); + $this->assertEquals(['bar', 'baz'], $request->getHeader('X-foo')); } public function testSetsBodyOnConstruction() { $body = new NullStream(); - $request = new Request(null, null, [], $body); + $request = new Request('GET', '/', [], $body); $this->assertSame($body, $request->getBody()); }