diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ce28640..6f9f191 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -41,6 +41,35 @@ jobs: - name: Run test suite run: composer run-script test + # Smoke-test the SDK against Symfony HttpClient as the discovered PSR-18 + # implementation, ensuring the discovery + integration path doesn't + # regress on a non-Guzzle stack. Single PHP version is sufficient since + # the SDK doesn't reference symfony types directly. + tests-symfony: + runs-on: ubuntu-latest + name: Tests with Symfony HttpClient (PSR-18) + steps: + - name: Checkout + uses: actions/checkout@ee0669bd1cc54295c223e0bb666b733df41de1c5 # v2 + + - name: Setup PHP + uses: shivammathur/setup-php@accd6127cb78bee3e8082180cb391013d204ef9f # v2 + with: + php-version: '8.3' + + - name: Swap PSR-18 implementation to Symfony HttpClient + run: | + composer remove --dev --no-update guzzlehttp/guzzle + composer require --dev --no-update \ + "symfony/http-client:^6.0 || ^7.0" \ + "nyholm/psr7:^1.0" + + - name: Install dependencies + run: composer install --no-interaction --no-progress + + - name: Run test suite + run: composer run-script test + static-analysis: strategy: fail-fast: false diff --git a/composer.json b/composer.json index 751200c..838a6fe 100644 --- a/composer.json +++ b/composer.json @@ -18,19 +18,32 @@ "license": "MIT", "minimum-stability": "dev", "prefer-stable": true, + "config": { + "allow-plugins": { + "php-http/discovery": false + } + }, "require": { "php": "^7.4 || ^8", "ext-json": "*", - "guzzlehttp/guzzle": "^7", + "php-http/discovery": "^1.15", "psr/cache": "^1 || ^2 || ^3", + "psr/http-client": "^1.0", + "psr/http-client-implementation": "^1.0", + "psr/http-factory": "^1.0", + "psr/http-factory-implementation": "^1.0", "psr/log": "^1 || ^2 || ^3", "symfony/cache": "^5.1.0 || ^6 || ^7 || ^8" }, "require-dev": { - "phpunit/phpunit": "=9.6.33", - "phpstan/phpstan": "=1.12.32" + "guzzlehttp/guzzle": "^7.4", + "guzzlehttp/psr7": "^2.0", + "phpstan/phpstan": "=1.12.32", + "phpunit/phpunit": "=9.6.33" }, "suggest": { + "guzzlehttp/guzzle": "A widely-used PSR-18 HTTP client; auto-discovered when installed.", + "symfony/http-client": "Symfony's PSR-18 HTTP client; auto-discovered when installed.", "monolog/monolog": "A PSR-3 logger to receive SDK log output; pass it via the logger config option. The SDK defaults to Psr\\Log\\NullLogger." }, "autoload": { @@ -43,8 +56,7 @@ "src/EvaluationCore/Util.php", "src/Flag/Util.php", "src/Assignment/AssignmentConstants.php", - "src/Exposure/ExposureConstants.php", - "src/Http/GuzzleConstants.php" + "src/Exposure/ExposureConstants.php" ] }, "autoload-dev": { diff --git a/src/Amplitude/Amplitude.php b/src/Amplitude/Amplitude.php index 40aeed4..07a0ed8 100644 --- a/src/Amplitude/Amplitude.php +++ b/src/Amplitude/Amplitude.php @@ -2,9 +2,11 @@ namespace AmplitudeExperiment\Amplitude; -use AmplitudeExperiment\Http\HttpClientInterface; -use AmplitudeExperiment\Http\GuzzleHttpClient; +use AmplitudeExperiment\Http\HttpClientFactory; use Psr\Http\Client\ClientExceptionInterface; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -21,7 +23,9 @@ class Amplitude * @var array> */ protected array $queue = []; - protected HttpClientInterface $httpClient; + protected ClientInterface $httpClient; + protected RequestFactoryInterface $requestFactory; + protected StreamFactoryInterface $streamFactory; private LoggerInterface $logger; private AmplitudeConfig $config; @@ -30,7 +34,11 @@ public function __construct(string $apiKey, ?AmplitudeConfig $config = null) $this->apiKey = $apiKey; $this->config = $config ?? AmplitudeConfig::builder()->build(); $this->logger = $this->config->logger ?? new NullLogger(); - $this->httpClient = $this->config->httpClient ?? new GuzzleHttpClient($this->config->guzzleClientConfig); + [$this->httpClient, $this->requestFactory, $this->streamFactory] = HttpClientFactory::resolveAll( + $this->config->httpClient, + $this->config->requestFactory, + $this->config->retryConfig + ); } public function flush(): void @@ -67,7 +75,6 @@ public function __destruct() */ private function post(string $url, array $payload): void { - $httpClient = $this->httpClient->getClient(); $payloadJson = json_encode($payload); if ($payloadJson === false) { @@ -77,12 +84,12 @@ private function post(string $url, array $payload): void $payloadString = $this->payloadToString($payload); - $request = $this->httpClient - ->createRequest('POST', $url, $payloadJson) + $request = $this->requestFactory->createRequest('POST', $url) + ->withBody($this->streamFactory->createStream($payloadJson)) ->withHeader('Content-Type', 'application/json'); try { - $response = $httpClient->sendRequest($request); + $response = $this->httpClient->sendRequest($request); if ($response->getStatusCode() != 200) { $this->logger->error('[Amplitude] Failed to send event: ' . $payloadString . ', ' . $response->getStatusCode() . ' ' . $response->getReasonPhrase()); return; diff --git a/src/Amplitude/AmplitudeConfig.php b/src/Amplitude/AmplitudeConfig.php index cd7424b..e9e4f75 100644 --- a/src/Amplitude/AmplitudeConfig.php +++ b/src/Amplitude/AmplitudeConfig.php @@ -4,7 +4,9 @@ use AmplitudeExperiment\Assignment\AssignmentConfig; use AmplitudeExperiment\Assignment\AssignmentConfigBuilder; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; /** @@ -35,14 +37,23 @@ class AmplitudeConfig */ public bool $useBatch; /** - * The underlying HTTP client to use for requests, if this is not set, the default {@link GuzzleHttpClient} will be used. + * The PSR-18 HTTP client to use for requests. If null, a PSR-18 + * implementation is auto-discovered via php-http/discovery and wrapped + * in {@link \AmplitudeExperiment\Http\RetryingClient} using + * {@link $retryConfig}. A user-supplied client is used verbatim with + * no retry wrap. */ - public ?HttpClientInterface $httpClient; + public ?ClientInterface $httpClient; /** - * @var array - * The configuration for the underlying default {@link GuzzleHttpClient} client (if used). See {@link GUZZLE_DEFAULTS} for defaults. + * The PSR-17 request factory used to construct requests. If null, a + * PSR-17 factory is auto-discovered. */ - public array $guzzleClientConfig; + public ?RequestFactoryInterface $requestFactory; + /** + * Retry configuration for the auto-wrapped client. Ignored when + * {@link $httpClient} is supplied — the user's client is used verbatim. + */ + public ?RetryConfig $retryConfig; /** * Set to use a custom PSR-3 logger. If not set, a {@link \Psr\Log\NullLogger} is used * and SDK log messages are discarded. Pass any PSR-3 implementation (e.g. Monolog, or @@ -66,30 +77,30 @@ class AmplitudeConfig 'minIdLength' => 5, 'flushQueueSize' => 200, 'httpClient' => null, - 'guzzleClientConfig' => [], + 'requestFactory' => null, + 'retryConfig' => null, 'logger' => null, ]; - /** - * @param array $guzzleClientConfig - */ public function __construct( - int $flushQueueSize, - int $minIdLength, - string $serverZone, - string $serverUrl, - bool $useBatch, - ?HttpClientInterface $httpClient, - array $guzzleClientConfig, - ?LoggerInterface $logger) - { + int $flushQueueSize, + int $minIdLength, + string $serverZone, + string $serverUrl, + bool $useBatch, + ?ClientInterface $httpClient, + ?RequestFactoryInterface $requestFactory, + ?RetryConfig $retryConfig, + ?LoggerInterface $logger + ) { $this->flushQueueSize = $flushQueueSize; $this->minIdLength = $minIdLength; $this->serverZone = $serverZone; $this->serverUrl = $serverUrl; $this->useBatch = $useBatch; $this->httpClient = $httpClient; - $this->guzzleClientConfig = $guzzleClientConfig; + $this->requestFactory = $requestFactory; + $this->retryConfig = $retryConfig; $this->logger = $logger; } diff --git a/src/Amplitude/AmplitudeConfigBuilder.php b/src/Amplitude/AmplitudeConfigBuilder.php index 3cdea16..7281536 100644 --- a/src/Amplitude/AmplitudeConfigBuilder.php +++ b/src/Amplitude/AmplitudeConfigBuilder.php @@ -2,7 +2,9 @@ namespace AmplitudeExperiment\Amplitude; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; class AmplitudeConfigBuilder @@ -12,11 +14,9 @@ class AmplitudeConfigBuilder protected string $serverZone = AmplitudeConfig::DEFAULTS['serverZone']; protected ?string $serverUrl = null; protected bool $useBatch = AmplitudeConfig::DEFAULTS['useBatch']; - protected ?HttpClientInterface $httpClient = AmplitudeConfig::DEFAULTS['httpClient']; - /** - * @var array - */ - protected array $guzzleClientConfig = AmplitudeConfig::DEFAULTS['guzzleClientConfig']; + protected ?ClientInterface $httpClient = AmplitudeConfig::DEFAULTS['httpClient']; + protected ?RequestFactoryInterface $requestFactory = AmplitudeConfig::DEFAULTS['requestFactory']; + protected ?RetryConfig $retryConfig = AmplitudeConfig::DEFAULTS['retryConfig']; protected ?LoggerInterface $logger = AmplitudeConfig::DEFAULTS['logger']; public function __construct() @@ -53,18 +53,33 @@ public function useBatch(bool $useBatch): AmplitudeConfigBuilder return $this; } - public function httpClient(HttpClientInterface $httpClient): AmplitudeConfigBuilder + /** + * Supply a PSR-18 HTTP client. The SDK uses it verbatim — no retry wrap. + * If omitted, a client is auto-discovered and wrapped in + * {@link \AmplitudeExperiment\Http\RetryingClient} using {@link retryConfig}. + */ + public function httpClient(ClientInterface $httpClient): AmplitudeConfigBuilder { $this->httpClient = $httpClient; return $this; } /** - * @param array $guzzleClientConfig + * Supply a PSR-17 request factory. If omitted, a factory is auto-discovered. + */ + public function requestFactory(RequestFactoryInterface $requestFactory): AmplitudeConfigBuilder + { + $this->requestFactory = $requestFactory; + return $this; + } + + /** + * Configure retry behavior for the auto-discovered client. Ignored when + * a client is supplied via {@link httpClient()}. */ - public function guzzleClientConfig(array $guzzleClientConfig): AmplitudeConfigBuilder + public function retryConfig(RetryConfig $retryConfig): AmplitudeConfigBuilder { - $this->guzzleClientConfig = $guzzleClientConfig; + $this->retryConfig = $retryConfig; return $this; } @@ -90,7 +105,8 @@ public function build(): AmplitudeConfig $this->serverUrl, $this->useBatch, $this->httpClient, - $this->guzzleClientConfig, + $this->requestFactory, + $this->retryConfig, $this->logger ); } diff --git a/src/Backoff.php b/src/Backoff.php deleted file mode 100644 index a701fc7..0000000 --- a/src/Backoff.php +++ /dev/null @@ -1,48 +0,0 @@ -attempts = $attempts; - $this->min = $min; - $this->max = $max; - $this->scalar = $scalar; - } - - public static function doWithBackoff(callable $action, Backoff $backoffPolicy): PromiseInterface - { - $delay = $backoffPolicy->min; - - $retry = function ($attempt) use ($delay, $action, $backoffPolicy, &$retry) { - return $action()->then( - // Success case - function ($result) { - return Create::promiseFor($result); - }, - function () use ($attempt, $backoffPolicy, $retry, &$delay) { - if ($attempt < $backoffPolicy->attempts - 1) { - usleep($delay * 1000); - $delay = min($delay * $backoffPolicy->scalar, $backoffPolicy->max); - return $retry($attempt + 1); - } else { - return Create::promiseFor(null); - } - } - ); - }; - - return $retry(0); - } -} diff --git a/src/Exception/MissingHttpImplementationException.php b/src/Exception/MissingHttpImplementationException.php new file mode 100644 index 0000000..62baaae --- /dev/null +++ b/src/Exception/MissingHttpImplementationException.php @@ -0,0 +1,16 @@ +apiKey = $apiKey; $this->serverUrl = $serverUrl; $this->logger = $logger; $this->httpClient = $httpClient; + $this->requestFactory = $requestFactory; } /** @@ -33,22 +41,19 @@ public function __construct(string $apiKey, LoggerInterface $logger, HttpClientI public function fetch(): array { $endpoint = $this->serverUrl . '/sdk/v2/flags?v=0'; - $request = $this->httpClient->createRequest('GET', $endpoint) + $request = $this->requestFactory->createRequest('GET', $endpoint) ->withHeader('Authorization', 'Api-Key ' . $this->apiKey) ->withHeader('Content-Type', 'application/json') ->withHeader('X-Amp-Exp-Library', 'experiment-php-server/' . VERSION); $this->logger->debug('[Experiment] Fetch flag configs'); - $httpClient = $this->httpClient->getClient(); - - $response = $httpClient->sendRequest($request); + $response = $this->httpClient->sendRequest($request); if ($response->getStatusCode() !== 200) { $this->logger->error('[Experiment] Fetch flag configs - received error response: ' . $response->getStatusCode() . ': ' . $response->getBody()); return []; } $this->logger->debug('[Experiment] Got flag configs: ' . $response->getBody()); return $this->parse(json_decode($response->getBody(), true)); - } /** diff --git a/src/Http/GuzzleConstants.php b/src/Http/GuzzleConstants.php deleted file mode 100644 index 0d4e267..0000000 --- a/src/Http/GuzzleConstants.php +++ /dev/null @@ -1,32 +0,0 @@ - 10000, - /** - * The number of retries to attempt before failing - */ - 'retries' => 8, - /** - * Retry backoff minimum (starting backoff delay) in milliseconds. The minimum backoff is scaled by - * `retryBackoffScalar` after each retry failure. - */ - 'retryBackoffMinMillis' => 500, - /** - * Retry backoff maximum in milliseconds. If the scaled backoff is greater than the max, the max is - * used for all subsequent retries. - */ - 'retryBackoffMaxMillis' => 10000, - /** - * Scales the minimum backoff exponentially. - */ - 'retryBackoffScalar' => 1.5, - /** - * @deprecated This parameter is not used. All requests (initial and retries) use timeoutMillis. - */ - 'retryTimeoutMillis' => 10000 -]; diff --git a/src/Http/GuzzleHttpClient.php b/src/Http/GuzzleHttpClient.php deleted file mode 100644 index d444896..0000000 --- a/src/Http/GuzzleHttpClient.php +++ /dev/null @@ -1,71 +0,0 @@ - - */ - private array $config; - - /** - * @param array $config - */ - public function __construct(array $config) - { - $handlerStack = HandlerStack::create(); - $this->config = array_merge(GUZZLE_DEFAULTS, $config); - - // Add middleware for retries - $handlerStack->push(Middleware::retry( - function ($retries, Request $request, $response = null, $exception = null) { - // Retry if the maximum number of retries is not reached and an exception occurred - return $retries < $this->config['retries'] && $exception instanceof Exception; - }, - function ($retries) { - // Calculate delay - return $this->calculateDelayMillis($retries); - } - )); - - // Create a Guzzle client with the custom handler stack - $this->client = new Client(['handler' => $handlerStack, RequestOptions::TIMEOUT => $this->config['timeoutMillis'] / 1000]); - } - - public function getClient(): ClientInterface - { - return $this->client; - } - - public function createRequest(string $method, string $uri, ?string $body = null): Request - { - return new Request($method, $uri, [], $body); - } - - - protected function calculateDelayMillis(int $iteration): int - { - $delayMillis = $this->config['retryBackoffMinMillis']; - - for ($i = 1; $i < $iteration; $i++) { - $delayMillis = min( - $delayMillis * $this->config['retryBackoffScalar'], - $this->config['retryBackoffMaxMillis'] - ); - } - return $delayMillis; - } -} diff --git a/src/Http/HttpClientFactory.php b/src/Http/HttpClientFactory.php new file mode 100644 index 0000000..0bd15ea --- /dev/null +++ b/src/Http/HttpClientFactory.php @@ -0,0 +1,57 @@ += 1'); + } + if ($backoffMinMillis < 0 || $backoffMaxMillis < $backoffMinMillis) { + throw new InvalidArgumentException('backoff bounds invalid'); + } + if ($backoffScalar < 1.0) { + throw new InvalidArgumentException('backoffScalar must be >= 1.0'); + } + $this->attempts = $attempts; + $this->backoffMinMillis = $backoffMinMillis; + $this->backoffMaxMillis = $backoffMaxMillis; + $this->backoffScalar = $backoffScalar; + $this->retryMethods = array_values(array_map('strtoupper', $retryMethods)); + } +} diff --git a/src/Http/RetryingClient.php b/src/Http/RetryingClient.php new file mode 100644 index 0000000..f84cf80 --- /dev/null +++ b/src/Http/RetryingClient.php @@ -0,0 +1,55 @@ +inner = $inner; + $this->config = $config ?? new RetryConfig(); + } + + public function sendRequest(RequestInterface $request): ResponseInterface + { + if (!in_array(strtoupper($request->getMethod()), $this->config->retryMethods, true)) { + return $this->inner->sendRequest($request); + } + + $delayMillis = $this->config->backoffMinMillis; + + for ($attempt = 1; $attempt < $this->config->attempts; $attempt++) { + try { + return $this->inner->sendRequest($request); + } catch (ClientExceptionInterface $e) { + usleep($delayMillis * 1000); + $delayMillis = (int) min( + $delayMillis * $this->config->backoffScalar, + $this->config->backoffMaxMillis + ); + } + } + + return $this->inner->sendRequest($request); + } +} diff --git a/src/Local/LocalEvaluationClient.php b/src/Local/LocalEvaluationClient.php index c14fbe1..c696929 100644 --- a/src/Local/LocalEvaluationClient.php +++ b/src/Local/LocalEvaluationClient.php @@ -5,21 +5,18 @@ use AmplitudeExperiment\Assignment\AssignmentConfig; use AmplitudeExperiment\Assignment\AssignmentService; use AmplitudeExperiment\EvaluationCore\EvaluationEngine; -use AmplitudeExperiment\EvaluationCore\Types\EvaluationFlag; -use AmplitudeExperiment\Exposure\DefaultExposureFilter; use AmplitudeExperiment\Exposure\DefaultExposureTrackingProvider; use AmplitudeExperiment\Exposure\Exposure; use AmplitudeExperiment\Exposure\ExposureConfig; use AmplitudeExperiment\Exposure\ExposureService; use AmplitudeExperiment\Flag\FlagConfigFetcher; use AmplitudeExperiment\Flag\FlagConfigService; -use AmplitudeExperiment\Http\GuzzleHttpClient; +use AmplitudeExperiment\Http\HttpClientFactory; use AmplitudeExperiment\Amplitude\Amplitude; use AmplitudeExperiment\User; use AmplitudeExperiment\Variant; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -use Symfony\Component\Cache\Adapter\ArrayAdapter; use function AmplitudeExperiment\EvaluationCore\topologicalSort; /** @@ -39,8 +36,12 @@ public function __construct(string $apiKey, ?LocalEvaluationConfig $config = nul { $this->config = $config ?? LocalEvaluationConfig::builder()->build(); $this->logger = $this->config->logger ?? new NullLogger(); - $httpClient = $config->httpClient ?? $this->config->httpClient ?? new GuzzleHttpClient($this->config->guzzleClientConfig); - $fetcher = new FlagConfigFetcher($apiKey, $this->logger, $httpClient, $this->config->serverUrl); + [$httpClient, $requestFactory] = HttpClientFactory::resolveAll( + $this->config->httpClient, + $this->config->requestFactory, + $this->config->retryConfig + ); + $fetcher = new FlagConfigFetcher($apiKey, $this->logger, $httpClient, $requestFactory, $this->config->serverUrl); $this->flagConfigService = new FlagConfigService($fetcher, $this->logger, $this->config->bootstrap); $this->initializeAssignmentService($this->config->assignmentConfig); $this->initializeExposureService($this->config->exposureConfig); diff --git a/src/Local/LocalEvaluationConfig.php b/src/Local/LocalEvaluationConfig.php index 73c8a90..a4e640c 100644 --- a/src/Local/LocalEvaluationConfig.php +++ b/src/Local/LocalEvaluationConfig.php @@ -4,7 +4,9 @@ use AmplitudeExperiment\Assignment\AssignmentConfig; use AmplitudeExperiment\Exposure\ExposureConfig; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; /** @@ -36,14 +38,23 @@ class LocalEvaluationConfig public ?AssignmentConfig $assignmentConfig; public ?ExposureConfig $exposureConfig; /** - * The underlying HTTP client to use for requests, if this is not set, the default {@link GuzzleHttpClient} will be used. + * The PSR-18 HTTP client to use for requests. If null, a PSR-18 + * implementation is auto-discovered via php-http/discovery and wrapped + * in {@link \AmplitudeExperiment\Http\RetryingClient} using + * {@link $retryConfig}. A user-supplied client is used verbatim with + * no retry wrap. */ - public ?HttpClientInterface $httpClient; + public ?ClientInterface $httpClient; /** - * @var array - * The configuration for the underlying default {@link GuzzleHttpClient} client (if used). See {@link GUZZLE_DEFAULTS} for defaults. + * The PSR-17 request factory used to construct requests. If null, a + * PSR-17 factory is auto-discovered. + */ + public ?RequestFactoryInterface $requestFactory; + /** + * Retry configuration for the auto-wrapped client. Ignored when + * {@link $httpClient} is supplied — the user's client is used verbatim. */ - public array $guzzleClientConfig; + public ?RetryConfig $retryConfig; const DEFAULTS = [ 'logger' => null, @@ -52,22 +63,31 @@ class LocalEvaluationConfig 'assignmentConfig' => null, 'exposureConfig' => null, 'httpClient' => null, - 'guzzleClientConfig' => [] + 'requestFactory' => null, + 'retryConfig' => null, ]; /** - * @param array $guzzleClientConfig * @param array $bootstrap */ - public function __construct(?LoggerInterface $logger, string $serverUrl, array $bootstrap, ?AssignmentConfig $assignmentConfig, ?ExposureConfig $exposureConfig, ?HttpClientInterface $httpClient, array $guzzleClientConfig) - { + public function __construct( + ?LoggerInterface $logger, + string $serverUrl, + array $bootstrap, + ?AssignmentConfig $assignmentConfig, + ?ExposureConfig $exposureConfig, + ?ClientInterface $httpClient, + ?RequestFactoryInterface $requestFactory, + ?RetryConfig $retryConfig + ) { $this->logger = $logger; $this->serverUrl = $serverUrl; $this->bootstrap = $bootstrap; $this->assignmentConfig = $assignmentConfig; $this->exposureConfig = $exposureConfig; $this->httpClient = $httpClient; - $this->guzzleClientConfig = $guzzleClientConfig; + $this->requestFactory = $requestFactory; + $this->retryConfig = $retryConfig; } public static function builder(): LocalEvaluationConfigBuilder diff --git a/src/Local/LocalEvaluationConfigBuilder.php b/src/Local/LocalEvaluationConfigBuilder.php index 9a30823..7375d3c 100644 --- a/src/Local/LocalEvaluationConfigBuilder.php +++ b/src/Local/LocalEvaluationConfigBuilder.php @@ -4,7 +4,9 @@ use AmplitudeExperiment\Assignment\AssignmentConfig; use AmplitudeExperiment\Exposure\ExposureConfig; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; class LocalEvaluationConfigBuilder @@ -17,11 +19,9 @@ class LocalEvaluationConfigBuilder protected array $bootstrap = LocalEvaluationConfig::DEFAULTS['bootstrap']; protected ?AssignmentConfig $assignmentConfig = LocalEvaluationConfig::DEFAULTS['assignmentConfig']; protected ?ExposureConfig $exposureConfig = LocalEvaluationConfig::DEFAULTS['exposureConfig']; - protected ?HttpClientInterface $httpClient = LocalEvaluationConfig::DEFAULTS['httpClient']; - /** - * @var array - */ - protected array $guzzleClientConfig = LocalEvaluationConfig::DEFAULTS['guzzleClientConfig']; + protected ?ClientInterface $httpClient = LocalEvaluationConfig::DEFAULTS['httpClient']; + protected ?RequestFactoryInterface $requestFactory = LocalEvaluationConfig::DEFAULTS['requestFactory']; + protected ?RetryConfig $retryConfig = LocalEvaluationConfig::DEFAULTS['retryConfig']; public function __construct() { @@ -60,18 +60,33 @@ public function exposureConfig(ExposureConfig $exposureConfig): LocalEvaluationC return $this; } - public function httpClient(HttpClientInterface $httpClient): LocalEvaluationConfigBuilder + /** + * Supply a PSR-18 HTTP client. The SDK uses it verbatim — no retry wrap. + * If omitted, a client is auto-discovered and wrapped in + * {@link \AmplitudeExperiment\Http\RetryingClient} using {@link retryConfig}. + */ + public function httpClient(ClientInterface $httpClient): LocalEvaluationConfigBuilder { $this->httpClient = $httpClient; return $this; } /** - * @param array $guzzleClientConfig + * Supply a PSR-17 request factory. If omitted, a factory is auto-discovered. + */ + public function requestFactory(RequestFactoryInterface $requestFactory): LocalEvaluationConfigBuilder + { + $this->requestFactory = $requestFactory; + return $this; + } + + /** + * Configure retry behavior for the auto-discovered client. Ignored when + * a client is supplied via {@link httpClient()}. */ - public function guzzleClientConfig(array $guzzleClientConfig): LocalEvaluationConfigBuilder + public function retryConfig(RetryConfig $retryConfig): LocalEvaluationConfigBuilder { - $this->guzzleClientConfig = $guzzleClientConfig; + $this->retryConfig = $retryConfig; return $this; } @@ -84,7 +99,8 @@ public function build(): LocalEvaluationConfig $this->assignmentConfig, $this->exposureConfig, $this->httpClient, - $this->guzzleClientConfig + $this->requestFactory, + $this->retryConfig ); } } diff --git a/src/Remote/RemoteEvaluationClient.php b/src/Remote/RemoteEvaluationClient.php index 9330d1a..1f2ea64 100644 --- a/src/Remote/RemoteEvaluationClient.php +++ b/src/Remote/RemoteEvaluationClient.php @@ -3,11 +3,12 @@ namespace AmplitudeExperiment\Remote; use AmplitudeExperiment\EvaluationCore\Types\EvaluationVariant; -use AmplitudeExperiment\Http\HttpClientInterface; -use AmplitudeExperiment\Http\GuzzleHttpClient; +use AmplitudeExperiment\Http\HttpClientFactory; use AmplitudeExperiment\User; use AmplitudeExperiment\Variant; use Psr\Http\Client\ClientExceptionInterface; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use const AmplitudeExperiment\VERSION; @@ -20,7 +21,8 @@ class RemoteEvaluationClient { private string $apiKey; private RemoteEvaluationConfig $config; - private HttpClientInterface $httpClient; + private ClientInterface $httpClient; + private RequestFactoryInterface $requestFactory; private LoggerInterface $logger; /** @@ -33,7 +35,11 @@ public function __construct(string $apiKey, ?RemoteEvaluationConfig $config = nu { $this->apiKey = $apiKey; $this->config = $config ?? RemoteEvaluationConfig::builder()->build(); - $this->httpClient = $config->httpClient ?? $this->config->httpClient ?? new GuzzleHttpClient($this->config->guzzleClientConfig); + [$this->httpClient, $this->requestFactory] = HttpClientFactory::resolveAll( + $this->config->httpClient, + $this->config->requestFactory, + $this->config->retryConfig + ); $this->logger = $this->config->logger ?? new NullLogger(); } @@ -66,7 +72,7 @@ private function fetchWithOptions(User $user, FetchOptions $options): array // Define the request URL $endpoint = $this->config->serverUrl . '/sdk/v2/vardata?v=0'; - $request = $this->httpClient->createRequest('GET', $endpoint) + $request = $this->requestFactory->createRequest('GET', $endpoint) ->withHeader('Authorization', 'Api-Key ' . $this->apiKey) ->withHeader('Content-Type', 'application/json') ->withHeader('X-Amp-Exp-User', $serializedUser); @@ -87,10 +93,8 @@ private function fetchWithOptions(User $user, FetchOptions $options): array $request = $request->withHeader('X-Amp-Exp-Exposure-Track', $tracksExposure ? 'track' : 'no-track'); } - $httpClient = $this->httpClient->getClient(); - try { - $response = $httpClient->sendRequest($request); + $response = $this->httpClient->sendRequest($request); if ($response->getStatusCode() != 200) { $this->logger->error('[Experiment] Failed to fetch variants: ' . $response->getBody()); return []; diff --git a/src/Remote/RemoteEvaluationConfig.php b/src/Remote/RemoteEvaluationConfig.php index 5b326a9..3068c8a 100644 --- a/src/Remote/RemoteEvaluationConfig.php +++ b/src/Remote/RemoteEvaluationConfig.php @@ -2,7 +2,9 @@ namespace AmplitudeExperiment\Remote; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; /** @@ -26,38 +28,45 @@ class RemoteEvaluationConfig */ public string $serverUrl; /** - * The underlying HTTP client to use for requests, if this is not set, the default {@link GuzzleHttpClient} will be used. + * The PSR-18 HTTP client to use for requests. If null, a PSR-18 + * implementation is auto-discovered via php-http/discovery and wrapped + * in {@link \AmplitudeExperiment\Http\RetryingClient} using + * {@link $retryConfig}. A user-supplied client is used verbatim with + * no retry wrap. */ - public ?HttpClientInterface $httpClient; + public ?ClientInterface $httpClient; /** - * @var array - * The configuration for the underlying default {@link GuzzleHttpClient} (if used). See {@link GUZZLE_DEFAULTS} for defaults. + * The PSR-17 request factory used to construct requests. If null, a + * PSR-17 factory is auto-discovered. */ - public array $guzzleClientConfig; + public ?RequestFactoryInterface $requestFactory; + /** + * Retry configuration for the auto-wrapped client. Ignored when + * {@link $httpClient} is supplied — the user's client is used verbatim. + */ + public ?RetryConfig $retryConfig; const DEFAULTS = [ 'logger' => null, 'debug' => false, 'serverUrl' => 'https://api.lab.amplitude.com', 'httpClient' => null, - 'guzzleClientConfig' => [] + 'requestFactory' => null, + 'retryConfig' => null, ]; - - /** - * @param array $guzzleClientConfig - */ public function __construct( - ?LoggerInterface $logger, - string $serverUrl, - ?HttpClientInterface $httpClient, - array $guzzleClientConfig - ) - { + ?LoggerInterface $logger, + string $serverUrl, + ?ClientInterface $httpClient, + ?RequestFactoryInterface $requestFactory, + ?RetryConfig $retryConfig + ) { $this->logger = $logger; $this->serverUrl = $serverUrl; $this->httpClient = $httpClient; - $this->guzzleClientConfig = $guzzleClientConfig; + $this->requestFactory = $requestFactory; + $this->retryConfig = $retryConfig; } public static function builder(): RemoteEvaluationConfigBuilder diff --git a/src/Remote/RemoteEvaluationConfigBuilder.php b/src/Remote/RemoteEvaluationConfigBuilder.php index 6f2bb03..84e1dc8 100644 --- a/src/Remote/RemoteEvaluationConfigBuilder.php +++ b/src/Remote/RemoteEvaluationConfigBuilder.php @@ -2,7 +2,9 @@ namespace AmplitudeExperiment\Remote; -use AmplitudeExperiment\Http\HttpClientInterface; +use AmplitudeExperiment\Http\RetryConfig; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Log\LoggerInterface; class RemoteEvaluationConfigBuilder @@ -10,11 +12,9 @@ class RemoteEvaluationConfigBuilder protected ?LoggerInterface $logger = RemoteEvaluationConfig::DEFAULTS['logger']; protected bool $debug = RemoteEvaluationConfig::DEFAULTS['debug']; protected string $serverUrl = RemoteEvaluationConfig::DEFAULTS['serverUrl']; - protected ?HttpClientInterface $httpClient = RemoteEvaluationConfig::DEFAULTS['httpClient']; - /** - * @var array - */ - protected array $guzzleClientConfig = RemoteEvaluationConfig::DEFAULTS['guzzleClientConfig']; + protected ?ClientInterface $httpClient = RemoteEvaluationConfig::DEFAULTS['httpClient']; + protected ?RequestFactoryInterface $requestFactory = RemoteEvaluationConfig::DEFAULTS['requestFactory']; + protected ?RetryConfig $retryConfig = RemoteEvaluationConfig::DEFAULTS['retryConfig']; public function __construct() { @@ -32,19 +32,33 @@ public function serverUrl(string $serverUrl): RemoteEvaluationConfigBuilder return $this; } - public function httpClient(HttpClientInterface $httpClient): RemoteEvaluationConfigBuilder + /** + * Supply a PSR-18 HTTP client. The SDK uses it verbatim — no retry wrap. + * If omitted, a client is auto-discovered and wrapped in + * {@link \AmplitudeExperiment\Http\RetryingClient} using {@link retryConfig}. + */ + public function httpClient(ClientInterface $httpClient): RemoteEvaluationConfigBuilder { $this->httpClient = $httpClient; return $this; } + /** + * Supply a PSR-17 request factory. If omitted, a factory is auto-discovered. + */ + public function requestFactory(RequestFactoryInterface $requestFactory): RemoteEvaluationConfigBuilder + { + $this->requestFactory = $requestFactory; + return $this; + } /** - * @param array $guzzleClientConfig + * Configure retry behavior for the auto-discovered client. Ignored when + * a client is supplied via {@link httpClient()}. */ - public function guzzleClientConfig(array $guzzleClientConfig): RemoteEvaluationConfigBuilder + public function retryConfig(RetryConfig $retryConfig): RemoteEvaluationConfigBuilder { - $this->guzzleClientConfig = $guzzleClientConfig; + $this->retryConfig = $retryConfig; return $this; } @@ -54,7 +68,8 @@ public function build(): RemoteEvaluationConfig $this->logger, $this->serverUrl, $this->httpClient, - $this->guzzleClientConfig + $this->requestFactory, + $this->retryConfig ); } } diff --git a/tests/Amplitude/AmplitudeTest.php b/tests/Amplitude/AmplitudeTest.php index a4d80af..2c6b8ae 100644 --- a/tests/Amplitude/AmplitudeTest.php +++ b/tests/Amplitude/AmplitudeTest.php @@ -4,21 +4,18 @@ use AmplitudeExperiment\Amplitude\AmplitudeConfig; use AmplitudeExperiment\Amplitude\Event; -use AmplitudeExperiment\Logger\DefaultLogger; -use AmplitudeExperiment\Test\Util\MockGuzzleHttpClient; -use GuzzleHttp\Exception\RequestException; -use GuzzleHttp\Handler\MockHandler; -use GuzzleHttp\HandlerStack; -use GuzzleHttp\Psr7\Response; +use AmplitudeExperiment\Http\RetryConfig; +use AmplitudeExperiment\Http\RetryingClient; +use AmplitudeExperiment\Test\Util\MockPsr18Client; +use AmplitudeExperiment\Test\Util\Psr7TestUtil; use PHPUnit\Framework\TestCase; use Psr\Http\Message\RequestInterface; -use Psr\Log\LoggerInterface; class AmplitudeTest extends TestCase { const API_KEY = 'test'; - public function testAmplitudeConfigServerUrl() + public function testAmplitudeConfigServerUrl(): void { $config = AmplitudeConfig::builder() ->build(); @@ -43,115 +40,115 @@ public function testAmplitudeConfigServerUrl() $this->assertEquals('test', $config->serverUrl); } - public function testEmptyQueueAfterFlushSuccess() + public function testEmptyQueueAfterFlushSuccess(): void { $client = new MockAmplitude(self::API_KEY); - $mock = new MockHandler([ - new Response(200, ['X-Foo' => 'Bar']), - ]); - $handlerStack = HandlerStack::create($mock); - $httpClient = new MockGuzzleHttpClient([], $handlerStack); - $client->setHttpClient($httpClient); - $event1 = new Event('test1'); - $event2 = new Event('test2'); - $event3 = new Event('test3'); - $client->logEvent($event1); - $client->logEvent($event2); - $client->logEvent($event3); + $client->setHttpClient(new MockPsr18Client([Psr7TestUtil::response(200, ['X-Foo' => 'Bar'])])); + + $client->logEvent(new Event('test1')); + $client->logEvent(new Event('test2')); + $client->logEvent(new Event('test3')); $this->assertEquals(3, $client->getQueueSize()); $client->flush(); $this->assertEquals(0, $client->getQueueSize()); } - public function testFlushAfterMaxQueue() + public function testFlushAfterMaxQueue(): void { - // Initialize the request counter $requestCounter = 0; - $config = AmplitudeConfig::builder() ->flushQueueSize(3) ->build(); $client = new MockAmplitude(self::API_KEY, $config); - $mockHandler = new MockHandler([ - function (RequestInterface $request, array $options) use (&$requestCounter) { + $mock = new MockPsr18Client([ + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - - return new Response(200, ['X-Foo' => 'Bar']); + return Psr7TestUtil::response(200, ['X-Foo' => 'Bar']); }, ]); + $client->setHttpClient($mock); - // Create a handler stack with the mock handler - $handlerStack = HandlerStack::create($mockHandler); - - // Create an instance of GuzzleFetchClient with the custom handler stack - $httpClient = new MockGuzzleHttpClient([], $handlerStack); - $client->setHttpClient($httpClient); - $event1 = new Event('test1'); - $event2 = new Event('test2'); - $event3 = new Event('test3'); - $client->logEvent($event1); - $client->logEvent($event2); + $client->logEvent(new Event('test1')); + $client->logEvent(new Event('test2')); $this->assertEquals(2, $client->getQueueSize()); - $client->logEvent($event3); + $client->logEvent(new Event('test3')); $this->assertEquals(1, $requestCounter); $this->assertEquals(0, $client->getQueueSize()); } - public function testBackoffRetriesToFailure() + public function testBackoffRetriesToFailureWhenPostRetryEnabled(): void { - // Initialize the request counter $requestCounter = 0; $config = AmplitudeConfig::builder()->build(); $client = new MockAmplitude(self::API_KEY, $config); - // Set up the mock handler with request counter incrementation logic - $mockHandler = new MockHandler(array_fill(1, 5, function (RequestInterface $request, array $options) use (&$requestCounter) { + $mock = new MockPsr18Client(array_fill(0, 5, function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - return new RequestException('Error Communicating with Server', $request); + return Psr7TestUtil::clientException('Error Communicating with Server'); })); + $retryConfig = new RetryConfig(5, 0, 0, 1.0, ['POST']); + $client->setHttpClient(new RetryingClient($mock, $retryConfig)); - $handlerStack = HandlerStack::create($mockHandler); - $httpClient = new MockGuzzleHttpClient(['retries' => 4], $handlerStack); - $client->setHttpClient($httpClient); - - $event1 = new Event('test'); - $event1->userId = 'user_id'; - $client->logEvent($event1); + $event = new Event('test'); + $event->userId = 'user_id'; + $client->logEvent($event); $client->flush(); - // Assert the number of requests sent (including retries) $this->assertEquals(5, $requestCounter); $this->assertEquals(1, $client->getQueueSize()); } - - public function testBackoffRetriesThenSuccess() + public function testBackoffRetriesThenSuccessWhenPostRetryEnabled(): void { - // Initialize the request counter $requestCounter = 0; $config = AmplitudeConfig::builder()->build(); $client = new MockAmplitude(self::API_KEY, $config); - // Set up the mock handler with request counter incrementation logic - $mockHandler = new MockHandler(array_fill(1, 2, function (RequestInterface $request, array $options) use (&$requestCounter) { + $mock = new MockPsr18Client([ + function (RequestInterface $request) use (&$requestCounter) { + $requestCounter++; + return Psr7TestUtil::clientException('Error Communicating with Server'); + }, + function (RequestInterface $request) use (&$requestCounter) { + $requestCounter++; + return Psr7TestUtil::clientException('Error Communicating with Server'); + }, + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - return new RequestException('Error Communicating with Server', $request); - }) + [ - function (RequestInterface $request, array $options) use (&$requestCounter) { - $requestCounter++; - - return new Response(200, ['X-Foo' => 'Bar']); - }, - ]); - - $handlerStack = HandlerStack::create($mockHandler); - $httpClient = new MockGuzzleHttpClient(['retries' => 4], $handlerStack); - $client->setHttpClient($httpClient); - $event1 = new Event('test'); - $event1->userId = 'user_id'; - $client->logEvent($event1); + return Psr7TestUtil::response(200, ['X-Foo' => 'Bar']); + }, + ]); + $retryConfig = new RetryConfig(5, 0, 0, 1.0, ['POST']); + $client->setHttpClient(new RetryingClient($mock, $retryConfig)); + + $event = new Event('test'); + $event->userId = 'user_id'; + $client->logEvent($event); $client->flush(); + $this->assertEquals(3, $requestCounter); $this->assertEquals(0, $client->getQueueSize()); } + + public function testPostNotRetriedByDefaultRetryConfig(): void + { + $requestCounter = 0; + $config = AmplitudeConfig::builder()->build(); + $client = new MockAmplitude(self::API_KEY, $config); + + $mock = new MockPsr18Client(array_fill(0, 3, function (RequestInterface $request) use (&$requestCounter) { + $requestCounter++; + return Psr7TestUtil::clientException('Error Communicating with Server'); + })); + // Default RetryConfig retries GET only — POST passes through with no retry. + $client->setHttpClient(new RetryingClient($mock, new RetryConfig(5, 0, 0, 1.0))); + + $event = new Event('test'); + $event->userId = 'user_id'; + $client->logEvent($event); + $client->flush(); + + $this->assertEquals(1, $requestCounter); + $this->assertEquals(1, $client->getQueueSize()); + } } diff --git a/tests/Amplitude/MockAmplitude.php b/tests/Amplitude/MockAmplitude.php index 966b6ed..ecfc3b6 100644 --- a/tests/Amplitude/MockAmplitude.php +++ b/tests/Amplitude/MockAmplitude.php @@ -4,8 +4,7 @@ use AmplitudeExperiment\Amplitude\Amplitude; use AmplitudeExperiment\Amplitude\AmplitudeConfig; -use AmplitudeExperiment\Http\HttpClientInterface; -use Psr\Log\LoggerInterface; +use Psr\Http\Client\ClientInterface; class MockAmplitude extends Amplitude { @@ -13,13 +12,16 @@ public function __construct(string $apiKey, ?AmplitudeConfig $config = null) { parent::__construct($apiKey, $config); } - public function setHttpClient(HttpClientInterface $httpClient) { + public function setHttpClient(ClientInterface $httpClient): void + { $this->httpClient = $httpClient; } - public function __destruct() { + public function __destruct() + { // Do nothing } - public function getQueueSize() : int { + public function getQueueSize(): int + { return count($this->queue); } } diff --git a/tests/EvaluationCore/EvaluateIntegrationTest.php b/tests/EvaluationCore/EvaluateIntegrationTest.php index fe14d31..2c9428a 100644 --- a/tests/EvaluationCore/EvaluateIntegrationTest.php +++ b/tests/EvaluationCore/EvaluateIntegrationTest.php @@ -6,9 +6,10 @@ use AmplitudeExperiment\EvaluationCore\Types\EvaluationFlag; use AmplitudeExperiment\Variant; use Exception; -use GuzzleHttp\Client; -use GuzzleHttp\Exception\GuzzleException; +use Http\Discovery\Psr17FactoryDiscovery; +use Http\Discovery\Psr18ClientDiscovery; use PHPUnit\Framework\TestCase; +use Psr\Http\Client\ClientExceptionInterface; use function AmplitudeExperiment\Flag\createFlagsFromArray; class EvaluateIntegrationTest extends TestCase @@ -21,7 +22,7 @@ class EvaluateIntegrationTest extends TestCase private array $flags; /** - * @throws GuzzleException + * @throws ClientExceptionInterface */ protected function setUp(): void { @@ -645,19 +646,17 @@ private function groupContext($groupType, $groupName, $groupProperties = []): ar } /** - * @throws GuzzleException + * @throws ClientExceptionInterface * @throws Exception */ private function getFlags($deploymentKey) { $serverUrl = 'https://api.lab.amplitude.com'; - $client = new Client(); + $request = Psr17FactoryDiscovery::findRequestFactory() + ->createRequest('GET', "{$serverUrl}/sdk/v2/flags?eval_mode=remote") + ->withHeader('Authorization', "Api-Key $deploymentKey"); - $response = $client->request('GET', "{$serverUrl}/sdk/v2/flags?eval_mode=remote", [ - 'headers' => [ - 'Authorization' => "Api-Key $deploymentKey", - ], - ]); + $response = Psr18ClientDiscovery::find()->sendRequest($request); if ($response->getStatusCode() !== 200) { throw new Exception("Response error {$response->getStatusCode()}"); diff --git a/tests/Http/RetryingClientTest.php b/tests/Http/RetryingClientTest.php new file mode 100644 index 0000000..2bc1e12 --- /dev/null +++ b/tests/Http/RetryingClientTest.php @@ -0,0 +1,184 @@ +fastConfig(['attempts' => 5])); + + $response = $client->sendRequest(Psr7TestUtil::request('GET')); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(1, $mock->callCount); + } + + public function testRetriesUntilSuccessWithinBudget(): void + { + $mock = new MockPsr18Client([ + Psr7TestUtil::clientException(), + Psr7TestUtil::clientException(), + Psr7TestUtil::response(200), + ]); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 5])); + + $response = $client->sendRequest(Psr7TestUtil::request('GET')); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(3, $mock->callCount); + } + + public function testThrowsLastExceptionWhenBudgetExhausted(): void + { + $queue = []; + for ($i = 0; $i < 4; $i++) { + $queue[] = Psr7TestUtil::clientException(); + } + $mock = new MockPsr18Client($queue); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 4])); + + try { + $client->sendRequest(Psr7TestUtil::request('GET')); + $this->fail('expected ClientExceptionInterface to be thrown'); + } catch (ClientExceptionInterface $e) { + $this->assertSame(4, $mock->callCount); + } + } + + public function testPostNotRetriedByDefault(): void + { + $mock = new MockPsr18Client([Psr7TestUtil::clientException()]); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 5])); + + try { + $client->sendRequest(Psr7TestUtil::request('POST')); + $this->fail('expected ClientExceptionInterface to be thrown'); + } catch (ClientExceptionInterface $e) { + $this->assertSame(1, $mock->callCount); + } + } + + public function testPostRetriedWhenInRetryMethods(): void + { + $mock = new MockPsr18Client([ + Psr7TestUtil::clientException(), + Psr7TestUtil::response(200), + ]); + $client = new RetryingClient($mock, $this->fastConfig([ + 'attempts' => 3, + 'retryMethods' => ['GET', 'POST'], + ])); + + $response = $client->sendRequest(Psr7TestUtil::request('POST')); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(2, $mock->callCount); + } + + public function testNonPsrExceptionPropagatesWithoutRetry(): void + { + $mock = new MockPsr18Client([new RuntimeException('not a PSR-18 exception')]); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 5])); + + try { + $client->sendRequest(Psr7TestUtil::request('GET')); + $this->fail('expected RuntimeException to be thrown'); + } catch (RuntimeException $e) { + $this->assertSame('not a PSR-18 exception', $e->getMessage()); + $this->assertSame(1, $mock->callCount); + } + } + + public function testHttpErrorResponsesNotRetried(): void + { + $mock = new MockPsr18Client([Psr7TestUtil::response(503)]); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 5])); + + $response = $client->sendRequest(Psr7TestUtil::request('GET')); + + $this->assertSame(503, $response->getStatusCode()); + $this->assertSame(1, $mock->callCount); + } + + public function testSingleAttemptDisablesRetry(): void + { + $mock = new MockPsr18Client([Psr7TestUtil::clientException()]); + $client = new RetryingClient($mock, $this->fastConfig(['attempts' => 1])); + + try { + $client->sendRequest(Psr7TestUtil::request('GET')); + $this->fail('expected ClientExceptionInterface to be thrown'); + } catch (ClientExceptionInterface $e) { + $this->assertSame(1, $mock->callCount); + } + } + + public function testMethodMatchIsCaseInsensitive(): void + { + $mock = new MockPsr18Client([ + Psr7TestUtil::clientException(), + Psr7TestUtil::response(200), + ]); + // retryMethods passed lowercase; constructor normalizes to uppercase + $client = new RetryingClient($mock, $this->fastConfig([ + 'attempts' => 3, + 'retryMethods' => ['get'], + ])); + + $response = $client->sendRequest(Psr7TestUtil::request('GET')); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(2, $mock->callCount); + } + + public function testRetryConfigRejectsZeroAttempts(): void + { + $this->expectException(InvalidArgumentException::class); + new RetryConfig(0); + } + + public function testRetryConfigRejectsInvalidBackoffBounds(): void + { + $this->expectException(InvalidArgumentException::class); + new RetryConfig(3, 5000, 1000); + } + + public function testRetryConfigRejectsScalarBelowOne(): void + { + $this->expectException(InvalidArgumentException::class); + new RetryConfig(3, 100, 1000, 0.5); + } + + /** + * @param array $overrides + */ + private function fastConfig(array $overrides = []): RetryConfig + { + $defaults = [ + 'attempts' => 3, + 'backoffMinMillis' => 0, + 'backoffMaxMillis' => 0, + 'backoffScalar' => 1.0, + 'retryMethods' => ['GET'], + ]; + $merged = array_merge($defaults, $overrides); + return new RetryConfig( + $merged['attempts'], + $merged['backoffMinMillis'], + $merged['backoffMaxMillis'], + $merged['backoffScalar'], + $merged['retryMethods'] + ); + } +} diff --git a/tests/Remote/RemoteEvaluationClientTest.php b/tests/Remote/RemoteEvaluationClientTest.php index a5660d7..ec99983 100644 --- a/tests/Remote/RemoteEvaluationClientTest.php +++ b/tests/Remote/RemoteEvaluationClientTest.php @@ -3,15 +3,14 @@ namespace AmplitudeExperiment\Test\Remote; use AmplitudeExperiment\Experiment; +use AmplitudeExperiment\Http\RetryConfig; +use AmplitudeExperiment\Http\RetryingClient; +use AmplitudeExperiment\Remote\FetchOptions; use AmplitudeExperiment\Remote\RemoteEvaluationClient; use AmplitudeExperiment\Remote\RemoteEvaluationConfig; -use AmplitudeExperiment\Remote\FetchOptions; -use AmplitudeExperiment\Test\Util\MockGuzzleHttpClient; +use AmplitudeExperiment\Test\Util\MockPsr18Client; +use AmplitudeExperiment\Test\Util\Psr7TestUtil; use AmplitudeExperiment\User; -use GuzzleHttp\Exception\RequestException; -use GuzzleHttp\Handler\MockHandler; -use GuzzleHttp\HandlerStack; -use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; use Psr\Http\Message\RequestInterface; @@ -28,7 +27,7 @@ public function __construct() ->build(); } - public function testFetchSuccess() + public function testFetchSuccess(): void { $client = new RemoteEvaluationClient($this->apiKey); $variants = $client->fetch($this->testUser); @@ -37,108 +36,77 @@ public function testFetchSuccess() $this->assertEquals("payload", $variant->payload); } - public function testFetchWithNoRetriesTimeoutFailure() + public function testFetchWithNoRetriesSurfacesTransportFailure(): void { - $guzzleConfig = ['retries' => 0, 'timeoutMillis' => 1]; + // Single attempt, no retry — a transport error short-circuits to empty result. + $mock = new MockPsr18Client([ + function (RequestInterface $request) { + return Psr7TestUtil::clientException('Simulated transport failure'); + }, + ]); $config = RemoteEvaluationConfig::builder() - ->guzzleClientConfig($guzzleConfig) + ->httpClient(new RetryingClient($mock, new RetryConfig(1, 0, 0, 1.0))) ->build(); $client = new RemoteEvaluationClient($this->apiKey, $config); $variants = $client->fetch($this->testUser); $this->assertEquals([], $variants); } - public function testFetchNoRetriesTimeoutFailureRetrySuccess() + public function testRetrySucceedsAfterTransientFailure(): void { - // Initialize the request counter $requestCounter = 0; - - // Set up the mock handler - $mockHandler = new MockHandler([ - // Simulate a failure (e.g., timeout) for the first request - function (RequestInterface $request, array $options) use (&$requestCounter) { + $mock = new MockPsr18Client([ + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - - return new RequestException('Error Communicating with Server', $request); + return Psr7TestUtil::clientException('Error Communicating with Server'); }, - // Simulate a successful response for the retried request - function (RequestInterface $request, array $options) use (&$requestCounter) { + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + return Psr7TestUtil::response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); }, ]); + $retryConfig = new RetryConfig(2, 100, 500, 2.0); - // Create a handler stack with the mock handler - $handlerStack = HandlerStack::create($mockHandler); - - // Create an instance of GuzzleFetchClient with the custom handler stack - $httpClient = new MockGuzzleHttpClient([ - 'retries' => 1, - 'timeoutMillis' => 10000, - 'retryBackoffMinMillis' => 100, - 'retryBackoffScalar' => 2, - 'retryBackoffMaxMillis' => 500, - ], $handlerStack); - - $client = new RemoteEvaluationClient($this->apiKey, RemoteEvaluationConfig::builder()->httpClient($httpClient)->build()); + $config = RemoteEvaluationConfig::builder() + ->httpClient(new RetryingClient($mock, $retryConfig)) + ->build(); + $client = new RemoteEvaluationClient($this->apiKey, $config); - // Expect a successful response after auto-retry $variants = $client->fetch($this->testUser); $variant = $variants['sdk-ci-test']; $this->assertEquals("on", $variant->key); $this->assertEquals("payload", $variant->payload); - - // Assert the number of requests sent (including retries) $this->assertEquals(2, $requestCounter); } - public function testretryOnceTimeoutFirstThenSucceedWithZeroBackoff() + public function testRetrySucceedsWithZeroBackoff(): void { - // Initialize the request counter $requestCounter = 0; - - // Set up the mock handler - $mockHandler = new MockHandler([ - // Simulate a failure (e.g., timeout) for the first request - function (RequestInterface $request, array $options) use (&$requestCounter) { + $mock = new MockPsr18Client([ + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - - return new RequestException('Error Communicating with Server', $request); + return Psr7TestUtil::clientException('Error Communicating with Server'); }, - // Simulate a successful response for the retried request - function (RequestInterface $request, array $options) use (&$requestCounter) { + function (RequestInterface $request) use (&$requestCounter) { $requestCounter++; - - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + return Psr7TestUtil::response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); }, ]); + $retryConfig = new RetryConfig(2, 0, 0, 1.0); - // Create a handler stack with the mock handler - $handlerStack = HandlerStack::create($mockHandler); - - // Create an instance of GuzzleFetchClient with the custom handler stack - $httpClient = new MockGuzzleHttpClient([ - 'retries' => 1, - 'timeoutMillis' => 10000, - 'retryBackoffMinMillis' => 0, - 'retryBackoffScalar' => 2, - 'retryBackoffMaxMillis' => 0, - ], $handlerStack); - - $client = new RemoteEvaluationClient($this->apiKey, RemoteEvaluationConfig::builder()->httpClient($httpClient)->build()); + $config = RemoteEvaluationConfig::builder() + ->httpClient(new RetryingClient($mock, $retryConfig)) + ->build(); + $client = new RemoteEvaluationClient($this->apiKey, $config); - // Expect a successful response after auto-retry $variants = $client->fetch($this->testUser); $variant = $variants['sdk-ci-test']; $this->assertEquals("on", $variant->key); $this->assertEquals("payload", $variant->payload); - - // Assert the number of requests sent (including retries) $this->assertEquals(2, $requestCounter); } - public function testFetchWithFlagKeysOptionsSuccess() + public function testFetchWithFlagKeysOptionsSuccess(): void { $client = new RemoteEvaluationClient($this->apiKey); $variants = $client->fetch($this->testUser, ['sdk-ci-test']); @@ -148,60 +116,55 @@ public function testFetchWithFlagKeysOptionsSuccess() $this->assertEquals("payload", $variant->payload); } - public function testExperimentInitializeRemote() + public function testExperimentInitializeRemote(): void { $experiment = new Experiment(); $client = $experiment->initializeRemote($this->apiKey); $this->assertEquals($client, $experiment->initializeRemote($this->apiKey)); } - public function testFetchWithFetchOptionsSuccess() + public function testFetchWithFetchOptionsSuccess(): void { - // Create an instance of GuzzleFetchClient with the custom handler stack - $mockHandler = new MockHandler([ - function (RequestInterface $request, array $options) { - $headers = $request->getHeaders(); - $this->assertEquals(base64_encode('["sdk-ci-test"]'), $headers['X-Amp-Exp-Flag-Keys'][0]); - $this->assertEquals('track', $headers['X-Amp-Exp-Track'][0]); - $this->assertEquals('track', $headers['X-Amp-Exp-Exposure-Track'][0]); - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); - }, - function (RequestInterface $request, array $options) { - $headers = $request->getHeaders(); - $this->assertEquals(base64_encode('["sdk-ci-test"]'), $headers['X-Amp-Exp-Flag-Keys'][0]); - $this->assertEquals('no-track', $headers['X-Amp-Exp-Track'][0]); - $this->assertEquals('no-track', $headers['X-Amp-Exp-Exposure-Track'][0]); - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); - }, - function (RequestInterface $request, array $options) { - $headers = $request->getHeaders(); - $this->assertEquals(base64_encode('["sdk-ci-test"]'), $headers['X-Amp-Exp-Flag-Keys'][0]); + $assertExpectedHeaders = function (RequestInterface $request, ?string $expectedTrack, ?string $expectedExposure) { + $headers = $request->getHeaders(); + $this->assertEquals(base64_encode('["sdk-ci-test"]'), $headers['X-Amp-Exp-Flag-Keys'][0]); + if ($expectedTrack === null) { $this->assertArrayNotHasKey('X-Amp-Exp-Track', $headers); + } else { + $this->assertEquals($expectedTrack, $headers['X-Amp-Exp-Track'][0]); + } + if ($expectedExposure === null) { $this->assertArrayNotHasKey('X-Amp-Exp-Exposure-Track', $headers); - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + } else { + $this->assertEquals($expectedExposure, $headers['X-Amp-Exp-Exposure-Track'][0]); + } + return Psr7TestUtil::response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + }; + + $mock = new MockPsr18Client([ + function (RequestInterface $request) use ($assertExpectedHeaders) { + return $assertExpectedHeaders($request, 'track', 'track'); }, - function (RequestInterface $request, array $options) { - $headers = $request->getHeaders(); - $this->assertEquals(base64_encode('["sdk-ci-test"]'), $headers['X-Amp-Exp-Flag-Keys'][0]); - $this->assertArrayNotHasKey('X-Amp-Exp-Track', $headers); - $this->assertArrayNotHasKey('X-Amp-Exp-Exposure-Track', $headers); - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + function (RequestInterface $request) use ($assertExpectedHeaders) { + return $assertExpectedHeaders($request, 'no-track', 'no-track'); + }, + function (RequestInterface $request) use ($assertExpectedHeaders) { + return $assertExpectedHeaders($request, null, null); }, - function (RequestInterface $request, array $options) { + function (RequestInterface $request) use ($assertExpectedHeaders) { + return $assertExpectedHeaders($request, null, null); + }, + function (RequestInterface $request) { $headers = $request->getHeaders(); $this->assertArrayNotHasKey('X-Amp-Exp-Flag-Keys', $headers); $this->assertArrayNotHasKey('X-Amp-Exp-Track', $headers); $this->assertArrayNotHasKey('X-Amp-Exp-Exposure-Track', $headers); - return new Response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); + return Psr7TestUtil::response(200, [], '{"sdk-ci-test":{"key":"on","payload":"payload"}}'); }, ]); - $handlerStack = HandlerStack::create($mockHandler); - $httpClient = new MockGuzzleHttpClient([ - 'retries' => 1, - 'timeoutMillis' => 1000, - ], $handlerStack); - $client = new RemoteEvaluationClient($this->apiKey, RemoteEvaluationConfig::builder()->httpClient($httpClient)->build()); + $config = RemoteEvaluationConfig::builder()->httpClient($mock)->build(); + $client = new RemoteEvaluationClient($this->apiKey, $config); $variants = $client->fetch($this->testUser, new FetchOptions(['sdk-ci-test'], true, true)); $this->assertEquals(1, sizeof($variants)); diff --git a/tests/Util/MockGuzzleHttpClient.php b/tests/Util/MockGuzzleHttpClient.php deleted file mode 100644 index d046f0d..0000000 --- a/tests/Util/MockGuzzleHttpClient.php +++ /dev/null @@ -1,62 +0,0 @@ -config = array_merge(GUZZLE_DEFAULTS, $config); - - // Add middleware for retries - $handlerStack->push(Middleware::retry( - function ($retries, Request $request, $response = null, $exception = null) { - // Retry if the maximum number of retries is not reached and an exception occurred - return $retries < $this->config['retries'] && $exception instanceof Exception; - }, - function ($retries) { - // Calculate delay - return $this->calculateDelayMillis($retries); - } - )); - - // Create a Guzzle client with the custom handler stack - $this->client = new Client(['handler' => $handlerStack, RequestOptions::TIMEOUT => $this->config['timeoutMillis'] / 1000]); - } - - public function getClient(): ClientInterface - { - return $this->client; - } - - public function createRequest(string $method, string $uri, ?string $body = null): Request - { - return new Request($method, $uri); - } - - protected function calculateDelayMillis($iteration): int - { - $delayMillis = $this->config['retryBackoffMinMillis']; - - for ($i = 0; $i < $iteration; $i++) { - $delayMillis = min( - $delayMillis * $this->config['retryBackoffScalar'], - $this->config['retryBackoffMaxMillis'] - ); - } - return $delayMillis; - } -} diff --git a/tests/Util/MockPsr18Client.php b/tests/Util/MockPsr18Client.php new file mode 100644 index 0000000..d08b85d --- /dev/null +++ b/tests/Util/MockPsr18Client.php @@ -0,0 +1,48 @@ + */ + private array $queue; + public int $callCount = 0; + + /** + * @param array $queue + */ + public function __construct(array $queue) + { + $this->queue = $queue; + } + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $this->callCount++; + if (empty($this->queue)) { + throw new RuntimeException('MockPsr18Client queue exhausted'); + } + $next = array_shift($this->queue); + if (is_callable($next)) { + $next = $next($request); + } + if ($next instanceof Throwable) { + throw $next; + } + return $next; + } +} diff --git a/tests/Util/Psr7TestUtil.php b/tests/Util/Psr7TestUtil.php new file mode 100644 index 0000000..22f876e --- /dev/null +++ b/tests/Util/Psr7TestUtil.php @@ -0,0 +1,45 @@ +createRequest($method, $uri); + } + + /** + * @param array $headers + */ + public static function response(int $status = 200, array $headers = [], ?string $body = null): ResponseInterface + { + $response = Psr17FactoryDiscovery::findResponseFactory()->createResponse($status); + foreach ($headers as $name => $value) { + $response = $response->withHeader($name, $value); + } + if ($body !== null) { + $stream = Psr17FactoryDiscovery::findStreamFactory()->createStream($body); + $response = $response->withBody($stream); + } + return $response; + } + + public static function clientException(string $message = 'Simulated transport error'): ClientExceptionInterface + { + return new class($message) extends RuntimeException implements ClientExceptionInterface { + }; + } +}