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

[BC Break] Add requestTimeout option to be shared between REST/gRPC #390

Merged
merged 3 commits into from
Mar 16, 2017
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
3 changes: 2 additions & 1 deletion src/BigQuery/Connection/Rest.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,9 @@ private function resolveUploadOptions(array $args)
unset($args['configuration']);

$uploaderOptionKeys = [
'httpOptions',
'restOptions',
'retries',
'requestTimeout',
'metadata'
];

Expand Down
7 changes: 7 additions & 0 deletions src/Core/GrpcRequestWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public function __construct(array $config = [])
* @param array $options [optional] {
* Request options.
*
* @type float $requestTimeout Seconds to wait before timing out the
* request. **Defaults to** `60`.
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type array $grpcOptions gRPC specific configuration options.
Expand All @@ -108,6 +110,7 @@ public function send(callable $request, array $args, array $options = [])
{
$retries = isset($options['retries']) ? $options['retries'] : $this->retries;
$grpcOptions = isset($options['grpcOptions']) ? $options['grpcOptions'] : $this->grpcOptions;
$timeout = isset($options['requestTimeout']) ? $options['requestTimeout'] : $this->requestTimeout;
$backoff = new ExponentialBackoff($retries, function (\Exception $ex) {
$statusCode = $ex->getCode();

Expand All @@ -122,6 +125,10 @@ public function send(callable $request, array $args, array $options = [])
$grpcOptions['retrySettings'] = new RetrySettings(null, null);
}

if ($timeout && !array_key_exists('timeoutMs', $grpcOptions)) {
$grpcOptions['timeoutMs'] = $timeout * 1000;
}

$optionalArgs = &$args[count($args) - 1];
$optionalArgs += $grpcOptions;

Expand Down
11 changes: 6 additions & 5 deletions src/Core/GrpcTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ public function setRequestWrapper(GrpcRequestWrapper $requestWrapper)
*/
public function send(callable $request, array $args)
{
$requestOptions = $args[count($args) - 1];
$requestOptions = $this->pluckArray([
'grpcOptions',
'retries',
'requestTimeout'
], $args[count($args) - 1]);

return $this->requestWrapper->send($request, $args, array_intersect_key($requestOptions, [
'grpcOptions' => null,
'retries' => null
]));
return $this->requestWrapper->send($request, $args, $requestOptions);
}

/**
Expand Down
21 changes: 14 additions & 7 deletions src/Core/RequestWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class RequestWrapper
/**
* @var array HTTP client specific configuration options.
*/
private $httpOptions;
private $restOptions;

/**
* @var array
Expand Down Expand Up @@ -96,7 +96,7 @@ class RequestWrapper
* @type callable $authHttpHandler A handler used to deliver Psr7
* requests specifically for authentication.
* @type callable $httpHandler A handler used to deliver Psr7 requests.
* @type array $httpOptions HTTP client specific configuration options.
* @type array $restOptions HTTP client specific configuration options.
* @type bool $shouldSignRequest Whether to enable request signing.
* }
*/
Expand All @@ -107,7 +107,7 @@ public function __construct(array $config = [])
'accessToken' => null,
'authHttpHandler' => null,
'httpHandler' => null,
'httpOptions' => [],
'restOptions' => [],
'shouldSignRequest' => true,
'componentVersion' => null
];
Expand All @@ -116,7 +116,7 @@ public function __construct(array $config = [])
$this->accessToken = $config['accessToken'];
$this->httpHandler = $config['httpHandler'] ?: HttpHandlerFactory::build();
$this->authHttpHandler = $config['authHttpHandler'] ?: $this->httpHandler;
$this->httpOptions = $config['httpOptions'];
$this->restOptions = $config['restOptions'];
$this->shouldSignRequest = $config['shouldSignRequest'];
}

Expand All @@ -127,20 +127,27 @@ public function __construct(array $config = [])
* @param array $options [optional] {
* Request options.
*
* @type float $requestTimeout Seconds to wait before timing out the
* request. **Defaults to** `0`.
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type array $httpOptions HTTP client specific configuration options.
* @type array $restOptions HTTP client specific configuration options.
* }
* @return ResponseInterface
*/
public function send(RequestInterface $request, array $options = [])
{
$retries = isset($options['retries']) ? $options['retries'] : $this->retries;
$httpOptions = isset($options['httpOptions']) ? $options['httpOptions'] : $this->httpOptions;
$restOptions = isset($options['restOptions']) ? $options['restOptions'] : $this->restOptions;
$timeout = isset($options['requestTimeout']) ? $options['requestTimeout'] : $this->requestTimeout;
$backoff = new ExponentialBackoff($retries, $this->getRetryFunction());

if ($timeout && !array_key_exists('timeout', $restOptions)) {
$restOptions['timeout'] = $timeout;
}

try {
return $backoff->execute($this->httpHandler, [$this->applyHeaders($request), $httpOptions]);
return $backoff->execute($this->httpHandler, [$this->applyHeaders($request), $restOptions]);
} catch (\Exception $ex) {
throw $this->convertToGoogleException($ex);
}
Expand Down
9 changes: 9 additions & 0 deletions src/Core/RequestWrapperTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ trait RequestWrapperTrait
*/
private $keyFile;

/**
* @var float Seconds to wait before timing out the request. **Defaults to**
* `0` with REST and `60` with gRPC.
*/
private $requestTimeout;

/**
* @var int Number of retries for a failed request. **Defaults to** `3`.
*/
Expand All @@ -74,6 +80,8 @@ trait RequestWrapperTrait
* @type array $keyFile The contents of the service account credentials
* .json file retrieved from the Google Developer's Console.
* Ex: `json_decode(file_get_contents($path), true)`.
* @type float $requestTimeout Seconds to wait before timing out the
* request. **Defaults to** `0` with REST and `60` with gRPC.
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type array $scopes Scopes to be used for the request.
Expand All @@ -87,6 +95,7 @@ public function setCommonDefaults(array $config)
'authCacheOptions' => [],
'credentialsFetcher' => null,
'keyFile' => null,
'requestTimeout' => null,
'retries' => null,
'scopes' => null
];
Expand Down
10 changes: 6 additions & 4 deletions src/Core/RestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/
trait RestTrait
{
use ArrayTrait;
use JsonTrait;

/**
Expand Down Expand Up @@ -67,10 +68,11 @@ public function setRequestWrapper(RequestWrapper $requestWrapper)
*/
public function send($resource, $method, array $options = [])
{
$requestOptions = array_intersect_key($options, [
'httpOptions' => null,
'retries' => null
]);
$requestOptions = $this->pluckArray([
'restOptions',
'retries',
'requestTimeout'
], $options);

return json_decode(
$this->requestWrapper->send(
Expand Down
9 changes: 6 additions & 3 deletions src/Core/Upload/AbstractUploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ abstract class AbstractUploader
* @type array $metadata Metadata on the resource.
* @type int $chunkSize Size of the chunks to send incrementally during
* a resumable upload. Must be in multiples of 262144 bytes.
* @type array $httpOptions HTTP client specific configuration options.
* @type array $restOptions HTTP client specific configuration options.
* @type float $requestTimeout Seconds to wait before timing out the
* request. **Defaults to** `0`.
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type string $contentType Content type of the resource.
Expand All @@ -96,8 +98,9 @@ public function __construct(
$this->metadata = isset($options['metadata']) ? $options['metadata'] : [];
$this->chunkSize = isset($options['chunkSize']) ? $options['chunkSize'] : null;
$this->requestOptions = array_intersect_key($options, [
'httpOptions' => null,
'retries' => null
'restOptions' => null,
'retries' => null,
'requestTimeout' => null
]);

$this->contentType = isset($options['contentType'])
Expand Down
5 changes: 3 additions & 2 deletions src/Storage/Connection/Rest.php
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public function downloadObject(array $args = [])
];

$requestOptions = array_intersect_key($args, [
'httpOptions' => null,
'restOptions' => null,
'retries' => null
]);

Expand Down Expand Up @@ -297,8 +297,9 @@ private function resolveUploadOptions(array $args)
: Psr7\mimetype_from_filename($args['metadata']['name']);

$uploaderOptionKeys = [
'httpOptions',
'restOptions',
'retries',
'requestTimeout',
'chunkSize',
'contentType',
'metadata'
Expand Down
6 changes: 3 additions & 3 deletions src/Storage/EncryptionTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ public function formatEncryptionHeaders(array $options)
+ $this->buildHeaders($destinationKey, $destinationKeySHA256, false);

if (!empty($encryptionHeaders)) {
if (isset($options['httpOptions']['headers'])) {
$options['httpOptions']['headers'] += $encryptionHeaders;
if (isset($options['restOptions']['headers'])) {
$options['restOptions']['headers'] += $encryptionHeaders;
} else {
$options['httpOptions']['headers'] = $encryptionHeaders;
$options['restOptions']['headers'] = $encryptionHeaders;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Storage/StorageObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ public function rename($name, array $options = [])

$this->delete(
array_intersect_key($options, [
'httpOptions' => null,
'restOptions' => null,
'retries' => null
])
);
Expand Down
2 changes: 1 addition & 1 deletion src/Storage/StreamWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public function stream_open($path, $mode, $flags, &$openedPath)
} elseif ($mode == 'r') {
try {
// Lazy read from the source
$options['httpOptions']['stream'] = true;
$options['restOptions']['stream'] = true;
$this->stream = new ReadStream(
$this->bucket->object($this->file)->downloadAsStream($options)
);
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/Core/GrpcRequestWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ public function setUp()
public function testSuccessfullySendsRequest($response, $expectedMessage)
{
$requestWrapper = new GrpcRequestWrapper();
$requestOptions = [
'requestTimeout' => 3.5
];

$actualResponse = $requestWrapper->send(
function ($test) use ($response) {
function ($test, $options) use ($response, $requestOptions) {
$this->assertEquals($requestOptions['requestTimeout'] * 1000, $options['timeoutMs']);
return $response;
},
['test', []]
['test', []],
$requestOptions
);

$this->assertEquals($expectedMessage, $actualResponse);
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/Core/GrpcTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,28 @@ public function testSendsRequest()
$this->assertEquals($message, $actualResponse);
}

public function testSendsRequestWithOptions()
{
$options = [
'requestTimeout' => 3.5,
'grpcOptions' => ['timeoutMs' => 100],
'retries' => 0
];
$message = ['successful' => 'message'];
$this->requestWrapper->send(
Argument::type('callable'),
Argument::type('array'),
$options
)->willReturn($message);

$this->implementation->setRequestWrapper($this->requestWrapper->reveal());
$actualResponse = $this->implementation->send(function () {
return true;
}, [$options]);

$this->assertEquals($message, $actualResponse);
}

public function testGetsGaxConfig()
{
$version = '1.0.0';
Expand Down
11 changes: 9 additions & 2 deletions tests/unit/Core/RequestWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,23 @@ public function testSuccessfullySendsRequest()
{
$expectedBody = 'responseBody';
$response = new Response(200, [], $expectedBody);
$requestOptions = [
'restOptions' => ['debug' => true],
'requestTimeout' => 3.5
];

$requestWrapper = new RequestWrapper([
'accessToken' => 'abc',
'httpHandler' => function ($request, $options = []) use ($response) {
'httpHandler' => function ($request, $options = []) use ($response, $requestOptions) {
$this->assertEquals($requestOptions['restOptions']['debug'], $options['debug']);
$this->assertEquals($requestOptions['requestTimeout'], $options['timeout']);
return $response;
}
]);

$actualResponse = $requestWrapper->send(
new Request('GET', 'http://www.test.com')
new Request('GET', 'http://www.test.com'),
$requestOptions
);

$this->assertEquals($expectedBody, (string) $actualResponse->getBody());
Expand Down
11 changes: 6 additions & 5 deletions tests/unit/Core/RestTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ public function testSendsRequest()

public function testSendsRequestWithOptions()
{
$httpOptions = [
'httpOptions' => ['debug' => true],
'retries' => 5
$restOptions = [
'restOptions' => ['debug' => true],
'retries' => 5,
'requestTimeout' => 3.5
];
$responseBody = '{"whatAWonderful": "response"}';
$this->requestWrapper->send(Argument::any(), $httpOptions)
$this->requestWrapper->send(Argument::any(), $restOptions)
->willReturn(new Response(200, [], $responseBody));

$this->implementation->setRequestBuilder($this->requestBuilder->reveal());
$this->implementation->setRequestWrapper($this->requestWrapper->reveal());
$actualResponse = $this->implementation->send('resource', 'method', $httpOptions);
$actualResponse = $this->implementation->send('resource', 'method', $restOptions);

$this->assertEquals(json_decode($responseBody, true), $actualResponse);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/Storage/Connection/RestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function ($args) use (&$actualRequest, $response) {
'bucket' => 'bigbucket',
'object' => 'myfile.txt',
'generation' => 100,
'httpOptions' => ['debug' => true],
'restOptions' => ['debug' => true],
'retries' => 0
]);

Expand Down
Loading