Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpClient] add ResponseInterface::toArray() #30499

Merged
merged 1 commit into from
Mar 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"symfony/polyfill-intl-idn": "^1.10",
"symfony/polyfill-mbstring": "~1.0",
"symfony/polyfill-php72": "~1.5",
"symfony/polyfill-php73": "^1.8"
"symfony/polyfill-php73": "^1.11"
},
"replace": {
"symfony/asset": "self.version",
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/HttpClient/Exception/JsonException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Exception;

use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;

/**
* Thrown by responses' toArray() method when their content cannot be JSON-decoded.
*
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
final class JsonException extends \JsonException implements TransportExceptionInterface
{
}
43 changes: 43 additions & 0 deletions src/Symfony/Component/HttpClient/Response/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpClient\Chunk\ErrorChunk;
use Symfony\Component\HttpClient\Chunk\LastChunk;
use Symfony\Component\HttpClient\Exception\ClientException;
use Symfony\Component\HttpClient\Exception\JsonException;
use Symfony\Component\HttpClient\Exception\RedirectionException;
use Symfony\Component\HttpClient\Exception\ServerException;
use Symfony\Component\HttpClient\Exception\TransportException;
Expand Down Expand Up @@ -52,6 +53,7 @@ trait ResponseTrait
private $timeout;
private $finalInfo;
private $offset = 0;
private $jsonData;

/**
* {@inheritdoc}
Expand Down Expand Up @@ -121,6 +123,47 @@ public function getContent(bool $throw = true): string
return stream_get_contents($this->content);
}

/**
* {@inheritdoc}
*/
public function toArray(bool $throw = true): array
{
if ('' === $content = $this->getContent($throw)) {
throw new TransportException('Response body is empty.');
}

if (null !== $this->jsonData) {
return $this->jsonData;
}

$contentType = $this->headers['content-type'][0] ?? 'application/json';

if (!preg_match('/\bjson\b/i', $contentType)) {
throw new JsonException(sprintf('Response content-type is "%s" while a JSON-compatible one was expected.', $contentType));
}

try {
$content = json_decode($content, true, 512, JSON_BIGINT_AS_STRING | (\PHP_VERSION_ID >= 70300 ? JSON_THROW_ON_ERROR : 0));
} catch (\JsonException $e) {
throw new JsonException($e->getMessage(), $e->getCode());
}

if (\PHP_VERSION_ID < 70300 && JSON_ERROR_NONE !== json_last_error()) {
throw new JsonException(json_last_error_msg(), json_last_error());
}

if (!\is_array($content)) {
throw new JsonException(sprintf('JSON content was expected to decode to an array, %s returned.', \gettype($content)));
}

if (null !== $this->content) {
// Option "buffer" is true
return $this->jsonData = $content;
}

return $content;
}

/**
* Closes the response and all its network handles.
*/
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/HttpClient/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
},
"require": {
"php": "^7.1.3",
"symfony/contracts": "^1.1"
"symfony/contracts": "^1.1",
"symfony/polyfill-php73": "^1.11"
},
"require-dev": {
"nyholm/psr7": "^1.0",
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Contracts/HttpClient/ResponseInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ public function getHeaders(bool $throw = true): array;
*/
public function getContent(bool $throw = true): string;

/**
* Gets the response body decoded as array, typically from a JSON payload.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean component will deal with e.g. XML if requested? What if ppl want to leverage a serializer for this?

Honestly, do want the API? :D

From #30502

ppl can always encode/decode on their own if they really want precise control on these

yet, we kinda want this 30+ line method :) should it be util instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration with Serializer should be provided by decoration.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Mar 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I see no reasons why integration with Serializer should implement HttpClientInterface: turning strings (even when coming from response bodies) to objects is not the domain of an http client to me.

Copy link
Contributor

@ro0NL ro0NL Mar 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case i think we need to update the description to something like Gets a decoded body array representation (typically ...).

Let's say i decorate this toArray to mutate data, but im passing this client globally around in my app; some services might be effected by this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'm not sure to spot the difference, semantically :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed your comment, you're right we can expect HTTP domain; thus the body decoded as-is.

👍 with a simple decoration solution for extension in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would also help #30494 btw

*
* @param bool $throw Whether an exception should be thrown on 3/4/5xx status codes
*
* @throws TransportExceptionInterface When the body cannot be decoded or when a network error occurs
* @throws RedirectionExceptionInterface On a 3xx when $throw is true and the "max_redirects" option has been reached
* @throws ClientExceptionInterface On a 4xx when $throw is true
* @throws ServerExceptionInterface On a 5xx when $throw is true
*/
public function toArray(bool $throw = true): array;

/**
* Returns info coming from the transport layer.
*
Expand Down
35 changes: 18 additions & 17 deletions src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public function testGetRequest()
$this->assertSame(['application/json'], $headers['content-type']);

$body = json_decode($response->getContent(), true);
$this->assertSame($body, $response->toArray());

$this->assertSame('HTTP/1.1', $body['SERVER_PROTOCOL']);
$this->assertSame('/', $body['REQUEST_URI']);
Expand All @@ -79,7 +80,7 @@ public function testNonBufferedGetRequest()
'headers' => ['Foo' => 'baR'],
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();
$this->assertSame('baR', $body['HTTP_FOO']);

$this->expectException(TransportExceptionInterface::class);
Expand All @@ -106,7 +107,7 @@ public function testHttpVersion()
$this->assertSame(200, $response->getStatusCode());
$this->assertSame('HTTP/1.0 200 OK', $response->getInfo('raw_headers')[0]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame('HTTP/1.0', $body['SERVER_PROTOCOL']);
$this->assertSame('GET', $body['REQUEST_METHOD']);
Expand Down Expand Up @@ -203,7 +204,7 @@ public function testInlineAuth()
$client = $this->getHttpClient();
$response = $client->request('GET', 'http://foo:bar%3Dbar@localhost:8057');

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame('foo', $body['PHP_AUTH_USER']);
$this->assertSame('bar=bar', $body['PHP_AUTH_PW']);
Expand All @@ -219,7 +220,7 @@ public function testRedirects()
},
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();
$this->assertSame('GET', $body['REQUEST_METHOD']);
$this->assertSame('Basic Zm9vOmJhcg==', $body['HTTP_AUTHORIZATION']);
$this->assertSame('http://localhost:8057/', $response->getInfo('url'));
Expand Down Expand Up @@ -250,7 +251,8 @@ public function testRelativeRedirects()
$client = $this->getHttpClient();
$response = $client->request('GET', 'http://localhost:8057/302/relative');

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame('/', $body['REQUEST_URI']);
$this->assertNull($response->getInfo('redirect_url'));

Expand Down Expand Up @@ -279,7 +281,7 @@ public function testRedirect307()
'body' => 'foo=bar',
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame(['foo' => 'bar', 'REQUEST_METHOD' => 'POST'], $body);
}
Expand Down Expand Up @@ -388,7 +390,7 @@ public function testOnProgress()
'on_progress' => function (...$state) use (&$steps) { $steps[] = $state; },
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame(['foo' => '0123456789', 'REQUEST_METHOD' => 'POST'], $body);
$this->assertSame([0, 0], \array_slice($steps[0], 0, 2));
Expand All @@ -405,7 +407,7 @@ public function testPostArray()
'body' => ['foo' => 'bar'],
]);

$this->assertSame(['foo' => 'bar', 'REQUEST_METHOD' => 'POST'], json_decode($response->getContent(), true));
$this->assertSame(['foo' => 'bar', 'REQUEST_METHOD' => 'POST'], $response->toArray());
}

public function testPostResource()
Expand All @@ -420,7 +422,7 @@ public function testPostResource()
'body' => $h,
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame(['foo' => '0123456789', 'REQUEST_METHOD' => 'POST'], $body);
}
Expand All @@ -438,7 +440,7 @@ public function testPostCallback()
},
]);

$this->assertSame(['foo' => '0123456789', 'REQUEST_METHOD' => 'POST'], json_decode($response->getContent(), true));
$this->assertSame(['foo' => '0123456789', 'REQUEST_METHOD' => 'POST'], $response->toArray());
}

public function testOnProgressCancel()
Expand Down Expand Up @@ -581,15 +583,15 @@ public function testProxy()
'proxy' => 'http://localhost:8057',
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();
$this->assertSame('localhost:8057', $body['HTTP_HOST']);
$this->assertRegexp('#^http://(localhost|127\.0\.0\.1):8057/$#', $body['REQUEST_URI']);

$response = $client->request('GET', 'http://localhost:8057/', [
'proxy' => 'http://foo:b%3Dar@localhost:8057',
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();
$this->assertSame('Basic Zm9vOmI9YXI=', $body['HTTP_PROXY_AUTHORIZATION']);
}

Expand All @@ -603,7 +605,7 @@ public function testNoProxy()
'proxy' => 'http://localhost:8057',
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertSame('HTTP/1.1', $body['SERVER_PROTOCOL']);
$this->assertSame('/', $body['REQUEST_URI']);
Expand All @@ -629,7 +631,7 @@ public function testAutoEncodingRequest()
$this->assertSame(['Accept-Encoding'], $headers['vary']);
$this->assertContains('gzip', $headers['content-encoding'][0]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();

$this->assertContains('gzip', $body['HTTP_ACCEPT_ENCODING']);
}
Expand All @@ -652,7 +654,7 @@ public function testQuery()
'query' => ['b' => 'b'],
]);

$body = json_decode($response->getContent(), true);
$body = $response->toArray();
$this->assertSame('GET', $body['REQUEST_METHOD']);
$this->assertSame('/?a=a&b=b', $body['REQUEST_URI']);
}
Expand All @@ -673,10 +675,9 @@ public function testUserlandEncodingRequest()
$this->assertContains('gzip', $headers['content-encoding'][0]);

$body = $response->getContent();

$this->assertSame("\x1F", $body[0]);
$body = json_decode(gzdecode($body), true);

$body = json_decode(gzdecode($body), true);
$this->assertSame('gzip', $body['HTTP_ACCEPT_ENCODING']);
}

Expand Down