From 3f7d2cf45ea85c81c993c50116ada662ba11491e Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Fri, 9 Jul 2021 11:42:48 -0400 Subject: [PATCH] [bug] fix HttpOptions query/parameters --- src/Browser/BrowserKitBrowser.php | 9 ++++++- src/Browser/HttpOptions.php | 42 ++++++++++++++++++++++++++++--- tests/BrowserKitBrowserTests.php | 8 ++++++ tests/Fixture/Kernel.php | 2 +- tests/HttpOptionsTest.php | 18 ++++++------- 5 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/Browser/BrowserKitBrowser.php b/src/Browser/BrowserKitBrowser.php index 821661f..cf9aced 100644 --- a/src/Browser/BrowserKitBrowser.php +++ b/src/Browser/BrowserKitBrowser.php @@ -104,7 +104,14 @@ final public function request(string $method, string $url, $options = []): self $options = HttpOptions::create($options); - $this->inner->request($method, $url, $options->query(), $options->files(), $options->server(), $options->body()); + $this->inner->request( + $method, + $options->addQueryToUrl($url), + $options->parameters(), + $options->files(), + $options->server(), + $options->body() + ); return $this; } diff --git a/src/Browser/HttpOptions.php b/src/Browser/HttpOptions.php index c3ecf61..50aac09 100644 --- a/src/Browser/HttpOptions.php +++ b/src/Browser/HttpOptions.php @@ -21,7 +21,7 @@ class HttpOptions // server variables 'server' => [], - // raw request body as string + // request body 'body' => null, // if set, will json_encode and use as the body and @@ -159,9 +159,11 @@ final public function withFiles(array $files): self } /** + * @param string|array|null $body + * * @return static */ - final public function withBody(?string $body): self + final public function withBody($body): self { $this->options['body'] = $body; @@ -190,12 +192,39 @@ final public function asAjax(): self return $this; } + final public function addQueryToUrl(string $url): string + { + $parts = \parse_url($url); + + if (isset($parts['query'])) { + \parse_str($parts['query'], $query); + } else { + $query = []; + } + + // merge query on url with the query option + $parts['query'] = \http_build_query(\array_merge($query, $this->options['query'])); + + $scheme = isset($parts['scheme']) ? $parts['scheme'].'://' : ''; + $host = $parts['host'] ?? ''; + $port = isset($parts['port']) ? ':'.$parts['port'] : ''; + $user = $parts['user'] ?? ''; + $pass = isset($parts['pass']) ? ':'.$parts['pass'] : ''; + $pass = ($user || $pass) ? "{$pass}@" : ''; + $path = $parts['path'] ?? ''; + $query = isset($parts['query']) && $parts['query'] ? '?'.$parts['query'] : ''; + $fragment = isset($parts['fragment']) ? '#'.$parts['fragment'] : ''; + + return $scheme.$user.$pass.$host.$port.$path.$query.$fragment; + } + /** * @internal */ - final public function query(): array + final public function parameters(): array { - return $this->options['query']; + // when body is array, use as request parameters + return \is_array($this->options['body']) ? $this->options['body'] : []; } /** @@ -251,6 +280,11 @@ final public function server(): array */ final public function body(): ?string { + if (\is_array($this->options['body'])) { + // when body is array, it's used as the request parameters + return null; + } + if (null === $this->options['json']) { return $this->options['body']; } diff --git a/tests/BrowserKitBrowserTests.php b/tests/BrowserKitBrowserTests.php index b83e9eb..025f699 100644 --- a/tests/BrowserKitBrowserTests.php +++ b/tests/BrowserKitBrowserTests.php @@ -206,6 +206,14 @@ public function http_method_actions(): void ->assertContains('"x-token":["my-token"]') ->post('/http-method', HttpOptions::json()->withHeader('content-type', 'application/ld+json')) ->assertContains('"content-type":["application\/ld+json"]') + ->post('/http-method?q1=qv1') + ->assertContains('"query":{"q1":"qv1"}') + ->post('/http-method', ['query' => ['q1' => 'qv1']]) + ->assertContains('"query":{"q1":"qv1"}') + ->post('/http-method?q1=qv1', ['query' => ['q2' => 'qv2']]) + ->assertContains('"query":{"q1":"qv1","q2":"qv2"}') + ->post('/http-method', ['body' => ['b1' => 'bv1']]) + ->assertContains('"request":{"b1":"bv1"}') ; } diff --git a/tests/Fixture/Kernel.php b/tests/Fixture/Kernel.php index 182cf88..bf2c4f0 100644 --- a/tests/Fixture/Kernel.php +++ b/tests/Fixture/Kernel.php @@ -67,7 +67,7 @@ public function httpMethod(Request $request): Response 'attributes' => $request->attributes->all(), 'files' => $request->files->all(), 'server' => $request->server->all(), - 'request' => $request->query->all(), + 'request' => $request->request->all(), 'content' => $request->getContent(), 'ajax' => $request->isXmlHttpRequest(), ]); diff --git a/tests/HttpOptionsTest.php b/tests/HttpOptionsTest.php index ff39599..4c48872 100644 --- a/tests/HttpOptionsTest.php +++ b/tests/HttpOptionsTest.php @@ -17,7 +17,7 @@ public function defaults(): void { $options = new HttpOptions(); - $this->assertEmpty($options->query()); + $this->assertSame('/', $options->addQueryToUrl('/')); $this->assertEmpty($options->files()); $this->assertEmpty($options->server()); $this->assertNull($options->body()); @@ -30,7 +30,7 @@ public function can_configure_with_constructor_array(): void { $options = new HttpOptions([ 'headers' => ['header' => 'header value'], - 'query' => $expectedQuery = ['param' => 'param value'], + 'query' => ['param' => 'param value'], 'files' => $expectedFiles = ['file' => 'file value'], 'server' => ['server' => 'server value'], 'body' => $expectedBody = 'body value', @@ -38,7 +38,7 @@ public function can_configure_with_constructor_array(): void 'ajax' => false, ]); - $this->assertSame($expectedQuery, $options->query()); + $this->assertSame('/?param=param+value', $options->addQueryToUrl('/')); $this->assertSame($expectedFiles, $options->files()); $this->assertSame(['server' => 'server value', 'HTTP_HEADER' => 'header value'], $options->server()); $this->assertSame($expectedBody, $options->body()); @@ -55,10 +55,10 @@ public function can_configure_via_withers(): void ->withHeader('header2', 'header2 value') ->withFiles($expectedFiles = ['file' => 'file value']) ->withServer(['server' => 'server value']) - ->withQuery($expectedQuery = ['param' => 'param value']) + ->withQuery(['param' => 'param value']) ; - $this->assertSame($expectedQuery, $options->query()); + $this->assertSame('/?param=param+value', $options->addQueryToUrl('/')); $this->assertSame($expectedFiles, $options->files()); $this->assertSame($expectedBody, $options->body()); $this->assertSame( @@ -199,7 +199,7 @@ public function can_merge_with_array(): void { $options = HttpOptions::create([ 'headers' => ['header1' => 'header1 value'], - 'query' => $expectedQuery = ['param' => 'param value'], + 'query' => ['param' => 'param value'], 'files' => $expectedFiles = ['file' => 'file value'], 'server' => ['server' => 'server value'], 'body' => null, @@ -211,7 +211,7 @@ public function can_merge_with_array(): void 'json' => $json = ['json' => 'body'], ]); - $this->assertSame($expectedQuery, $options->query()); + $this->assertSame('/?param=param+value', $options->addQueryToUrl('/')); $this->assertSame($expectedFiles, $options->files()); $this->assertSame(\json_encode($json), $options->body()); $this->assertSame( @@ -234,7 +234,7 @@ public function can_merge_with_http_options_object(): void { $options = HttpOptions::create([ 'headers' => ['header1' => 'header1 value'], - 'query' => $expectedQuery = ['param' => 'param value'], + 'query' => ['param' => 'param value'], 'files' => $expectedFiles = ['file' => 'file value'], 'server' => ['server' => 'server value'], 'body' => null, @@ -243,7 +243,7 @@ public function can_merge_with_http_options_object(): void ]); $options = $options->merge(new class(['headers' => ['header2' => 'header2 value']]) extends HttpOptions {}); - $this->assertSame($expectedQuery, $options->query()); + $this->assertSame('/?param=param+value', $options->addQueryToUrl('/')); $this->assertSame($expectedFiles, $options->files()); $this->assertNull($options->body()); $this->assertSame(