Skip to content
Open
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
16 changes: 15 additions & 1 deletion Core/src/RequestWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -465,12 +465,26 @@ private function getExceptionMessage(\Exception $ex)
/**
* Gets a set of request options.
*
* Per-call options are merged over the client-level ones rather than replacing
* them, so that transport settings configured once on the client (`proxy`,
* `verify`, `cert`, ...) still apply to calls that supply their own options.
* The merge is shallow, because Guzzle options are a flat list of mostly
* scalar values; `headers` is the one nested array worth merging on its own,
* so that a per-call header does not drop the client's default headers.
*
* @param array $options
* @return array
*/
private function getRequestOptions(array $options)
{
$restOptions = $options['restOptions'] ?? $this->restOptions;
$perCallRestOptions = $options['restOptions'] ?? [];
$headers = ($perCallRestOptions['headers'] ?? []) + ($this->restOptions['headers'] ?? []);
$restOptions = $perCallRestOptions + $this->restOptions;

if ($headers) {
$restOptions['headers'] = $headers;
}
Comment on lines +480 to +486

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This logic would be cleaner like this:

Suggested change
$perCallRestOptions = $options['restOptions'] ?? [];
$headers = ($perCallRestOptions['headers'] ?? []) + ($this->restOptions['headers'] ?? []);
$restOptions = $perCallRestOptions + $this->restOptions;
if ($headers) {
$restOptions['headers'] = $headers;
}
$restOptions = ($options['restOptions'] ?? []) + $this->restOptions;
if ($headers = ($options['restOptions']['headers'] ?? []) + ($this->restOptions['headers'] ?? [])) {
$restOptions['headers'] = $headers;
}


$timeout = $options['requestTimeout'] ?? $this->requestTimeout;

if ($timeout && !array_key_exists('timeout', $restOptions)) {
Expand Down
67 changes: 67 additions & 0 deletions Core/tests/Unit/RequestWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,73 @@ public function testSuccessfullySendsAsyncRequest()
);
}

public function testClientLevelRestOptionsSurvivePerCallRestOptions()
{
$actualOptions = [];
$requestWrapper = new RequestWrapper([
'accessToken' => 'abc',
'restOptions' => ['proxy' => 'http://proxy.example.com:8080', 'verify' => false],
'httpHandler' => function ($request, $options = []) use (&$actualOptions) {
$actualOptions = $options;
return new Response(200);
}
]);

$requestWrapper->send(
new Request('GET', 'http://www.example.com'),
['restOptions' => ['headers' => ['X-Goog-Hash' => 'crc32c=abc']]]
);

$this->assertEquals('http://proxy.example.com:8080', $actualOptions['proxy']);
$this->assertFalse($actualOptions['verify']);
$this->assertEquals(['X-Goog-Hash' => 'crc32c=abc'], $actualOptions['headers']);
}

public function testPerCallRestOptionsTakePrecedenceOverClientLevelOnes()
{
$actualOptions = [];
$requestWrapper = new RequestWrapper([
'accessToken' => 'abc',
'restOptions' => ['proxy' => 'http://proxy.example.com:8080', 'debug' => false],
'httpHandler' => function ($request, $options = []) use (&$actualOptions) {
$actualOptions = $options;
return new Response(200);
}
]);

$requestWrapper->send(
new Request('GET', 'http://www.example.com'),
['restOptions' => ['debug' => true]]
);

$this->assertTrue($actualOptions['debug']);
$this->assertEquals('http://proxy.example.com:8080', $actualOptions['proxy']);
}

public function testRestOptionHeadersAreMergedRatherThanReplaced()
{
$actualOptions = [];
$requestWrapper = new RequestWrapper([
'accessToken' => 'abc',
'restOptions' => ['headers' => ['X-Client' => 'default', 'X-Keep' => 'yes']],
'httpHandler' => function ($request, $options = []) use (&$actualOptions) {
$actualOptions = $options;
return new Response(200);
}
]);

$requestWrapper->send(
new Request('GET', 'http://www.example.com'),
['restOptions' => ['headers' => ['X-Client' => 'per-call', 'X-Goog-Hash' => 'crc32c=abc']]]
);

$this->assertEquals([
'X-Client' => 'per-call',
'X-Goog-Hash' => 'crc32c=abc',
'X-Keep' => 'yes',
], $actualOptions['headers']);
}

public function testSendAsyncRetriesOnFailure()
{
$actualDelays = 0;
Expand Down
Loading