Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 4fc0f42610 | |||
| 29645b7a5f | |||
| 012a4e780d | |||
| a35888a6f3 | |||
| a81f6b0d21 | |||
| 88760ed127 |
@@ -1,3 +1,38 @@
|
||||
# Alfa Bank SDK - Release Notes v1.1.0
|
||||
|
||||
## ⚠️ Behaviour change: safe retries
|
||||
|
||||
Saloon retries a request whenever an exception is thrown (`handleRetry` returns `true` by
|
||||
default) and both SDK connectors set `tries = 3`. As a result, non-idempotent operations
|
||||
(`POST /payoffs`, `PATCH /payoffs/{id}/accept`, SBP `QrcReversal`) were sent to the bank up to
|
||||
three times while the calling code believed it had made a single request. For payouts a repeated
|
||||
POST is a duplicate-payment risk, not a harmless retry — the SDK now stops this at the connector
|
||||
level, because Saloon only retries when the per-send callable, the request **and** the connector
|
||||
all allow it.
|
||||
|
||||
The new trait `Apoloz\AlfaBank\Traits\HasSafeRetries` replaces the default with an explicit policy:
|
||||
|
||||
1. A deterministic bank verdict (4xx/5xx) is never retried.
|
||||
2. An explicit deny-list of money operations is never retried.
|
||||
3. An explicit allow-list keeps retries for read-only SBP operations (that API uses POST for reads).
|
||||
4. By default only read-only HTTP methods (GET/HEAD/OPTIONS) are retried.
|
||||
|
||||
Every decision is reported through the connector's PSR-3 logger (`[alfa-sdk][retry]`), which makes
|
||||
the number of physical requests countable per operation via the `X-Request-ID` header.
|
||||
|
||||
## 🐞 Fixes
|
||||
|
||||
- Restored the missing `HasStringBody` and `CmsSigner` imports in
|
||||
`Sbp\Requests\DigitalRuble\CashLink\*` requests: the classes failed with a fatal error when loaded.
|
||||
|
||||
## 🧪 Tests
|
||||
|
||||
- Added `tests/Feature/RetryPolicyTest.php` (23 tests): money operations and bank verdicts are never
|
||||
repeated, read-only operations keep their retries, and the real Saloon retry loop is asserted
|
||||
(a failing non-idempotent POST reaches the wire exactly once).
|
||||
|
||||
---
|
||||
|
||||
# Alfa Bank SDK - Release Notes v1.0.0
|
||||
|
||||
## 🚀 Overview
|
||||
|
||||
@@ -13,7 +13,12 @@ use Apoloz\AlfaBank\Http\Middleware\LoggingResponseMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Middleware\RequestIdMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Middleware\VerifyJwsResponseMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Pagination\AlfaOffsetPaginator;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\CreatePayoffRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\CreateTaxRefundRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\PayoffAcceptRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\TaxRefundAcceptRequest;
|
||||
use Apoloz\AlfaBank\Traits\HasAlfaErrorHandling;
|
||||
use Apoloz\AlfaBank\Traits\HasSafeRetries;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Psr\Log\NullLogger;
|
||||
use Saloon\Helpers\OAuth2\OAuthConfig;
|
||||
@@ -26,7 +31,6 @@ use Saloon\PaginationPlugin\OffsetPaginator;
|
||||
use Saloon\Traits\OAuth2\AuthorizationCodeGrant;
|
||||
use Saloon\Traits\Plugins\AlwaysThrowOnErrors;
|
||||
use Saloon\Traits\Plugins\HasTimeout;
|
||||
use Saloon\Traits\RequestProperties\HasTries;
|
||||
|
||||
class AlfaBankConnector extends Connector implements HasPagination
|
||||
{
|
||||
@@ -34,7 +38,9 @@ class AlfaBankConnector extends Connector implements HasPagination
|
||||
use AuthorizationCodeGrant;
|
||||
use HasAlfaErrorHandling;
|
||||
use HasTimeout;
|
||||
use HasTries;
|
||||
// Replaces the HasTries default handleRetry (always retry) with a safe policy:
|
||||
// transport failures are retried for read-only requests only, bank verdicts never.
|
||||
use HasSafeRetries;
|
||||
|
||||
public function __construct(
|
||||
protected AlfaConfig $alfaConfig,
|
||||
@@ -45,6 +51,15 @@ class AlfaBankConnector extends Connector implements HasPagination
|
||||
$this->tries = 3;
|
||||
$this->retryInterval = 500;
|
||||
$this->logger = $this->logger ?? new NullLogger;
|
||||
|
||||
// Money operations: repeating them is a duplicate-payment risk, so they are
|
||||
// denied explicitly in addition to the read-only-method rule.
|
||||
$this->denyRetriesFor([
|
||||
CreatePayoffRequest::class,
|
||||
PayoffAcceptRequest::class,
|
||||
CreateTaxRefundRequest::class,
|
||||
TaxRefundAcceptRequest::class,
|
||||
]);
|
||||
}
|
||||
|
||||
public function resolveBaseUrl(): string
|
||||
|
||||
@@ -23,7 +23,7 @@ class UpdateRequisiteRequest extends Request implements HasBody
|
||||
{
|
||||
use HasJsonBody;
|
||||
|
||||
protected Method $method = Method::PUT;
|
||||
protected Method $method = Method::PATCH;
|
||||
|
||||
public function __construct(
|
||||
protected string $beneficiaryId,
|
||||
|
||||
@@ -4,7 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace Apoloz\AlfaBank\Requests\NominalAccounts\Payouts;
|
||||
|
||||
use Apoloz\AlfaBank\Data\Shared\ResultResponseData;
|
||||
use Apoloz\AlfaBank\Data\NominalAccounts\Payouts\PayoffResponseData;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Http\Response;
|
||||
@@ -25,8 +25,8 @@ class PayoffAcceptRequest extends Request
|
||||
return "/api/na/jp/v1/payoffs/{$this->payoffId}/accept?{$query}";
|
||||
}
|
||||
|
||||
public function createDtoFromResponse(Response $response): ResultResponseData
|
||||
public function createDtoFromResponse(Response $response): PayoffResponseData
|
||||
{
|
||||
return ResultResponseData::from($response->json());
|
||||
return PayoffResponseData::from($response->json());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,10 +14,12 @@ namespace Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\CashLink;
|
||||
// <BELIEF_STATE "This POST request requires specific DTOs for its JSON body and its response, along with Saloon's body-handling traits.">
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\ChangeAccForCashUniQrRequestData;
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\ChangeAccForCashUniQrResponseData;
|
||||
use Apoloz\AlfaBank\Utils\CmsSigner;
|
||||
use Saloon\Contracts\Body\HasBody;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Http\Response;
|
||||
use Saloon\Traits\Body\HasStringBody;
|
||||
|
||||
// </SECTION>
|
||||
|
||||
|
||||
@@ -14,10 +14,12 @@ namespace Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\CashLink;
|
||||
// <BELIEF_STATE "This POST request requires specific DTOs for its JSON body and its response, along with Saloon's body-handling traits.">
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\CreateCashUniQrRequestData;
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\CreateCashUniQrResponseData;
|
||||
use Apoloz\AlfaBank\Utils\CmsSigner;
|
||||
use Saloon\Contracts\Body\HasBody;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Http\Response;
|
||||
use Saloon\Traits\Body\HasStringBody;
|
||||
|
||||
// </SECTION>
|
||||
|
||||
|
||||
@@ -14,10 +14,12 @@ namespace Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\CashLink;
|
||||
// <BELIEF_STATE "This POST request requires specific DTOs for its JSON body and its response, along with Saloon's body-handling traits.">
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\GetCashUniQrStatusRequestData;
|
||||
use Apoloz\AlfaBank\Sbp\Data\DigitalRuble\CashLink\GetCashUniQrStatusResponseData;
|
||||
use Apoloz\AlfaBank\Utils\CmsSigner;
|
||||
use Saloon\Contracts\Body\HasBody;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Http\Response;
|
||||
use Saloon\Traits\Body\HasStringBody;
|
||||
|
||||
// </SECTION>
|
||||
|
||||
|
||||
@@ -9,7 +9,20 @@ use Apoloz\AlfaBank\Http\Middleware\LoggingRequestMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Middleware\LoggingResponseMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Middleware\RequestIdMiddleware;
|
||||
use Apoloz\AlfaBank\Http\Pagination\AlfaOffsetPaginator;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\CashLink\GetCashQrHistoryRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\CashLink\GetCashQrStatusRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\CashLink\GetCashUniQrStatusRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\GetUniQrStatusRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Management\GetBlockedQrListRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Registry\GetBankListRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalDataRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalHistoryRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalStatusRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\QrcReversalRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Status\GetQrcStatusRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Subscription\GetQrSubStatusRequest;
|
||||
use Apoloz\AlfaBank\Traits\HasAlfaErrorHandling;
|
||||
use Apoloz\AlfaBank\Traits\HasSafeRetries;
|
||||
use Apoloz\AlfaBank\Utils\CmsSigner;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Psr\Log\NullLogger;
|
||||
@@ -20,14 +33,14 @@ use Saloon\PaginationPlugin\Contracts\HasPagination;
|
||||
use Saloon\PaginationPlugin\OffsetPaginator;
|
||||
use Saloon\Traits\Plugins\AlwaysThrowOnErrors;
|
||||
use Saloon\Traits\Plugins\HasTimeout;
|
||||
use Saloon\Traits\RequestProperties\HasTries;
|
||||
|
||||
class SbpConnector extends Connector implements HasPagination
|
||||
{
|
||||
use AlwaysThrowOnErrors;
|
||||
use HasAlfaErrorHandling;
|
||||
use HasTimeout;
|
||||
use HasTries;
|
||||
// Replaces the HasTries default handleRetry (always retry) with a safe policy.
|
||||
use HasSafeRetries;
|
||||
|
||||
public function __construct(
|
||||
protected AlfaConfig $alfaConfig,
|
||||
@@ -38,6 +51,32 @@ class SbpConnector extends Connector implements HasPagination
|
||||
$this->tries = 3;
|
||||
$this->retryInterval = 500;
|
||||
$this->logger = $this->logger ?? new NullLogger;
|
||||
|
||||
// The SBP API uses POST even for read-only operations, so the read-only method
|
||||
// rule alone would silently disable retries for status/list queries. These are
|
||||
// pure reads: repeating them creates no entity at the bank.
|
||||
$this->allowRetriesFor([
|
||||
GetQrcStatusRequest::class,
|
||||
GetQrcReversalStatusRequest::class,
|
||||
GetQrcReversalHistoryRequest::class,
|
||||
GetCashQrStatusRequest::class,
|
||||
GetCashQrHistoryRequest::class,
|
||||
GetQrSubStatusRequest::class,
|
||||
GetBlockedQrListRequest::class,
|
||||
GetBankListRequest::class,
|
||||
GetUniQrStatusRequest::class,
|
||||
GetCashUniQrStatusRequest::class,
|
||||
]);
|
||||
|
||||
// Money-mutating operations and calls that create/modify a document on the
|
||||
// bank side are denied explicitly. GetQrcReversalDataRequest is deliberately
|
||||
// NOT retried: the bank treats its message as the "previous message" the
|
||||
// following QRCreversal is matched against (EBD 44 / I14044), so it is not a
|
||||
// pure read despite its name.
|
||||
$this->denyRetriesFor([
|
||||
QrcReversalRequest::class,
|
||||
GetQrcReversalDataRequest::class,
|
||||
]);
|
||||
}
|
||||
|
||||
public function resolveBaseUrl(): string
|
||||
|
||||
147
src/Traits/HasSafeRetries.php
Normal file
147
src/Traits/HasSafeRetries.php
Normal file
@@ -0,0 +1,147 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Apoloz\AlfaBank\Traits;
|
||||
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Exceptions\Request\FatalRequestException;
|
||||
use Saloon\Exceptions\Request\RequestException;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Traits\RequestProperties\HasTries;
|
||||
|
||||
// <SECTION name="safe_retries" intent="Override Saloon's default 'retry anything' behaviour with a policy that never repeats non-idempotent money operations and never repeats deterministic bank verdicts.">
|
||||
// <BELIEF_STATE "Saloon retries by default (handleRetry returns true for any exception) and both SDK connectors set tries = 3, so POST /payoffs (CreatePayoffRequest), PATCH /payoffs/{id}/accept (PayoffAcceptRequest) and SBP QrcReversalRequest were sent up to three times without the calling code knowing. For payouts a repeated POST is a duplicate-payment risk, not a harmless retry, therefore the connector must be the level that stops it: Saloon only retries when the per-send callable, the request and the connector all return true.">
|
||||
trait HasSafeRetries
|
||||
{
|
||||
// HasTries provides $tries, $retryInterval and the retry loop contract.
|
||||
// We replace its handleRetry default (always true) with the policy below.
|
||||
use HasTries;
|
||||
|
||||
/**
|
||||
* HTTP methods whose repetition is idempotent by specification: they read state
|
||||
* and never create or mutate a bank entity.
|
||||
*
|
||||
* @var list<\Saloon\Enums\Method>
|
||||
*/
|
||||
protected const RETRYABLE_METHODS = [Method::GET, Method::HEAD, Method::OPTIONS];
|
||||
|
||||
/**
|
||||
* Explicit deny-list. Wins over every other rule: keeps a non-idempotent
|
||||
* operation safe even if it is ever declared with a read-only HTTP method.
|
||||
*
|
||||
* @var list<class-string<Request>>
|
||||
*/
|
||||
protected array $nonRetryableRequests = [];
|
||||
|
||||
/**
|
||||
* Explicit allow-list for safe operations that are not read-only by method.
|
||||
*
|
||||
* @var list<class-string<Request>>
|
||||
*/
|
||||
protected array $retryableRequests = [];
|
||||
|
||||
/**
|
||||
* Retry decision, evaluated by Saloon inside the retry loop.
|
||||
*
|
||||
* Called for every caught FatalRequestException (transport: timeout, DNS, TLS)
|
||||
* and RequestException (HTTP verdict: 4xx/5xx).
|
||||
*/
|
||||
public function handleRetry(FatalRequestException|RequestException $exception, Request $request): bool
|
||||
{
|
||||
$decision = $this->shouldRetryRequest($exception, $request);
|
||||
|
||||
$this->logRetryDecision($exception, $request, $decision);
|
||||
|
||||
return $decision;
|
||||
}
|
||||
|
||||
/**
|
||||
* Policy itself, free of side effects (used directly by tests and subclasses).
|
||||
*
|
||||
* Rules, in priority order:
|
||||
* 1. The bank answered with a verdict (4xx/5xx) — never retry: repeating a
|
||||
* deterministic rejection only multiplies requests.
|
||||
* 2. Explicit deny-list — never retry.
|
||||
* 3. Explicit allow-list — retry on transport failure.
|
||||
* 4. Default — retry read-only methods only (GET/HEAD/OPTIONS).
|
||||
*/
|
||||
public function shouldRetryRequest(FatalRequestException|RequestException $exception, Request $request): bool
|
||||
{
|
||||
// Rule 1: bank verdict, not a transport failure.
|
||||
if ($exception instanceof RequestException) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$requestClass = $request::class;
|
||||
|
||||
// Rule 2: explicit deny-list wins over everything.
|
||||
if (in_array($requestClass, $this->nonRetryableRequests, true)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Rule 3: explicit allow-list.
|
||||
if (in_array($requestClass, $this->retryableRequests, true)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Rule 4: read-only methods only.
|
||||
return in_array($request->getMethod(), self::RETRYABLE_METHODS, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark requests as non-retryable (operations that create or mutate bank entities).
|
||||
*
|
||||
* @param list<class-string<Request>> $requestClasses
|
||||
*/
|
||||
public function denyRetriesFor(array $requestClasses): static
|
||||
{
|
||||
$this->nonRetryableRequests = array_values(array_unique([...$this->nonRetryableRequests, ...$requestClasses]));
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark requests as retryable even when their HTTP method is not read-only.
|
||||
*
|
||||
* @param list<class-string<Request>> $requestClasses
|
||||
*/
|
||||
public function allowRetriesFor(array $requestClasses): static
|
||||
{
|
||||
$this->retryableRequests = array_values(array_unique([...$this->retryableRequests, ...$requestClasses]));
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Observability: record every retry decision through the connector's PSR-3 logger
|
||||
* (in the Laravel application this is the finance_alfabank channel).
|
||||
*
|
||||
* X-Request-ID is stable across the attempts of one operation, so the number of
|
||||
* physical requests per operation is 1 + the number of records sharing that id.
|
||||
*
|
||||
* Requires the using connector to expose $logger (both SDK connectors do).
|
||||
*/
|
||||
protected function logRetryDecision(
|
||||
FatalRequestException|RequestException $exception,
|
||||
Request $request,
|
||||
bool $decision
|
||||
): void {
|
||||
if ($this->logger === null) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->logger->warning('[alfa-sdk][retry]', [
|
||||
'connector' => static::class,
|
||||
'request' => $request::class,
|
||||
'http_method' => $request->getMethod()->value,
|
||||
'exception' => $exception::class,
|
||||
'http_status' => $exception instanceof RequestException ? $exception->getResponse()->status() : null,
|
||||
'decision' => $decision ? 'retry' : 'stop',
|
||||
'request_id' => $exception instanceof RequestException
|
||||
? $exception->getResponse()->header('X-Request-ID')
|
||||
: $exception->getPendingRequest()->headers()->get('X-Request-ID'),
|
||||
]);
|
||||
}
|
||||
}
|
||||
// </SECTION>
|
||||
@@ -5,7 +5,9 @@ use Apoloz\AlfaBank\Core\AlfaConfig;
|
||||
use Apoloz\AlfaBank\Data\NominalAccounts\Beneficiaries\AddressData;
|
||||
use Apoloz\AlfaBank\Data\NominalAccounts\Beneficiaries\CreateBeneficiaryRequestData;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Beneficiaries\CreateBeneficiaryRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Beneficiaries\Requisites\UpdateRequisiteRequest;
|
||||
use Apoloz\AlfaBank\Services\OpenSslCmsSigner;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Http\Faking\MockResponse;
|
||||
use Spatie\LaravelData\DataCollection;
|
||||
|
||||
@@ -58,3 +60,14 @@ test('can create a beneficiary with automatic signing', function (): void {
|
||||
expect($response->status())->toBe(202)
|
||||
->and($response->dto()->requestId)->toBe('req-123');
|
||||
});
|
||||
|
||||
test('update requisite request uses PATCH', function (): void {
|
||||
$request = new UpdateRequisiteRequest(
|
||||
beneficiaryId: 'ben-1',
|
||||
requisiteId: 'req-1',
|
||||
data: ['phoneNumber' => '79031234567'],
|
||||
);
|
||||
|
||||
expect($request->getMethod())->toBe(Method::PATCH)
|
||||
->and($request->resolveEndpoint())->toBe('/api/na/jp/v1/beneficiaries/ben-1/requisites/req-1');
|
||||
});
|
||||
|
||||
407
tests/Feature/RetryPolicyTest.php
Normal file
407
tests/Feature/RetryPolicyTest.php
Normal file
@@ -0,0 +1,407 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use Apoloz\AlfaBank\Core\AlfaBankConnector;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\CreatePayoffRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\CreateTaxRefundRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\PayoffAcceptRequest;
|
||||
use Apoloz\AlfaBank\Requests\NominalAccounts\Payouts\TaxRefundAcceptRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalDataRequest;
|
||||
use Apoloz\AlfaBank\Sbp\Requests\Reversal\QrcReversalRequest;
|
||||
use Apoloz\AlfaBank\Sbp\SbpConnector;
|
||||
use GuzzleHttp\Psr7\Request as PsrRequest;
|
||||
use GuzzleHttp\Psr7\Response as PsrResponse;
|
||||
use Psr\Log\AbstractLogger;
|
||||
use Saloon\Enums\Method;
|
||||
use Saloon\Exceptions\Request\FatalRequestException;
|
||||
use Saloon\Exceptions\Request\RequestException;
|
||||
use Saloon\Http\Connector;
|
||||
use Saloon\Http\PendingRequest;
|
||||
use Saloon\Http\Request;
|
||||
use Saloon\Http\Response;
|
||||
use Saloon\Http\Senders\GuzzleSender;
|
||||
|
||||
// <SECTION name="retry_policy_tests" intent="Lock down the safe-retry policy: money operations and deterministic bank verdicts are never repeated, read-only operations still retry transport failures.">
|
||||
|
||||
/**
|
||||
* PSR-3 logger double: the SDK is framework-agnostic, so retry decisions are asserted
|
||||
* on the records the connector sends to its logger (in the app: channel finance_alfabank).
|
||||
*/
|
||||
class RetryRecordingLogger extends AbstractLogger
|
||||
{
|
||||
/** @var list<array{level: mixed, message: string, context: array<string, mixed>}> */
|
||||
public array $records = [];
|
||||
|
||||
public function log($level, string|Stringable $message, array $context = []): void
|
||||
{
|
||||
$this->records[] = [
|
||||
'level' => $level,
|
||||
'message' => (string) $message,
|
||||
'context' => $context,
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sender that fails on every attempt with a transport error (timeout/DNS/TLS),
|
||||
* which is exactly the case where Saloon's retry loop asks the connector.
|
||||
* Counts physical attempts so a test can prove how many requests reached the wire.
|
||||
*/
|
||||
class RetryCountingFailingSender extends GuzzleSender
|
||||
{
|
||||
public static int $attempts = 0;
|
||||
|
||||
public function send(PendingRequest $pendingRequest): Response
|
||||
{
|
||||
self::$attempts++;
|
||||
|
||||
throw new FatalRequestException(new RuntimeException('Connection timed out'), $pendingRequest);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Requests that create or move money. Repeating any of them is a duplicate-payment
|
||||
* risk, so this list is enumerated on purpose: if a money operation ever becomes
|
||||
* retryable, this test must fail and force a conscious review.
|
||||
*
|
||||
* @return array<string, array{0: class-string<Request>}>
|
||||
*/
|
||||
function retryMoneyOperations(): array
|
||||
{
|
||||
return [
|
||||
'BaaS: POST /payoffs' => [CreatePayoffRequest::class],
|
||||
'BaaS: PATCH accept' => [PayoffAcceptRequest::class],
|
||||
'BaaS: POST tax refund' => [CreateTaxRefundRequest::class],
|
||||
'BaaS: PATCH tax refund accept' => [TaxRefundAcceptRequest::class],
|
||||
'SBP: QRCreversal' => [QrcReversalRequest::class],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* SBP uses POST even for read-only endpoints. These calls are safe to repeat because
|
||||
* they create nothing at the bank; the list is duplicated here deliberately so that
|
||||
* widening the connector's allow-list requires updating the test too.
|
||||
*
|
||||
* @return list<class-string<Request>>
|
||||
*/
|
||||
function retrySbpReadOnlyPostOperations(): array
|
||||
{
|
||||
return [
|
||||
Apoloz\AlfaBank\Sbp\Requests\Status\GetQrcStatusRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalStatusRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\Reversal\GetQrcReversalHistoryRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\CashLink\GetCashQrStatusRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\CashLink\GetCashQrHistoryRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\Subscription\GetQrSubStatusRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\Management\GetBlockedQrListRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\Registry\GetBankListRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\GetUniQrStatusRequest::class,
|
||||
Apoloz\AlfaBank\Sbp\Requests\DigitalRuble\CashLink\GetCashUniQrStatusRequest::class,
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* All request classes shipped by the SDK with the HTTP method declared in their defaults.
|
||||
*
|
||||
* @return array<class-string<Request>, Method>
|
||||
*/
|
||||
function retryAllRequestClasses(): array
|
||||
{
|
||||
$sourceRoot = dirname(__DIR__, 2).'/src';
|
||||
$classes = [];
|
||||
|
||||
foreach (['Requests', 'Sbp/Requests'] as $directory) {
|
||||
$path = $sourceRoot.'/'.$directory;
|
||||
|
||||
if (! is_dir($path)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$files = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path));
|
||||
|
||||
foreach ($files as $file) {
|
||||
if ($file->getExtension() !== 'php') {
|
||||
continue;
|
||||
}
|
||||
|
||||
$relative = substr($file->getPathname(), strlen($sourceRoot) + 1, -4);
|
||||
$class = 'Apoloz\\AlfaBank\\'.str_replace('/', '\\', $relative);
|
||||
|
||||
if (! class_exists($class)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$reflection = new ReflectionClass($class);
|
||||
|
||||
if ($reflection->isAbstract() || ! $reflection->isSubclassOf(Request::class)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$classes[$class] = $reflection->getDefaultProperties()['method'] ?? Method::GET;
|
||||
}
|
||||
}
|
||||
|
||||
ksort($classes);
|
||||
|
||||
return $classes;
|
||||
}
|
||||
|
||||
function retryConnectorFor(string $requestClass): Connector
|
||||
{
|
||||
return str_starts_with($requestClass, 'Apoloz\\AlfaBank\\Sbp\\')
|
||||
? new SbpConnector(createTestConfig(), 'test-key-alias')
|
||||
: new AlfaBankConnector(createTestConfig());
|
||||
}
|
||||
|
||||
function retryRequestInstance(string $requestClass): Request
|
||||
{
|
||||
return (new ReflectionClass($requestClass))->newInstanceWithoutConstructor();
|
||||
}
|
||||
|
||||
/** Pending request built from a harmless GET so that no body is signed or merged. */
|
||||
function retryNeutralPendingRequest(Connector $connector): PendingRequest
|
||||
{
|
||||
$neutral = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/';
|
||||
}
|
||||
};
|
||||
|
||||
return new PendingRequest($connector, $neutral);
|
||||
}
|
||||
|
||||
function retryTransportFailure(Connector $connector): FatalRequestException
|
||||
{
|
||||
return new FatalRequestException(
|
||||
new RuntimeException('Connection timed out'),
|
||||
retryNeutralPendingRequest($connector),
|
||||
);
|
||||
}
|
||||
|
||||
function retryBankVerdict(Connector $connector, int $status = 400): RequestException
|
||||
{
|
||||
$pending = retryNeutralPendingRequest($connector);
|
||||
|
||||
$response = new Response(
|
||||
new PsrResponse($status, ['Content-Type' => 'application/json', 'X-Request-ID' => 'test-request-id'], '{"error":"bad_request"}'),
|
||||
$pending,
|
||||
new PsrRequest('GET', 'https://test.alfabank.ru/'),
|
||||
);
|
||||
|
||||
return new RequestException($response);
|
||||
}
|
||||
|
||||
// --- money operations are never repeated -------------------------------------
|
||||
|
||||
test('money operations are never retried on transport failure', function (string $requestClass): void {
|
||||
$connector = retryConnectorFor($requestClass);
|
||||
|
||||
$decision = $connector->handleRetry(
|
||||
retryTransportFailure($connector),
|
||||
retryRequestInstance($requestClass),
|
||||
);
|
||||
|
||||
expect($decision)->toBeFalse();
|
||||
})->with(retryMoneyOperations());
|
||||
|
||||
test('QRCreversalData is never retried: the bank treats it as the previous message of a reversal', function (): void {
|
||||
$connector = new SbpConnector(createTestConfig(), 'test-key-alias');
|
||||
|
||||
$decision = $connector->handleRetry(
|
||||
retryTransportFailure($connector),
|
||||
retryRequestInstance(GetQrcReversalDataRequest::class),
|
||||
);
|
||||
|
||||
expect($decision)->toBeFalse();
|
||||
});
|
||||
|
||||
// --- read-only operations keep their resilience ------------------------------
|
||||
|
||||
test('read-only GET requests are retried on transport failure', function (): void {
|
||||
$connector = new AlfaBankConnector(createTestConfig());
|
||||
|
||||
$getRequest = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/payoffs';
|
||||
}
|
||||
};
|
||||
|
||||
expect($connector->handleRetry(retryTransportFailure($connector), $getRequest))->toBeTrue();
|
||||
});
|
||||
|
||||
test('SBP read-only POST operations are still retried on transport failure', function (string $requestClass): void {
|
||||
$connector = new SbpConnector(createTestConfig(), 'test-key-alias');
|
||||
|
||||
$decision = $connector->handleRetry(
|
||||
retryTransportFailure($connector),
|
||||
retryRequestInstance($requestClass),
|
||||
);
|
||||
|
||||
expect($decision)->toBeTrue();
|
||||
})->with(retrySbpReadOnlyPostOperations());
|
||||
|
||||
test('every mutating request class in the SDK is non-retryable by default', function (): void {
|
||||
$sbpReadOnlyPost = retrySbpReadOnlyPostOperations();
|
||||
|
||||
$mutating = array_filter(
|
||||
retryAllRequestClasses(),
|
||||
static fn (Method $method, string $class): bool => ! in_array($method, [Method::GET, Method::HEAD, Method::OPTIONS], true)
|
||||
&& ! in_array($class, $sbpReadOnlyPost, true),
|
||||
ARRAY_FILTER_USE_BOTH,
|
||||
);
|
||||
|
||||
expect($mutating)->not->toBeEmpty();
|
||||
|
||||
foreach ($mutating as $class => $method) {
|
||||
$connector = retryConnectorFor($class);
|
||||
|
||||
$decision = $connector->handleRetry(
|
||||
retryTransportFailure($connector),
|
||||
retryRequestInstance($class),
|
||||
);
|
||||
|
||||
expect($decision)->toBeFalse("{$method->value} {$class} must not be retried");
|
||||
}
|
||||
});
|
||||
|
||||
// --- deterministic bank verdicts ---------------------------------------------
|
||||
|
||||
test('bank verdict responses are never retried, even for read-only requests', function (): void {
|
||||
$connector = new AlfaBankConnector(createTestConfig());
|
||||
|
||||
$getRequest = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/payoffs';
|
||||
}
|
||||
};
|
||||
|
||||
expect($connector->handleRetry(retryBankVerdict($connector, 400), $getRequest))->toBeFalse()
|
||||
->and($connector->handleRetry(retryBankVerdict($connector, 500), $getRequest))->toBeFalse();
|
||||
});
|
||||
|
||||
// --- the decision is actually enforced by Saloon's retry loop ----------------
|
||||
|
||||
test('a failing non-idempotent POST is sent exactly once', function (): void {
|
||||
$connector = new class(createTestConfig()) extends AlfaBankConnector
|
||||
{
|
||||
protected string $defaultSender = RetryCountingFailingSender::class;
|
||||
};
|
||||
$connector->retryInterval = 0;
|
||||
|
||||
$request = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::POST;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/payoffs';
|
||||
}
|
||||
};
|
||||
|
||||
RetryCountingFailingSender::$attempts = 0;
|
||||
|
||||
try {
|
||||
$connector->send($request);
|
||||
} catch (FatalRequestException) {
|
||||
// expected: the original transport failure is surfaced to the caller
|
||||
}
|
||||
|
||||
expect(RetryCountingFailingSender::$attempts)->toBe(1);
|
||||
});
|
||||
|
||||
test('a failing read-only GET still uses all configured attempts', function (): void {
|
||||
$connector = new class(createTestConfig()) extends AlfaBankConnector
|
||||
{
|
||||
protected string $defaultSender = RetryCountingFailingSender::class;
|
||||
};
|
||||
$connector->retryInterval = 0;
|
||||
|
||||
$request = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/payoffs';
|
||||
}
|
||||
};
|
||||
|
||||
RetryCountingFailingSender::$attempts = 0;
|
||||
|
||||
try {
|
||||
$connector->send($request);
|
||||
} catch (FatalRequestException) {
|
||||
// expected: retries exhausted, original failure surfaced
|
||||
}
|
||||
|
||||
expect(RetryCountingFailingSender::$attempts)->toBe(3);
|
||||
});
|
||||
|
||||
// --- observability -----------------------------------------------------------
|
||||
|
||||
test('the retry decision is logged with the request, method and verdict', function (): void {
|
||||
$logger = new RetryRecordingLogger;
|
||||
$connector = new AlfaBankConnector(createTestConfig());
|
||||
$connector->withLogging($logger);
|
||||
|
||||
$getRequest = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/payoffs';
|
||||
}
|
||||
};
|
||||
|
||||
$connector->handleRetry(retryTransportFailure($connector), $getRequest);
|
||||
$connector->handleRetry(retryBankVerdict($connector, 400), $getRequest);
|
||||
|
||||
// The connector's logging middleware shares this logger, so keep only retry records.
|
||||
$records = array_values(array_filter(
|
||||
$logger->records,
|
||||
static fn (array $record): bool => $record['message'] === '[alfa-sdk][retry]',
|
||||
));
|
||||
|
||||
expect($records)->toHaveCount(2)
|
||||
->and($records[0]['context']['decision'])->toBe('retry')
|
||||
->and($records[0]['context']['http_method'])->toBe('GET')
|
||||
->and($records[1]['context']['decision'])->toBe('stop')
|
||||
->and($records[1]['context']['http_status'])->toBe(400)
|
||||
->and($records[1]['context']['request_id'])->toBe('test-request-id');
|
||||
});
|
||||
|
||||
// --- explicit policy overrides ------------------------------------------------
|
||||
|
||||
test('the deny-list overrides the read-only default', function (): void {
|
||||
$readOnly = new class extends Request
|
||||
{
|
||||
protected Method $method = Method::GET;
|
||||
|
||||
public function resolveEndpoint(): string
|
||||
{
|
||||
return '/';
|
||||
}
|
||||
};
|
||||
|
||||
$connector = new AlfaBankConnector(createTestConfig());
|
||||
expect($connector->handleRetry(retryTransportFailure($connector), $readOnly))->toBeTrue();
|
||||
|
||||
$connector->denyRetriesFor([$readOnly::class]);
|
||||
expect($connector->handleRetry(retryTransportFailure($connector), $readOnly))->toBeFalse();
|
||||
});
|
||||
|
||||
// </SECTION>
|
||||
Reference in New Issue
Block a user