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

Add support for default max tries setting for Http requests #282

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions docs/getting_started.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`:

Expand Down
2 changes: 1 addition & 1 deletion src/Clients/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
7 changes: 6 additions & 1 deletion src/Context.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
*/
Expand All @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down
66 changes: 66 additions & 0 deletions tests/Clients/HttpTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
19 changes: 19 additions & 0 deletions tests/ContextTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}