diff --git a/CLAUDE.md b/CLAUDE.md index d664781..fb9e22a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,7 +11,8 @@ Guidance for Claude Code in this repo. This file covers what rarely changes; dee ## V2 architecture - **Facade**: `src/SyncV2Sdk.php`. -- **Config**: `src/Config/SyncConfigV2.php` — `appId` (must be a UUID), `apiUrl`, `token`, optional `targetIndex`. +- **Config**: `src/Config/SyncConfigV2.php` — `appId` (must be a UUID), `apiUrl`, `token`, optional `targetIndex`, `timeout` (30s), `connectTimeout` (10s), `retryPolicy`. +- **Transport**: `src/Client/HttpClient.php` builds an `HttpRequest`, hands it to a `Client\Transport\Transport` (default `CurlTransport`), and retries idempotent calls per `Client\RetryPolicy` (transport errors, 5xx, 429; equal-jitter exponential backoff). POST is non-idempotent unless the facade passes `idempotent: true` — only bulk-operations, V1 sync/delete-products and normalize do. Never flag a configuration POST idempotent. `TransportException extends ApiException` with status 0 for "no response at all". Tests inject a fake `Transport` and `Sleeper` (see `tests/Client/Support/`). - **Endpoints**: `/api/v2/applications/{appId}/...`. - **Payloads**: strict immutable readonly ValueObjects in `src/V2/ValueObjects/` (BulkOperations, Index, Normalize, Product, Response, Search, SearchSettings, Synonym, Common), each with constructor validation, a builder, and a `jsonSerialize()` verified against a fixture. See `src/V2/ValueObjects/CLAUDE.md` for the conventions. - **Adapters**: `PrestaShopAdapterV2`, `MagentoAdapterV2` (GraphQL-fed via `src/Magento/`), `ShopifyAdapter` — transform platform product data into V2 payloads. @@ -87,7 +88,7 @@ vendor/bin/phpstan analyse # level 4, src/ only (phpstan.neon); expect "[O vendor/bin/phpcs src tests # PSR-12 (phpcs.xml); expect empty output / exit 0 ``` -`laravel/pint` is in require-dev but NOT wired into CI — phpcs is the authority. No Makefile, no docker-compose, no `.env`; tests are fully offline (HTTP is mocked). +`laravel/pint` is in require-dev but NOT wired into CI — phpcs is the authority. No Makefile, no docker-compose, no `.env`; tests are fully offline (HTTP is mocked at the facade level, or scripted through `tests/Client/Support/FakeTransport.php` at the transport level). ### Install ```bash diff --git a/README.md b/README.md index 59291c0..c5ecbbc 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ use BradSearch\SyncSdk\Models\FieldConfigBuilder; $config = new SyncConfig( baseUrl: 'https://your-api-endpoint.com', authToken: 'your-auth-token', - timeout: 30, + timeout: 30, // total request timeout, seconds + connectTimeout: 10, // connection-establishment timeout, seconds verifySSL: true ); @@ -206,6 +207,34 @@ try { } ``` +`TransportException` (a subclass of `ApiException`) means no HTTP response arrived at all: connection refused, connect or read timeout, DNS or TLS failure. Its `statusCode` is `0` and `responseBody` is `null`. + +## Timeouts and retries + +Every request carries two timeouts from the config: `connectTimeout` (default 10s) bounds the TCP/TLS handshake, `timeout` (default 30s) bounds the whole request. Raise `timeout` for large bulk payloads; keep `connectTimeout` short so a stalled network fails fast. + +Idempotent requests are retried automatically on transport errors, HTTP 5xx and 429 with jittered exponential backoff (1s doubling to an 8s cap, 3 attempts by default). Idempotent means GET, PUT, DELETE, PATCH and the bulk-style POSTs (`bulk-operations`, V1 `sync/` and `delete-products`, `normalize`). Configuration POSTs (`configuration`, `configuration/refresh`, `synonyms`, `index`, `index/activate`, V1 `reindex`) are never retried by the SDK. After the last attempt the exception from the final response is thrown, with its status code and body intact. + +Tune or disable the budget per config: + +```php +use BradSearch\SyncSdk\Client\RetryPolicy; + +new SyncConfigV2( + appId: $appId, + apiUrl: $apiUrl, + token: $token, + timeout: 120, + connectTimeout: 10, + retryPolicy: new RetryPolicy(maxAttempts: 3, baseDelaySeconds: 1.0, maxDelaySeconds: 8.0), +); + +// Caller owns every retry decision: +new SyncConfig($baseUrl, $token, retryPolicy: RetryPolicy::none()); +``` + +For tests, pass an implementation of `BradSearch\SyncSdk\Client\Transport\Transport` as the second constructor argument of `SyncV2Sdk` or `AdminSdk` to script responses without touching the network. + ## Advanced Usage ### Field Filtering diff --git a/src/AdminSdk.php b/src/AdminSdk.php index e996538..461cd8e 100644 --- a/src/AdminSdk.php +++ b/src/AdminSdk.php @@ -5,6 +5,7 @@ namespace BradSearch\SyncSdk; use BradSearch\SyncSdk\Client\AdminHttpClient; +use BradSearch\SyncSdk\Client\Transport\Transport; use BradSearch\SyncSdk\Config\SyncConfig; use BradSearch\SyncSdk\V2\ValueObjects\Response\AllIndicesResponse; @@ -18,9 +19,12 @@ class AdminSdk { private readonly AdminHttpClient $httpClient; - public function __construct(SyncConfig $config) + /** + * @param Transport|null $transport Override the HTTP transport (tests, custom clients); null uses cURL + */ + public function __construct(SyncConfig $config, ?Transport $transport = null) { - $this->httpClient = new AdminHttpClient($config); + $this->httpClient = new AdminHttpClient($config, $transport); } /** diff --git a/src/Client/AdminHttpClient.php b/src/Client/AdminHttpClient.php index f0c6b1b..25da912 100644 --- a/src/Client/AdminHttpClient.php +++ b/src/Client/AdminHttpClient.php @@ -4,99 +4,34 @@ namespace BradSearch\SyncSdk\Client; +use BradSearch\SyncSdk\Client\Transport\Sleeper; +use BradSearch\SyncSdk\Client\Transport\Transport; use BradSearch\SyncSdk\Config\SyncConfig; -use BradSearch\SyncSdk\Exceptions\ApiException; /** * HTTP client for admin operations that includes the X-Admin-Action header. + * + * Thin wrapper over HttpClient so admin calls share its timeouts and retry policy. */ class AdminHttpClient { + private readonly HttpClient $httpClient; + public function __construct( - private readonly SyncConfig $config + SyncConfig $config, + ?Transport $transport = null, + ?Sleeper $sleeper = null, ) { + $this->httpClient = new HttpClient($config, $transport, $sleeper, ['X-Admin-Action: true']); } public function get(string $endpoint): array { - return $this->request('GET', $endpoint); + return $this->httpClient->get($endpoint); } public function delete(string $endpoint): array { - return $this->request('DELETE', $endpoint); - } - - private function request(string $method, string $endpoint, ?array $data = null): array - { - $curl = curl_init(); - - if ($curl === false) { - throw new ApiException('Failed to initialize cURL'); - } - - try { - $url = rtrim($this->config->baseUrl, '/') . '/' . ltrim($endpoint, '/'); - - $options = [ - CURLOPT_URL => $url, - CURLOPT_RETURNTRANSFER => true, - CURLOPT_TIMEOUT => $this->config->timeout, - CURLOPT_CUSTOMREQUEST => $method, - CURLOPT_HTTPHEADER => [ - 'Content-Type: application/json', - 'Authorization: Bearer ' . $this->config->authToken, - 'X-Admin-Action: true', - ], - CURLOPT_SSL_VERIFYPEER => $this->config->verifySSL, - CURLOPT_SSL_VERIFYHOST => $this->config->verifySSL ? 2 : 0, - ]; - - if ($data !== null) { - $json = json_encode($data, JSON_THROW_ON_ERROR); - $options[CURLOPT_POSTFIELDS] = $json; - } - - curl_setopt_array($curl, $options); - - $response = curl_exec($curl); - - if ($response === false) { - $error = curl_error($curl); - throw new ApiException("cURL error: {$error}"); - } - - $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE); - - if (!is_string($response)) { - throw new ApiException('Invalid response from server'); - } - - if ($statusCode < 200 || $statusCode >= 300) { - throw new ApiException( - "API request failed with status {$statusCode}", - $statusCode, - $response - ); - } - - if (empty($response)) { - return []; - } - - try { - $decoded = json_decode($response, true, 512, JSON_THROW_ON_ERROR); - } catch (\JsonException $e) { - throw new ApiException("Failed to decode JSON response: {$e->getMessage()}", $statusCode, $response); - } - - if (!is_array($decoded)) { - throw new ApiException('Expected JSON object in response', $statusCode, $response); - } - - return $decoded; - } finally { - curl_close($curl); - } + return $this->httpClient->delete($endpoint); } } diff --git a/src/Client/HttpClient.php b/src/Client/HttpClient.php index 429bb9d..99bf7ff 100644 --- a/src/Client/HttpClient.php +++ b/src/Client/HttpClient.php @@ -4,15 +4,33 @@ namespace BradSearch\SyncSdk\Client; +use BradSearch\SyncSdk\Client\Transport\CurlTransport; +use BradSearch\SyncSdk\Client\Transport\HttpRequest; +use BradSearch\SyncSdk\Client\Transport\HttpResponse; +use BradSearch\SyncSdk\Client\Transport\NativeSleeper; +use BradSearch\SyncSdk\Client\Transport\Sleeper; +use BradSearch\SyncSdk\Client\Transport\Transport; use BradSearch\SyncSdk\Config\SyncConfig; use BradSearch\SyncSdk\Exceptions\ApiException; -use CurlHandle; +use BradSearch\SyncSdk\Exceptions\TransportException; class HttpClient { + private readonly Transport $transport; + + private readonly Sleeper $sleeper; + + /** + * @param list $extraHeaders Additional raw header lines sent with every request + */ public function __construct( - private readonly SyncConfig $config + private readonly SyncConfig $config, + ?Transport $transport = null, + ?Sleeper $sleeper = null, + private readonly array $extraHeaders = [], ) { + $this->transport = $transport ?? new CurlTransport(); + $this->sleeper = $sleeper ?? new NativeSleeper(); } /** @@ -20,15 +38,18 @@ public function __construct( */ public function get(string $endpoint): array { - return $this->request('GET', $endpoint); + return $this->request('GET', $endpoint, null, true); } /** - * Make a POST request + * Make a POST request. + * + * POST is not retried unless the caller marks it idempotent (bulk operations keyed by id, + * pure computations). Configuration mutations must leave the flag false. */ - public function post(string $endpoint, array $data = []): array + public function post(string $endpoint, array $data = [], bool $idempotent = false): array { - return $this->request('POST', $endpoint, $data); + return $this->request('POST', $endpoint, $data, $idempotent); } /** @@ -36,7 +57,7 @@ public function post(string $endpoint, array $data = []): array */ public function put(string $endpoint, array $data = []): array { - return $this->request('PUT', $endpoint, $data); + return $this->request('PUT', $endpoint, $data, true); } /** @@ -44,7 +65,7 @@ public function put(string $endpoint, array $data = []): array */ public function delete(string $endpoint): array { - return $this->request('DELETE', $endpoint); + return $this->request('DELETE', $endpoint, null, true); } /** @@ -52,82 +73,89 @@ public function delete(string $endpoint): array */ public function patch(string $endpoint, array $data = []): array { - return $this->request('PATCH', $endpoint, $data); + return $this->request('PATCH', $endpoint, $data, true); } /** - * Make HTTP request + * Send the request, retrying transport failures, 5xx and 429 when the call is idempotent. */ - private function request(string $method, string $endpoint, ?array $data = null): array + private function request(string $method, string $endpoint, ?array $data, bool $idempotent): array { - $curl = curl_init(); + $request = $this->buildRequest($method, $endpoint, $data); + $policy = $this->config->retryPolicy; + $maxAttempts = $idempotent ? $policy->maxAttempts : 1; + $attempt = 0; - if ($curl === false) { - throw new ApiException('Failed to initialize cURL'); - } + while (true) { + $attempt++; - try { - $url = rtrim($this->config->baseUrl, '/') . '/' . ltrim($endpoint, '/'); - - $options = [ - CURLOPT_URL => $url, - CURLOPT_RETURNTRANSFER => true, - CURLOPT_TIMEOUT => $this->config->timeout, - CURLOPT_CUSTOMREQUEST => $method, - CURLOPT_HTTPHEADER => [ - 'Content-Type: application/json', - 'Authorization: Bearer ' . $this->config->authToken, - ], - CURLOPT_SSL_VERIFYPEER => $this->config->verifySSL, - CURLOPT_SSL_VERIFYHOST => $this->config->verifySSL ? 2 : 0, - ]; - - if ($data !== null) { - $json = json_encode($data, JSON_THROW_ON_ERROR); - $options[CURLOPT_POSTFIELDS] = $json; + try { + $response = $this->transport->send($request); + } catch (TransportException $e) { + if ($attempt >= $maxAttempts) { + throw $e; + } + + $this->sleeper->sleep($policy->delayBeforeRetry($attempt)); + continue; } - curl_setopt_array($curl, $options); - - $response = curl_exec($curl); - - if ($response === false) { - $error = curl_error($curl); - throw new ApiException("cURL error: {$error}"); + if ($attempt < $maxAttempts && $policy->isRetryableStatus($response->statusCode)) { + $this->sleeper->sleep($policy->delayBeforeRetry($attempt)); + continue; } - $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE); - - if (!is_string($response)) { - throw new ApiException('Invalid response from server'); - } + return $this->decode($response); + } + } - if ($statusCode < 200 || $statusCode >= 300) { - throw new ApiException( - "API request failed with status {$statusCode}", - $statusCode, - $response - ); - } + private function buildRequest(string $method, string $endpoint, ?array $data): HttpRequest + { + $headers = [ + 'Content-Type: application/json', + 'Authorization: Bearer ' . $this->config->authToken, + ...$this->extraHeaders, + ]; + + return new HttpRequest( + method: $method, + url: rtrim($this->config->baseUrl, '/') . '/' . ltrim($endpoint, '/'), + headers: $headers, + body: $data === null ? null : json_encode($data, JSON_THROW_ON_ERROR), + timeout: $this->config->timeout, + connectTimeout: $this->config->connectTimeout, + verifySSL: $this->config->verifySSL, + ); + } - // Handle empty responses (e.g., from DELETE requests) - if (empty($response)) { - return []; - } + private function decode(HttpResponse $response): array + { + $statusCode = $response->statusCode; + $body = $response->body; + + if ($statusCode < 200 || $statusCode >= 300) { + throw new ApiException( + "API request failed with status {$statusCode}", + $statusCode, + $body + ); + } - try { - $decoded = json_decode($response, true, 512, JSON_THROW_ON_ERROR); - } catch (\JsonException $e) { - throw new ApiException("Failed to decode JSON response: {$e->getMessage()}", $statusCode, $response); - } + // Handle empty responses (e.g., from DELETE requests) + if (empty($body)) { + return []; + } - if (!is_array($decoded)) { - throw new ApiException('Expected JSON object in response', $statusCode, $response); - } + try { + $decoded = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + throw new ApiException("Failed to decode JSON response: {$e->getMessage()}", $statusCode, $body); + } - return $decoded; - } finally { - curl_close($curl); + if (!is_array($decoded)) { + throw new ApiException('Expected JSON object in response', $statusCode, $body); } + + return $decoded; } } diff --git a/src/Client/RetryPolicy.php b/src/Client/RetryPolicy.php new file mode 100644 index 0000000..e5d6dab --- /dev/null +++ b/src/Client/RetryPolicy.php @@ -0,0 +1,58 @@ +maxAttempts < 1) { + throw new InvalidFieldConfigException('Retry maxAttempts must be at least 1'); + } + + if ($this->baseDelaySeconds <= 0) { + throw new InvalidFieldConfigException('Retry baseDelaySeconds must be greater than 0'); + } + + if ($this->maxDelaySeconds < $this->baseDelaySeconds) { + throw new InvalidFieldConfigException('Retry maxDelaySeconds must not be lower than baseDelaySeconds'); + } + } + + /** + * A single attempt and no waiting: the caller owns every retry decision. + */ + public static function none(): self + { + return new self(maxAttempts: 1); + } + + public function isRetryableStatus(int $statusCode): bool + { + return $statusCode === 429 || $statusCode >= 500; + } + + /** + * Seconds to wait after the given (1-based) failed attempt before the next one. + */ + public function delayBeforeRetry(int $attempt): float + { + $ceiling = min($this->baseDelaySeconds * (2 ** max(0, $attempt - 1)), $this->maxDelaySeconds); + $unit = random_int(0, 1_000_000) / 1_000_000; + + return $ceiling / 2 + ($ceiling / 2) * $unit; + } +} diff --git a/src/Client/Transport/CurlTransport.php b/src/Client/Transport/CurlTransport.php new file mode 100644 index 0000000..f3cbfe2 --- /dev/null +++ b/src/Client/Transport/CurlTransport.php @@ -0,0 +1,49 @@ + $request->url, + CURLOPT_RETURNTRANSFER => true, + CURLOPT_TIMEOUT => $request->timeout, + CURLOPT_CONNECTTIMEOUT => $request->connectTimeout, + CURLOPT_CUSTOMREQUEST => $request->method, + CURLOPT_HTTPHEADER => $request->headers, + CURLOPT_SSL_VERIFYPEER => $request->verifySSL, + CURLOPT_SSL_VERIFYHOST => $request->verifySSL ? 2 : 0, + ]; + + if ($request->body !== null) { + $options[CURLOPT_POSTFIELDS] = $request->body; + } + + curl_setopt_array($curl, $options); + + $response = curl_exec($curl); + + if ($response === false) { + throw new TransportException('cURL error: ' . curl_error($curl)); + } + + if (!is_string($response)) { + throw new TransportException('Invalid response from server'); + } + + return new HttpResponse(curl_getinfo($curl, CURLINFO_HTTP_CODE), $response); + } +} diff --git a/src/Client/Transport/HttpRequest.php b/src/Client/Transport/HttpRequest.php new file mode 100644 index 0000000..098f13e --- /dev/null +++ b/src/Client/Transport/HttpRequest.php @@ -0,0 +1,25 @@ + $headers Raw header lines, e.g. "Content-Type: application/json" + * @param string|null $body Encoded request body, or null to send none + * @param int $timeout Total request timeout in seconds + * @param int $connectTimeout Connection-establishment timeout in seconds + */ + public function __construct( + public string $method, + public string $url, + public array $headers, + public ?string $body, + public int $timeout, + public int $connectTimeout, + public bool $verifySSL, + ) { + } +} diff --git a/src/Client/Transport/HttpResponse.php b/src/Client/Transport/HttpResponse.php new file mode 100644 index 0000000..27e1925 --- /dev/null +++ b/src/Client/Transport/HttpResponse.php @@ -0,0 +1,14 @@ +validate(); } @@ -34,5 +42,9 @@ private function validate(): void if ($this->timeout <= 0) { throw new InvalidFieldConfigException('Timeout must be greater than 0'); } + + if ($this->connectTimeout <= 0) { + throw new InvalidFieldConfigException('Connect timeout must be greater than 0'); + } } } diff --git a/src/Config/SyncConfigV2.php b/src/Config/SyncConfigV2.php index 060e150..60c60a8 100644 --- a/src/Config/SyncConfigV2.php +++ b/src/Config/SyncConfigV2.php @@ -4,15 +4,24 @@ namespace BradSearch\SyncSdk\Config; +use BradSearch\SyncSdk\Client\RetryPolicy; use BradSearch\SyncSdk\Exceptions\InvalidFieldConfigException; readonly class SyncConfigV2 { + /** + * @param int $timeout Total request timeout in seconds + * @param int $connectTimeout Connection-establishment timeout in seconds + * @param RetryPolicy $retryPolicy Retry budget applied to idempotent requests only + */ public function __construct( public string $appId, public string $apiUrl, public string $token, public ?string $targetIndex = null, + public int $timeout = 30, + public int $connectTimeout = 10, + public RetryPolicy $retryPolicy = new RetryPolicy(), ) { $this->validate(); } @@ -38,6 +47,14 @@ private function validate(): void if (!filter_var($this->apiUrl, FILTER_VALIDATE_URL)) { throw new InvalidFieldConfigException('API URL must be a valid URL'); } + + if ($this->timeout <= 0) { + throw new InvalidFieldConfigException('Timeout must be greater than 0'); + } + + if ($this->connectTimeout <= 0) { + throw new InvalidFieldConfigException('Connect timeout must be greater than 0'); + } } private function isValidUuid(string $uuid): bool diff --git a/src/Exceptions/TransportException.php b/src/Exceptions/TransportException.php new file mode 100644 index 0000000..5bc90da --- /dev/null +++ b/src/Exceptions/TransportException.php @@ -0,0 +1,19 @@ +validate(); } @@ -42,6 +44,10 @@ private function validate(): void throw new InvalidFieldConfigException('Timeout must be greater than 0'); } + if ($this->connectTimeout <= 0) { + throw new InvalidFieldConfigException('Connect timeout must be greater than 0'); + } + if ($this->defaultPageSize <= 0) { throw new InvalidFieldConfigException('Default page size must be greater than 0'); } diff --git a/src/Magento/MagentoGraphQLClient.php b/src/Magento/MagentoGraphQLClient.php index fff42ce..952c37c 100644 --- a/src/Magento/MagentoGraphQLClient.php +++ b/src/Magento/MagentoGraphQLClient.php @@ -52,6 +52,7 @@ public function query(string $query, array $variables = []): array CURLOPT_URL => $this->config->graphqlUrl, CURLOPT_RETURNTRANSFER => true, CURLOPT_TIMEOUT => $this->config->timeout, + CURLOPT_CONNECTTIMEOUT => $this->config->connectTimeout, CURLOPT_POST => true, CURLOPT_POSTFIELDS => json_encode($payload, JSON_THROW_ON_ERROR), CURLOPT_HTTPHEADER => $headers, diff --git a/src/SyncV2Sdk.php b/src/SyncV2Sdk.php index b359b41..7dcf8d6 100644 --- a/src/SyncV2Sdk.php +++ b/src/SyncV2Sdk.php @@ -5,6 +5,7 @@ namespace BradSearch\SyncSdk; use BradSearch\SyncSdk\Client\HttpClient; +use BradSearch\SyncSdk\Client\Transport\Transport; use BradSearch\SyncSdk\Config\SyncConfig; use BradSearch\SyncSdk\Config\SyncConfigV2; use BradSearch\SyncSdk\V2\ValueObjects\BulkOperations\BulkOperationsRequest; @@ -28,15 +29,22 @@ class SyncV2Sdk private readonly string $baseApiPath; + /** + * @param Transport|null $transport Override the HTTP transport (tests, custom clients); null uses cURL + */ public function __construct( - private readonly SyncConfigV2 $config + private readonly SyncConfigV2 $config, + ?Transport $transport = null, ) { $syncConfig = new SyncConfig( baseUrl: $this->config->apiUrl, - authToken: $this->config->token + authToken: $this->config->token, + timeout: $this->config->timeout, + connectTimeout: $this->config->connectTimeout, + retryPolicy: $this->config->retryPolicy, ); - $this->httpClient = new HttpClient($syncConfig); + $this->httpClient = new HttpClient($syncConfig, $transport); $this->baseApiPath = "api/v2/applications/{$this->config->appId}/"; } @@ -269,9 +277,11 @@ public function bulkOperations(BulkOperationsRequest $request): BulkOperationsRe $indexName = $this->config->targetIndex ?? $this->config->appId; $path = $this->baseApiPath . 'index/' . urlencode($indexName) . '/bulk-operations'; + // Idempotent: every operation is keyed by product id, so a retried batch is a harmless overwrite. $response = $this->getHttpClient()->post( $path, - $request->jsonSerialize() + $request->jsonSerialize(), + idempotent: true ); return BulkOperationsResponse::fromArray($response); @@ -341,9 +351,11 @@ public function deleteSearchSettings(): array */ public function normalize(NormalizeRequest $request): NormalizeResponse { + // Idempotent: a pure computation over the request payload, safe to repeat. $response = $this->getHttpClient()->post( $this->baseApiPath . 'normalize', - $request->jsonSerialize() + $request->jsonSerialize(), + idempotent: true ); return NormalizeResponse::fromArray($response); diff --git a/src/SynchronizationApiSdk.php b/src/SynchronizationApiSdk.php index 8aeb3c9..9d23a03 100644 --- a/src/SynchronizationApiSdk.php +++ b/src/SynchronizationApiSdk.php @@ -161,7 +161,7 @@ private function sendBatch(string $index, array $products): void $data['endpoint'] = $this->endpoint; } - $this->httpClient->post('api/v1/sync/', $data); + $this->httpClient->post('api/v1/sync/', $data, idempotent: true); } /** @@ -193,7 +193,7 @@ private function sendDeleteBatch(string $index, array $productsIds): void 'product_ids' => $productsIds, ]; - $this->httpClient->post('api/v1/sync/delete-products', $data); + $this->httpClient->post('api/v1/sync/delete-products', $data, idempotent: true); } /** @@ -294,7 +294,7 @@ public function bulkOperations(array $operations): BulkOperationResult 'operations' => array_map(fn(BulkOperation $op) => $op->toArray(), $operations) ]; - $response = $this->httpClient->post("{$this->apiStartUrl}sync/bulk-operations", $data); + $response = $this->httpClient->post("{$this->apiStartUrl}sync/bulk-operations", $data, idempotent: true); return BulkOperationResult::fromApiResponse($response); } diff --git a/tests/AdminSdkTransportTest.php b/tests/AdminSdkTransportTest.php new file mode 100644 index 0000000..ee0e3a0 --- /dev/null +++ b/tests/AdminSdkTransportTest.php @@ -0,0 +1,54 @@ +deleteIndex('app_x_v1'); + + $request = $transport->requests[0]; + $this->assertSame('DELETE', $request->method); + $this->assertSame('https://api.example.com/api/v2/admin/indices/app_x_v1', $request->url); + $this->assertSame(30, $request->timeout); + $this->assertSame(10, $request->connectTimeout); + $this->assertContains('X-Admin-Action: true', $request->headers); + $this->assertContains('Authorization: Bearer secret', $request->headers); + } + + public function testAdminDeleteIsRetriedOn503(): void + { + $config = new SyncConfig( + baseUrl: 'https://api.example.com', + authToken: 'secret', + retryPolicy: new RetryPolicy(maxAttempts: 2, baseDelaySeconds: 0.001, maxDelaySeconds: 0.001), + ); + $transport = new FakeTransport([ + new HttpResponse(503, 'busy'), + new HttpResponse(200, '{"status":"ok"}'), + ]); + + $result = (new AdminSdk($config, $transport))->deleteIndex('app_x_v1'); + + $this->assertSame(['status' => 'ok'], $result); + $this->assertCount(2, $transport->requests); + } +} diff --git a/tests/Client/HttpClientRetryTest.php b/tests/Client/HttpClientRetryTest.php new file mode 100644 index 0000000..0e900a4 --- /dev/null +++ b/tests/Client/HttpClientRetryTest.php @@ -0,0 +1,282 @@ +sleeper = new FakeSleeper(); + } + + /** + * @param list $outcomes + * @return array{HttpClient, FakeTransport} + */ + private function client(array $outcomes, ?RetryPolicy $policy = null): array + { + $config = new SyncConfig( + baseUrl: 'https://api.example.com', + authToken: 'token', + timeout: 120, + connectTimeout: 7, + retryPolicy: $policy ?? new RetryPolicy(), + ); + $transport = new FakeTransport($outcomes); + + return [new HttpClient($config, $transport, $this->sleeper), $transport]; + } + + public function testRetriesIdempotentCallOn502ThenSucceeds(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(502, 'bad gateway'), + new HttpResponse(200, '{"ok":true}'), + ]); + + $result = $client->get('api/v2/thing'); + + $this->assertSame(['ok' => true], $result); + $this->assertCount(2, $transport->requests); + $this->assertCount(1, $this->sleeper->delays); + } + + public function testRetriesIdempotentCallOn429(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(429, 'slow down'), + new HttpResponse(200, '{}'), + ]); + + $client->put('api/v2/thing', ['a' => 1]); + + $this->assertCount(2, $transport->requests); + } + + public function testRetriesIdempotentCallOnTransportError(): void + { + [$client, $transport] = $this->client([ + new TransportException('cURL error: Connection timed out after 7001 milliseconds'), + new HttpResponse(200, '{}'), + ]); + + $client->delete('api/v2/thing'); + + $this->assertCount(2, $transport->requests); + } + + public function testDoesNotRetryOn400(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(400, '{"error":"bad request"}'), + ]); + + try { + $client->get('api/v2/thing'); + $this->fail('Expected ApiException'); + } catch (ApiException $e) { + $this->assertSame(400, $e->statusCode); + $this->assertSame('{"error":"bad request"}', $e->responseBody); + } + + $this->assertCount(1, $transport->requests); + $this->assertSame([], $this->sleeper->delays); + } + + public function testDoesNotRetryNonIdempotentPostOn503(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(503, 'unavailable'), + ]); + + try { + $client->post('api/v2/configuration', ['x' => 1]); + $this->fail('Expected ApiException'); + } catch (ApiException $e) { + $this->assertSame(503, $e->statusCode); + $this->assertSame('unavailable', $e->responseBody); + } + + $this->assertCount(1, $transport->requests); + $this->assertSame([], $this->sleeper->delays); + } + + public function testDoesNotRetryNonIdempotentPostOnTransportError(): void + { + [$client, $transport] = $this->client([ + new TransportException('cURL error: Connection refused'), + ]); + + $this->expectException(TransportException::class); + $this->expectExceptionMessage('cURL error: Connection refused'); + + try { + $client->post('api/v2/configuration/refresh'); + } finally { + $this->assertCount(1, $transport->requests); + } + } + + public function testRetriesPostFlaggedIdempotent(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(503, 'unavailable'), + new HttpResponse(200, '{"status":"success"}'), + ]); + + $result = $client->post('api/v2/index/x/bulk-operations', ['operations' => []], idempotent: true); + + $this->assertSame(['status' => 'success'], $result); + $this->assertCount(2, $transport->requests); + } + + public function testThrowsLastResponseWhenAttemptsExhausted(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(503, 'first'), + new HttpResponse(502, 'second'), + new HttpResponse(500, '{"error":"third"}'), + ]); + + try { + $client->get('api/v2/thing'); + $this->fail('Expected ApiException'); + } catch (ApiException $e) { + $this->assertSame(500, $e->statusCode); + $this->assertSame('{"error":"third"}', $e->responseBody); + } + + $this->assertCount(3, $transport->requests); + $this->assertCount(2, $this->sleeper->delays); + } + + public function testThrowsLastTransportErrorWhenAttemptsExhausted(): void + { + [$client, $transport] = $this->client([ + new TransportException('cURL error: first'), + new TransportException('cURL error: second'), + new TransportException('cURL error: third'), + ]); + + try { + $client->get('api/v2/thing'); + $this->fail('Expected TransportException'); + } catch (TransportException $e) { + $this->assertSame('cURL error: third', $e->getMessage()); + $this->assertSame(0, $e->statusCode); + $this->assertNull($e->responseBody); + } + + $this->assertCount(3, $transport->requests); + } + + public function testRetryPolicyNoneDisablesRetries(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(503, 'unavailable'), + ], RetryPolicy::none()); + + $this->expectException(ApiException::class); + + try { + $client->get('api/v2/thing'); + } finally { + $this->assertCount(1, $transport->requests); + } + } + + public function testBackoffDelaysStayWithinJitterBounds(): void + { + [$client] = $this->client([ + new HttpResponse(503, ''), + new HttpResponse(503, ''), + new HttpResponse(503, ''), + new HttpResponse(503, ''), + new HttpResponse(503, ''), + ], new RetryPolicy(maxAttempts: 5)); + + try { + $client->get('api/v2/thing'); + } catch (ApiException) { + // expected after 5 attempts + } + + $this->assertCount(4, $this->sleeper->delays); + + // Exponential 1, 2, 4, 8 with equal jitter: each delay lands in [d/2, d]. + $expected = [1.0, 2.0, 4.0, 8.0]; + foreach ($this->sleeper->delays as $i => $delay) { + $this->assertGreaterThanOrEqual($expected[$i] / 2, $delay, "delay #{$i}"); + $this->assertLessThanOrEqual($expected[$i], $delay, "delay #{$i}"); + } + } + + public function testRequestCarriesTimeoutsHeadersAndBody(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(200, '{}'), + ]); + + $client->post('api/v2/thing', ['a' => 1]); + + $request = $transport->requests[0]; + $this->assertSame('POST', $request->method); + $this->assertSame('https://api.example.com/api/v2/thing', $request->url); + $this->assertSame(120, $request->timeout); + $this->assertSame(7, $request->connectTimeout); + $this->assertSame('{"a":1}', $request->body); + $this->assertContains('Authorization: Bearer token', $request->headers); + $this->assertContains('Content-Type: application/json', $request->headers); + } + + public function testGetSendsNoBody(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(200, '{}'), + ]); + + $client->get('api/v2/thing'); + + $this->assertNull($transport->requests[0]->body); + } + + public function testEmptyResponseBodyDecodesToEmptyArray(): void + { + [$client] = $this->client([ + new HttpResponse(204, ''), + ]); + + $this->assertSame([], $client->delete('api/v2/thing')); + } + + public function testInvalidJsonIsNotRetried(): void + { + [$client, $transport] = $this->client([ + new HttpResponse(200, 'not json'), + ]); + + try { + $client->get('api/v2/thing'); + $this->fail('Expected ApiException'); + } catch (ApiException $e) { + $this->assertSame(200, $e->statusCode); + $this->assertSame('not json', $e->responseBody); + } + + $this->assertCount(1, $transport->requests); + } +} diff --git a/tests/Client/RetryPolicyTest.php b/tests/Client/RetryPolicyTest.php new file mode 100644 index 0000000..0d2fb63 --- /dev/null +++ b/tests/Client/RetryPolicyTest.php @@ -0,0 +1,67 @@ +assertSame(3, $policy->maxAttempts); + $this->assertSame(1.0, $policy->baseDelaySeconds); + $this->assertSame(8.0, $policy->maxDelaySeconds); + } + + public function testNoneMeansSingleAttempt(): void + { + $this->assertSame(1, RetryPolicy::none()->maxAttempts); + } + + public function testRetryableStatuses(): void + { + $policy = new RetryPolicy(); + + $this->assertTrue($policy->isRetryableStatus(429)); + $this->assertTrue($policy->isRetryableStatus(500)); + $this->assertTrue($policy->isRetryableStatus(503)); + $this->assertFalse($policy->isRetryableStatus(400)); + $this->assertFalse($policy->isRetryableStatus(404)); + $this->assertFalse($policy->isRetryableStatus(422)); + $this->assertFalse($policy->isRetryableStatus(200)); + } + + public function testDelayGrowsExponentiallyWithEqualJitterAndCap(): void + { + $policy = new RetryPolicy(maxAttempts: 6, baseDelaySeconds: 1.0, maxDelaySeconds: 8.0); + $ceilings = [1 => 1.0, 2 => 2.0, 3 => 4.0, 4 => 8.0, 5 => 8.0]; + + foreach ($ceilings as $attempt => $ceiling) { + for ($i = 0; $i < 50; $i++) { + $delay = $policy->delayBeforeRetry($attempt); + $this->assertGreaterThanOrEqual($ceiling / 2, $delay, "attempt {$attempt}"); + $this->assertLessThanOrEqual($ceiling, $delay, "attempt {$attempt}"); + } + } + } + + public function testRejectsZeroAttempts(): void + { + $this->expectException(InvalidFieldConfigException::class); + + new RetryPolicy(maxAttempts: 0); + } + + public function testRejectsCapBelowBase(): void + { + $this->expectException(InvalidFieldConfigException::class); + + new RetryPolicy(baseDelaySeconds: 2.0, maxDelaySeconds: 1.0); + } +} diff --git a/tests/Client/Support/FakeSleeper.php b/tests/Client/Support/FakeSleeper.php new file mode 100644 index 0000000..663fd79 --- /dev/null +++ b/tests/Client/Support/FakeSleeper.php @@ -0,0 +1,21 @@ + */ + public array $delays = []; + + public function sleep(float $seconds): void + { + $this->delays[] = $seconds; + } +} diff --git a/tests/Client/Support/FakeTransport.php b/tests/Client/Support/FakeTransport.php new file mode 100644 index 0000000..6072add --- /dev/null +++ b/tests/Client/Support/FakeTransport.php @@ -0,0 +1,48 @@ + */ + public array $requests = []; + + /** @var list */ + private array $outcomes; + + /** + * @param list $outcomes + */ + public function __construct(array $outcomes) + { + $this->outcomes = $outcomes; + } + + public function send(HttpRequest $request): HttpResponse + { + $this->requests[] = $request; + + if ($this->outcomes === []) { + throw new LogicException('FakeTransport received more requests than scripted outcomes'); + } + + $outcome = array_shift($this->outcomes); + + if ($outcome instanceof TransportException) { + throw $outcome; + } + + return $outcome; + } +} diff --git a/tests/Config/ConnectTimeoutConfigTest.php b/tests/Config/ConnectTimeoutConfigTest.php new file mode 100644 index 0000000..1f5fb65 --- /dev/null +++ b/tests/Config/ConnectTimeoutConfigTest.php @@ -0,0 +1,94 @@ +assertSame(30, $config->timeout); + $this->assertSame(10, $config->connectTimeout); + $this->assertSame(3, $config->retryPolicy->maxAttempts); + } + + public function testSyncConfigPositionalCallersStillWork(): void + { + $config = new SyncConfig('https://api.example.com', 'token', 45, false); + + $this->assertSame(45, $config->timeout); + $this->assertFalse($config->verifySSL); + $this->assertSame(10, $config->connectTimeout); + } + + public function testSyncConfigRejectsNonPositiveConnectTimeout(): void + { + $this->expectException(InvalidFieldConfigException::class); + $this->expectExceptionMessage('Connect timeout must be greater than 0'); + + new SyncConfig('https://api.example.com', 'token', connectTimeout: 0); + } + + public function testSyncConfigV2Defaults(): void + { + $config = new SyncConfigV2(self::APP_ID, 'https://api.example.com', 'token'); + + $this->assertNull($config->targetIndex); + $this->assertSame(30, $config->timeout); + $this->assertSame(10, $config->connectTimeout); + $this->assertSame(3, $config->retryPolicy->maxAttempts); + } + + public function testSyncConfigV2AcceptsTimeoutsAndPolicy(): void + { + $config = new SyncConfigV2( + appId: self::APP_ID, + apiUrl: 'https://api.example.com', + token: 'token', + targetIndex: 'idx_v2', + timeout: 120, + connectTimeout: 5, + retryPolicy: RetryPolicy::none(), + ); + + $this->assertSame('idx_v2', $config->targetIndex); + $this->assertSame(120, $config->timeout); + $this->assertSame(5, $config->connectTimeout); + $this->assertSame(1, $config->retryPolicy->maxAttempts); + } + + public function testSyncConfigV2RejectsNonPositiveTimeouts(): void + { + $this->expectException(InvalidFieldConfigException::class); + + new SyncConfigV2(self::APP_ID, 'https://api.example.com', 'token', timeout: -1); + } + + public function testMagentoConfigHasConnectTimeout(): void + { + $config = new MagentoConfig('https://shop.example.com/graphql'); + $this->assertSame(10, $config->connectTimeout); + + $custom = new MagentoConfig('https://shop.example.com/graphql', connectTimeout: 3); + $this->assertSame(3, $custom->connectTimeout); + } + + public function testMagentoConfigRejectsNonPositiveConnectTimeout(): void + { + $this->expectException(InvalidFieldConfigException::class); + + new MagentoConfig('https://shop.example.com/graphql', connectTimeout: 0); + } +} diff --git a/tests/SyncV2SdkTransportTest.php b/tests/SyncV2SdkTransportTest.php new file mode 100644 index 0000000..3b3c4e9 --- /dev/null +++ b/tests/SyncV2SdkTransportTest.php @@ -0,0 +1,129 @@ +getSearchSettings(); + + $request = $transport->requests[0]; + $this->assertSame('GET', $request->method); + $this->assertSame('https://api.example.com/api/v2/applications/' . self::APP_ID . '/configuration', $request->url); + $this->assertSame(120, $request->timeout); + $this->assertSame(10, $request->connectTimeout); + $this->assertContains('Authorization: Bearer secret', $request->headers); + } + + public function testDefaultsAreThirtySecondReadAndTenSecondConnect(): void + { + $config = new SyncConfigV2(self::APP_ID, 'https://api.example.com', 'secret'); + $transport = new FakeTransport([new HttpResponse(200, '{}')]); + + (new SyncV2Sdk($config, $transport))->getSearchSettings(); + + $this->assertSame(30, $transport->requests[0]->timeout); + $this->assertSame(10, $transport->requests[0]->connectTimeout); + } + + public function testBulkOperationsIsRetriedOn503(): void + { + $config = $this->fastRetryConfig(); + $transport = new FakeTransport([ + new HttpResponse(503, 'unavailable'), + new HttpResponse(200, json_encode([ + 'status' => 'success', + 'total_operations' => 1, + 'successful_operations' => 1, + 'failed_operations' => 0, + 'results' => [ + ['id' => '1', 'operation' => 'index_products', 'status' => 'created'], + ], + ], JSON_THROW_ON_ERROR)), + ]); + + $response = (new SyncV2Sdk($config, $transport))->bulkOperations($this->bulkRequest()); + + $this->assertSame(1, $response->successfulOperations); + $this->assertCount(2, $transport->requests); + $this->assertSame('POST', $transport->requests[1]->method); + } + + public function testConfigurationRefreshIsNotRetriedOn503(): void + { + $config = $this->fastRetryConfig(); + $transport = new FakeTransport([new HttpResponse(503, 'unavailable')]); + + try { + (new SyncV2Sdk($config, $transport))->refreshConfiguration(); + $this->fail('Expected ApiException'); + } catch (ApiException $e) { + $this->assertSame(503, $e->statusCode); + $this->assertSame('unavailable', $e->responseBody); + } + + $this->assertCount(1, $transport->requests); + } + + private function fastRetryConfig(): SyncConfigV2 + { + return new SyncConfigV2( + appId: self::APP_ID, + apiUrl: 'https://api.example.com', + token: 'secret', + retryPolicy: new RetryPolicy( + maxAttempts: 3, + baseDelaySeconds: 0.001, + maxDelaySeconds: 0.001, + ), + ); + } + + private function bulkRequest(): BulkOperationsRequest + { + return new BulkOperationsRequest([ + BulkOperation::indexProducts([ + new Product( + id: '1', + sku: 'SKU-001', + pricing: new ProductPricing(10.00, 12.00, 8.00, 10.00), + imageUrl: new ImageUrl('https://example.com/s.jpg', 'https://example.com/m.jpg') + ), + ]), + ]); + } +}