From 8727ddc38cbbc001131ddcea5d81eada4ee79379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Vannicatte?= Date: Thu, 12 May 2022 12:13:34 +0200 Subject: [PATCH] fix(php): make `requestOptions` match other clients (#497) --- .../src/transporter/createTransporter.ts | 4 +- .../lib/RequestOptions/RequestOptions.php | 21 +++--- .../RequestOptions/RequestOptionsFactory.php | 66 +++---------------- .../lib/RetryStrategy/ApiWrapper.php | 29 +++----- .../lib/RetryStrategy/ApiWrapperInterface.php | 15 +---- templates/php/api.mustache | 22 +++++-- 6 files changed, 50 insertions(+), 107 deletions(-) diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts index ce4a832332..fa9a44c48f 100644 --- a/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts +++ b/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts @@ -240,13 +240,13 @@ export function createTransporter({ cacheable: baseRequestOptions?.cacheable, timeout: baseRequestOptions?.timeout, queryParameters: { - ...baseRequestOptions?.queryParameters, ...methodOptions.queryParameters, + ...baseRequestOptions?.queryParameters, }, headers: { Accept: 'application/json', - ...baseRequestOptions?.headers, ...methodOptions.headers, + ...baseRequestOptions?.headers, }, }; diff --git a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptions.php b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptions.php index 45b176223c..e406f11506 100644 --- a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptions.php +++ b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptions.php @@ -8,7 +8,7 @@ final class RequestOptions { private $headers = []; - private $query = []; + private $queryParameters = []; private $body = []; @@ -20,7 +20,7 @@ final class RequestOptions public function __construct(array $options = []) { - foreach (['headers', 'query', 'body'] as $name) { + foreach (['headers', 'queryParameters', 'body'] as $name) { if (isset($options[$name]) && !empty($options[$name])) { $this->{$name} = $options[$name]; } @@ -120,7 +120,7 @@ public function setHeaders($headers) */ public function getQueryParameters() { - return $this->query; + return $this->queryParameters; } /** @@ -128,7 +128,7 @@ public function getQueryParameters() */ public function getBuiltQueryParameters() { - return Helpers::buildQuery($this->query); + return Helpers::buildQuery($this->queryParameters); } /** @@ -141,7 +141,7 @@ public function getBuiltQueryParameters() */ public function addQueryParameter($name, $value) { - $this->query[$name] = $value; + $this->queryParameters[$name] = $value; return $this; } @@ -156,7 +156,10 @@ public function addQueryParameter($name, $value) */ public function addQueryParameters($parameters) { - $this->query = array_merge($this->query, $parameters); + $this->queryParameters = array_merge( + $this->queryParameters, + $parameters + ); return $this; } @@ -171,8 +174,8 @@ public function addQueryParameters($parameters) */ public function addDefaultQueryParameter($name, $value) { - if (!isset($this->query[$name])) { - $this->query[$name] = $value; + if (!isset($this->queryParameters[$name])) { + $this->queryParameters[$name] = $value; } return $this; @@ -203,7 +206,7 @@ public function addDefaultQueryParameters($queryParameters) */ public function setQueryParameters($queryParameters) { - $this->query = $queryParameters; + $this->queryParameters = $queryParameters; return $this; } diff --git a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php index e116499509..3b7a14f48e 100644 --- a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php +++ b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php @@ -9,8 +9,6 @@ final class RequestOptionsFactory { private $config; - private $validHeaders = ['Content-type', 'User-Agent']; - public function __construct(Configuration $config) { $this->config = $config; @@ -22,21 +20,14 @@ public function __construct(Configuration $config) * * @return \Algolia\AlgoliaSearch\RequestOptions\RequestOptions */ - public function create($options, $defaults = []) + public function create($options) { if (is_array($options)) { - $options += $defaults; - $options = $this->format($options); $options = $this->normalize($options); $options = new RequestOptions($options); } elseif ($options instanceof RequestOptions) { - $defaults = $this->create($defaults); - $options->addDefaultHeaders($defaults->getHeaders()); - $options->addDefaultQueryParameters( - $defaults->getQueryParameters() - ); - $options->addDefaultBodyParameters($defaults->getBody()); + $options = $this->create($options); } else { throw new \InvalidArgumentException( 'RequestOptions can only be created from array or from RequestOptions object' @@ -46,9 +37,9 @@ public function create($options, $defaults = []) return $options->addDefaultHeaders($this->config->getDefaultHeaders()); } - public function createBodyLess($options, $defaults = []) + public function createBodyLess($options) { - $options = $this->create($options, $defaults); + $options = $this->create($options); return $options->addQueryParameters($options->getBody())->setBody([]); } @@ -59,12 +50,13 @@ private function normalize($options) 'headers' => [ 'X-Algolia-Application-Id' => $this->config->getAppId(), 'X-Algolia-API-Key' => $this->config->getAlgoliaApiKey(), - 'User-Agent' => $this->config->getUserAgent() !== null + 'User-Agent' => + $this->config->getUserAgent() !== null ? $this->config->getUserAgent() : UserAgent::get(), 'Content-Type' => 'application/json', ], - 'query' => [], + 'queryParameters' => [], 'body' => [], 'readTimeout' => $this->config->getReadTimeout(), 'writeTimeout' => $this->config->getWriteTimeout(), @@ -72,18 +64,10 @@ private function normalize($options) ]; foreach ($options as $optionName => $value) { - $type = $this->getOptionType($optionName); - - if ( - in_array( - $type, - ['readTimeout', 'writeTimeout', 'connectTimeout'], - true - ) - ) { - $normalized[$type] = $value; + if (is_array($value)) { + $normalized[$optionName] = $this->format($value); } else { - $normalized[$type][$optionName] = $value; + $normalized[$optionName] = $value; } } @@ -103,36 +87,6 @@ private function format($options) return $options; } - private function getOptionType($optionName) - { - if ($this->isValidHeaderName($optionName)) { - return 'headers'; - } elseif ( - in_array( - $optionName, - ['connectTimeout', 'readTimeout', 'writeTimeout'], - true - ) - ) { - return $optionName; - } - - return 'query'; - } - - private function isValidHeaderName($name) - { - if (preg_match('/^X-[a-zA-Z-]+/', $name)) { - return true; - } - - if (in_array($name, $this->validHeaders, true)) { - return true; - } - - return false; - } - public static function getAttributesToFormat() { return ['attributesToRetrieve', 'type']; diff --git a/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php b/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php index 2400c98a59..e3c1c86cfa 100644 --- a/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php +++ b/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php @@ -66,21 +66,15 @@ public function __construct( } } - public function read( - $method, - $path, - $requestOptions = [], - $defaultRequestOptions = [] - ) { + public function read($method, $path, $requestOptions = []) + { if ('GET' === mb_strtoupper($method)) { $requestOptions = $this->requestOptionsFactory->createBodyLess( - $requestOptions, - $defaultRequestOptions + $requestOptions ); } else { $requestOptions = $this->requestOptionsFactory->create( - $requestOptions, - $defaultRequestOptions + $requestOptions ); } @@ -93,23 +87,16 @@ public function read( ); } - public function write( - $method, - $path, - $data = [], - $requestOptions = [], - $defaultRequestOptions = [] - ) { + public function write($method, $path, $data = [], $requestOptions = []) + { if ('DELETE' === mb_strtoupper($method)) { $requestOptions = $this->requestOptionsFactory->createBodyLess( - $requestOptions, - $defaultRequestOptions + $requestOptions ); $data = []; } else { $requestOptions = $this->requestOptionsFactory->create( - $requestOptions, - $defaultRequestOptions + $requestOptions ); } diff --git a/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapperInterface.php b/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapperInterface.php index 347e69e236..6c526eae59 100644 --- a/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapperInterface.php +++ b/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapperInterface.php @@ -4,20 +4,9 @@ interface ApiWrapperInterface { - public function read( - $method, - $path, - $requestOptions = [], - $defaultRequestOptions = [] - ); + public function read($method, $path, $requestOptions = []); - public function write( - $method, - $path, - $data = [], - $requestOptions = [], - $defaultRequestOptions = [] - ); + public function write($method, $path, $data = [], $requestOptions = []); public function send($method, $path, $requestOptions = [], $hosts = null); } diff --git a/templates/php/api.mustache b/templates/php/api.mustache index c2eef3cd26..cb9b481392 100644 --- a/templates/php/api.mustache +++ b/templates/php/api.mustache @@ -139,7 +139,7 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts; * @see {{{dataType}}} {{/isModel}} {{/allParams}} - * @param array $requestOptions Request Options + * @param array $requestOptions The requestOptions to send along with the query, they will be merged with the transporter requestOptions. * * @return {{#returnType}}{{#responses}}{{#dataType}}{{#-first}}array|{{{dataType}}}{{/-first}}{{/dataType}}{{/responses}}{{/returnType}}{{^returnType}}void{{/returnType}} {{#isDeprecated}} @@ -206,6 +206,7 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts; $resourcePath = '{{{path}}}'; $queryParameters = []; + $headers = []; $httpBody = []; {{#vendorExtensions}}{{#queryParams}} @@ -257,7 +258,7 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts; } {{/bodyParams}} {{#headerParams}} - $queryParameters['{{baseName}}'] = ${{paramName}}; + $headers['{{baseName}}'] = ${{paramName}}; {{/headerParams}} {{#servers.0}} $operationHosts = [{{#servers}}"{{{url}}}"{{^-last}}, {{/-last}}{{/servers}}]; @@ -267,16 +268,25 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts; $operationHost = $operationHosts[$this->hostIndex]; {{/servers.0}} - $requestOptions += $queryParameters; - return $this->sendRequest('{{httpMethod}}', $resourcePath, $queryParameters, $httpBody, $requestOptions); + return $this->sendRequest('{{httpMethod}}', $resourcePath, $headers, $queryParameters, $httpBody, $requestOptions); } {{/operation}} - private function sendRequest($method, $resourcePath, $queryParameters, $httpBody, $requestOptions) + private function sendRequest($method, $resourcePath, $headers, $queryParameters, $httpBody, $requestOptions) { - $query = \GuzzleHttp\Psr7\Query::build($queryParameters); + if (!isset($requestOptions['headers'])) { + $requestOptions['headers'] = []; + } + if (!isset($requestOptions['queryParameters'])) { + $requestOptions['queryParameters'] = []; + } + + $requestOptions['headers'] = array_merge($headers, $requestOptions['headers']); + $requestOptions['queryParameters'] = array_merge($queryParameters, $requestOptions['queryParameters']); + + $query = \GuzzleHttp\Psr7\Query::build($requestOptions['queryParameters']); if($method == 'GET') { $request = $this->api->read(