Repository navigation
Split AcquirersClient, refresh tokens on 401, fix CI and README for 2.0 - #2
Merged
Merged
Conversation
Every public AcquirersClient method is now asserted (method, path, query, headers, decoded body) against tests/fixtures/requests/*.json on a Guzzle MockHandler, and the parsed DTOs are checked field by field. The 2.0 clients load the same fixtures, so a request that differs is a regression rather than a test to update. Ruling: fixtures hold request and response together, one file per exchange, and headers are compared case-insensitively without Host, User-Agent and Content-Length (Guzzle adds those) — if a Guzzle-added header ever matters, the helper's exclusion list is the one place to change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ApiTransport sends the same headers and JSON body as 1.0 (checked against the step 0 fixtures), maps every failure to DiiaApiException (HTTP status, Diia's body "code", raw body), and on a 401 drops the cached token, fetches a new one and retries exactly once. BearerTokenProvider caches for 7200 - 300 s and gains invalidate(). Ruling: failures with no response (connection refused, timeout) are a DiiaApiException with status 0 and the Guzzle exception as previous, so callers catch one type — if callers want Guzzle's own, unwrap getPrevious(). Ruling: a successful response whose body is not a JSON object is a DiiaApiException too, not UnexpectedValueException — one failure type for the whole API; a 2.0 break, listed in the changelog. Ruling: HttpHeadersProvider stays until AcquirersClient is removed in the next commit, so this commit keeps the suite green. Co-Authored-By: Claude Opus 5.5 <[email protected]>
BranchClient, OfferClient, OfferRequestClient and DocumentRequestClient each take one typed mapper per job over the shared ApiTransport, so no constructor takes two parameters of the same class (asserted by reflection). ClientFactory is the one place that wires them; clients from one factory share one transport and one token. Removed: AcquirersClient, Dependency/ (DependencyResolver and SupportedDependencyInterface), RequestJsonMapper, ResponseJsonMapper, HttpHeadersProvider. The 1.0 cases moved into one test per client and still replay the step 0 fixtures unchanged. Ruling: ResponseJsonMapper is deleted too, not only RequestJsonMapper — ApiTransport now decodes JSON, so it had nothing left to do — if anyone used it directly, json_decode + the item mapper replaces it. Ruling: a status response without "status" is a DiiaApiException with HTTP 200 (was UnexpectedValueException), via an @internal StatusResponse helper shared by both status endpoints. Ruling: the clients and ClientFactory are not final, so applications can mock them; ApiTransport is final. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Runs on push and pull_request to main, on demand, and monthly, so the ?branch=main badge always has a recent main run. A keepalive job on the scheduled run re-enables the workflow before GitHub's 60-day inactivity auto-disable. Replaces `php -l src/ tests/`, which lints nothing and exits 0, with a per-file lint on every PHP version in the matrix, and php-actions/composer (its own image) with shivammathur/setup-php. Ruling: the lint runs inside each matrix job rather than once — syntax support differs between 7.4 and 8.5 — at the cost of seven lint runs. Ruling: dependencies are resolved with `composer update` since composer.lock is not committed for this library. Co-Authored-By: Claude Opus 5.5 <[email protected]>
README: the title loses "[Unofficial]" and gets a [!NOTE] community notice and CI / PHP / Packagist badges. The 45 lines of manual wiring become one ClientFactory call with timeouts from config. Fixed: the four-backtick fence, the missing `;`, `$brancId`, the non-existent `hashFile()` (now euspe 2.0 Hasher, base64-encoded as registries does), and `makeOfferRequest($branchId, $offerId)`. New: Errors, Callbacks (what Diia posts, what the receiver must verify), Upgrading from 1.x. The scopes remark is kept as observed behaviour, dated 2024, since Diia's current docs could not be checked. tests/ReadmeExamplesTest.php runs every ```php block of the README in order against a MockHandler, so the examples cannot drift again. composer.json: description "Diia (Дія) API client for PHP"; php ^7.4 || ^8.0; guzzle ^7.8; psr/simple-cache and psr/log 1 to 3. Ruling: the test extracts and evals the README blocks instead of keeping hand-made copies — a copy is what drifted in 1.x — at the cost of one str_replace that injects the mock handler into the Guzzle client. Ruling: euspe is stubbed from its published 2.0 contract (tests/Support/euspe-stubs.php), not required — it needs IIT's proprietary extension and 2.0 is not released; if its API changes before release, the stubs and the README change together. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The token endpoint carries the acquirer token in its URL (/api/v1/auth/acquirer/<token>), and Guzzle quotes the URL in its messages. DiiaApiException copied that message, and kept the Guzzle exception as previous, so logging a failed call wrote the token to the logs (reproduced with real cURL against a closed port). Token-request failures now get their own mapping: the message names the API call that needed the token and says the token request failed, and the Guzzle exception is not chained. For failures of the API call itself, the message uses cURL's error text instead of Guzzle's message; the Guzzle exception stays as previous, as its URL holds no secret. This also stops naming the API path as the failed request when it was the token request that failed (the review's first nit). Ruling: token-request failures drop the previous exception entirely — loggers such as Monolog print previous exceptions — at the cost of the Guzzle stack trace for that case; status, Diia's code and body are kept. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The euspe stubs record calls in static arrays that were never reset, so `--repeat 2` failed (1 failure) because the second run saw both runs' calls. The test now empties them before running the examples. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The signing callback example indexed $hashes[$item['name']] directly, so a copied handler passed null to base64_decode for a name it never sent. It now rejects the callback when the name has no stored hash. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ApiTransport encodes with JSON_THROW_ON_ERROR, so a DTO holding invalid UTF-8 throws JsonException before sending; 1.x's json_encode returned false and Guzzle sent an empty body (checked). Documented as a 2.0 change rather than wrapped: it is a caller error, not an API failure. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The clients built paths with sprintf('/…/branch/%s', $branchId), so an
ID holding `/`, `?` or `../` changed the endpoint: get('a/../b?c')
requested /api/v2/acquirers/branch/b, with the acquirer's bearer token.
Every path ID now goes through rawurlencode(). Diia's own IDs are
alphanumeric, so the step 0 fixtures still match unchanged; same
behaviour as 1.0 until now, so it is listed in the CHANGELOG.
Ruling: rawurlencode() at each call site rather than a path builder in
ApiTransport — seven one-line changes, each visible where the path is
written — if more resources are added, a helper would remove the repetition.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
AcquirersClienthandled every Diia resource and took fiveResponseJsonMapperarguments of the same type, so swapping two compiled fine and failed at runtime.BearerTokenProvidercached the token for its full 2-hour lifetime and never refreshed it, so requests close to expiry failed with 401. The README described wiring and methods that no longer existed. CI linted nothing and never ran on a schedule, so the badge showed "no status".This is 2.0. It splits the client by API resource, refreshes the token, and rewrites the README and CI. Endpoints, request bodies, DTO names and enum names stay the same.
How
tests/fixtures/requests/. The 2.0 clients are tested against the same fixtures, so any request that differs from 1.0 fails a test.BranchClient,OfferClient,OfferRequestClient,DocumentRequestClienteach take one typed mapper per job, andClientFactorybuilds them. A reflection test checks that no client constructor takes two parameters of the same class.Http\ApiTransportsets the headers, encodes and decodes JSON, and turns every failure intoDiiaApiException(HTTP status, Diia's errorcode, raw body). On a 401 it drops the cached token, gets a new one and retries once. The token is now cached for 7200 − 300 s.get('a/../b?c')requested/api/v2/acquirers/branch/b. Real Diia IDs come out the same, so the fixtures recorded from 1.0 still match.tests/ReadmeExamplesTest.phpextracts every PHP block from the README and runs them in order against mocked responses.The table mapping each old method to its new one is in
CHANGELOG.mdand in the README under Upgrading from 1.x.Testing
Run locally in Docker, on the current HEAD:
composer:lts):OK (46 tests, 297 assertions)php:7.4-cli, dependencies resolved for 7.4):OK (46 tests, 297 assertions)OK (42 tests, 279 assertions)) but were not re-run after them.composer validate --strict: valid.CI (run 36274838984): all 7 PHP jobs pass, each with
OK (46 tests, 297 assertions)and 45 files linted, and the Composer validation job passes.Reviewed with
/gku:reviewin the same session that wrote the code. That is a self-review, so weaker than a fresh reviewer. Its findings are fixed in the last five commits.Acceptance criteria
DiiaApiException(tested with GuzzleMockHandler)[!NOTE]notice;composer.jsondescription updated?branch=main), PHP versions, Packagist releaseReadmeExamplesTestOK (46 tests, 297 assertions)and 45 files lintedmainrun: can only be confirmed after merge and the first monthly runCHANGELOG.md2.0.0 maps everyAcquirersClientmethod to its new client method2.0.0and visible on Packagist: manual, after mergeNotes
AcquirersClient,Dependency/,RequestJsonMapper,ResponseJsonMapper,HttpHeadersProvider. HTTP and transport failures now throwDiiaApiExceptioninstead of Guzzle exceptions.UnexpectedValueException: a list response missing its items key, and a token response missing its token. Documented in the CHANGELOG; worth deciding before tagging.tests/Support/euspe-stubs.php), written from its published contract, because the real library needs IIT's proprietary extension and 2.0 isn't released yet. If euspe's API changes before its release, the README and the stubs change together.2.0.0.ScopesMapperandItemsListResponseraise deprecation notices on PHP 8.4 and later.OfferMapperreadsreturnLinkwithout a null check, so an offer with no return link raises a warning.barcode,otp).'token', so two factories with different credentials sharing one cache would overwrite each other's token (same as 1.x).f6cb741, the recorded 1.0 requests) also sits on a local1.xbranch, which has not been pushed.🤖 Generated with Claude Code