Skip to content

Commit

Permalink
[bug] fix HttpOptions query/parameters (#45)
Browse files Browse the repository at this point in the history
  • Loading branch information
kbond authored Jul 9, 2021
1 parent ecbb08b commit 2b63095
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 15 deletions.
9 changes: 8 additions & 1 deletion src/Browser/BrowserKitBrowser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
42 changes: 38 additions & 4 deletions src/Browser/HttpOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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'] : [];
}

/**
Expand Down Expand Up @@ -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'];
}
Expand Down
8 changes: 8 additions & 0 deletions tests/BrowserKitBrowserTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -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"}')
;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Fixture/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
]);
Expand Down
18 changes: 9 additions & 9 deletions tests/HttpOptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -30,15 +30,15 @@ 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',
'json' => null,
'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());
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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(
Expand Down

0 comments on commit 2b63095

Please sign in to comment.