From 24ac8f6cdae946f8348c07d74b292d04fe76abb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?U=C4=9Fur=20Ar=C4=B1c=C4=B1?= Date: Sat, 17 Jun 2023 20:41:39 +0300 Subject: [PATCH 1/2] Add support for default max tries setting for Http requests --- docs/getting_started.md | 25 ++++++++------- src/Clients/Http.php | 2 +- src/Context.php | 7 +++- tests/Clients/HttpTest.php | 66 ++++++++++++++++++++++++++++++++++++++ tests/ContextTest.php | 19 +++++++++++ 5 files changed, 105 insertions(+), 14 deletions(-) diff --git a/docs/getting_started.md b/docs/getting_started.md index 96f583ec..94986126 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -12,18 +12,19 @@ The library is available on [Packagist](https://packagist.org/packages/shopify/s The first thing your app will need to do to use this library is to set up your configurations. You can do that by calling the `Shopify\Context::initialize` method, which accepts the following settings: -| Param | Type | Required? | Default | Notes | -| ----------------- | ----------------- | :-------: | :----------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `apiKey` | `string` | Yes | - | App API key from the Partners dashboard | -| `apiSecretKey` | `string` | Yes | - | App API secret from the Partners dashboard | -| `scopes` | `string \| array` | Yes | - | App scopes | -| `hostName` | `string` | Yes | - | App host name e.g. `my-app.my-domain.ca`. You may optionally include `https://` or `http://` to determine which scheme to use | -| `sessionStorage` | `SessionStorage` | Yes | - | Session storage strategy. Read our [notes on session handling](issues.md#notes-on-session-handling) for more information | -| `apiVersion` | `string` | No | `'unstable'` | App API version, defaults to unstable | -| `isEmbeddedApp` | `bool` | No | `true` | Whether the app is an embedded app | -| `isPrivateApp` | `bool` | No | `false` | Whether the app is a private app | -| `userAgentPrefix` | `string` | No | - | Prefix for user agent header sent with a request | -| `logger` | `LoggerInterface` | No | - | App logger, so the library can add its own logs to it. Must implement the [PSR-3](https://www.php-fig.org/psr/psr-3/) `Psr\Log\LoggerInterface` interface from the `psr/log` package | +| Param | Type | Required? | Default | Notes | +| ----------------- | ----------------- | :-------: | :----------: | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `apiKey` | `string` | Yes | - | App API key from the Partners dashboard | +| `apiSecretKey` | `string` | Yes | - | App API secret from the Partners dashboard | +| `scopes` | `string \| array` | Yes | - | App scopes | +| `hostName` | `string` | Yes | - | App host name e.g. `my-app.my-domain.ca`. You may optionally include `https://` or `http://` to determine which scheme to use | +| `sessionStorage` | `SessionStorage` | Yes | - | Session storage strategy. Read our [notes on session handling](issues.md#notes-on-session-handling) for more information | +| `apiVersion` | `string` | No | `'unstable'` | App API version, defaults to unstable | +| `isEmbeddedApp` | `bool` | No | `true` | Whether the app is an embedded app | +| `isPrivateApp` | `bool` | No | `false` | Whether the app is a private app | +| `userAgentPrefix` | `string` | No | - | Prefix for user agent header sent with a request | +| `logger` | `LoggerInterface` | No | - | App logger, so the library can add its own logs to it. Must implement the [PSR-3](https://www.php-fig.org/psr/psr-3/) `Psr\Log\LoggerInterface` interface from the `psr/log` package | +| `maxTries` | `int` | No | 1 | Default max try count for `Shopify\Clients\Http` requests, so if a response has a retriable status code it can be tried at least this much times, unless overrided by request methods' own parameter | You should call this method as early as possible in your application, as none of the library's features are available until it is initialized. Below is an example call to `initialize`: diff --git a/src/Clients/Http.php b/src/Clients/Http.php index ee443a0a..628085b3 100644 --- a/src/Clients/Http.php +++ b/src/Clients/Http.php @@ -167,7 +167,7 @@ protected function request( ?int $tries = null, string $dataType = self::DATA_TYPE_JSON ) { - $maxTries = $tries ?? 1; + $maxTries = $tries ?? Context::$MAX_TRIES; $version = require dirname(__FILE__) . '/../version.php'; $userAgentParts = ["Shopify Admin API Library for PHP v$version"]; diff --git a/src/Context.php b/src/Context.php index 03b1f38f..3148c527 100644 --- a/src/Context.php +++ b/src/Context.php @@ -43,6 +43,8 @@ class Context public static $LOGGER = null; /** @var string[] */ public static $CUSTOM_SHOP_DOMAINS = null; + /** @var int */ + public static $MAX_TRIES = 1; /** @var int */ public static $RETRY_TIME_IN_SECONDS = 1; @@ -67,6 +69,7 @@ class Context * @param LoggerInterface|null $logger App logger, so the library can add its own logs to * it * @param string[] $customShopDomains One or more regexps to use when validating domains + * @param int $maxTries Maximum number of times to retry a HTTP request * * @throws \Shopify\Exception\MissingArgumentException */ @@ -82,7 +85,8 @@ public static function initialize( string $privateAppStorefrontAccessToken = null, string $userAgentPrefix = '', LoggerInterface $logger = null, - array $customShopDomains = [] + array $customShopDomains = [], + int $maxTries = 1, ): void { $authScopes = new Scopes($scopes); @@ -135,6 +139,7 @@ public static function initialize( self::$USER_AGENT_PREFIX = $userAgentPrefix; self::$LOGGER = $logger; self::$CUSTOM_SHOP_DOMAINS = $customShopDomains; + self::$MAX_TRIES = $maxTries; self::$IS_INITIALIZED = true; } diff --git a/tests/Clients/HttpTest.php b/tests/Clients/HttpTest.php index c4a9c4d7..273662c1 100644 --- a/tests/Clients/HttpTest.php +++ b/tests/Clients/HttpTest.php @@ -334,6 +334,72 @@ public function testRetryAfterCanBeFloat() $this->assertThat($response, new HttpResponseMatcher(200, [], $this->successResponse)); } + public function testDefaultMaxTriesCanBeSetOnContext() + { + Context::$MAX_TRIES = 2; + + $this->mockTransportRequests([ + new MockRequest( + $this->buildMockHttpResponse(429, null, ['Retry-After' => 0]), + "https://$this->domain/test/path", + "GET", + "^Shopify Admin API Library for PHP v$this->version$", + ), + new MockRequest( + $this->buildMockHttpResponse(200, $this->successResponse), + "https://$this->domain/test/path", + "GET", + null, + [], + null, + null, + true, + true, + ), + ]); + + $client = new Http($this->domain); + + $response = $client->get(path: 'test/path'); + $this->assertThat($response, new HttpResponseMatcher(200, [], $this->successResponse)); + } + + public function testTriesParameterCanOverrideMaxTriesOfContext() + { + Context::$MAX_TRIES = 2; + + $this->mockTransportRequests([ + new MockRequest( + $this->buildMockHttpResponse(429, null, ['Retry-After' => 0]), + "https://$this->domain/test/path", + "GET", + "^Shopify Admin API Library for PHP v$this->version$", + ), + new MockRequest( + $this->buildMockHttpResponse(429, null, ['Retry-After' => 0]), + "https://$this->domain/test/path", + "GET", + "^Shopify Admin API Library for PHP v$this->version$", + ), + new MockRequest( + $this->buildMockHttpResponse(200, $this->successResponse), + "https://$this->domain/test/path", + "GET", + null, + [], + null, + null, + true, + true, + ), + ]); + + $client = new Http($this->domain); + + $response = $client->get(path: 'test/path', tries: 3); + $this->assertThat($response, new HttpResponseMatcher(200, [], $this->successResponse)); + } + public function testRetryLogicForAllRetriableCodes() { $this->mockTransportRequests([ diff --git a/tests/ContextTest.php b/tests/ContextTest.php index 1440b895..916cc032 100644 --- a/tests/ContextTest.php +++ b/tests/ContextTest.php @@ -264,4 +264,23 @@ public function testCanSetCustomShopDomains() $this->assertEquals($domains, Context::$CUSTOM_SHOP_DOMAINS); } + + public function testCanSetMaxTries() + { + $maxTries = 5; + + Context::initialize( + apiKey: 'ash', + apiSecretKey: 'steffi', + scopes: ['sleepy', 'kitty'], + hostName: 'my-friends-cats', + sessionStorage: new MockSessionStorage(), + apiVersion: ApiVersion::LATEST, + isPrivateApp: false, + customShopDomains: [], + maxTries: $maxTries, + ); + + $this->assertEquals($maxTries, Context::$MAX_TRIES); + } } From 29dc7278217d3e0622d5678d591ae98b49d2b390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?U=C4=9Fur=20Ar=C4=B1c=C4=B1?= Date: Sat, 17 Jun 2023 21:00:46 +0300 Subject: [PATCH 2/2] Update CHANGELOG with pr number --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3dcb952..19f04104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## Unreleased +- [#282](https://github.com/Shopify/shopify-api-php/pull/282) Add support for a default `Context::$MAX_TRIES` setting to use in `Shopify\Clients\Http` requests + ## v5.0.0 - 2023-05-10 - [#269](https://github.com/Shopify/shopify-api-php/pull/269) [bugfix] Refactored query string building to account for Shopify-specific array format