From 51b7d6c552862d0affc5af4fe41adf332238dc03 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 02:51:29 +0100 Subject: [PATCH 01/21] chore(core): implement spec 0.2.0 Spec 0.2.0 adds the client contract, the fixtures and the contract scenarios that the PHP client must pass. Its grammar and vectors are the same as before, so the core passes them unchanged and only its SPEC_VERSION moves. CONTEXT.md takes the new terms of the glossary. Signed-off-by: MobileMage --- .changes/unreleased/Changed-20261003-024952.yaml | 3 +++ CONTEXT.md | 14 +++++++++++++- README.md | 2 +- docs/api-dump.md | 2 +- spec | 2 +- src/Postcode.php | 2 +- 6 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 .changes/unreleased/Changed-20261003-024952.yaml diff --git a/.changes/unreleased/Changed-20261003-024952.yaml b/.changes/unreleased/Changed-20261003-024952.yaml new file mode 100644 index 0000000..cdedd2e --- /dev/null +++ b/.changes/unreleased/Changed-20261003-024952.yaml @@ -0,0 +1,3 @@ +kind: Changed +body: Implement spec 0.2.0. It adds the client contract, and core results stay the same. +time: 2026-10-03T02:49:52.051073+01:00 diff --git a/CONTEXT.md b/CONTEXT.md index 64a0f59..fcc8182 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -122,9 +122,21 @@ _Avoid_: using this word for a Gatepost component ### Gatepost **Spec**: -The `spec` repo. It holds the grammar, the data, the vectors and the standards. The API fixtures and the completed OpenAPI file come later. +The `spec` repo. It holds the grammar, the client contract, the data, the vectors, the fixtures, the completed OpenAPI file and the standards. _Avoid_: schema, contract (for the repo) +**Contract scenario**: +One shared test case in `spec/contract/`. It names the calls, the mock server's responses and the outcome that every client must reach. +_Avoid_: contract test (for the file), vector, fixture + +**Attempt**: +One request that a client sends for one call. A retry is the second or third attempt. +_Avoid_: try, request (when you count them) + +**Evidence**: +How Gatepost knows the shape of a gateway response: `observed` in its own calls, `documented` by NIPOST only, or `assumed` by the mock server. +_Avoid_: source, confidence, proof + **Vector**: One shared test case in `spec/vectors/` that every SDK must pass. _Avoid_: fixture, example, golden file diff --git a/README.md b/README.md index 396e2ee..3c1fbce 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ echo Postcode::parse(str_pad($code, 65))->error?->code->value, "\n"; // bad_leng | Requirement | Version | |---|---| | PHP | 8.1 or later | -| Gatepost spec | 0.1.0 | +| Gatepost spec | 0.2.0 | ## Docs diff --git a/docs/api-dump.md b/docs/api-dump.md index c00ff7b..7d2cfa4 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -31,7 +31,7 @@ final class ParseResult final class Postcode { - public const SPEC_VERSION = '0.1.0'; + public const SPEC_VERSION = '0.2.0'; public readonly string $compact; public readonly string $canonical; public readonly string $display; diff --git a/spec b/spec index ca4afdd..a4f601d 160000 --- a/spec +++ b/spec @@ -1 +1 @@ -Subproject commit ca4afddf65e2897c71bb49ef7bed71b1a9741408 +Subproject commit a4f601d0c58240318a4cce68302574c8bb021cae diff --git a/src/Postcode.php b/src/Postcode.php index 3d7443b..eb45f64 100644 --- a/src/Postcode.php +++ b/src/Postcode.php @@ -24,7 +24,7 @@ final class Postcode /** * The version of the Gatepost spec that this package implements. */ - public const SPEC_VERSION = '0.1.0'; + public const SPEC_VERSION = '0.2.0'; // In UTF-8, a code point takes at most 4 bytes. Longer input is over the limit without a // count, so a 10 MB string costs no more than a short one. From f6b27a48268136a3530862df9da8fa89d92bcce3 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 02:56:58 +0100 Subject: [PATCH 02/21] feat(client): map gateway errors and retry failed attempts The client must give the same errors and the same retries as every other Gatepost client, so the status map and the retry rules follow the spec's client contract. PSR-18 has no timeout, so the client times each attempt and reports a failure at the timeout as a timeout. A call stops at its total deadline, and a Retry-After of 10 seconds or less on a 429, 502, 503 or 504 sets the wait. A fake clock drives the tests, so none of them sleeps. The package now requires the PSR interfaces for HTTP and caching, which ADR 0003 records. Signed-off-by: MobileMage --- composer.json | 7 +- composer.lock | 591 +++++++++++++++++- ...03-the-client-depends-on-psr-interfaces.md | 11 + docs/api-dump.md | 24 + infection.json5 | 10 +- src/Client/ErrorCode.php | 36 ++ src/Client/Internal/Sender.php | 194 ++++++ src/Client/Internal/SystemTimer.php | 31 + src/Client/Internal/Timer.php | 32 + src/Client/PostcodeException.php | 162 +++++ tests/Unit/Client/FakeTimer.php | 51 ++ tests/Unit/Client/PostcodeExceptionTest.php | 157 +++++ tests/Unit/Client/ScriptedTransport.php | 72 +++ tests/Unit/Client/SenderRateLimitTest.php | 79 +++ tests/Unit/Client/SenderRetryTest.php | 157 +++++ tests/Unit/Client/SenderTest.php | 100 +++ tests/Unit/Client/SenderTestCase.php | 64 ++ tests/Unit/Client/SenderTimeoutTest.php | 94 +++ tests/Unit/Client/SystemTimerTest.php | 53 ++ tests/Unit/PortabilityTest.php | 34 +- 20 files changed, 1948 insertions(+), 11 deletions(-) create mode 100644 docs/adr/0003-the-client-depends-on-psr-interfaces.md create mode 100644 src/Client/ErrorCode.php create mode 100644 src/Client/Internal/Sender.php create mode 100644 src/Client/Internal/SystemTimer.php create mode 100644 src/Client/Internal/Timer.php create mode 100644 src/Client/PostcodeException.php create mode 100644 tests/Unit/Client/FakeTimer.php create mode 100644 tests/Unit/Client/PostcodeExceptionTest.php create mode 100644 tests/Unit/Client/ScriptedTransport.php create mode 100644 tests/Unit/Client/SenderRateLimitTest.php create mode 100644 tests/Unit/Client/SenderRetryTest.php create mode 100644 tests/Unit/Client/SenderTest.php create mode 100644 tests/Unit/Client/SenderTestCase.php create mode 100644 tests/Unit/Client/SenderTimeoutTest.php create mode 100644 tests/Unit/Client/SystemTimerTest.php diff --git a/composer.json b/composer.json index 465caf1..affe6e3 100644 --- a/composer.json +++ b/composer.json @@ -4,12 +4,17 @@ "license": "Apache-2.0", "type": "library", "require": { - "php": "^8.1" + "php": "^8.1", + "psr/http-client": "^1.0.3", + "psr/http-factory": "^1.1.0", + "psr/http-message": "^1.1 || ^2.0", + "psr/simple-cache": "^2.0 || ^3.0" }, "require-dev": { "composer/semver": "^3.5.0", "ergebnis/composer-normalize": "^2.54.0", "friendsofphp/php-cs-fixer": "^3.95.27", + "guzzlehttp/guzzle": "^7.15.5", "phpmd/phpmd": "^2.15.0", "phpstan/phpstan": "^2.2.16", "phpstan/phpstan-deprecation-rules": "^2.0.5", diff --git a/composer.lock b/composer.lock index f04e432..ec3ef2f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,220 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8437dea3b19e96c2b2e727b8cb79e4bd", - "packages": [], + "content-hash": "60d5277d45890209fe9ccd29e35900fb", + "packages": [ + { + "name": "psr/http-client", + "version": "1.0.3", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-client.git", + "reference": "bb5906edc1c324c9a05aa0873d40117941e5fa90" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-client/zipball/bb5906edc1c324c9a05aa0873d40117941e5fa90", + "reference": "bb5906edc1c324c9a05aa0873d40117941e5fa90", + "shasum": "" + }, + "require": { + "php": "^7.0 || ^8.0", + "psr/http-message": "^1.0 || ^2.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Client\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interface for HTTP clients", + "homepage": "https://github.com/php-fig/http-client", + "keywords": [ + "http", + "http-client", + "psr", + "psr-18" + ], + "support": { + "source": "https://github.com/php-fig/http-client" + }, + "time": "2023-09-23T14:17:50+00:00" + }, + { + "name": "psr/http-factory", + "version": "1.1.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-factory.git", + "reference": "2b4765fddfe3b508ac62f829e852b1501d3f6e8a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-factory/zipball/2b4765fddfe3b508ac62f829e852b1501d3f6e8a", + "reference": "2b4765fddfe3b508ac62f829e852b1501d3f6e8a", + "shasum": "" + }, + "require": { + "php": ">=7.1", + "psr/http-message": "^1.0 || ^2.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Message\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "PSR-17: Common interfaces for PSR-7 HTTP message factories", + "keywords": [ + "factory", + "http", + "message", + "psr", + "psr-17", + "psr-7", + "request", + "response" + ], + "support": { + "source": "https://github.com/php-fig/http-factory" + }, + "time": "2024-04-15T12:06:14+00:00" + }, + { + "name": "psr/http-message", + "version": "2.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-message.git", + "reference": "402d35bcb92c70c026d1a6a9883f06b2ead23d71" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-message/zipball/402d35bcb92c70c026d1a6a9883f06b2ead23d71", + "reference": "402d35bcb92c70c026d1a6a9883f06b2ead23d71", + "shasum": "" + }, + "require": { + "php": "^7.2 || ^8.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Message\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interface for HTTP messages", + "homepage": "https://github.com/php-fig/http-message", + "keywords": [ + "http", + "http-message", + "psr", + "psr-7", + "request", + "response" + ], + "support": { + "source": "https://github.com/php-fig/http-message/tree/2.0" + }, + "time": "2023-04-04T09:54:51+00:00" + }, + { + "name": "psr/simple-cache", + "version": "3.0.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/simple-cache.git", + "reference": "764e0b3939f5ca87cb904f570ef9be2d78a07865" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/simple-cache/zipball/764e0b3939f5ca87cb904f570ef9be2d78a07865", + "reference": "764e0b3939f5ca87cb904f570ef9be2d78a07865", + "shasum": "" + }, + "require": { + "php": ">=8.0.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\SimpleCache\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interfaces for simple caching", + "keywords": [ + "cache", + "caching", + "psr", + "psr-16", + "simple-cache" + ], + "support": { + "source": "https://github.com/php-fig/simple-cache/tree/3.0.0" + }, + "time": "2021-10-29T13:26:27+00:00" + } + ], "packages-dev": [ { "name": "clue/ndjson-react", @@ -1038,6 +1250,337 @@ ], "time": "2026-09-22T09:54:55+00:00" }, + { + "name": "guzzlehttp/guzzle", + "version": "7.15.5", + "source": { + "type": "git", + "url": "https://github.com/guzzle/guzzle.git", + "reference": "ee80339fd9177ba44c49cdb653ff02a4d1106b9a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/guzzle/zipball/ee80339fd9177ba44c49cdb653ff02a4d1106b9a", + "reference": "ee80339fd9177ba44c49cdb653ff02a4d1106b9a", + "shasum": "" + }, + "require": { + "ext-json": "*", + "guzzlehttp/promises": "^2.5.3", + "guzzlehttp/psr7": "^2.13.1", + "php": "^7.2.5 || ^8.0", + "psr/http-client": "^1.0", + "symfony/deprecation-contracts": "^2.5 || ^3.0", + "symfony/polyfill-php80": "^1.25" + }, + "provide": { + "psr/http-client-implementation": "1.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "ext-curl": "*", + "guzzle/client-integration-tests": "3.0.3", + "guzzlehttp/test-server": "^0.7", + "php-http/message-factory": "^1.1", + "phpunit/phpunit": "^8.5.52 || ^9.6.34", + "psr/log": "^1.1 || ^2.0 || ^3.0" + }, + "suggest": { + "ext-curl": "Required for CURL handler support", + "ext-intl": "Required for Internationalized Domain Name (IDN) support", + "psr/log": "Required for using the Log middleware" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "files": [ + "src/functions_include.php" + ], + "psr-4": { + "GuzzleHttp\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "Jeremy Lindblom", + "email": "jeremeamia@gmail.com", + "homepage": "https://github.com/jeremeamia" + }, + { + "name": "George Mponos", + "email": "gmponos@gmail.com", + "homepage": "https://github.com/gmponos" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://github.com/sagikazarmark" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + } + ], + "description": "Guzzle is a PHP HTTP client library", + "keywords": [ + "client", + "curl", + "framework", + "http", + "http client", + "psr-18", + "psr-7", + "rest", + "web service" + ], + "support": { + "issues": "https://github.com/guzzle/guzzle/issues", + "source": "https://github.com/guzzle/guzzle/tree/7.15.5" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/guzzle", + "type": "tidelift" + } + ], + "time": "2026-08-24T09:21:06+00:00" + }, + { + "name": "guzzlehttp/promises", + "version": "2.5.3", + "source": { + "type": "git", + "url": "https://github.com/guzzle/promises.git", + "reference": "cde49999552d185d64715fe9c1f77a2aadd2f9f1" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/promises/zipball/cde49999552d185d64715fe9c1f77a2aadd2f9f1", + "reference": "cde49999552d185d64715fe9c1f77a2aadd2f9f1", + "shasum": "" + }, + "require": { + "php": "^7.2.5 || ^8.0", + "symfony/deprecation-contracts": "^2.5 || ^3.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "phpunit/phpunit": "^8.5.52 || ^9.6.34" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "psr-4": { + "GuzzleHttp\\Promise\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + } + ], + "description": "Guzzle promises library", + "keywords": [ + "promise" + ], + "support": { + "issues": "https://github.com/guzzle/promises/issues", + "source": "https://github.com/guzzle/promises/tree/2.5.3" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/promises", + "type": "tidelift" + } + ], + "time": "2026-08-24T09:11:28+00:00" + }, + { + "name": "guzzlehttp/psr7", + "version": "2.13.1", + "source": { + "type": "git", + "url": "https://github.com/guzzle/psr7.git", + "reference": "95e7828100de18b4e269fb1703be530082d5166d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/psr7/zipball/95e7828100de18b4e269fb1703be530082d5166d", + "reference": "95e7828100de18b4e269fb1703be530082d5166d", + "shasum": "" + }, + "require": { + "php": "^7.2.5 || ^8.0", + "psr/http-factory": "^1.0", + "psr/http-message": "^1.1 || ^2.0", + "ralouphie/getallheaders": "^3.0", + "symfony/deprecation-contracts": "^2.5 || ^3.0", + "symfony/polyfill-php80": "^1.25" + }, + "provide": { + "psr/http-factory-implementation": "1.0", + "psr/http-message-implementation": "1.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "http-interop/http-factory-tests": "1.1.0", + "jshttp/mime-db": "1.54.0.1", + "phpunit/phpunit": "^8.5.52 || ^9.6.34" + }, + "suggest": { + "laminas/laminas-httphandlerrunner": "Emit PSR-7 responses" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "psr-4": { + "GuzzleHttp\\Psr7\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "George Mponos", + "email": "gmponos@gmail.com", + "homepage": "https://github.com/gmponos" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://github.com/sagikazarmark" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://sagikazarmark.hu" + } + ], + "description": "PSR-7 message implementation that also provides common utility methods", + "keywords": [ + "http", + "message", + "psr-7", + "request", + "response", + "stream", + "uri", + "url" + ], + "support": { + "issues": "https://github.com/guzzle/psr7/issues", + "source": "https://github.com/guzzle/psr7/tree/2.13.1" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/psr7", + "type": "tidelift" + } + ], + "time": "2026-08-24T09:13:11+00:00" + }, { "name": "justinrainbow/json-schema", "version": "6.13.1", @@ -2404,6 +2947,50 @@ }, "time": "2024-09-11T13:17:53+00:00" }, + { + "name": "ralouphie/getallheaders", + "version": "3.0.3", + "source": { + "type": "git", + "url": "https://github.com/ralouphie/getallheaders.git", + "reference": "120b605dfeb996808c31b6477290a714d356e822" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ralouphie/getallheaders/zipball/120b605dfeb996808c31b6477290a714d356e822", + "reference": "120b605dfeb996808c31b6477290a714d356e822", + "shasum": "" + }, + "require": { + "php": ">=5.6" + }, + "require-dev": { + "php-coveralls/php-coveralls": "^2.1", + "phpunit/phpunit": "^5 || ^6.5" + }, + "type": "library", + "autoload": { + "files": [ + "src/getallheaders.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Ralph Khattar", + "email": "ralph.khattar@gmail.com" + } + ], + "description": "A polyfill for getallheaders.", + "support": { + "issues": "https://github.com/ralouphie/getallheaders/issues", + "source": "https://github.com/ralouphie/getallheaders/tree/develop" + }, + "time": "2019-03-08T08:55:37+00:00" + }, { "name": "react/cache", "version": "v1.2.0", diff --git a/docs/adr/0003-the-client-depends-on-psr-interfaces.md b/docs/adr/0003-the-client-depends-on-psr-interfaces.md new file mode 100644 index 0000000..9137351 --- /dev/null +++ b/docs/adr/0003-the-client-depends-on-psr-interfaces.md @@ -0,0 +1,11 @@ +# The client depends on the PSR interfaces for HTTP and caching + +The client must send HTTP requests and, when the caller turns the cache on, keep results. DEP-3 asks for an ADR for each new runtime dependency. + +The package requires four packages from the PHP-FIG: `psr/http-client` (PSR-18), `psr/http-factory` (PSR-17), `psr/http-message` (PSR-7) and `psr/simple-cache` (PSR-16). Each one holds only interfaces, with no code that runs, under the MIT licence. PHP-6 names them as the only dependencies of the client. The caller brings the HTTP client and the cache, such as Guzzle or Symfony HttpClient, and the cache of the framework. The package bundles no HTTP client, so a WordPress plugin that prefixes the package with Strauss (WP-4) does not ship a second copy of Guzzle. + +Guzzle is a dev dependency only. The contract tests use it as the transport. + +The constraints allow PSR-7 1.1 and 2.0, and PSR-16 2.0 and 3.0, because older WordPress hosts and frameworks still pin the earlier major versions. The client calls only methods that both majors have. + +PSR-18 has no timeout. The caller sets the timeout on the HTTP client, and passes the same value as `timeoutMs`. The client times each attempt, and it reports a failed attempt that took `timeoutMs` or more as `timeout`. diff --git a/docs/api-dump.md b/docs/api-dump.md index 7d2cfa4..bfc2886 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -3,6 +3,30 @@ Do not edit this file. `composer api:dump` writes it from the public symbols of the package. It leaves out each `Internal` namespace and each symbol with `@internal` in its doc comment. It shows native types only, so a type that only a doc comment gives is not here. `composer api:dump:check` fails when this file is out of date. ```php +namespace Gatepost\Postcode\Client; + +enum ErrorCode: string +{ + case InvalidInput = 'invalid_input'; + case Unauthorized = 'unauthorized'; + case InsufficientCredits = 'insufficient_credits'; + case OriginNotAllowed = 'origin_not_allowed'; + case Forbidden = 'forbidden'; + case RateLimited = 'rate_limited'; + case ServerError = 'server_error'; + case UnexpectedResponse = 'unexpected_response'; + case NetworkError = 'network_error'; + case Timeout = 'timeout'; +} + +final class PostcodeException extends \RuntimeException +{ + public function errorCode(): ErrorCode; + public function status(): ?int; + public function apiCode(): ?string; + public function retryAfterMs(): ?int; +} + namespace Gatepost\Postcode; final class ParseError diff --git a/infection.json5 b/infection.json5 index 9530fc8..3e88344 100644 --- a/infection.json5 +++ b/infection.json5 @@ -4,8 +4,14 @@ "$schema": "tools/infection/vendor/infection/infection/resources/schema.json", "source": { "directories": ["src"], - // The generator writes these files, and its --check mode keeps them in step. - "excludes": ["Internal/NfkcTable.php", "Internal/SpecData.php"], + // The generator writes the first two files, and its --check mode keeps them in step. The + // unit tests never sleep (T-5), so they cannot measure the real waits of SystemTimer. The + // contract scenarios measure them against the mock server. + "excludes": [ + "Internal/NfkcTable.php", + "Internal/SpecData.php", + "Client/Internal/SystemTimer.php", + ], }, "phpUnit": { "customPath": "vendor/bin/phpunit", diff --git a/src/Client/ErrorCode.php b/src/Client/ErrorCode.php new file mode 100644 index 0000000..b2ea49a --- /dev/null +++ b/src/Client/ErrorCode.php @@ -0,0 +1,36 @@ +timer->nowMs() + $this->budgetMs(); + for ($retry = 1; ; ++$retry) { + try { + return $this->attempt($request); + } catch (PostcodeException $error) { + $waitMs = $this->waitMs($retry, $error, $retryTimeouts); + if ($waitMs === null || $this->timer->nowMs() + $waitMs > $deadlineMs) { + throw $error; + } + $this->timer->wait($waitMs); + } + } + } + + /** + * The total time of a call: every attempt takes the timeout, and every retry waits the + * longest backoff. + */ + private function budgetMs(): float + { + $budgetMs = (1 + $this->maxRetries) * $this->timeoutMs; + for ($retry = 1; $retry <= $this->maxRetries; ++$retry) { + $budgetMs += self::backoffBaseMs($retry) + self::MAX_JITTER_MS; + } + + return $budgetMs; + } + + private function attempt(RequestInterface $request): mixed + { + $startedMs = $this->timer->nowMs(); + try { + $response = $this->transport->sendRequest($request); + } catch (ClientExceptionInterface $failure) { + $tookMs = $this->timer->nowMs() - $startedMs; + throw $tookMs >= $this->timeoutMs + ? PostcodeException::timeout($this->timeoutMs, $failure) + : PostcodeException::networkError($failure); + } + + return self::dataField($response, $this->retryAfterMs($response)); + } + + /** + * The wait before a retry, or null when the error is final. + */ + private function waitMs(int $retry, PostcodeException $error, bool $retryTimeouts): ?int + { + if ($retry > $this->maxRetries) { + return null; + } + + return match ($error->errorCode()) { + ErrorCode::NetworkError => $this->backoffMs($retry), + ErrorCode::Timeout => $retryTimeouts ? $this->backoffMs($retry) : null, + ErrorCode::ServerError => \in_array($error->status(), self::RETRIED_STATUSES, true) + ? self::shortRetryAfterMs($error) ?? $this->backoffMs($retry) + : null, + // With no Retry-After, or a long one, a retry in the same clock minute would fail too. + ErrorCode::RateLimited => self::shortRetryAfterMs($error), + default => null, + }; + } + + /** + * The wait that Retry-After asked for, or null when there is none or it is too long. + */ + private static function shortRetryAfterMs(PostcodeException $error): ?int + { + $retryAfterMs = $error->retryAfterMs(); + + return $retryAfterMs !== null && $retryAfterMs <= self::LONGEST_RETRY_AFTER_MS + ? $retryAfterMs + : null; + } + + /** + * The first retry waits 500 to 750 ms, and each later one waits twice as long. + */ + private function backoffMs(int $retry): int + { + return self::backoffBaseMs($retry) + $this->timer->jitterMs(self::MAX_JITTER_MS); + } + + private static function backoffBaseMs(int $retry): int + { + return self::FIRST_WAIT_MS * 2 ** ($retry - 1); + } + + /** + * @throws PostcodeException + */ + private static function dataField(ResponseInterface $response, ?int $retryAfterMs): mixed + { + $status = $response->getStatusCode(); + $body = \json_decode((string) $response->getBody(), true); + if ($status !== 200) { + throw PostcodeException::fromStatus($status, self::apiCode($body), $retryAfterMs); + } + if (!\is_array($body) || !\array_key_exists('data', $body)) { + throw PostcodeException::unreadable($status); + } + + return $body['data']; + } + + /** + * The error.code of an error body. The client never fails on an unexpected error body. + */ + private static function apiCode(mixed $body): ?string + { + $error = \is_array($body) ? ($body['error'] ?? null) : null; + $code = \is_array($error) ? ($error['code'] ?? null) : null; + + return \is_string($code) ? $code : null; + } + + /** + * The wait that the Retry-After header of a 429, 502, 503 or 504 asks for, as seconds or as + * an HTTP date. A header in another form counts as no header. A date in the past is valid + * for a 429 only. + */ + private function retryAfterMs(ResponseInterface $response): ?int + { + $status = $response->getStatusCode(); + $value = \trim($response->getHeaderLine('Retry-After')); + $mayWait = $status === 429 || \in_array($status, self::RETRIED_STATUSES, true); + if ($value === '' || !$mayWait) { + return null; + } + if (\preg_match('/\A\d{1,9}\z/', $value) === 1) { + return (int) $value * 1000; + } + $utc = new DateTimeZone('UTC'); + $date = DateTimeImmutable::createFromFormat(self::HTTP_DATE, $value, $utc); + if ($date === false) { + return null; + } + $waitMs = (int) \ceil($date->getTimestamp() * 1000 - $this->timer->nowMs()); + + return $waitMs < 0 && $status !== 429 ? null : \max(0, $waitMs); + } +} diff --git a/src/Client/Internal/SystemTimer.php b/src/Client/Internal/SystemTimer.php new file mode 100644 index 0000000..a14c3a2 --- /dev/null +++ b/src/Client/Internal/SystemTimer.php @@ -0,0 +1,31 @@ +status = $status; + $error->apiCode = $apiCode; + $error->retryAfterMs = $retryAfterMs; + + return $error; + } + + /** + * @internal A response with status 200 must hold a JSON object with a data field, and the + * parts that the contract uses. + */ + public static function unreadable(int $status): self + { + $message = "The gateway sent a response that the client cannot read (HTTP {$status}). " + . 'Try again later, and report it if it happens again.'; + + $error = new self($message, ErrorCode::UnexpectedResponse); + $error->status = $status; + + return $error; + } + + /** + * @internal The transport failed before any response arrived. + */ + public static function networkError(Throwable $previous): self + { + $message = 'The client could not reach the gateway. Check the network and the base URL.'; + + return new self($message, ErrorCode::NetworkError, $previous); + } + + /** + * @internal The transport failed after the attempt reached the timeout. + */ + public static function timeout(int $timeoutMs, Throwable $previous): self + { + $message = "The gateway sent no response within {$timeoutMs} ms. Try again later."; + + return new self($message, ErrorCode::Timeout, $previous); + } + + /** + * The reason for the failure, as a stable code. + */ + public function errorCode(): ErrorCode + { + return $this->errorCode; + } + + /** + * The HTTP status of the response, or null when no response arrived. + */ + public function status(): ?int + { + return $this->status; + } + + /** + * The error.code of the response body, such as level_not_granted, or null. + */ + public function apiCode(): ?string + { + return $this->apiCode; + } + + /** + * The wait in milliseconds that the Retry-After header of a 429 asked for, or null. Tell + * the user when to try again. + */ + public function retryAfterMs(): ?int + { + return $this->retryAfterMs; + } + + private static function errorCodeFor(int $status, ?string $apiCode): ErrorCode + { + return match (true) { + $status === 401 => ErrorCode::Unauthorized, + $status === 402 => ErrorCode::InsufficientCredits, + $status === 403 && $apiCode === 'origin_not_allowed' => ErrorCode::OriginNotAllowed, + $status === 403 => ErrorCode::Forbidden, + $status === 429 => ErrorCode::RateLimited, + $status >= 400 && $status <= 499 => ErrorCode::InvalidInput, + default => ErrorCode::ServerError, + }; + } + + private static function messageFor( + ErrorCode $errorCode, + int $status, + ?int $retryAfterMs, + ): string { + $wait = $retryAfterMs === null ? 'Wait' : 'Wait ' . \ceil($retryAfterMs / 1000) . ' s'; + + return match ($errorCode) { + ErrorCode::Unauthorized => "The gateway refused the API key (HTTP {$status}). " + . 'Check the key that the client sends.', + ErrorCode::InsufficientCredits => "The account has no credits for this lookup level " + . "(HTTP {$status}). Add credits, or ask for a lower level.", + ErrorCode::OriginNotAllowed => "The key does not allow the origin of this request " + . "(HTTP {$status}). Add the origin to the key in the NIPOST dashboard.", + ErrorCode::Forbidden => "The key does not allow this request (HTTP {$status}). " + . 'Check the scope and the lookup level of the key.', + ErrorCode::RateLimited => "The gateway refused the request because the key sent too " + . "many (HTTP {$status}). {$wait}, then try again.", + ErrorCode::InvalidInput => "The gateway rejected the request (HTTP {$status}). " + . 'Check the input of the call.', + default => "The gateway failed (HTTP {$status}). Try again later.", + }; + } +} diff --git a/tests/Unit/Client/FakeTimer.php b/tests/Unit/Client/FakeTimer.php new file mode 100644 index 0000000..1a5713c --- /dev/null +++ b/tests/Unit/Client/FakeTimer.php @@ -0,0 +1,51 @@ + */ + public array $waits = []; + + private float $nowMs = self::START_MS; + + /** + * @param int $jitterMs The random part that each wait gets. + */ + public function __construct(private readonly int $jitterMs = 0) {} + + public function nowMs(): float + { + return $this->nowMs; + } + + public function wait(int $ms): void + { + $this->waits[] = $ms; + $this->advance($ms); + } + + public function jitterMs(int $maxMs): int + { + return \min($this->jitterMs, $maxMs); + } + + public function advance(float $ms): void + { + $this->nowMs += $ms; + } +} diff --git a/tests/Unit/Client/PostcodeExceptionTest.php b/tests/Unit/Client/PostcodeExceptionTest.php new file mode 100644 index 0000000..18bb154 --- /dev/null +++ b/tests/Unit/Client/PostcodeExceptionTest.php @@ -0,0 +1,157 @@ + + */ + public static function statuses(): array + { + return [ + '401 with no key' => [401, 'auth_required', ErrorCode::Unauthorized], + '401 with a wrong key' => [401, 'invalid_api_key', ErrorCode::Unauthorized], + 'status 402' => [402, 'insufficient_credits', ErrorCode::InsufficientCredits], + '403 for an origin' => [403, 'origin_not_allowed', ErrorCode::OriginNotAllowed], + '403 for a level' => [403, 'level_not_granted', ErrorCode::Forbidden], + '403 with no body' => [403, null, ErrorCode::Forbidden], + 'status 429' => [429, 'rate_limited', ErrorCode::RateLimited], + 'status 502' => [502, null, ErrorCode::ServerError], + 'status 503' => [503, null, ErrorCode::ServerError], + 'status 504' => [504, null, ErrorCode::ServerError], + 'status 400' => [400, 'invalid_request', ErrorCode::InvalidInput], + 'status 404' => [404, 'not_found', ErrorCode::InvalidInput], + 'status 499' => [499, null, ErrorCode::InvalidInput], + 'status 500' => [500, 'internal', ErrorCode::ServerError], + 'status 501' => [501, null, ErrorCode::ServerError], + 'status 505' => [505, null, ErrorCode::ServerError], + 'status 204' => [204, null, ErrorCode::ServerError], + 'status 301' => [301, null, ErrorCode::ServerError], + ]; + } + + #[Test] + #[DataProvider('statuses')] + public function mapsEachStatusToOneErrorCode( + int $status, + ?string $apiCode, + ErrorCode $code, + ): void { + $error = PostcodeException::fromStatus($status, $apiCode, null); + + self::assertSame($code, $error->errorCode()); + self::assertSame($status, $error->status()); + self::assertSame($apiCode, $error->apiCode()); + } + + /** + * @return array + */ + public static function messages(): array + { + $failure = new RuntimeException('A synthetic failure.'); + + return [ + 'status 401' => [ + PostcodeException::fromStatus(401, 'auth_required', null), + 'The gateway refused the API key (HTTP 401). Check the key that the client sends.', + ], + 'status 402' => [ + PostcodeException::fromStatus(402, 'insufficient_credits', null), + 'The account has no credits for this lookup level (HTTP 402). Add credits, or ask ' + . 'for a lower level.', + ], + 'status 403 for an origin' => [ + PostcodeException::fromStatus(403, 'origin_not_allowed', null), + 'The key does not allow the origin of this request (HTTP 403). Add the origin to ' + . 'the key in the NIPOST dashboard.', + ], + 'status 403 for a level' => [ + PostcodeException::fromStatus(403, 'level_not_granted', null), + 'The key does not allow this request (HTTP 403). Check the scope and the lookup ' + . 'level of the key.', + ], + 'status 429 with no Retry-After' => [ + PostcodeException::fromStatus(429, 'rate_limited', null), + 'The gateway refused the request because the key sent too many (HTTP 429). Wait, ' + . 'then try again.', + ], + 'status 429 that asks for 11.4 seconds' => [ + PostcodeException::fromStatus(429, 'rate_limited', 11_400), + 'The gateway refused the request because the key sent too many (HTTP 429). Wait ' + . '12 s, then try again.', + ], + 'status 400' => [ + PostcodeException::fromStatus(400, 'invalid_request', null), + 'The gateway rejected the request (HTTP 400). Check the input of the call.', + ], + 'status 503' => [ + PostcodeException::fromStatus(503, null, null), + 'The gateway failed (HTTP 503). Try again later.', + ], + 'an unreadable response' => [ + PostcodeException::unreadable(200), + 'The gateway sent a response that the client cannot read (HTTP 200). Try again ' + . 'later, and report it if it happens again.', + ], + 'a failed connection' => [ + PostcodeException::networkError($failure), + 'The client could not reach the gateway. Check the network and the base URL.', + ], + 'a timeout' => [ + PostcodeException::timeout(8000, $failure), + 'The gateway sent no response within 8000 ms. Try again later.', + ], + ]; + } + + #[Test] + #[DataProvider('messages')] + public function saysWhatHappenedAndWhatToDo(PostcodeException $error, string $message): void + { + self::assertSame($message, $error->getMessage()); + self::assertSame(0, $error->getCode()); + } + + #[Test] + public function keepsTheStatusOfAnUnreadableResponse(): void + { + $error = PostcodeException::unreadable(200); + + self::assertSame(ErrorCode::UnexpectedResponse, $error->errorCode()); + self::assertSame(200, $error->status()); + self::assertNull($error->apiCode()); + self::assertNull($error->retryAfterMs()); + } + + #[Test] + public function hasOneCaseForEachCodeOfTheErrorTableInTheSpec(): void + { + $client = \file_get_contents(\dirname(__DIR__, 3) . '/spec/client.md'); + self::assertIsString($client); + $errors = \explode("\n## ", \explode("\n## Errors\n", $client)[1])[0]; + \preg_match_all('/^\| [^|]+ \| `([a-z_]+)` \|/m', $errors, $codes); + + self::assertSame( + \array_values(\array_unique($codes[1])), + \array_map(static fn(ErrorCode $code): string => $code->value, ErrorCode::cases()), + ); + } +} diff --git a/tests/Unit/Client/ScriptedTransport.php b/tests/Unit/Client/ScriptedTransport.php new file mode 100644 index 0000000..03b1c0c --- /dev/null +++ b/tests/Unit/Client/ScriptedTransport.php @@ -0,0 +1,72 @@ + */ + public array $requests = []; + + /** + * @param list $answers + */ + public function __construct( + private array $answers, + private readonly ?FakeTimer $timer = null, + ) {} + + /** + * A response with a JSON body. + * + * @param array $headers + */ + public static function json(int $status, mixed $body, array $headers = []): ResponseInterface + { + $headers['Content-Type'] = 'application/json'; + + return new Response($status, $headers, \json_encode($body, \JSON_THROW_ON_ERROR)); + } + + /** + * A response with the gateway's error body. + * + * @param array $headers + */ + public static function error(int $status, string $code, array $headers = []): ResponseInterface + { + $body = ['error' => ['code' => $code, 'message' => 'A synthetic error.']]; + + return self::json($status, $body, $headers); + } + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $this->requests[] = $request; + $answer = \array_shift($this->answers); + if ($answer === null) { + throw new LogicException('The test expected no more requests.'); + } + if ($answer instanceof ResponseInterface) { + return $answer; + } + $this->timer?->advance($answer); + + throw new ConnectException('The synthetic connection failed.', $request); + } +} diff --git a/tests/Unit/Client/SenderRateLimitTest.php b/tests/Unit/Client/SenderRateLimitTest.php new file mode 100644 index 0000000..2ff69f1 --- /dev/null +++ b/tests/Unit/Client/SenderRateLimitTest.php @@ -0,0 +1,79 @@ + + */ + public static function shortRetryAfters(): array + { + return [ + 'one second' => ['1', 1000], + 'ten seconds, the longest that the client waits' => ['10', 10_000], + 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], + 'an HTTP date in the past' => ['Mon, 12 Oct 2026 08:59:00 GMT', 0], + ]; + } + + #[Test] + #[DataProvider('shortRetryAfters')] + public function waitsExactlyAsLongAsAShortRetryAfterAsks(string $header, int $waitMs): void + { + $timer = new FakeTimer(jitterMs: 200); + $transport = new ScriptedTransport([ + ScriptedTransport::error(429, 'rate_limited', ['Retry-After' => $header]), + ScriptedTransport::json(200, self::VALID), + ]); + + self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); + self::assertSame([$waitMs], $timer->waits); + } + + /** + * @return array, ?int}> + */ + public static function finalRateLimits(): array + { + return [ + 'no Retry-After' => [[], null], + 'a Retry-After of eleven seconds' => [['Retry-After' => '11'], 11_000], + 'a Retry-After of two minutes' => [['Retry-After' => '120'], 120_000], + 'a Retry-After that is not a number or a date' => [['Retry-After' => 'soon'], null], + ]; + } + + /** + * @param array $headers + */ + #[Test] + #[DataProvider('finalRateLimits')] + public function failsAtOnceWithRateLimitedWhenTheWaitIsLongOrUnknown( + array $headers, + ?int $retryAfterMs, + ): void { + $transport = new ScriptedTransport([ + ScriptedTransport::error(429, 'rate_limited', $headers), + ]); + + $error = $this->failure($transport); + + self::assertSame(ErrorCode::RateLimited, $error->errorCode()); + self::assertSame($retryAfterMs, $error->retryAfterMs()); + self::assertCount(1, $transport->requests); + } +} diff --git a/tests/Unit/Client/SenderRetryTest.php b/tests/Unit/Client/SenderRetryTest.php new file mode 100644 index 0000000..c7fdade --- /dev/null +++ b/tests/Unit/Client/SenderRetryTest.php @@ -0,0 +1,157 @@ +send($transport, timer: $timer)); + self::assertSame([680], $timer->waits); + } + + #[Test] + public function doublesTheWaitBeforeEachLaterRetry(): void + { + $timer = new FakeTimer(jitterMs: 250); + $transport = new ScriptedTransport([ + ScriptedTransport::error(502, 'unavailable'), + ScriptedTransport::error(504, 'unavailable'), + ScriptedTransport::json(200, self::VALID), + ]); + + self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); + self::assertSame([750, 1250], $timer->waits); + } + + #[Test] + public function stopsAfterTheFirstAttemptAndTwoRetries(): void + { + $busy = ScriptedTransport::error(503, 'busy'); + $transport = new ScriptedTransport([$busy, $busy, $busy]); + + $error = $this->failure($transport); + + self::assertSame(ErrorCode::ServerError, $error->errorCode()); + self::assertSame(503, $error->status()); + self::assertSame('busy', $error->apiCode()); + self::assertCount(3, $transport->requests); + self::assertSame([500, 1000], $this->timer->waits); + } + + #[Test] + public function sendsOneAttemptWhenMaxRetriesIsZero(): void + { + $transport = new ScriptedTransport([ScriptedTransport::error(503, 'busy')]); + + $error = $this->failure($transport, maxRetries: 0); + + self::assertSame(ErrorCode::ServerError, $error->errorCode()); + self::assertCount(1, $transport->requests); + } + + /** + * @return array + */ + public static function finalStatuses(): array + { + return [ + 'status 400' => [400, 'invalid_request'], + 'status 401' => [401, 'auth_required'], + 'status 402' => [402, 'insufficient_credits'], + 'status 403' => [403, 'level_not_granted'], + 'status 500' => [500, 'internal'], + ]; + } + + #[Test] + #[DataProvider('finalStatuses')] + public function neverRetriesAStatusThatTheSpecCallsFinal(int $status, string $apiCode): void + { + $transport = new ScriptedTransport([ScriptedTransport::error($status, $apiCode)]); + + $error = $this->failure($transport); + + self::assertSame($status, $error->status()); + self::assertSame($apiCode, $error->apiCode()); + self::assertCount(1, $transport->requests); + } + + /** + * @return array + */ + public static function validRetryAfters(): array + { + return [ + 'zero seconds' => ['0', 0], + 'four seconds' => ['4', 4000], + 'ten seconds, the longest that the client waits' => ['10', 10_000], + 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], + ]; + } + + #[Test] + #[DataProvider('validRetryAfters')] + public function waitsExactlyAsLongAsAValidRetryAfterAsksOnA503( + string $header, + int $waitMs, + ): void { + $timer = new FakeTimer(jitterMs: 200); + $transport = new ScriptedTransport([ + ScriptedTransport::error(503, 'unavailable', ['Retry-After' => $header]), + ScriptedTransport::json(200, self::VALID), + ]); + + self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); + self::assertSame([$waitMs], $timer->waits); + } + + /** + * @return array + */ + public static function otherRetryAfters(): array + { + return [ + 'eleven seconds' => ['11', 11_000], + 'a number that is not whole' => ['1.5', null], + 'a negative number' => ['-1', null], + 'text' => ['soon', null], + 'an HTTP date in the past' => ['Mon, 12 Oct 2026 08:59:00 GMT', null], + ]; + } + + #[Test] + #[DataProvider('otherRetryAfters')] + public function usesItsOwnWaitForALongOrInvalidRetryAfterOnA504( + string $header, + ?int $retryAfterMs, + ): void { + $timer = new FakeTimer(jitterMs: 100); + $busy = ScriptedTransport::error(504, 'busy', ['Retry-After' => $header]); + $transport = new ScriptedTransport([$busy, $busy, $busy]); + + $error = $this->failure($transport, timer: $timer); + + self::assertSame($retryAfterMs, $error->retryAfterMs()); + self::assertSame([600, 1100], $timer->waits); + } +} diff --git a/tests/Unit/Client/SenderTest.php b/tests/Unit/Client/SenderTest.php new file mode 100644 index 0000000..2a98a7f --- /dev/null +++ b/tests/Unit/Client/SenderTest.php @@ -0,0 +1,100 @@ +send($transport)); + self::assertCount(1, $transport->requests); + } + + /** + * @return array + */ + public static function unreadableResponses(): array + { + return [ + 'a captive portal page' => [new Response(200, [], 'Sign in')], + 'a JSON list' => [ScriptedTransport::json(200, [1, 2])], + 'a JSON object with no data field' => [ScriptedTransport::json(200, ['valid' => true])], + 'an empty body' => [new Response(200)], + ]; + } + + #[Test] + #[DataProvider('unreadableResponses')] + public function failsWithUnexpectedResponseOnA200WithNoData(ResponseInterface $response): void + { + $error = $this->failure(new ScriptedTransport([$response])); + + self::assertSame(ErrorCode::UnexpectedResponse, $error->errorCode()); + self::assertSame(200, $error->status()); + self::assertSame([], $this->timer->waits); + } + + /** + * @return array + */ + public static function unexpectedErrorBodies(): array + { + return [ + 'plain text' => [new Response(403, [], 'Forbidden')], + 'a JSON list' => [ScriptedTransport::json(403, ['error'])], + 'an error that is text' => [ScriptedTransport::json(403, ['error' => 'denied'])], + 'a code that is a number' => [ScriptedTransport::json(403, ['error' => ['code' => 7]])], + ]; + } + + #[Test] + #[DataProvider('unexpectedErrorBodies')] + public function keepsTheStatusAndNoApiCodeForAnUnexpectedErrorBody( + ResponseInterface $response, + ): void { + $error = $this->failure(new ScriptedTransport([$response])); + + self::assertSame(ErrorCode::Forbidden, $error->errorCode()); + self::assertNull($error->apiCode()); + } + + #[Test] + public function ignoresRetryAfterOnAStatusOtherThan429(): void + { + $transport = new ScriptedTransport([ + ScriptedTransport::error(500, 'internal', ['Retry-After' => '1']), + ]); + + self::assertNull($this->failure($transport)->retryAfterMs()); + } + + #[Test] + public function sendsTheSameRequestOnEachAttempt(): void + { + $transport = new ScriptedTransport([ + ScriptedTransport::error(503, 'busy'), + ScriptedTransport::json(200, self::VALID), + ]); + + $this->send($transport); + + self::assertSame($transport->requests[0], $transport->requests[1]); + } +} diff --git a/tests/Unit/Client/SenderTestCase.php b/tests/Unit/Client/SenderTestCase.php new file mode 100644 index 0000000..0b31938 --- /dev/null +++ b/tests/Unit/Client/SenderTestCase.php @@ -0,0 +1,64 @@ + ['postcode' => 'FC-01-Z99-ZZ-01', 'valid' => true]]; + protected const TIMEOUT_MS = 8000; + + protected FakeTimer $timer; + + protected function setUp(): void + { + $this->timer = new FakeTimer(); + } + + /** + * Sends one call with the default options of the client, unless the test names others. + */ + protected function send( + ScriptedTransport $transport, + int $maxRetries = 2, + bool $retryTimeouts = true, + ?FakeTimer $timer = null, + ): mixed { + $sender = new Sender($transport, self::TIMEOUT_MS, $maxRetries, $timer ?? $this->timer); + $request = new Request('GET', 'https://gateway.invalid/v1/lookup'); + + return $sender->send($request, $retryTimeouts); + } + + /** + * Sends one call that must fail, and returns its error. + */ + protected function failure( + ScriptedTransport $transport, + int $maxRetries = 2, + bool $retryTimeouts = true, + ?FakeTimer $timer = null, + ): PostcodeException { + try { + $this->send($transport, $maxRetries, $retryTimeouts, $timer); + } catch (PostcodeException $error) { + return $error; + } + + self::fail('The call did not fail.'); + } +} diff --git a/tests/Unit/Client/SenderTimeoutTest.php b/tests/Unit/Client/SenderTimeoutTest.php new file mode 100644 index 0000000..8cc9f44 --- /dev/null +++ b/tests/Unit/Client/SenderTimeoutTest.php @@ -0,0 +1,94 @@ +timer); + + $error = $this->failure($transport); + + self::assertSame(ErrorCode::NetworkError, $error->errorCode()); + self::assertNull($error->status()); + self::assertInstanceOf(NetworkExceptionInterface::class, $error->getPrevious()); + self::assertSame([500, 1000], $this->timer->waits); + } + + #[Test] + public function callsAFailureAtTheTimeoutATimeoutAndRetriesIt(): void + { + $transport = new ScriptedTransport([8000.0, 8000.0, 8000.0], $this->timer); + + $error = $this->failure($transport); + + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertNull($error->status()); + self::assertCount(3, $transport->requests); + } + + #[Test] + public function callsAFailureJustBeforeTheTimeoutANetworkError(): void + { + $transport = new ScriptedTransport([7999.0], $this->timer); + + $error = $this->failure($transport, maxRetries: 0); + + self::assertSame(ErrorCode::NetworkError, $error->errorCode()); + } + + #[Test] + public function doesNotRetryATimeoutWhenTheCallAsksForNone(): void + { + $transport = new ScriptedTransport([8000.0], $this->timer); + + $error = $this->failure($transport, retryTimeouts: false); + + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertCount(1, $transport->requests); + self::assertSame([], $this->timer->waits); + } + + #[Test] + public function stopsAtOnceWhenTheNextWaitWouldEndAfterTheDeadline(): void + { + // The deadline is 3 x 8000 ms plus 750 ms and 1250 ms: 26 000 ms after the first attempt. + // The first attempt ends at 25 400 ms, and the wait of 750 ms would end at 26 150 ms. + $timer = new FakeTimer(jitterMs: 250); + $transport = new ScriptedTransport([25_400.0], $timer); + + $error = $this->failure($transport, timer: $timer); + + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertCount(1, $transport->requests); + self::assertSame([], $timer->waits); + } + + #[Test] + public function retriesWhenTheNextWaitEndsExactlyAtTheDeadline(): void + { + $timer = new FakeTimer(jitterMs: 250); + $transport = new ScriptedTransport([ + 25_250.0, + ScriptedTransport::json(200, self::VALID), + ], $timer); + + self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); + self::assertSame([750], $timer->waits); + } +} diff --git a/tests/Unit/Client/SystemTimerTest.php b/tests/Unit/Client/SystemTimerTest.php new file mode 100644 index 0000000..9701610 --- /dev/null +++ b/tests/Unit/Client/SystemTimerTest.php @@ -0,0 +1,53 @@ +nowMs(); + + self::assertGreaterThanOrEqual($before, $nowMs); + self::assertLessThan($before + 2000, $nowMs); + } + + #[Test] + public function returnsAtOnceFromAWaitOfZero(): void + { + $timer = new SystemTimer(); + $before = $timer->nowMs(); + + $timer->wait(0); + + self::assertLessThan($before + 50, $timer->nowMs()); + } + + #[Test] + public function keepsEachRandomPartOfAWaitWithinItsLimit(): void + { + $timer = new SystemTimer(); + $parts = []; + for ($draw = 0; $draw < 200; ++$draw) { + $parts[$timer->jitterMs(2)] = true; + } + \ksort($parts); + + self::assertSame([0, 1, 2], \array_keys($parts)); + } +} diff --git a/tests/Unit/PortabilityTest.php b/tests/Unit/PortabilityTest.php index 7ff39cf..61bccca 100644 --- a/tests/Unit/PortabilityTest.php +++ b/tests/Unit/PortabilityTest.php @@ -18,7 +18,10 @@ * The package runs on shared WordPress hosts. Many of them lack ext-intl or ext-mbstring, and * on PHP 8.1 the case functions follow the locale. A Packagist install has no spec submodule, * so the package must read no files, and the core has no I/O, no clock and no randomness (CS-2). - * A string such as 'strtoupper' is a callable, so a string literal counts as a name too. + * A string such as 'strtoupper' is a callable, so a string literal counts as a name too. The + * client in src/Client/ must wait between retries and time each attempt, so it may use the clock + * and randomness. It still sends each request through the caller's PSR-18 transport, so the + * other lists apply to it. * * The check reads names from lists, because a tool that reads symbols leaves most of them out. * ComposerRequireChecker flags only a symbol of an extension that composer.json does not require. @@ -27,6 +30,9 @@ */ final class PortabilityTest extends TestCase { + private const CLOCK_GROUP = 'the clock and randomness (CS-2)'; + private const CLIENT_FOLDER = 'src/Client/'; + private const NAME_TOKENS = [ \T_STRING, \T_NAME_QUALIFIED, @@ -100,7 +106,7 @@ public static function bannedNames(): array ], ['curl', 'gethostby', 'proc_', 'socket_'], ], - 'the clock and randomness (CS-2)' => [ + self::CLOCK_GROUP => [ [ 'date', 'date_create', 'date_create_immutable', 'date_default_timezone_get', 'date_default_timezone_set', 'datetime', 'datetimeimmutable', 'getdate', @@ -174,9 +180,11 @@ public static function plantedCode(): array #[Test] public function keepsTheSourceFreeOfEveryBannedName(): void { - $lookup = self::lookup(); + $coreLookup = self::lookup(); + $clientLookup = self::lookup(leaveOut: self::CLOCK_GROUP); $found = []; foreach (self::sourceFiles('src') as $file => $code) { + $lookup = \str_starts_with($file, self::CLIENT_FOLDER) ? $clientLookup : $coreLookup; foreach (self::bannedIn($code, $lookup) as [$name, $line]) { $found[] = "{$name} in {$file}:{$line}"; } @@ -197,6 +205,16 @@ public function findsABannedNameInPlantedCode(string $statements, array $expecte self::assertSame($expected, \array_column($found, 0)); } + #[Test] + public function letsTheClientWaitButNotOpenAFile(): void + { + $code = ', string} */ - private static function lookup(): array + private static function lookup(?string $leaveOut = null): array { $names = []; $prefixes = []; - foreach (self::bannedNames() as [$groupNames, $groupPrefixes]) { + foreach (self::bannedNames() as $group => [$groupNames, $groupPrefixes]) { + if ($group === $leaveOut) { + continue; + } $names = [...$names, ...$groupNames]; $prefixes = [...$prefixes, ...$groupPrefixes]; } From 70a953a0c6854422d9a4e50b31e645dcdb2f19f2 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:00:54 +0100 Subject: [PATCH 03/21] feat(client): look up a postcode through the gateway A lookup is the call that every integrator needs first. The client checks the code with the core, so a typo never costs a request, and it reads the level from the fields of the response, because the gateway does not state it. A postcode that the gateway does not know is a result, not an error. A status stays text, so a new gateway word is not lost, and a body that the client cannot read is an unexpected_response. Levels 2 to 5 follow NIPOST's docs, since the test keys hold level 1 only. Signed-off-by: MobileMage --- docs/api-dump.md | 26 ++++ src/Client/AdministrativeAddress.php | 45 +++++++ src/Client/Internal/Fields.php | 98 ++++++++++++++ src/Client/LookupResult.php | 98 ++++++++++++++ src/Client/PostcodeClient.php | 151 +++++++++++++++++++++ tests/Unit/Client/ClientOptionsTest.php | 139 +++++++++++++++++++ tests/Unit/Client/FieldsTest.php | 61 +++++++++ tests/Unit/Client/FixtureGateway.php | 85 ++++++++++++ tests/Unit/Client/LookupInputTest.php | 94 +++++++++++++ tests/Unit/Client/LookupRequestTest.php | 102 ++++++++++++++ tests/Unit/Client/LookupResultTest.php | 169 ++++++++++++++++++++++++ tests/Unit/DocExamplesTest.php | 5 +- 12 files changed, 1072 insertions(+), 1 deletion(-) create mode 100644 src/Client/AdministrativeAddress.php create mode 100644 src/Client/Internal/Fields.php create mode 100644 src/Client/LookupResult.php create mode 100644 src/Client/PostcodeClient.php create mode 100644 tests/Unit/Client/ClientOptionsTest.php create mode 100644 tests/Unit/Client/FieldsTest.php create mode 100644 tests/Unit/Client/FixtureGateway.php create mode 100644 tests/Unit/Client/LookupInputTest.php create mode 100644 tests/Unit/Client/LookupRequestTest.php create mode 100644 tests/Unit/Client/LookupResultTest.php diff --git a/docs/api-dump.md b/docs/api-dump.md index bfc2886..6617dfe 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -5,6 +5,14 @@ Do not edit this file. `composer api:dump` writes it from the public symbols of ```php namespace Gatepost\Postcode\Client; +final class AdministrativeAddress +{ + public readonly string $stateName; + public readonly string $lgaName; + public readonly string $localityName; + public readonly string $zone; +} + enum ErrorCode: string { case InvalidInput = 'invalid_input'; @@ -19,6 +27,24 @@ enum ErrorCode: string case Timeout = 'timeout'; } +final class LookupResult +{ + public readonly \Gatepost\Postcode\Postcode $postcode; + public readonly bool $valid; + public readonly ?string $status; + public readonly int $levelRequested; + public readonly int $levelReceived; + public readonly ?AdministrativeAddress $administrativeAddress; + public readonly ?string $recentHouseAddress; + public readonly ?string $buildingUseStatus; +} + +final class PostcodeClient +{ + public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', int $timeoutMs = 8000, int $maxRetries = 2); + public function lookup(\Gatepost\Postcode\Postcode|string $code, int $level = 1): LookupResult; +} + final class PostcodeException extends \RuntimeException { public function errorCode(): ErrorCode; diff --git a/src/Client/AdministrativeAddress.php b/src/Client/AdministrativeAddress.php new file mode 100644 index 0000000..c3e51fb --- /dev/null +++ b/src/Client/AdministrativeAddress.php @@ -0,0 +1,45 @@ + $fields + * + * @throws PostcodeException When a field is missing or is not text. + */ + public static function fromFields(array $fields): self + { + return new self( + Fields::string($fields, 'state_name'), + Fields::string($fields, 'lga_name'), + Fields::string($fields, 'locality_name'), + Fields::string($fields, 'zone'), + ); + } +} diff --git a/src/Client/Internal/Fields.php b/src/Client/Internal/Fields.php new file mode 100644 index 0000000..120204d --- /dev/null +++ b/src/Client/Internal/Fields.php @@ -0,0 +1,98 @@ + + * + * @throws PostcodeException When the value is not a JSON object. + */ + public static function object(mixed $value): array + { + if (!\is_array($value) || ($value !== [] && \array_is_list($value))) { + throw PostcodeException::unreadable(200); + } + + return $value; + } + + /** + * @return ?array The object, or null when the field is absent or null. + * + * @param array $fields + * + * @throws PostcodeException When the field is not a JSON object. + */ + public static function optionalObject(array $fields, string $name): ?array + { + $value = $fields[$name] ?? null; + + return $value === null ? null : self::object($value); + } + + /** + * @param array $fields + * + * @throws PostcodeException When the field is missing or is not true or false. + */ + public static function bool(array $fields, string $name): bool + { + $value = $fields[$name] ?? null; + if (!\is_bool($value)) { + throw PostcodeException::unreadable(200); + } + + return $value; + } + + /** + * @param array $fields + * + * @throws PostcodeException When the field is missing or is not text. + */ + public static function string(array $fields, string $name): string + { + return self::optionalString($fields, $name) ?? throw PostcodeException::unreadable(200); + } + + /** + * @param array $fields + * + * @return ?string The text, or null when the field is absent or null. + * + * @throws PostcodeException When the field is not text. + */ + public static function optionalString(array $fields, string $name): ?string + { + $value = $fields[$name] ?? null; + if ($value !== null && !\is_string($value)) { + throw PostcodeException::unreadable(200); + } + + return $value; + } + + /** + * @param array $fields + */ + public static function has(array $fields, string $name): bool + { + return ($fields[$name] ?? null) !== null; + } +} diff --git a/src/Client/LookupResult.php b/src/Client/LookupResult.php new file mode 100644 index 0000000..b11dbe9 --- /dev/null +++ b/src/Client/LookupResult.php @@ -0,0 +1,98 @@ + ['point_geometry'], + 4 => ['other_building_info'], + 3 => ['building_use_status'], + 2 => ['administrative_address', 'recent_house_address'], + ]; + + /** + * Each parameter is a field of the result, and only fromResponse() calls this constructor. + * + * @SuppressWarnings("PHPMD.ExcessiveParameterList") + */ + private function __construct( + /** The postcode that the caller asked for. The client never takes it from the response. */ + public readonly Postcode $postcode, + /** True when the gateway knows the postcode. */ + public readonly bool $valid, + /** + * The gateway's word for the postcode, such as valid, invalid, not_found or restricted. + * It stays text, so a new word is not lost. It is null when the gateway sent none. + */ + public readonly ?string $status, + /** The lookup level that the caller asked for, from 1 to 5. */ + public readonly int $levelRequested, + /** The lookup level of the data in the response, from 1 to 5. */ + public readonly int $levelReceived, + /** The names of the places that hold the postcode, from level 2, or null. */ + public readonly ?AdministrativeAddress $administrativeAddress, + /** The most recent house address of the unit, from level 2, or null. */ + public readonly ?string $recentHouseAddress, + /** What the building is used for, such as residential, from level 3, or null. */ + public readonly ?string $buildingUseStatus, + ) {} + + /** + * @internal PostcodeClient reads each lookup response with this method. + * + * @throws PostcodeException When a known field is missing or has the wrong type. + */ + public static function fromResponse( + mixed $response, + Postcode $postcode, + int $levelRequested, + ): self { + $fields = Fields::object($response); + $address = Fields::optionalObject($fields, 'administrative_address'); + $recent = Fields::optionalObject($fields, 'recent_house_address'); + + return new self( + $postcode, + Fields::bool($fields, 'valid'), + Fields::optionalString($fields, 'status'), + $levelRequested, + self::levelReceived($fields), + $address === null ? null : AdministrativeAddress::fromFields($address), + $recent === null ? null : Fields::optionalString($recent, 'recent'), + Fields::optionalString($fields, 'building_use_status'), + ); + } + + /** + * The response does not state its level, so the client reads it from the fields. + * + * @param array $fields + */ + private static function levelReceived(array $fields): int + { + foreach (self::LEVEL_FIELDS as $level => $names) { + foreach ($names as $name) { + if (Fields::has($fields, $name)) { + return $level; + } + } + } + + return 1; + } +} diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php new file mode 100644 index 0000000..e0b73d3 --- /dev/null +++ b/src/Client/PostcodeClient.php @@ -0,0 +1,151 @@ + 8]), + * new \GuzzleHttp\Psr7\HttpFactory(), + * apiKey: \getenv('NIPOST_API_KEY') ?: null, + * ); + * ``` + */ +final class PostcodeClient +{ + private const DEFAULT_BASE_URL = 'https://api.postcode.gov.ng'; + + private readonly string $baseUrl; + + private readonly Sender $sender; + + /** + * @param ClientInterface $transport A PSR-18 HTTP client. Set its timeout to + * timeoutMs, because PSR-18 has no timeout. + * @param RequestFactoryInterface $requestFactory A PSR-17 factory for the requests. + * @param ?string $apiKey The key for the X-API-Key header. With null, + * the client sends no key, and the gateway + * answers 401. + * @param string $baseUrl The gateway's address. + * @param int $timeoutMs The longest wait for one attempt. A failed + * attempt that took this long is a timeout. + * @param int $maxRetries The most retries after the first attempt. + * + * @throws InvalidArgumentException When an option is out of range, or the key is empty. + * + * Callers name each option, and TELL-3 counts no named or defaulted parameter, so PHPMD's + * count of parameters does not apply here. + * + * @SuppressWarnings("PHPMD.ExcessiveParameterList") + */ + public function __construct( + ClientInterface $transport, + private readonly RequestFactoryInterface $requestFactory, + private readonly ?string $apiKey = null, + string $baseUrl = self::DEFAULT_BASE_URL, + int $timeoutMs = 8000, + int $maxRetries = 2, + ) { + if ($apiKey === '') { + throw new InvalidArgumentException('apiKey is empty. Pass null to send no key.'); + } + if ($timeoutMs <= 0) { + throw new InvalidArgumentException("timeoutMs is {$timeoutMs}. Use 1 or more."); + } + if ($maxRetries < 0) { + throw new InvalidArgumentException("maxRetries is {$maxRetries}. Use 0 or more."); + } + $this->baseUrl = \rtrim($baseUrl, '/'); + $this->sender = new Sender($transport, $timeoutMs, $maxRetries, new SystemTimer()); + } + + /** + * Asks the gateway what it knows about a postcode. The client checks the code with + * Postcode::parse() first, and sends no request for a code that fails. Levels above 1 use + * credits, and a key holds a level grant. A level above the grant fails with Forbidden. + * The shapes of levels 2 to 5 follow NIPOST's docs. Gatepost has seen only level 1. + * + * ```php + * $result = $client->lookup('fc 01 z99 zz 01'); + * $result->valid; // true + * echo $result->postcode->canonical; // FC-01-Z99-ZZ-01 + * ``` + * + * @param string|Postcode $code A full postcode, as text or as a parsed postcode. + * @param int $level The lookup level, from 1 to 5. + * + * @throws PostcodeException For a code that fails the check, for a level outside 1 to 5, + * and for each failure of the gateway. The client sends no + * request for bad input. + */ + public function lookup(string|Postcode $code, int $level = 1): LookupResult + { + if ($level < 1 || $level > 5) { + throw PostcodeException::invalidInput("The lookup level is {$level}. Use 1 to 5."); + } + $postcode = self::fullPostcode($code); + $query = ['code' => $postcode->canonical, 'level' => $level]; + $response = $this->send('/v1/lookup', $query, retryTimeouts: true); + + return LookupResult::fromResponse($response, $postcode, $level); + } + + /** + * @throws PostcodeException + */ + private static function fullPostcode(string|Postcode $code): Postcode + { + if (\is_string($code)) { + $parsed = Postcode::parse($code); + if (!$parsed->isOk()) { + throw PostcodeException::invalidInput( + "The postcode failed the format check with {$parsed->error->code->value}. " + . 'Call Postcode::parse() to show the user the reason.', + ); + } + $code = $parsed->value; + } + if ($code->precision !== Precision::Unit) { + throw PostcodeException::invalidInput( + 'The postcode stops before the unit. A lookup needs all five segments.', + ); + } + + return $code; + } + + /** + * @param array $query + * + * @throws PostcodeException + */ + private function send(string $path, array $query, bool $retryTimeouts): mixed + { + $url = $this->baseUrl . $path . '?' . \http_build_query($query); + $request = $this->requestFactory->createRequest('GET', $url); + if ($this->apiKey !== null) { + $request = $request->withHeader('X-API-Key', $this->apiKey); + } + + return $this->sender->send($request, $retryTimeouts); + } +} diff --git a/tests/Unit/Client/ClientOptionsTest.php b/tests/Unit/Client/ClientOptionsTest.php new file mode 100644 index 0000000..223745c --- /dev/null +++ b/tests/Unit/Client/ClientOptionsTest.php @@ -0,0 +1,139 @@ + + */ + public static function badOptions(): array + { + $gateway = new FixtureGateway(); + $factory = new HttpFactory(); + + return [ + 'a timeout of 0' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + timeoutMs: 0, + ), + 'timeoutMs is 0. Use 1 or more.', + ], + 'a negative timeout' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + timeoutMs: -1, + ), + 'timeoutMs is -1. Use 1 or more.', + ], + 'negative retries' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + maxRetries: -1, + ), + 'maxRetries is -1. Use 0 or more.', + ], + 'an empty key' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + apiKey: '', + ), + 'apiKey is empty. Pass null to send no key.', + ], + ]; + } + + /** + * @param Closure(): PostcodeClient $create + */ + #[Test] + #[DataProvider('badOptions')] + public function rejectsAnOptionOutOfRangeAsAProgrammerError( + Closure $create, + string $message, + ): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + + $create(); + } + + #[Test] + public function acceptsNoRetriesAndTheShortestTimeout(): void + { + $client = new PostcodeClient( + new FixtureGateway(), + new HttpFactory(), + timeoutMs: 1, + maxRetries: 0, + ); + + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + } + + /** + * @return array + */ + public static function failures(): array + { + $answers = [ + 'status 401' => ScriptedTransport::error(401, 'invalid_api_key'), + 'status 403' => ScriptedTransport::error(403, 'level_not_granted'), + 'status 429' => ScriptedTransport::error(429, 'rate_limited'), + 'a failed connection' => 1.0, + ]; + + return \array_map( + static fn(ResponseInterface|float $answer): array => [new ScriptedTransport([$answer])], + $answers, + ); + } + + #[Test] + #[DataProvider('failures')] + public function keepsTheKeyOutOfEachErrorMessage(ScriptedTransport $transport): void + { + $client = new PostcodeClient( + $transport, + new HttpFactory(), + apiKey: self::KEY, + maxRetries: 0, + ); + + try { + $client->lookup(FixtureGateway::UNIT); + self::fail('The lookup did not fail.'); + } catch (PostcodeException $error) { + self::assertNotSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertStringNotContainsString(self::KEY, $error->getMessage()); + self::assertStringNotContainsString('nipost_', $error->getMessage()); + } + } +} diff --git a/tests/Unit/Client/FieldsTest.php b/tests/Unit/Client/FieldsTest.php new file mode 100644 index 0000000..36f8108 --- /dev/null +++ b/tests/Unit/Client/FieldsTest.php @@ -0,0 +1,61 @@ + + */ + public static function objects(): array + { + return [ + 'an object with fields' => [['valid' => true]], + 'an empty object, which json_decode() reads as an empty array' => [[]], + ]; + } + + #[Test] + #[DataProvider('objects')] + public function readsAJsonObject(mixed $value): void + { + self::assertSame($value, Fields::object($value)); + } + + /** + * @return array + */ + public static function notObjects(): array + { + return [ + 'a list' => [[1, 2]], + 'a list of one object' => [[['valid' => true]]], + 'text' => ['valid'], + 'null' => [null], + ]; + } + + #[Test] + #[DataProvider('notObjects')] + public function refusesAValueThatIsNotAJsonObject(mixed $value): void + { + $this->expectExceptionObject(PostcodeException::unreadable(200)); + + Fields::object($value); + } +} diff --git a/tests/Unit/Client/FixtureGateway.php b/tests/Unit/Client/FixtureGateway.php new file mode 100644 index 0000000..e5c8e81 --- /dev/null +++ b/tests/Unit/Client/FixtureGateway.php @@ -0,0 +1,85 @@ + */ + public array $requests = []; + + /** + * A client that sends each request to a new FixtureGateway. + */ + public static function client(): PostcodeClient + { + return new PostcodeClient( + new self(), + new HttpFactory(), + apiKey: 'nipost_test_mock_l3', + baseUrl: 'https://gateway.invalid', + ); + } + + /** + * Reads one fixture, such as lookup/valid-level-1, as a response. + */ + public static function fixture(string $name): ResponseInterface + { + $text = \file_get_contents(\dirname(__DIR__, 3) . "/spec/fixtures/{$name}.json"); + $fixture = \json_decode((string) $text, true, 512, \JSON_THROW_ON_ERROR); + if (!\is_array($fixture) || !\is_int($fixture['status'] ?? null)) { + throw new UnexpectedValueException("The fixture {$name} has no status."); + } + $body = \json_encode($fixture['body'], \JSON_THROW_ON_ERROR); + + return new Response($fixture['status'], ['Content-Type' => 'application/json'], $body); + } + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $this->requests[] = $request; + \parse_str($request->getUri()->getQuery(), $query); + + return match ($request->getUri()->getPath()) { + '/v1/lookup' => self::lookup($query), + default => self::fixture('errors/unknown-path'), + }; + } + + /** + * @param array $query + */ + private static function lookup(array $query): ResponseInterface + { + if (($query['code'] ?? null) !== self::UNIT) { + return self::fixture('lookup/not-found'); + } + // The client of this gateway holds a level 3 key, as the mock key nipost_test_mock_l3 does. + $level = \is_string($query['level'] ?? null) ? $query['level'] : '1'; + if ((int) $level > 3) { + return self::fixture('errors/level-not-granted-3'); + } + + return self::fixture("lookup/valid-level-{$level}"); + } +} diff --git a/tests/Unit/Client/LookupInputTest.php b/tests/Unit/Client/LookupInputTest.php new file mode 100644 index 0000000..595b1d4 --- /dev/null +++ b/tests/Unit/Client/LookupInputTest.php @@ -0,0 +1,94 @@ + + */ + public static function badCodes(): array + { + $district = Postcode::parse('FC-01-Z99', allowPartial: true)->value; + self::assertNotNull($district); + + return [ + 'a unit of one digit' => ['FC-01-Z99-ZZ-1'], + 'empty text' => [''], + 'a legacy postcode' => ['900108'], + 'a partial postcode as text' => ['FC01Z99'], + 'a partial postcode that parse accepted' => [$district], + ]; + } + + #[Test] + #[DataProvider('badCodes')] + public function sendsNoRequestForACodeThatFailsTheCheck(string|Postcode $code): void + { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + try { + $client->lookup($code); + self::fail('The lookup did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertNull($error->status()); + } + self::assertSame([], $gateway->requests); + } + + #[Test] + public function namesTheReasonOfTheFormatCheckInTheMessage(): void + { + $this->expectExceptionMessage( + 'The postcode failed the format check with bad_length. Call Postcode::parse() to show ' + . 'the user the reason.', + ); + + FixtureGateway::client()->lookup('FC-01-Z99-ZZ-1'); + } + + /** + * @return array + */ + public static function badLevels(): array + { + return ['level 0' => [0], 'level 6' => [6], 'a negative level' => [-1]]; + } + + #[Test] + #[DataProvider('badLevels')] + public function sendsNoRequestForALevelOutsideOneToFive(int $level): void + { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + try { + $client->lookup(FixtureGateway::UNIT, $level); + self::fail('The lookup did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertNull($error->status()); + } + self::assertSame([], $gateway->requests); + } +} diff --git a/tests/Unit/Client/LookupRequestTest.php b/tests/Unit/Client/LookupRequestTest.php new file mode 100644 index 0000000..d3b7174 --- /dev/null +++ b/tests/Unit/Client/LookupRequestTest.php @@ -0,0 +1,102 @@ +lookup('fc01 z99 zz02', level: 2); + + $request = $gateway->requests[0]; + self::assertSame('GET', $request->getMethod()); + self::assertSame( + 'https://api.postcode.gov.ng/v1/lookup?code=FC-01-Z99-ZZ-02&level=2', + (string) $request->getUri(), + ); + self::assertSame([self::KEY], $request->getHeader('X-API-Key')); + } + + #[Test] + public function sendsNoKeyHeaderWhenTheClientHasNoKey(): void + { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + $client->lookup(FixtureGateway::UNIT); + + self::assertFalse($gateway->requests[0]->hasHeader('X-API-Key')); + self::assertSame( + 'code=FC-01-Z99-ZZ-01&level=1', + $gateway->requests[0]->getUri()->getQuery(), + 'A lookup asks for level 1 unless the caller names a level.', + ); + } + + #[Test] + public function joinsTheBaseUrlAndThePathWithOneSlash(): void + { + $gateway = new FixtureGateway(); + $baseUrl = 'http://127.0.0.1:4010/'; + $client = new PostcodeClient($gateway, new HttpFactory(), baseUrl: $baseUrl); + + $client->lookup(FixtureGateway::UNIT); + + self::assertSame('/v1/lookup', $gateway->requests[0]->getUri()->getPath()); + self::assertSame('127.0.0.1', $gateway->requests[0]->getUri()->getHost()); + } + + #[Test] + public function acceptsAParsedPostcode(): void + { + $parsed = Postcode::parse(FixtureGateway::UNIT); + self::assertTrue($parsed->isOk()); + + $result = FixtureGateway::client()->lookup($parsed->value); + + self::assertSame($parsed->value, $result->postcode); + } + + #[Test] + public function sendsLevel5ToTheGatewayAndReportsItsRefusal(): void + { + try { + FixtureGateway::client()->lookup(FixtureGateway::UNIT, level: 5); + self::fail('The lookup did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::Forbidden, $error->errorCode()); + self::assertSame('level_not_granted', $error->apiCode()); + } + } + + #[Test] + public function returnsAResultWithValidFalseForAnUnknownPostcode(): void + { + $result = FixtureGateway::client()->lookup('FC-01-Z99-ZZ-02'); + + self::assertFalse($result->valid); + self::assertSame('FC-01-Z99-ZZ-02', $result->postcode->canonical); + } +} diff --git a/tests/Unit/Client/LookupResultTest.php b/tests/Unit/Client/LookupResultTest.php new file mode 100644 index 0000000..ec45b99 --- /dev/null +++ b/tests/Unit/Client/LookupResultTest.php @@ -0,0 +1,169 @@ + 'FC01Z99ZZ01', 'valid' => true, 'status' => 'valid']; + + $result = LookupResult::fromResponse($response, self::unit(), 1); + + self::assertSame('FC-01-Z99-ZZ-01', $result->postcode->canonical); + self::assertTrue($result->valid); + self::assertSame('valid', $result->status); + self::assertSame(1, $result->levelReceived); + self::assertNull($result->administrativeAddress); + self::assertNull($result->recentHouseAddress); + self::assertNull($result->buildingUseStatus); + } + + #[Test] + public function readsTheAddressOfTheDocumentedLevel3Shape(): void + { + $result = FixtureGateway::client()->lookup(FixtureGateway::UNIT, level: 3); + + self::assertSame(3, $result->levelRequested); + self::assertSame(3, $result->levelReceived); + self::assertNotNull($result->administrativeAddress); + self::assertSame('SYNTHETIC LGA', $result->administrativeAddress->lgaName); + self::assertSame('NORTH CENTRAL', $result->administrativeAddress->zone); + self::assertSame('1 SYNTHETIC STREET, SYNTHETIC LOCALITY', $result->recentHouseAddress); + self::assertSame('residential', $result->buildingUseStatus); + self::assertNull($result->status); + } + + #[Test] + public function ignoresEachFieldThatTheSpecDoesNotUse(): void + { + $body = ['valid' => true, 'status' => 'valid', 'verified' => false, 'new_field' => [1]]; + + $result = LookupResult::fromResponse($body, self::unit(), 1); + + self::assertTrue($result->valid); + self::assertSame(1, $result->levelReceived); + } + + /** + * @return array, int}> + */ + public static function levelFields(): array + { + $address = ['state_name' => 'S', 'lga_name' => 'L', 'locality_name' => 'C', 'zone' => 'Z']; + + return [ + 'no field of a higher level' => [[], 1], + 'an administrative address' => [['administrative_address' => $address], 2], + 'a recent house address' => [['recent_house_address' => ['recent' => 'SYNTHETIC']], 2], + 'a building use status' => [['building_use_status' => 'residential'], 3], + 'other building information' => [['other_building_info' => ['floors' => 2]], 4], + 'a point geometry' => [['point_geometry' => ['type' => 'Point']], 5], + 'a field of level 2 that is null' => [['administrative_address' => null], 1], + ]; + } + + /** + * @param array $fields + */ + #[Test] + #[DataProvider('levelFields')] + public function readsTheLevelReceivedFromTheMostPreciseField(array $fields, int $level): void + { + $result = LookupResult::fromResponse(['valid' => true, ...$fields], self::unit(), 5); + + self::assertSame($level, $result->levelReceived); + self::assertSame(5, $result->levelRequested); + } + + /** + * @return array + */ + public static function statuses(): array + { + return [ + 'valid' => ['valid', 'valid'], + 'not_found' => ['not_found', 'not_found'], + 'invalid' => ['invalid', 'invalid'], + 'restricted' => ['restricted', 'restricted'], + 'a word that the spec does not list' => ['pending', 'pending'], + 'an empty word' => ['', ''], + 'null' => [null, null], + ]; + } + + #[Test] + #[DataProvider('statuses')] + public function keepsAnyStatusTextAsItIs(mixed $status, ?string $expected): void + { + $body = ['valid' => false, 'status' => $status, 'verified' => false, 'new_field' => [1]]; + + $result = LookupResult::fromResponse($body, self::unit(), 1); + + self::assertFalse($result->valid); + self::assertSame($expected, $result->status); + } + + /** + * @return array + */ + public static function unreadableBodies(): array + { + $noZone = ['state_name' => 'S', 'lga_name' => 'L', 'locality_name' => 'C']; + + return [ + 'a list in place of an object' => [[['valid' => true]]], + 'no valid field' => [['status' => 'valid']], + 'valid as text' => [['valid' => 'true']], + 'an address as text' => [['valid' => true, 'administrative_address' => 'SYNTHETIC']], + 'an address with no zone' => [['valid' => true, 'administrative_address' => $noZone]], + 'a status that is a number' => [['valid' => true, 'status' => 5]], + 'a status that is a list' => [['valid' => true, 'status' => ['valid']]], + 'valid as null' => [['valid' => null]], + 'an address that is a list' => [['valid' => true, 'administrative_address' => [1]]], + 'a recent address that is a number' => [ + ['valid' => true, 'recent_house_address' => ['recent' => 7]], + ], + ]; + } + + #[Test] + #[DataProvider('unreadableBodies')] + public function failsWithUnexpectedResponseWhenAKnownFieldHasTheWrongType(mixed $response): void + { + try { + LookupResult::fromResponse($response, self::unit(), 1); + self::fail('The body was read.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::UnexpectedResponse, $error->errorCode()); + self::assertSame(200, $error->status()); + } + } + + private static function unit(): Postcode + { + $parsed = Postcode::parse(FixtureGateway::UNIT); + self::assertTrue($parsed->isOk()); + + return $parsed->value; + } +} diff --git a/tests/Unit/DocExamplesTest.php b/tests/Unit/DocExamplesTest.php index bcc128b..0949053 100644 --- a/tests/Unit/DocExamplesTest.php +++ b/tests/Unit/DocExamplesTest.php @@ -24,7 +24,8 @@ * A line that ends with a comment shows a result. `echo $postcode->redact(); // EK-01-A03-FK-**` * shows the text that the line prints, and `$district->contains($building); // true` shows a value. * The doc comment of a method may use the object of that method with no line that makes it, so each - * example runs after the lines of SETUP. + * example runs after the lines of SETUP. The client in SETUP answers from the synthetic fixtures of + * the spec, with no network. */ final class DocExamplesTest extends TestCase { @@ -33,6 +34,7 @@ final class DocExamplesTest extends TestCase '$postcode = Postcode::parse(\'EK-01-A03-FK-01\')->value;', '$building = $postcode;', '$district = Postcode::parse(\'EK-01-A03\', allowPartial: true)->value;', + '$client = \\Gatepost\\Postcode\\Tests\\Unit\\Client\\FixtureGateway::client();', ]; private const COMMENT_MARK = '/^\s*(?:\/\*\*|\*\/|\*)\s?/'; @@ -161,6 +163,7 @@ private static function module(string $example): string }, \explode("\n", $example)); return " Date: Sat, 3 Oct 2026 03:07:21 +0100 Subject: [PATCH 04/21] fix(client): treat a past Retry-After date as invalid on every status The spec calls a Retry-After date in the past invalid, so a 429 with one now fails at once with rate_limited, and a 502, 503 or 504 uses the client's own wait. A date that does not exist is invalid too, and a whole number of up to 12 digits is valid. A test now pins the 250 ms limit of the random part of a wait. Signed-off-by: MobileMage --- src/Client/Internal/Sender.php | 11 ++-- src/Client/PostcodeException.php | 7 +- tests/Unit/Client/SenderRateLimitTest.php | 11 +++- tests/Unit/Client/SenderRetryAfterTest.php | 76 ++++++++++++++++++++++ tests/Unit/Client/SenderRetryTest.php | 74 +++++---------------- 5 files changed, 112 insertions(+), 67 deletions(-) create mode 100644 tests/Unit/Client/SenderRetryAfterTest.php diff --git a/src/Client/Internal/Sender.php b/src/Client/Internal/Sender.php index ea41e95..faf676a 100644 --- a/src/Client/Internal/Sender.php +++ b/src/Client/Internal/Sender.php @@ -168,8 +168,8 @@ private static function apiCode(mixed $body): ?string /** * The wait that the Retry-After header of a 429, 502, 503 or 504 asks for, as seconds or as - * an HTTP date. A header in another form counts as no header. A date in the past is valid - * for a 429 only. + * an HTTP date. A header in another form counts as no header, and so does a date in the past + * or a date that does not exist. */ private function retryAfterMs(ResponseInterface $response): ?int { @@ -179,16 +179,17 @@ private function retryAfterMs(ResponseInterface $response): ?int if ($value === '' || !$mayWait) { return null; } - if (\preg_match('/\A\d{1,9}\z/', $value) === 1) { + if (\preg_match('/\A\d{1,12}\z/', $value) === 1) { return (int) $value * 1000; } $utc = new DateTimeZone('UTC'); $date = DateTimeImmutable::createFromFormat(self::HTTP_DATE, $value, $utc); - if ($date === false) { + $problems = DateTimeImmutable::getLastErrors(); + if ($date === false || ($problems !== false && $problems['warning_count'] > 0)) { return null; } $waitMs = (int) \ceil($date->getTimestamp() * 1000 - $this->timer->nowMs()); - return $waitMs < 0 && $status !== 429 ? null : \max(0, $waitMs); + return $waitMs < 0 ? null : $waitMs; } } diff --git a/src/Client/PostcodeException.php b/src/Client/PostcodeException.php index e7a5657..da3bc86 100644 --- a/src/Client/PostcodeException.php +++ b/src/Client/PostcodeException.php @@ -115,7 +115,8 @@ public function apiCode(): ?string } /** - * The wait in milliseconds that the Retry-After header of a 429 asked for, or null. Tell + * The wait in milliseconds that a valid Retry-After header of a 429, 502, + * 503 or 504 asked for, or null. Tell * the user when to try again. */ public function retryAfterMs(): ?int @@ -141,7 +142,9 @@ private static function messageFor( int $status, ?int $retryAfterMs, ): string { - $wait = $retryAfterMs === null ? 'Wait' : 'Wait ' . \ceil($retryAfterMs / 1000) . ' s'; + $wait = $retryAfterMs === null || $retryAfterMs === 0 + ? 'Wait' + : 'Wait ' . \ceil($retryAfterMs / 1000) . ' s'; return match ($errorCode) { ErrorCode::Unauthorized => "The gateway refused the API key (HTTP {$status}). " diff --git a/tests/Unit/Client/SenderRateLimitTest.php b/tests/Unit/Client/SenderRateLimitTest.php index 2ff69f1..ce0fc2c 100644 --- a/tests/Unit/Client/SenderRateLimitTest.php +++ b/tests/Unit/Client/SenderRateLimitTest.php @@ -26,7 +26,7 @@ public static function shortRetryAfters(): array 'one second' => ['1', 1000], 'ten seconds, the longest that the client waits' => ['10', 10_000], 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], - 'an HTTP date in the past' => ['Mon, 12 Oct 2026 08:59:00 GMT', 0], + 'an HTTP date in the current second' => ['Mon, 12 Oct 2026 09:00:00 GMT', 0], ]; } @@ -54,6 +54,15 @@ public static function finalRateLimits(): array 'a Retry-After of eleven seconds' => [['Retry-After' => '11'], 11_000], 'a Retry-After of two minutes' => [['Retry-After' => '120'], 120_000], 'a Retry-After that is not a number or a date' => [['Retry-After' => 'soon'], null], + 'an HTTP date in the past' => [ + ['Retry-After' => 'Mon, 12 Oct 2026 08:59:00 GMT'], + null, + ], + 'a date that does not exist' => [ + ['Retry-After' => 'Sat, 31 Feb 2027 09:00:03 GMT'], + null, + ], + 'a number of ten digits' => [['Retry-After' => '1000000000'], 1_000_000_000_000], ]; } diff --git a/tests/Unit/Client/SenderRetryAfterTest.php b/tests/Unit/Client/SenderRetryAfterTest.php new file mode 100644 index 0000000..3737f66 --- /dev/null +++ b/tests/Unit/Client/SenderRetryAfterTest.php @@ -0,0 +1,76 @@ + + */ + public static function validRetryAfters(): array + { + return [ + 'zero seconds' => ['0', 0], + 'four seconds' => ['4', 4000], + 'ten seconds, the longest that the client waits' => ['10', 10_000], + 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], + ]; + } + + #[Test] + #[DataProvider('validRetryAfters')] + public function waitsExactlyAsLongAsAValidRetryAfterAsksOnA503( + string $header, + int $waitMs, + ): void { + $timer = new FakeTimer(jitterMs: 200); + $transport = new ScriptedTransport([ + ScriptedTransport::error(503, 'unavailable', ['Retry-After' => $header]), + ScriptedTransport::json(200, self::VALID), + ]); + + self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); + self::assertSame([$waitMs], $timer->waits); + } + + /** + * @return array + */ + public static function otherRetryAfters(): array + { + return [ + 'eleven seconds' => ['11', 11_000], + 'a number that is not whole' => ['1.5', null], + 'a negative number' => ['-1', null], + 'text' => ['soon', null], + 'an HTTP date in the past' => ['Mon, 12 Oct 2026 08:59:00 GMT', null], + ]; + } + + #[Test] + #[DataProvider('otherRetryAfters')] + public function usesItsOwnWaitForALongOrInvalidRetryAfterOnA504( + string $header, + ?int $retryAfterMs, + ): void { + $timer = new FakeTimer(jitterMs: 100); + $busy = ScriptedTransport::error(504, 'busy', ['Retry-After' => $header]); + $transport = new ScriptedTransport([$busy, $busy, $busy]); + + $error = $this->failure($transport, timer: $timer); + + self::assertSame($retryAfterMs, $error->retryAfterMs()); + self::assertSame([600, 1100], $timer->waits); + } +} diff --git a/tests/Unit/Client/SenderRetryTest.php b/tests/Unit/Client/SenderRetryTest.php index c7fdade..b2d8f46 100644 --- a/tests/Unit/Client/SenderRetryTest.php +++ b/tests/Unit/Client/SenderRetryTest.php @@ -29,6 +29,21 @@ public function retriesA503AfterTheFirstWaitAndReturnsTheNextAnswer(): void self::assertSame([680], $timer->waits); } + #[Test] + public function keepsTheRandomPartOfEachWaitWithin250Ms(): void + { + $timer = new FakeTimer(jitterMs: 1000); + $transport = new ScriptedTransport([ + ScriptedTransport::error(503, 'busy'), + ScriptedTransport::error(503, 'busy'), + ScriptedTransport::json(200, self::VALID), + ]); + + $this->send($transport, timer: $timer); + + self::assertSame([750, 1250], $timer->waits); + } + #[Test] public function doublesTheWaitBeforeEachLaterRetry(): void { @@ -95,63 +110,4 @@ public function neverRetriesAStatusThatTheSpecCallsFinal(int $status, string $ap self::assertSame($apiCode, $error->apiCode()); self::assertCount(1, $transport->requests); } - - /** - * @return array - */ - public static function validRetryAfters(): array - { - return [ - 'zero seconds' => ['0', 0], - 'four seconds' => ['4', 4000], - 'ten seconds, the longest that the client waits' => ['10', 10_000], - 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], - ]; - } - - #[Test] - #[DataProvider('validRetryAfters')] - public function waitsExactlyAsLongAsAValidRetryAfterAsksOnA503( - string $header, - int $waitMs, - ): void { - $timer = new FakeTimer(jitterMs: 200); - $transport = new ScriptedTransport([ - ScriptedTransport::error(503, 'unavailable', ['Retry-After' => $header]), - ScriptedTransport::json(200, self::VALID), - ]); - - self::assertSame(self::VALID['data'], $this->send($transport, timer: $timer)); - self::assertSame([$waitMs], $timer->waits); - } - - /** - * @return array - */ - public static function otherRetryAfters(): array - { - return [ - 'eleven seconds' => ['11', 11_000], - 'a number that is not whole' => ['1.5', null], - 'a negative number' => ['-1', null], - 'text' => ['soon', null], - 'an HTTP date in the past' => ['Mon, 12 Oct 2026 08:59:00 GMT', null], - ]; - } - - #[Test] - #[DataProvider('otherRetryAfters')] - public function usesItsOwnWaitForALongOrInvalidRetryAfterOnA504( - string $header, - ?int $retryAfterMs, - ): void { - $timer = new FakeTimer(jitterMs: 100); - $busy = ScriptedTransport::error(504, 'busy', ['Retry-After' => $header]); - $transport = new ScriptedTransport([$busy, $busy, $busy]); - - $error = $this->failure($transport, timer: $timer); - - self::assertSame($retryAfterMs, $error->retryAfterMs()); - self::assertSame([600, 1100], $timer->waits); - } } From 73b643486cd109a39863696a991511896433bbbb Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:25:20 +0100 Subject: [PATCH 05/21] fix(client): give null for lookup parts outside the closed list A level 2 or 3 body can lack one address name, or hold a recent address or a building use of another type. The gateway has charged for such a call, so failing it would lose a paid answer. Only valid and status can now fail a lookup, as the contract lists. The tests also check that the client keeps the caller's postcode when the body echoes another one. Signed-off-by: MobileMage --- docs/api-dump.md | 8 +- src/Client/AdministrativeAddress.php | 26 ++-- src/Client/Internal/Fields.php | 21 ++-- src/Client/LookupResult.php | 11 +- src/Client/PostcodeClient.php | 3 +- tests/Unit/Client/FieldsTest.php | 24 ++++ tests/Unit/Client/LookupResultLenientTest.php | 114 ++++++++++++++++++ tests/Unit/Client/LookupResultTest.php | 10 +- 8 files changed, 177 insertions(+), 40 deletions(-) create mode 100644 tests/Unit/Client/LookupResultLenientTest.php diff --git a/docs/api-dump.md b/docs/api-dump.md index 6617dfe..88c48bc 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -7,10 +7,10 @@ namespace Gatepost\Postcode\Client; final class AdministrativeAddress { - public readonly string $stateName; - public readonly string $lgaName; - public readonly string $localityName; - public readonly string $zone; + public readonly ?string $stateName; + public readonly ?string $lgaName; + public readonly ?string $localityName; + public readonly ?string $zone; } enum ErrorCode: string diff --git a/src/Client/AdministrativeAddress.php b/src/Client/AdministrativeAddress.php index c3e51fb..32a510b 100644 --- a/src/Client/AdministrativeAddress.php +++ b/src/Client/AdministrativeAddress.php @@ -16,14 +16,14 @@ final class AdministrativeAddress { private function __construct( - /** The name of the state. */ - public readonly string $stateName, - /** The name of the local government area. */ - public readonly string $lgaName, - /** The name of the locality. */ - public readonly string $localityName, - /** The geopolitical zone, such as NORTH CENTRAL. */ - public readonly string $zone, + /** The name of the state, or null when the gateway sent none. */ + public readonly ?string $stateName, + /** The name of the local government area, or null. */ + public readonly ?string $lgaName, + /** The name of the locality, or null. */ + public readonly ?string $localityName, + /** The geopolitical zone, such as NORTH CENTRAL, or null. */ + public readonly ?string $zone, ) {} /** @@ -31,15 +31,15 @@ private function __construct( * * @param array $fields * - * @throws PostcodeException When a field is missing or is not text. + * A name that is missing or is not text is null. */ public static function fromFields(array $fields): self { return new self( - Fields::string($fields, 'state_name'), - Fields::string($fields, 'lga_name'), - Fields::string($fields, 'locality_name'), - Fields::string($fields, 'zone'), + Fields::textOrNull($fields, 'state_name'), + Fields::textOrNull($fields, 'lga_name'), + Fields::textOrNull($fields, 'locality_name'), + Fields::textOrNull($fields, 'zone'), ); } } diff --git a/src/Client/Internal/Fields.php b/src/Client/Internal/Fields.php index 120204d..11393c1 100644 --- a/src/Client/Internal/Fields.php +++ b/src/Client/Internal/Fields.php @@ -19,6 +19,9 @@ final class Fields { /** + * A JSON object whose keys are only 0, 1, 2 and so on reads as a list, so it fails here too. + * No shape of the gateway has such keys. + * * @return array * * @throws PostcodeException When the value is not a JSON object. @@ -33,17 +36,17 @@ public static function object(mixed $value): array } /** - * @return ?array The object, or null when the field is absent or null. - * * @param array $fields * - * @throws PostcodeException When the field is not a JSON object. + * @return ?array The object, or null when the field is absent, null or not + * an object. The parts outside the closed list of + * spec/client.md never fail a call that the gateway charged. */ - public static function optionalObject(array $fields, string $name): ?array + public static function objectOrNull(array $fields, string $name): ?array { $value = $fields[$name] ?? null; - return $value === null ? null : self::object($value); + return \is_array($value) && ($value === [] || !\array_is_list($value)) ? $value : null; } /** @@ -64,11 +67,13 @@ public static function bool(array $fields, string $name): bool /** * @param array $fields * - * @throws PostcodeException When the field is missing or is not text. + * @return ?string The text, or null when the field is absent, null or not text. */ - public static function string(array $fields, string $name): string + public static function textOrNull(array $fields, string $name): ?string { - return self::optionalString($fields, $name) ?? throw PostcodeException::unreadable(200); + $value = $fields[$name] ?? null; + + return \is_string($value) ? $value : null; } /** diff --git a/src/Client/LookupResult.php b/src/Client/LookupResult.php index b11dbe9..7063166 100644 --- a/src/Client/LookupResult.php +++ b/src/Client/LookupResult.php @@ -55,7 +55,8 @@ private function __construct( /** * @internal PostcodeClient reads each lookup response with this method. * - * @throws PostcodeException When a known field is missing or has the wrong type. + * @throws PostcodeException When valid is missing or is not true or false, or when status is + * neither null nor text. Other parts of the wrong type give null. */ public static function fromResponse( mixed $response, @@ -63,8 +64,8 @@ public static function fromResponse( int $levelRequested, ): self { $fields = Fields::object($response); - $address = Fields::optionalObject($fields, 'administrative_address'); - $recent = Fields::optionalObject($fields, 'recent_house_address'); + $address = Fields::objectOrNull($fields, 'administrative_address'); + $recent = Fields::objectOrNull($fields, 'recent_house_address'); return new self( $postcode, @@ -73,8 +74,8 @@ public static function fromResponse( $levelRequested, self::levelReceived($fields), $address === null ? null : AdministrativeAddress::fromFields($address), - $recent === null ? null : Fields::optionalString($recent, 'recent'), - Fields::optionalString($fields, 'building_use_status'), + $recent === null ? null : Fields::textOrNull($recent, 'recent'), + Fields::textOrNull($fields, 'building_use_status'), ); } diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index e0b73d3..c8c90e3 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -91,7 +91,8 @@ public function __construct( * ``` * * @param string|Postcode $code A full postcode, as text or as a parsed postcode. - * @param int $level The lookup level, from 1 to 5. + * @param int $level The lookup level, from 1 to 5. PHP's own type check + * refuses a value that is not a whole number. * * @throws PostcodeException For a code that fails the check, for a level outside 1 to 5, * and for each failure of the gateway. The client sends no diff --git a/tests/Unit/Client/FieldsTest.php b/tests/Unit/Client/FieldsTest.php index 36f8108..3976af2 100644 --- a/tests/Unit/Client/FieldsTest.php +++ b/tests/Unit/Client/FieldsTest.php @@ -37,6 +37,30 @@ public function readsAJsonObject(mixed $value): void self::assertSame($value, Fields::object($value)); } + #[Test] + public function readsAnObjectFieldAsNullWhenItIsNotAnObject(): void + { + $fields = ['a' => ['x' => 1], 'b' => 'text', 'c' => [1], 'd' => null]; + + self::assertSame(['x' => 1], Fields::objectOrNull($fields, 'a')); + self::assertNull(Fields::objectOrNull($fields, 'b')); + self::assertNull(Fields::objectOrNull($fields, 'c')); + self::assertNull(Fields::objectOrNull($fields, 'd')); + self::assertNull(Fields::objectOrNull($fields, 'missing')); + } + + #[Test] + public function readsATextFieldAsNullWhenItIsNotText(): void + { + $fields = ['a' => 'x', 'b' => 5, 'c' => ['x'], 'd' => null]; + + self::assertSame('x', Fields::textOrNull($fields, 'a')); + self::assertNull(Fields::textOrNull($fields, 'b')); + self::assertNull(Fields::textOrNull($fields, 'c')); + self::assertNull(Fields::textOrNull($fields, 'd')); + self::assertNull(Fields::textOrNull($fields, 'missing')); + } + /** * @return array */ diff --git a/tests/Unit/Client/LookupResultLenientTest.php b/tests/Unit/Client/LookupResultLenientTest.php new file mode 100644 index 0000000..ee7da16 --- /dev/null +++ b/tests/Unit/Client/LookupResultLenientTest.php @@ -0,0 +1,114 @@ +, int}> + */ + public static function lenientBodies(): array + { + return [ + 'an address as text' => [['administrative_address' => 'SYNTHETIC'], 2], + 'an address that is a list' => [['administrative_address' => [1]], 2], + 'a recent address as text' => [['recent_house_address' => 'SYNTHETIC'], 2], + 'a recent address that is a list' => [['recent_house_address' => ['x']], 2], + 'a recent text that is a number' => [['recent_house_address' => ['recent' => 7]], 2], + 'a building use status that is a number' => [['building_use_status' => 5], 3], + 'a building use status that is a list' => [['building_use_status' => ['a']], 3], + ]; + } + + /** + * The gateway has charged for the call, so a part outside the closed list of client.md + * gives null and never an error. + * + * @param array $fields + */ + #[Test] + #[DataProvider('lenientBodies')] + public function givesNullForAPartOfTheWrongType(array $fields, int $level): void + { + $result = LookupResult::fromResponse(['valid' => true, ...$fields], self::unit(), 3); + + self::assertNull($result->administrativeAddress); + self::assertNull($result->recentHouseAddress); + self::assertNull($result->buildingUseStatus); + self::assertSame($level, $result->levelReceived); + } + + /** + * @return array, ?string, ?string}> + */ + public static function addressNames(): array + { + $names = ['state_name' => 'S', 'lga_name' => 'L', 'locality_name' => 'C', 'zone' => 'Z']; + + return [ + 'no zone' => [ + ['state_name' => 'S', 'lga_name' => 'L', 'locality_name' => 'C'], + 'S', + null, + ], + 'a zone that is a number' => [[...$names, 'zone' => 5], 'S', null], + 'an empty address' => [[], null, null], + 'a state that is null' => [[...$names, 'state_name' => null], null, 'Z'], + ]; + } + + /** + * @param array $address + */ + #[Test] + #[DataProvider('addressNames')] + public function givesNullForEachNameThatIsMissingOrNotText( + array $address, + ?string $state, + ?string $zone, + ): void { + $body = ['valid' => true, 'administrative_address' => $address]; + + $result = LookupResult::fromResponse($body, self::unit(), 2); + + self::assertNotNull($result->administrativeAddress); + self::assertSame($state, $result->administrativeAddress->stateName); + self::assertSame($zone, $result->administrativeAddress->zone); + } + + #[Test] + public function keepsTheCallersPostcodeWhenTheBodyEchoesAnotherOne(): void + { + foreach (['FC-01-Z99-ZZ-02', 'hello', 7] as $echo) { + $body = ['postcode' => $echo, 'valid' => true]; + + $result = LookupResult::fromResponse($body, self::unit(), 1); + + self::assertSame('FC-01-Z99-ZZ-01', $result->postcode->canonical); + } + } + + private static function unit(): Postcode + { + $parsed = Postcode::parse(FixtureGateway::UNIT); + self::assertTrue($parsed->isOk()); + + return $parsed->value; + } +} diff --git a/tests/Unit/Client/LookupResultTest.php b/tests/Unit/Client/LookupResultTest.php index ec45b99..2ec510b 100644 --- a/tests/Unit/Client/LookupResultTest.php +++ b/tests/Unit/Client/LookupResultTest.php @@ -128,27 +128,19 @@ public function keepsAnyStatusTextAsItIs(mixed $status, ?string $expected): void */ public static function unreadableBodies(): array { - $noZone = ['state_name' => 'S', 'lga_name' => 'L', 'locality_name' => 'C']; - return [ 'a list in place of an object' => [[['valid' => true]]], 'no valid field' => [['status' => 'valid']], 'valid as text' => [['valid' => 'true']], - 'an address as text' => [['valid' => true, 'administrative_address' => 'SYNTHETIC']], - 'an address with no zone' => [['valid' => true, 'administrative_address' => $noZone]], 'a status that is a number' => [['valid' => true, 'status' => 5]], 'a status that is a list' => [['valid' => true, 'status' => ['valid']]], 'valid as null' => [['valid' => null]], - 'an address that is a list' => [['valid' => true, 'administrative_address' => [1]]], - 'a recent address that is a number' => [ - ['valid' => true, 'recent_house_address' => ['recent' => 7]], - ], ]; } #[Test] #[DataProvider('unreadableBodies')] - public function failsWithUnexpectedResponseWhenAKnownFieldHasTheWrongType(mixed $response): void + public function failsWithUnexpectedResponseWhenValidOrStatusIsUnreadable(mixed $response): void { try { LookupResult::fromResponse($response, self::unit(), 1); From 518d10da77c3004ac342c3e2c355a1b13bff20a0 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:33:58 +0100 Subject: [PATCH 06/21] feat(client): add reverse search and autocomplete A checkout field needs the postcode at a GPS fix and suggestions while the user types. The gateway can find an area with no unit in the radius, so a reverse result allows found with no unit. The client refuses a coordinate or a radius outside its range, because the gateway cuts a larger radius without a sign, and it writes each number in its shortest decimal form, so every client sends the same query. Autocomplete sends no empty text, because the gateway never answers it, and it builds the postcode of each suggestion, because the gateway sends the value of one segment only. It waits 15 s by default, and it does not retry a timeout, because the next keystroke replaces the call. Signed-off-by: MobileMage --- docs/api-dump.md | 45 +++++- src/Client/AutocompleteResult.php | 49 ++++++ src/Client/Confidence.php | 19 +++ src/Client/Internal/Decimal.php | 62 +++++++ src/Client/Internal/Fields.php | 54 +++++++ src/Client/PostcodeClient.php | 125 ++++++++++++++- src/Client/ReverseResult.php | 58 +++++++ src/Client/ReverseUnit.php | 64 ++++++++ src/Client/Suggestion.php | 60 +++++++ tests/Unit/Client/AutocompleteTest.php | 151 ++++++++++++++++++ tests/Unit/Client/AutocompleteTimeoutTest.php | 84 ++++++++++ tests/Unit/Client/DecimalTest.php | 52 ++++++ tests/Unit/Client/FieldsTest.php | 49 ++++++ tests/Unit/Client/FixtureGateway.php | 56 ++++++- tests/Unit/Client/ReverseInputTest.php | 95 +++++++++++ tests/Unit/Client/ReverseLenientTest.php | 56 +++++++ tests/Unit/Client/ReverseTest.php | 151 ++++++++++++++++++ 17 files changed, 1220 insertions(+), 10 deletions(-) create mode 100644 src/Client/AutocompleteResult.php create mode 100644 src/Client/Confidence.php create mode 100644 src/Client/Internal/Decimal.php create mode 100644 src/Client/ReverseResult.php create mode 100644 src/Client/ReverseUnit.php create mode 100644 src/Client/Suggestion.php create mode 100644 tests/Unit/Client/AutocompleteTest.php create mode 100644 tests/Unit/Client/AutocompleteTimeoutTest.php create mode 100644 tests/Unit/Client/DecimalTest.php create mode 100644 tests/Unit/Client/ReverseInputTest.php create mode 100644 tests/Unit/Client/ReverseLenientTest.php create mode 100644 tests/Unit/Client/ReverseTest.php diff --git a/docs/api-dump.md b/docs/api-dump.md index 88c48bc..f8528e9 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -13,6 +13,19 @@ final class AdministrativeAddress public readonly ?string $zone; } +final class AutocompleteResult +{ + public readonly \Gatepost\Postcode\Precision $segment; + public readonly array $suggestions; +} + +enum Confidence: string +{ + case High = 'high'; + case Medium = 'medium'; + case Low = 'low'; +} + enum ErrorCode: string { case InvalidInput = 'invalid_input'; @@ -41,8 +54,10 @@ final class LookupResult final class PostcodeClient { - public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', int $timeoutMs = 8000, int $maxRetries = 2); + public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', ?int $timeoutMs = null, int $maxRetries = 2); public function lookup(\Gatepost\Postcode\Postcode|string $code, int $level = 1): LookupResult; + public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): ReverseResult; + public function autocomplete(string $q): AutocompleteResult; } final class PostcodeException extends \RuntimeException @@ -53,6 +68,34 @@ final class PostcodeException extends \RuntimeException public function retryAfterMs(): ?int; } +final class ReverseResult +{ + public readonly bool $found; + public readonly ?float $radiusM; + public readonly ?ReverseUnit $unit; + public readonly ?string $area; + public readonly ?string $district; + public readonly ?string $state; +} + +final class ReverseUnit +{ + public readonly \Gatepost\Postcode\Postcode $postcode; + public readonly float $distanceM; + public readonly Confidence $confidence; + public readonly ?string $stateName; + public readonly ?string $lgaName; + public readonly ?string $localityName; + public readonly ?string $address; +} + +final class Suggestion +{ + public readonly string $code; + public readonly ?string $label; + public readonly ?\Gatepost\Postcode\Postcode $postcode; +} + namespace Gatepost\Postcode; final class ParseError diff --git a/src/Client/AutocompleteResult.php b/src/Client/AutocompleteResult.php new file mode 100644 index 0000000..e469253 --- /dev/null +++ b/src/Client/AutocompleteResult.php @@ -0,0 +1,49 @@ + $suggestions + */ + private function __construct( + /** The segment that the suggestions complete. */ + public readonly Precision $segment, + /** @var list The suggestions, in the gateway's order. */ + public readonly array $suggestions, + ) {} + + /** + * @internal PostcodeClient reads each autocomplete response with this method. + * + * @param string $typed The normalised text that the client sent as q. + * + * @throws PostcodeException When the segment is not one of the five, suggestions is not a + * list, or a suggestion has no text in code. + */ + public static function fromResponse(mixed $response, string $typed): self + { + $fields = Fields::object($response); + $segment = Precision::tryFrom(Fields::string($fields, 'segment')) + ?? throw PostcodeException::unreadable(200); + $suggestions = []; + foreach (Fields::list($fields, 'suggestions') as $suggestion) { + $suggestions[] = Suggestion::fromFields($suggestion, $segment, $typed); + } + + return new self($segment, $suggestions); + } +} diff --git a/src/Client/Confidence.php b/src/Client/Confidence.php new file mode 100644 index 0000000..70b7cdb --- /dev/null +++ b/src/Client/Confidence.php @@ -0,0 +1,19 @@ + $fields + * + * @return list + * + * @throws PostcodeException When the field is missing or is not a JSON array. + */ + public static function list(array $fields, string $name): array + { + $value = $fields[$name] ?? null; + if (!\is_array($value) || !\array_is_list($value)) { + throw PostcodeException::unreadable(200); + } + + return $value; + } + + /** + * @param array $fields + * + * @throws PostcodeException When the field is missing or is not text. + */ + public static function string(array $fields, string $name): string + { + $value = $fields[$name] ?? null; + if (!\is_string($value)) { + throw PostcodeException::unreadable(200); + } + + return $value; + } + + /** + * @param array $fields + * + * @throws PostcodeException When the field is missing or is not a finite number. + */ + public static function number(array $fields, string $name): float + { + return self::numberOrNull($fields, $name) ?? throw PostcodeException::unreadable(200); + } + + /** + * @param array $fields + * + * @return ?float The number, or null when the field is absent, null or not a finite number. + */ + public static function numberOrNull(array $fields, string $name): ?float + { + $value = $fields[$name] ?? null; + + return (\is_int($value) || \is_float($value)) && \is_finite($value) ? (float) $value : null; + } + /** * @param array $fields * diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index c8c90e3..a61e06a 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -7,8 +7,10 @@ namespace Gatepost\Postcode\Client; +use Gatepost\Postcode\Client\Internal\Decimal; use Gatepost\Postcode\Client\Internal\Sender; use Gatepost\Postcode\Client\Internal\SystemTimer; +use Gatepost\Postcode\Internal\SpecData; use Gatepost\Postcode\Postcode; use Gatepost\Postcode\Precision; use InvalidArgumentException; @@ -33,11 +35,18 @@ final class PostcodeClient { private const DEFAULT_BASE_URL = 'https://api.postcode.gov.ng'; + private const AUTOCOMPLETE_TIMEOUT_MS = 15000; + // The gateway answers no empty q, and a postcode has 11 characters (spec/client.md). + private const AUTOCOMPLETE_TEXT = '/\A[A-Z0-9]{1,11}\z/'; + private const WITHIN_INPUT_LIMIT = '/\A.{0,' . SpecData::MAX_INPUT_CODE_POINTS . '}\z/su'; + private const MAX_RADIUS_M = 250; private readonly string $baseUrl; private readonly Sender $sender; + private readonly Sender $autocompleteSender; + /** * @param ClientInterface $transport A PSR-18 HTTP client. Set its timeout to * timeoutMs, because PSR-18 has no timeout. @@ -46,8 +55,10 @@ final class PostcodeClient * the client sends no key, and the gateway * answers 401. * @param string $baseUrl The gateway's address. - * @param int $timeoutMs The longest wait for one attempt. A failed + * @param ?int $timeoutMs The longest wait for one attempt. A failed * attempt that took this long is a timeout. + * With null, it is 8000, and 15000 for + * autocomplete. * @param int $maxRetries The most retries after the first attempt. * * @throws InvalidArgumentException When an option is out of range, or the key is empty. @@ -62,20 +73,27 @@ public function __construct( private readonly RequestFactoryInterface $requestFactory, private readonly ?string $apiKey = null, string $baseUrl = self::DEFAULT_BASE_URL, - int $timeoutMs = 8000, + ?int $timeoutMs = null, int $maxRetries = 2, ) { if ($apiKey === '') { throw new InvalidArgumentException('apiKey is empty. Pass null to send no key.'); } - if ($timeoutMs <= 0) { + if ($timeoutMs !== null && $timeoutMs <= 0) { throw new InvalidArgumentException("timeoutMs is {$timeoutMs}. Use 1 or more."); } if ($maxRetries < 0) { throw new InvalidArgumentException("maxRetries is {$maxRetries}. Use 0 or more."); } $this->baseUrl = \rtrim($baseUrl, '/'); - $this->sender = new Sender($transport, $timeoutMs, $maxRetries, new SystemTimer()); + $timer = new SystemTimer(); + $this->sender = new Sender($transport, $timeoutMs ?? 8000, $maxRetries, $timer); + $this->autocompleteSender = new Sender( + $transport, + $timeoutMs ?? self::AUTOCOMPLETE_TIMEOUT_MS, + $maxRetries, + $timer, + ); } /** @@ -105,11 +123,87 @@ public function lookup(string|Postcode $code, int $level = 1): LookupResult } $postcode = self::fullPostcode($code); $query = ['code' => $postcode->canonical, 'level' => $level]; - $response = $this->send('/v1/lookup', $query, retryTimeouts: true); + $response = $this->send($this->sender, '/v1/lookup', $query, retryTimeouts: true); return LookupResult::fromResponse($response, $postcode, $level); } + /** + * Finds the postcode at a point, such as a GPS fix. A point with no postcode within the + * radius is a result with found false, not an error. + * + * ```php + * $result = $client->reverse(9.0, 7.0); + * echo $result->unit?->postcode->canonical; // FC-01-Z99-ZZ-01 + * echo $result->area; // FC-01-Z99-ZZ + * ``` + * + * @param float $lat The latitude in degrees, from -90 to 90. + * @param float $lng The longitude in degrees, from -180 to 180. + * @param ?float $maxDistanceM The radius in metres, from 0 to 250. With null, the gateway + * applies its own default. The gateway cuts a larger radius to 250 + * without a sign, so the client refuses it. + * + * @throws PostcodeException For a number outside its range or not finite, and for each + * failure of the gateway. The client sends no request for bad input. + */ + public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): ReverseResult + { + $query = [ + 'lat' => self::decimal('The latitude', $lat, 90), + 'lng' => self::decimal('The longitude', $lng, 180), + ]; + if ($maxDistanceM !== null) { + $query['max_distance_m'] = self::decimal( + 'The radius', + $maxDistanceM, + self::MAX_RADIUS_M, + 0, + ); + } + $response = $this->send($this->sender, '/v1/search/reverse', $query, retryTimeouts: true); + + return ReverseResult::fromResponse($response); + } + + /** + * Suggests ways to complete the segment that the user types. The client normalises the text + * with Postcode::normalize(). It sends no request for text that is empty, longer than a + * postcode, or holds a character other than A to Z and 0 to 9. A timeout ends the call + * without a retry, because the next keystroke replaces it. + * + * ```php + * $result = $client->autocomplete('fc01z'); + * echo $result->segment->value; // district + * echo $result->suggestions[0]->postcode?->canonical; // FC-01-Z99 + * ``` + * + * @param string $q The text that the user typed. + * + * @throws PostcodeException For text that fails the check, and for each failure of the + * gateway. + */ + public function autocomplete(string $q): AutocompleteResult + { + // Like the other clients, refuse text over the input limit of the core before the + // normalisation, which has no limit of its own. A string that is not UTF-8 fails here too. + $typed = \preg_match(self::WITHIN_INPUT_LIMIT, $q) === 1 ? Postcode::normalize($q) : ''; + if (\preg_match(self::AUTOCOMPLETE_TEXT, $typed) !== 1) { + throw PostcodeException::invalidInput( + 'The text must hold 1 to 11 letters A to Z and digits 0 to 9, after spaces and ' + . 'hyphens are removed. Send no request until the user types one.', + ); + } + $response = $this->send( + $this->autocompleteSender, + '/v1/search/autocomplete', + ['q' => $typed], + retryTimeouts: false, + ); + + return AutocompleteResult::fromResponse($response, $typed); + } + /** * @throws PostcodeException */ @@ -134,12 +228,29 @@ private static function fullPostcode(string|Postcode $code): Postcode return $code; } + /** + * The number in its shortest decimal form, after a check of its range. + * + * @throws PostcodeException For a number that is not finite or outside its range. + */ + private static function decimal(string $name, float $number, int $max, ?int $min = null): string + { + $min ??= -$max; + if (!\is_finite($number) || $number < $min || $number > $max) { + throw PostcodeException::invalidInput( + "{$name} must be a number from {$min} to {$max}.", + ); + } + + return Decimal::shortest($number); + } + /** * @param array $query * * @throws PostcodeException */ - private function send(string $path, array $query, bool $retryTimeouts): mixed + private function send(Sender $sender, string $path, array $query, bool $retryTimeouts): mixed { $url = $this->baseUrl . $path . '?' . \http_build_query($query); $request = $this->requestFactory->createRequest('GET', $url); @@ -147,6 +258,6 @@ private function send(string $path, array $query, bool $retryTimeouts): mixed $request = $request->withHeader('X-API-Key', $this->apiKey); } - return $this->sender->send($request, $retryTimeouts); + return $sender->send($request, $retryTimeouts); } } diff --git a/src/Client/ReverseResult.php b/src/Client/ReverseResult.php new file mode 100644 index 0000000..ce25d0c --- /dev/null +++ b/src/Client/ReverseResult.php @@ -0,0 +1,58 @@ +value ?? throw PostcodeException::unreadable(200), + $distanceM, + ($confidence === null ? null : Confidence::tryFrom($confidence)) ?? Confidence::Low, + Fields::textOrNull($fields, 'state_name'), + Fields::textOrNull($fields, 'lga_name'), + Fields::textOrNull($fields, 'locality_name'), + Fields::textOrNull($fields, 'address'), + ); + } +} diff --git a/src/Client/Suggestion.php b/src/Client/Suggestion.php new file mode 100644 index 0000000..09830d3 --- /dev/null +++ b/src/Client/Suggestion.php @@ -0,0 +1,60 @@ +value]['start']); + $postcode = Postcode::parse($before . $code, allowPartial: true)->value; + + return $postcode?->precision === $segment ? $postcode : null; + } +} diff --git a/tests/Unit/Client/AutocompleteTest.php b/tests/Unit/Client/AutocompleteTest.php new file mode 100644 index 0000000..fe35b75 --- /dev/null +++ b/tests/Unit/Client/AutocompleteTest.php @@ -0,0 +1,151 @@ +autocomplete('fc 01-z'); + + self::assertSame('/v1/search/autocomplete', $gateway->requests[0]->getUri()->getPath()); + self::assertSame('q=FC01Z', $gateway->requests[0]->getUri()->getQuery()); + self::assertSame(Precision::District, $result->segment); + self::assertCount(1, $result->suggestions); + self::assertSame('Z99', $result->suggestions[0]->code); + self::assertNull($result->suggestions[0]->label); + self::assertSame('FC-01-Z99', $result->suggestions[0]->postcode?->canonical); + } + + #[Test] + public function buildsAStatePostcodeFromTheFirstLetter(): void + { + $response = ['segment' => 'state', 'suggestions' => [['code' => 'FC']]]; + + $result = AutocompleteResult::fromResponse($response, 'F'); + + self::assertSame('FC', $result->suggestions[0]->postcode?->canonical); + } + + #[Test] + public function keepsALabelAndGivesNullForACodeThatMakesNoPostcode(): void + { + $response = [ + 'segment' => 'district', + 'suggestions' => [['code' => 'Z99', 'label' => 'SYNTHETIC'], ['code' => 'ZZZZ']], + 'limit' => 20, + ]; + + $result = AutocompleteResult::fromResponse($response, 'FC01Z'); + + self::assertSame('SYNTHETIC', $result->suggestions[0]->label); + self::assertSame('FC-01-Z99', $result->suggestions[0]->postcode?->canonical); + self::assertNull($result->suggestions[1]->postcode); + } + + #[Test] + public function readsALabelThatIsNotTextAsNull(): void + { + $response = ['segment' => 'state', 'suggestions' => [['code' => 'FC', 'label' => 7]]]; + + $result = AutocompleteResult::fromResponse($response, 'F'); + + self::assertNull($result->suggestions[0]->label); + } + + #[Test] + public function readsAnEmptyListAsNoSuggestions(): void + { + $response = ['segment' => 'area', 'suggestions' => []]; + + $result = AutocompleteResult::fromResponse($response, 'FC01Z99Z'); + + self::assertSame(Precision::Area, $result->segment); + self::assertSame([], $result->suggestions); + } + + /** + * @return array + */ + public static function badTexts(): array + { + return [ + 'empty text' => [''], + 'only separators' => [' - '], + 'a character that no postcode has' => ['FC!'], + 'more than 11 characters' => ['FC01Z99ZZ011'], + 'separators that push the text over the input limit' => ['FC' . \str_repeat(' ', 70)], + 'bytes that are not UTF-8' => ["FC\xFF"], + ]; + } + + #[Test] + #[DataProvider('badTexts')] + public function sendsNoRequestForTextThatFailsTheCheck(string $q): void + { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + try { + $client->autocomplete($q); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertNull($error->status()); + } + self::assertSame([], $gateway->requests); + } + + /** + * @return array}> + */ + public static function unreadableBodies(): array + { + return [ + 'a segment that is not one of the five' => [['segment' => 'zone', 'suggestions' => []]], + 'a segment that is a number' => [['segment' => 3, 'suggestions' => []]], + 'no suggestions field' => [['segment' => 'state']], + 'suggestions as an object' => [['segment' => 'state', 'suggestions' => ['a' => 1]]], + 'a code that is a number' => [['segment' => 'lga', 'suggestions' => [['code' => 1]]]], + 'a suggestion with no code' => [ + ['segment' => 'lga', 'suggestions' => [['label' => 'a']]], + ], + 'a suggestion that is text' => [['segment' => 'lga', 'suggestions' => ['01']]], + ]; + } + + /** + * @param array $response + */ + #[Test] + #[DataProvider('unreadableBodies')] + public function failsWithUnexpectedResponseWhenAKnownFieldHasTheWrongType(array $response): void + { + $this->expectExceptionObject(PostcodeException::unreadable(200)); + + AutocompleteResult::fromResponse($response, 'FC01'); + } +} diff --git a/tests/Unit/Client/AutocompleteTimeoutTest.php b/tests/Unit/Client/AutocompleteTimeoutTest.php new file mode 100644 index 0000000..56f39cc --- /dev/null +++ b/tests/Unit/Client/AutocompleteTimeoutTest.php @@ -0,0 +1,84 @@ +attempts; + \usleep(30_000); + + throw new ConnectException('The connection timed out.', $request); + } + }; + $client = new PostcodeClient($transport, new HttpFactory(), timeoutMs: 20); + + try { + $client->autocomplete('F'); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + } + self::assertSame(1, $transport->attempts); + } + + /** + * Reads the timeout of the sender that autocomplete uses. A timeout takes real time, and the + * default one is 15 s, so a test cannot wait for it. + */ + private static function timeoutOfAutocomplete(PostcodeClient $client): int + { + $sender = (new ReflectionProperty(PostcodeClient::class, 'autocompleteSender')) + ->getValue($client); + self::assertInstanceOf(Sender::class, $sender); + $timeout = (new ReflectionProperty(Sender::class, 'timeoutMs'))->getValue($sender); + self::assertIsInt($timeout); + + return $timeout; + } +} diff --git a/tests/Unit/Client/DecimalTest.php b/tests/Unit/Client/DecimalTest.php new file mode 100644 index 0000000..08fdbf9 --- /dev/null +++ b/tests/Unit/Client/DecimalTest.php @@ -0,0 +1,52 @@ + + */ + public static function numbers(): array + { + return [ + 'a whole number' => [9.0, '9'], + 'a whole number with zeros' => [250.0, '250'], + 'a negative whole number' => [-90.0, '-90'], + 'zero' => [0.0, '0'], + 'negative zero' => [-0.0, '0'], + 'one decimal' => [0.5, '0.5'], + 'three decimals' => [9.001, '9.001'], + 'a negative decimal' => [-7.25, '-7.25'], + 'the sum 0.1 + 0.2' => [0.1 + 0.2, '0.30000000000000004'], + 'five zeros after the point' => [0.00001, '0.00001'], + 'a number that PHP prints as 1.0E-7' => [0.0000001, '0.0000001'], + 'a number with several digits after an exponent' => [1.5e-9, '0.0000000015'], + 'a negative number with an exponent' => [-2.5e-8, '-0.000000025'], + 'a number that PHP prints as 1.0E+25' => [1e25, '10000000000000000000000000'], + 'a large number with digits' => [1.5e20, '150000000000000000000'], + ]; + } + + #[Test] + #[DataProvider('numbers')] + public function writesTheShortestDecimalWithNoExponent(float $number, string $expected): void + { + self::assertSame($expected, Decimal::shortest($number)); + self::assertSame($number + 0.0, (float) Decimal::shortest($number)); + } +} diff --git a/tests/Unit/Client/FieldsTest.php b/tests/Unit/Client/FieldsTest.php index 3976af2..036e0f1 100644 --- a/tests/Unit/Client/FieldsTest.php +++ b/tests/Unit/Client/FieldsTest.php @@ -82,4 +82,53 @@ public function refusesAValueThatIsNotAJsonObject(mixed $value): void Fields::object($value); } + + #[Test] + public function readsAListAndRefusesAnythingElse(): void + { + self::assertSame([1, 'a'], Fields::list(['a' => [1, 'a']], 'a')); + self::assertSame([], Fields::list(['a' => []], 'a')); + foreach ([['a' => ['k' => 1]], ['a' => 'text'], ['a' => null], []] as $fields) { + try { + Fields::list($fields, 'a'); + self::fail('The reader accepted a value that is not a list.'); + } catch (PostcodeException $error) { + self::assertEquals(PostcodeException::unreadable(200), $error); + } + } + } + + #[Test] + public function readsATextFieldAndRefusesAnythingElse(): void + { + self::assertSame('x', Fields::string(['a' => 'x'], 'a')); + foreach ([['a' => 1], ['a' => null], []] as $fields) { + try { + Fields::string($fields, 'a'); + self::fail('The reader accepted a value that is not text.'); + } catch (PostcodeException $error) { + self::assertEquals(PostcodeException::unreadable(200), $error); + } + } + } + + #[Test] + public function readsANumberAsAFloatAndGivesNullForAnythingElse(): void + { + $fields = ['i' => 4, 'f' => 4.5, 's' => '4', 'n' => null, 'big' => \INF, 'b' => true]; + + self::assertSame(4.0, Fields::number($fields, 'i')); + self::assertSame(4.5, Fields::numberOrNull($fields, 'f')); + foreach (['s', 'n', 'big', 'b', 'missing'] as $name) { + self::assertNull(Fields::numberOrNull($fields, $name)); + } + } + + #[Test] + public function refusesAFieldThatIsNotANumberWhenTheNumberIsRequired(): void + { + $this->expectExceptionObject(PostcodeException::unreadable(200)); + + Fields::number(['a' => '4'], 'a'); + } } diff --git a/tests/Unit/Client/FixtureGateway.php b/tests/Unit/Client/FixtureGateway.php index e5c8e81..49f5da6 100644 --- a/tests/Unit/Client/FixtureGateway.php +++ b/tests/Unit/Client/FixtureGateway.php @@ -8,6 +8,7 @@ namespace Gatepost\Postcode\Tests\Unit\Client; use Gatepost\Postcode\Client\PostcodeClient; +use Gatepost\Postcode\Internal\SpecData; use GuzzleHttp\Psr7\HttpFactory; use GuzzleHttp\Psr7\Response; use Psr\Http\Client\ClientInterface; @@ -62,6 +63,8 @@ public function sendRequest(RequestInterface $request): ResponseInterface return match ($request->getUri()->getPath()) { '/v1/lookup' => self::lookup($query), + '/v1/search/reverse' => self::reverse($query), + '/v1/search/autocomplete' => self::autocomplete($query), default => self::fixture('errors/unknown-path'), }; } @@ -71,15 +74,64 @@ public function sendRequest(RequestInterface $request): ResponseInterface */ private static function lookup(array $query): ResponseInterface { - if (($query['code'] ?? null) !== self::UNIT) { + if (self::text($query, 'code') !== self::UNIT) { return self::fixture('lookup/not-found'); } // The client of this gateway holds a level 3 key, as the mock key nipost_test_mock_l3 does. - $level = \is_string($query['level'] ?? null) ? $query['level'] : '1'; + $level = self::text($query, 'level'); if ((int) $level > 3) { return self::fixture('errors/level-not-granted-3'); } return self::fixture("lookup/valid-level-{$level}"); } + + /** + * @param array $query + */ + private static function reverse(array $query): ResponseInterface + { + $point = [(float) self::text($query, 'lat'), (float) self::text($query, 'lng')]; + + return match ($point) { + [9.0, 7.0] => self::fixture('reverse/unit'), + [9.001, 7.001] => self::fixture('reverse/area'), + default => self::fixture('reverse/not-found'), + }; + } + + /** + * The segment of the last typed character, and the value of that segment in the synthetic + * unit when the text is a prefix of it. The spec has no autocomplete fixture, so this builds + * the answer, as the mock server does. + * + * @param array $query + */ + private static function autocomplete(array $query): ResponseInterface + { + $typed = self::text($query, 'q'); + $unit = \str_replace('-', '', self::UNIT); + foreach (SpecData::SEGMENTS as $name => $segment) { + if (\strlen($typed) <= $segment['end']) { + $value = \substr($unit, $segment['start'], $segment['length']); + $suggestions = \str_starts_with($unit, $typed) ? [['code' => $value]] : []; + + return ScriptedTransport::json(200, [ + 'data' => ['segment' => $name, 'suggestions' => $suggestions], + ]); + } + } + + return self::fixture('errors/invalid-request'); + } + + /** + * @param array $query + */ + private static function text(array $query, string $name): string + { + $value = $query[$name] ?? ''; + + return \is_string($value) ? $value : ''; + } } diff --git a/tests/Unit/Client/ReverseInputTest.php b/tests/Unit/Client/ReverseInputTest.php new file mode 100644 index 0000000..2063835 --- /dev/null +++ b/tests/Unit/Client/ReverseInputTest.php @@ -0,0 +1,95 @@ + + */ + public static function points(): array + { + return [ + 'whole degrees' => [9.0, 7.0, null, 'lat=9&lng=7'], + 'a radius' => [9.001, 7.001, 25.0, 'lat=9.001&lng=7.001&max_distance_m=25'], + 'a small number with no exponent' => [0.00001, -0.5, null, 'lat=0.00001&lng=-0.5'], + 'a negative zero' => [-0.0, 0.0, 0.0, 'lat=0&lng=0&max_distance_m=0'], + 'the limits' => [-90.0, 180.0, 250.0, 'lat=-90&lng=180&max_distance_m=250'], + 'the other limits' => [90.0, -180.0, null, 'lat=90&lng=-180'], + ]; + } + + #[Test] + #[DataProvider('points')] + public function writesEachNumberInItsShortestDecimalForm( + float $lat, + float $lng, + ?float $maxDistanceM, + string $query, + ): void { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + $client->reverse($lat, $lng, $maxDistanceM); + + self::assertSame('/v1/search/reverse', $gateway->requests[0]->getUri()->getPath()); + self::assertSame($query, $gateway->requests[0]->getUri()->getQuery()); + } + + /** + * @return array + */ + public static function badInputs(): array + { + return [ + 'a latitude that is NAN' => [\NAN, 7.0, null], + 'a latitude that is infinite' => [\INF, 7.0, null], + 'a latitude above 90' => [90.0000001, 7.0, null], + 'a latitude below -90' => [-90.0000001, 7.0, null], + 'a longitude that is NAN' => [9.0, \NAN, null], + 'a longitude that is infinite' => [9.0, -\INF, null], + 'a longitude above 180' => [9.0, 180.0000001, null], + 'a longitude below -180' => [9.0, -180.0000001, null], + 'a radius that is NAN' => [9.0, 7.0, \NAN], + 'a radius below 0' => [9.0, 7.0, -0.0000001], + 'a radius above 250' => [9.0, 7.0, 250.0000001], + ]; + } + + #[Test] + #[DataProvider('badInputs')] + public function sendsNoRequestForANumberOutsideItsRange( + float $lat, + float $lng, + ?float $maxDistanceM, + ): void { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + try { + $client->reverse($lat, $lng, $maxDistanceM); + self::fail('The search did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertNull($error->status()); + } + self::assertSame([], $gateway->requests); + } +} diff --git a/tests/Unit/Client/ReverseLenientTest.php b/tests/Unit/Client/ReverseLenientTest.php new file mode 100644 index 0000000..2108d06 --- /dev/null +++ b/tests/Unit/Client/ReverseLenientTest.php @@ -0,0 +1,56 @@ + false]); + $text = ReverseResult::fromResponse(['found' => false, 'radius_m' => '25']); + $zero = ReverseResult::fromResponse(['found' => false, 'radius_m' => 0]); + + self::assertNull($absent->radiusM); + self::assertNull($text->radiusM); + self::assertSame(0.0, $zero->radiusM); + } + + #[Test] + public function keepsTheAreaAndTheDistrictAsTheGatewaySentThem(): void + { + $response = [ + 'found' => true, + 'area' => 'not a postcode', + 'district' => 7, + 'state' => ['FC'], + ]; + + $result = ReverseResult::fromResponse($response); + + self::assertSame('not a postcode', $result->area); + self::assertNull($result->district); + self::assertNull($result->state); + } + + #[Test] + public function readsAUnitThatIsNullAsNoUnit(): void + { + $result = ReverseResult::fromResponse(['found' => true, 'unit' => null]); + + self::assertNull($result->unit); + } +} diff --git a/tests/Unit/Client/ReverseTest.php b/tests/Unit/Client/ReverseTest.php new file mode 100644 index 0000000..f885d36 --- /dev/null +++ b/tests/Unit/Client/ReverseTest.php @@ -0,0 +1,151 @@ +reverse(9.0, 7.0); + + self::assertTrue($result->found); + self::assertSame(25.0, $result->radiusM); + self::assertNotNull($result->unit); + self::assertSame(FixtureGateway::UNIT, $result->unit->postcode->canonical); + self::assertSame(4.2, $result->unit->distanceM); + self::assertSame(Confidence::Low, $result->unit->confidence); + self::assertNull($result->unit->stateName); + self::assertNull($result->unit->lgaName); + self::assertNull($result->unit->localityName); + self::assertNull($result->unit->address); + self::assertSame('FC-01-Z99-ZZ', $result->area); + self::assertSame('FC-01-Z99', $result->district); + self::assertSame('FC', $result->state); + } + + #[Test] + public function findsAnAreaWithNoUnitWithinTheRadius(): void + { + $result = FixtureGateway::client()->reverse(9.001, 7.001, maxDistanceM: 25); + + self::assertTrue($result->found); + self::assertNull($result->unit); + self::assertSame('FC-01-Z99-ZZ', $result->area); + } + + #[Test] + public function returnsFoundFalseForAPointWithNoPostcode(): void + { + $result = FixtureGateway::client()->reverse(0.0, 0.0); + + self::assertFalse($result->found); + self::assertNull($result->unit); + self::assertNull($result->area); + self::assertNull($result->district); + self::assertNull($result->state); + } + + #[Test] + public function readsTheLevelTwoNamesOfAUnit(): void + { + $unit = [ + 'postcode' => FixtureGateway::UNIT, + 'distance_m' => 4, + 'confidence' => 'high', + 'state_name' => 'SYNTHETIC STATE', + 'lga_name' => 'SYNTHETIC LGA', + 'locality_name' => 'SYNTHETIC LOCALITY', + 'address' => 'SYNTHETIC ADDRESS', + ]; + + $result = ReverseResult::fromResponse(['found' => true, 'radius_m' => 25, 'unit' => $unit]); + + self::assertNotNull($result->unit); + self::assertSame(Confidence::High, $result->unit->confidence); + self::assertSame(4.0, $result->unit->distanceM); + self::assertSame('SYNTHETIC STATE', $result->unit->stateName); + self::assertSame('SYNTHETIC LGA', $result->unit->lgaName); + self::assertSame('SYNTHETIC LOCALITY', $result->unit->localityName); + self::assertSame('SYNTHETIC ADDRESS', $result->unit->address); + } + + /** + * @return array + */ + public static function confidences(): array + { + return [ + 'a word that the spec does not list' => ['very_high'], + 'a number' => [3], + 'no confidence' => [null], + ]; + } + + #[Test] + #[DataProvider('confidences')] + public function readsAnUnknownConfidenceAsLow(mixed $confidence): void + { + $unit = ['postcode' => FixtureGateway::UNIT, 'distance_m' => 4]; + $unit['confidence'] = $confidence; + + $result = ReverseResult::fromResponse(['found' => true, 'radius_m' => 25, 'unit' => $unit]); + + self::assertNotNull($result->unit); + self::assertSame(Confidence::Low, $result->unit->confidence); + } + + /** + * @return array}> + */ + public static function unreadableBodies(): array + { + $found = ['found' => true, 'radius_m' => 25]; + $unit = ['postcode' => FixtureGateway::UNIT, 'distance_m' => 4.2, 'confidence' => 'high']; + + return [ + 'no found field' => [['radius_m' => 25]], + 'a found that is text' => [['found' => 'true']], + 'a unit with the postcode of an area' => [ + $found + ['unit' => ['postcode' => 'FC-01-Z99-ZZ'] + $unit], + ], + 'a unit with a postcode that does not parse' => [ + $found + ['unit' => ['postcode' => 'HELLO'] + $unit], + ], + 'a unit with no postcode' => [$found + ['unit' => ['postcode' => null] + $unit]], + 'a unit with no distance' => [$found + ['unit' => ['distance_m' => null] + $unit]], + 'a unit with a distance as text' => [ + $found + ['unit' => ['distance_m' => '4'] + $unit], + ], + 'a unit that is a list' => [$found + ['unit' => [1, 2]]], + 'a unit that is text' => [$found + ['unit' => 'FC-01-Z99-ZZ-01']], + ]; + } + + /** + * @param array $response + */ + #[Test] + #[DataProvider('unreadableBodies')] + public function failsWithUnexpectedResponseWhenAListedPartIsBroken(array $response): void + { + $this->expectExceptionObject(\Gatepost\Postcode\Client\PostcodeException::unreadable(200)); + + ReverseResult::fromResponse($response); + } +} From ef14f72ab7f543cabab151f8cd8553db98e78403 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:38:57 +0100 Subject: [PATCH 07/21] feat(client): keep results in a PSR-16 cache when asked The cache stays off unless the caller sets cacheTtlMs and passes a PSR-16 cache, as the spec asks. Each key of the caller's cache holds a generation, so clearCache() hides every kept result without wiping the caller's other entries. A call takes its key before it sends the request, so a request that started before clearCache() keeps no result. Each API key keeps its own results, because two keys can hold different lookup levels, and the client keeps no error. Signed-off-by: MobileMage --- docs/api-dump.md | 3 +- src/Client/Internal/ResultCache.php | 102 ++++++++++++++++ src/Client/PostcodeClient.php | 111 ++++++++++++++--- tests/Unit/Client/ArrayCache.php | 100 +++++++++++++++ tests/Unit/Client/CacheTest.php | 154 ++++++++++++++++++++++++ tests/Unit/Client/ClientOptionsTest.php | 16 +++ tests/Unit/Client/ResultCacheTest.php | 136 +++++++++++++++++++++ 7 files changed, 607 insertions(+), 15 deletions(-) create mode 100644 src/Client/Internal/ResultCache.php create mode 100644 tests/Unit/Client/ArrayCache.php create mode 100644 tests/Unit/Client/CacheTest.php create mode 100644 tests/Unit/Client/ResultCacheTest.php diff --git a/docs/api-dump.md b/docs/api-dump.md index f8528e9..b15e073 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -54,10 +54,11 @@ final class LookupResult final class PostcodeClient { - public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', ?int $timeoutMs = null, int $maxRetries = 2); + public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', ?int $timeoutMs = null, int $maxRetries = 2, int $cacheTtlMs = 0, ?\Psr\SimpleCache\CacheInterface $cache = null); public function lookup(\Gatepost\Postcode\Postcode|string $code, int $level = 1): LookupResult; public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): ReverseResult; public function autocomplete(string $q): AutocompleteResult; + public function clearCache(): void; } final class PostcodeException extends \RuntimeException diff --git a/src/Client/Internal/ResultCache.php b/src/Client/Internal/ResultCache.php new file mode 100644 index 0000000..78b6900 --- /dev/null +++ b/src/Client/Internal/ResultCache.php @@ -0,0 +1,102 @@ +generation() . '.' . $hash; + } + + /** + * @return mixed The kept data, or null when nothing is kept. A result never has null data. + */ + public function get(string $slot): mixed + { + $entry = $this->cache->get($slot); + $expiresAtMs = \is_array($entry) ? ($entry['expiresAtMs'] ?? null) : null; + if (!\is_float($expiresAtMs) || $expiresAtMs <= $this->timer->nowMs()) { + return null; + } + + return $entry['data'] ?? null; + } + + /** + * Keeps the data of a call that the client read without an error. + */ + public function put(string $slot, mixed $data): void + { + $entry = ['expiresAtMs' => $this->timer->nowMs() + $this->ttlMs, 'data' => $data]; + // PSR-16 counts whole seconds, so the entry can outlive its TTL by up to one second. + // get() checks the time in milliseconds. + $this->cache->set($slot, $entry, (int) \ceil($this->ttlMs / 1000)); + } + + /** + * Makes every result that this cache kept unreachable. + */ + public function clear(): void + { + $this->cache->set(self::GENERATION_KEY, self::newGeneration()); + } + + // A cache can evict the generation like any other entry. A new one then hides the old + // entries, so an old result never comes back after clear(). + private function generation(): string + { + $generation = $this->cache->get(self::GENERATION_KEY); + if (\is_string($generation) && \preg_match(self::GENERATION, $generation) === 1) { + return $generation; + } + $generation = self::newGeneration(); + $this->cache->set(self::GENERATION_KEY, $generation); + + return $generation; + } + + private static function newGeneration(): string + { + return \bin2hex(\random_bytes(4)); + } +} diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index a61e06a..87b1676 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -7,7 +7,9 @@ namespace Gatepost\Postcode\Client; +use Closure; use Gatepost\Postcode\Client\Internal\Decimal; +use Gatepost\Postcode\Client\Internal\ResultCache; use Gatepost\Postcode\Client\Internal\Sender; use Gatepost\Postcode\Client\Internal\SystemTimer; use Gatepost\Postcode\Internal\SpecData; @@ -16,11 +18,12 @@ use InvalidArgumentException; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestFactoryInterface; +use Psr\SimpleCache\CacheInterface; /** * Calls NIPOST's postcode gateway with your own API key. Each call blocks until it has a result * or a PostcodeException. A call that fails for a reason that can pass, such as a 503, is tried - * again, up to maxRetries times. + * again, up to maxRetries times. The client keeps no result unless cacheTtlMs is above 0. * * Unofficial. Not made or endorsed by NIPOST. * @@ -47,6 +50,8 @@ final class PostcodeClient private readonly Sender $autocompleteSender; + private readonly ?ResultCache $cache; + /** * @param ClientInterface $transport A PSR-18 HTTP client. Set its timeout to * timeoutMs, because PSR-18 has no timeout. @@ -60,8 +65,13 @@ final class PostcodeClient * With null, it is 8000, and 15000 for * autocomplete. * @param int $maxRetries The most retries after the first attempt. + * @param int $cacheTtlMs How long the client keeps a result. 0 turns + * the cache off. + * @param ?CacheInterface $cache A PSR-16 cache for the results. The client + * needs one when cacheTtlMs is above 0. * - * @throws InvalidArgumentException When an option is out of range, or the key is empty. + * @throws InvalidArgumentException When an option is out of range, the key is empty, or + * cacheTtlMs is above 0 with no cache. * * Callers name each option, and TELL-3 counts no named or defaulted parameter, so PHPMD's * count of parameters does not apply here. @@ -75,6 +85,8 @@ public function __construct( string $baseUrl = self::DEFAULT_BASE_URL, ?int $timeoutMs = null, int $maxRetries = 2, + int $cacheTtlMs = 0, + ?CacheInterface $cache = null, ) { if ($apiKey === '') { throw new InvalidArgumentException('apiKey is empty. Pass null to send no key.'); @@ -85,6 +97,12 @@ public function __construct( if ($maxRetries < 0) { throw new InvalidArgumentException("maxRetries is {$maxRetries}. Use 0 or more."); } + if ($cacheTtlMs < 0) { + throw new InvalidArgumentException("cacheTtlMs is {$cacheTtlMs}. Use 0 or more."); + } + if ($cacheTtlMs > 0 && $cache === null) { + throw new InvalidArgumentException('cacheTtlMs is above 0. Pass a PSR-16 cache too.'); + } $this->baseUrl = \rtrim($baseUrl, '/'); $timer = new SystemTimer(); $this->sender = new Sender($transport, $timeoutMs ?? 8000, $maxRetries, $timer); @@ -94,6 +112,9 @@ public function __construct( $maxRetries, $timer, ); + $this->cache = $cache !== null && $cacheTtlMs > 0 + ? new ResultCache($cache, $cacheTtlMs, $timer) + : null; } /** @@ -123,9 +144,18 @@ public function lookup(string|Postcode $code, int $level = 1): LookupResult } $postcode = self::fullPostcode($code); $query = ['code' => $postcode->canonical, 'level' => $level]; - $response = $this->send($this->sender, '/v1/lookup', $query, retryTimeouts: true); + $read = static fn(mixed $response): LookupResult => LookupResult::fromResponse( + $response, + $postcode, + $level, + ); - return LookupResult::fromResponse($response, $postcode, $level); + return $this->call( + $this->sender, + $this->url('/v1/lookup', $query), + $read, + retryTimeouts: true, + ); } /** @@ -161,9 +191,14 @@ public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): Re 0, ); } - $response = $this->send($this->sender, '/v1/search/reverse', $query, retryTimeouts: true); + $read = ReverseResult::fromResponse(...); - return ReverseResult::fromResponse($response); + return $this->call( + $this->sender, + $this->url('/v1/search/reverse', $query), + $read, + retryTimeouts: true, + ); } /** @@ -194,14 +229,30 @@ public function autocomplete(string $q): AutocompleteResult . 'hyphens are removed. Send no request until the user types one.', ); } - $response = $this->send( + $read = static fn(mixed $response): AutocompleteResult => AutocompleteResult::fromResponse( + $response, + $typed, + ); + + return $this->call( $this->autocompleteSender, - '/v1/search/autocomplete', - ['q' => $typed], + $this->url('/v1/search/autocomplete', ['q' => $typed]), + $read, retryTimeouts: false, ); + } - return AutocompleteResult::fromResponse($response, $typed); + /** + * Removes every result that the client kept. The caller's other cache entries stay. A request + * that started before this call keeps no result. + * + * ```php + * $client->clearCache(); + * ``` + */ + public function clearCache(): void + { + $this->cache?->clear(); } /** @@ -247,17 +298,49 @@ private static function decimal(string $name, float $number, int $max, ?int $min /** * @param array $query + */ + private function url(string $path, array $query): string + { + return $this->baseUrl . $path . '?' . \http_build_query($query); + } + + /** + * Gives the kept result of the call, or sends it, reads the response and keeps the data. + * The client keeps no error, and it keeps data only after it read the data without one. + * + * @template T + * + * @param Closure(mixed): T $read + * + * @return T * * @throws PostcodeException */ - private function send(Sender $sender, string $path, array $query, bool $retryTimeouts): mixed - { - $url = $this->baseUrl . $path . '?' . \http_build_query($query); + private function call( + Sender $sender, + string $url, + Closure $read, + bool $retryTimeouts, + ): mixed { $request = $this->requestFactory->createRequest('GET', $url); if ($this->apiKey !== null) { $request = $request->withHeader('X-API-Key', $this->apiKey); } + $cache = $this->cache; + if ($cache === null) { + return $read($sender->send($request, $retryTimeouts)); + } + // The query holds the canonical postcode and the level. Two keys can hold different + // levels, and a cache can serve two clients, so each API key keeps its own results. + $slot = $cache->slot($url . ' ' . ($this->apiKey ?? '')); + $kept = $cache->get($slot); + if ($kept !== null) { + return $read($kept); + } + $response = $sender->send($request, $retryTimeouts); + $result = $read($response); + $cache->put($slot, $response); - return $sender->send($request, $retryTimeouts); + return $result; } } diff --git a/tests/Unit/Client/ArrayCache.php b/tests/Unit/Client/ArrayCache.php new file mode 100644 index 0000000..1d4f6bf --- /dev/null +++ b/tests/Unit/Client/ArrayCache.php @@ -0,0 +1,100 @@ + */ + public array $entries = []; + + /** @var array */ + public array $ttls = []; + + public function get(string $key, mixed $default = null): mixed + { + return \array_key_exists($key, $this->entries) ? $this->entries[$key] : $default; + } + + public function set(string $key, mixed $value, DateInterval|int|null $ttl = null): bool + { + $this->entries[$key] = $value; + $this->ttls[$key] = $ttl; + + return true; + } + + public function delete(string $key): bool + { + unset($this->entries[$key], $this->ttls[$key]); + + return true; + } + + public function clear(): bool + { + $this->entries = []; + $this->ttls = []; + + return true; + } + + /** + * @param iterable $keys + * + * @return array + */ + public function getMultiple(iterable $keys, mixed $default = null): iterable + { + $values = []; + foreach ($keys as $key) { + $values[$key] = $this->get($key, $default); + } + + return $values; + } + + /** + * @param iterable $values + */ + public function setMultiple(iterable $values, DateInterval|int|null $ttl = null): bool + { + foreach ($values as $key => $value) { + if (!\is_string($key)) { + throw new InvalidArgumentException('PSR-16 keys are text.'); + } + $this->set($key, $value, $ttl); + } + + return true; + } + + /** + * @param iterable $keys + */ + public function deleteMultiple(iterable $keys): bool + { + foreach ($keys as $key) { + $this->delete($key); + } + + return true; + } + + public function has(string $key): bool + { + return \array_key_exists($key, $this->entries); + } +} diff --git a/tests/Unit/Client/CacheTest.php b/tests/Unit/Client/CacheTest.php new file mode 100644 index 0000000..fd7a694 --- /dev/null +++ b/tests/Unit/Client/CacheTest.php @@ -0,0 +1,154 @@ +lookup(FixtureGateway::UNIT); + $client->lookup(FixtureGateway::UNIT); + + self::assertCount(2, $gateway->requests); + self::assertSame([], $store->entries); + } + + #[Test] + public function answersASecondIdenticalCallFromTheCache(): void + { + $gateway = new FixtureGateway(); + $client = self::cachingClient($gateway, new ArrayCache()); + + $first = $client->lookup(FixtureGateway::UNIT); + $second = $client->lookup('fc01z99zz01'); + $client->reverse(9.0, 7.0); + $client->reverse(9.0, 7.0); + + self::assertCount(2, $gateway->requests); + self::assertEquals($first, $second); + } + + #[Test] + public function keepsEachLevelAndEachKeyApart(): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $client = self::cachingClient($gateway, $store); + $other = self::cachingClient($gateway, $store, 'nipost_test_mock_l2'); + + $client->lookup(FixtureGateway::UNIT, level: 1); + $client->lookup(FixtureGateway::UNIT, level: 2); + $other->lookup(FixtureGateway::UNIT, level: 1); + + self::assertCount(3, $gateway->requests); + } + + #[Test] + public function keepsNoError(): void + { + $transport = new ScriptedTransport([ + ScriptedTransport::error(400, 'invalid_request'), + FixtureGateway::fixture('lookup/valid-level-1'), + ]); + $client = self::cachingClient($transport, new ArrayCache()); + + try { + $client->lookup(FixtureGateway::UNIT); + self::fail('The first lookup did not fail.'); + } catch (PostcodeException) { + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + } + self::assertCount(2, $transport->requests); + } + + #[Test] + public function sendsARequestAgainAfterClearCache(): void + { + $gateway = new FixtureGateway(); + $client = self::cachingClient($gateway, new ArrayCache()); + + $client->lookup(FixtureGateway::UNIT); + $client->clearCache(); + $client->lookup(FixtureGateway::UNIT); + + self::assertCount(2, $gateway->requests); + } + + #[Test] + public function keepsNoResultOfARequestThatStartedBeforeClearCache(): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + // The transport calls clearCache() while the request is in flight, as a fiber can. + $transport = new class ($gateway) implements ClientInterface { + public ?PostcodeClient $client = null; + + public function __construct(private readonly ClientInterface $inner) {} + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $this->client?->clearCache(); + + return $this->inner->sendRequest($request); + } + }; + $client = self::cachingClient($transport, $store); + $transport->client = $client; + + $client->lookup(FixtureGateway::UNIT); + $client->lookup(FixtureGateway::UNIT); + + self::assertCount(2, $gateway->requests); + } + + #[Test] + public function sharesTheResultsOfTwoClientsWithTheSameKeyAndCache(): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + + self::cachingClient($gateway, $store)->lookup(FixtureGateway::UNIT); + self::cachingClient($gateway, $store)->lookup(FixtureGateway::UNIT); + + self::assertCount(1, $gateway->requests); + } + + private static function cachingClient( + ClientInterface $transport, + ArrayCache $store, + string $apiKey = 'nipost_test_mock_l3', + ): PostcodeClient { + return new PostcodeClient( + $transport, + new HttpFactory(), + apiKey: $apiKey, + cacheTtlMs: self::TTL_MS, + cache: $store, + ); + } +} diff --git a/tests/Unit/Client/ClientOptionsTest.php b/tests/Unit/Client/ClientOptionsTest.php index 223745c..ae54ef8 100644 --- a/tests/Unit/Client/ClientOptionsTest.php +++ b/tests/Unit/Client/ClientOptionsTest.php @@ -59,6 +59,22 @@ public static function badOptions(): array ), 'maxRetries is -1. Use 0 or more.', ], + 'a negative cache time' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + cacheTtlMs: -1, + ), + 'cacheTtlMs is -1. Use 0 or more.', + ], + 'a cache time with no cache' => [ + static fn(): PostcodeClient => new PostcodeClient( + $gateway, + $factory, + cacheTtlMs: 1000, + ), + 'cacheTtlMs is above 0. Pass a PSR-16 cache too.', + ], 'an empty key' => [ static fn(): PostcodeClient => new PostcodeClient( $gateway, diff --git a/tests/Unit/Client/ResultCacheTest.php b/tests/Unit/Client/ResultCacheTest.php new file mode 100644 index 0000000..9f5d32a --- /dev/null +++ b/tests/Unit/Client/ResultCacheTest.php @@ -0,0 +1,136 @@ + 'FC-01-Z99-ZZ-01', 'valid' => true]; + + private ArrayCache $store; + + private FakeTimer $timer; + + private ResultCache $cache; + + protected function setUp(): void + { + $this->store = new ArrayCache(); + $this->timer = new FakeTimer(); + $this->cache = new ResultCache($this->store, 1500, $this->timer); + } + + #[Test] + public function givesBackTheDataUntilTheTtlEnds(): void + { + $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->timer->advance(1499); + + self::assertSame(self::DATA, $this->cache->get($this->cache->slot(self::CALL))); + + $this->timer->advance(1); + + self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + } + + /** + * @return array + */ + public static function ttls(): array + { + return [ + 'one millisecond' => [1, 1], + 'just over one second' => [1001, 2], + 'two seconds' => [2000, 2], + ]; + } + + #[Test] + #[DataProvider('ttls')] + public function asksTheCacheForWholeSecondsRoundedUp(int $ttlMs, int $seconds): void + { + $cache = new ResultCache($this->store, $ttlMs, $this->timer); + + $cache->put($cache->slot(self::CALL), self::DATA); + + $resultTtls = \array_filter($this->store->ttls, static fn($ttl): bool => $ttl !== null); + self::assertSame([$seconds], \array_values($resultTtls)); + } + + #[Test] + public function keepsEachCallUnderAKeyThatPsr16Allows(): void + { + $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->cache->put($this->cache->slot(self::CALL . ' other'), self::DATA); + + $keys = \array_keys($this->store->entries); + self::assertCount(3, $keys); + self::assertContains('gatepost.generation', $keys); + foreach (\array_diff($keys, ['gatepost.generation']) as $key) { + // A hash of the call, so that no key holds the postcode or the API key. + self::assertMatchesRegularExpression('/\Agatepost\.[0-9a-f]{8}\.[0-9a-f]{40}\z/', $key); + } + } + + #[Test] + public function hidesEveryKeptResultAfterClearAndKeepsTheOtherEntries(): void + { + $this->store->set('app.session', 'kept'); + $this->cache->put($this->cache->slot(self::CALL), self::DATA); + + $this->cache->clear(); + + self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertSame('kept', $this->store->get('app.session')); + } + + #[Test] + public function keepsNoResultOfACallThatStartedBeforeClear(): void + { + $slot = $this->cache->slot(self::CALL); + + $this->cache->clear(); + $this->cache->put($slot, self::DATA); + + self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + } + + #[Test] + public function hidesTheOldResultsWhenTheCacheEvictsTheGeneration(): void + { + $this->cache->put($this->cache->slot(self::CALL), self::DATA); + + $this->store->delete('gatepost.generation'); + + self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + } + + #[Test] + public function readsAnEntryOfAnotherShapeAsNothingKept(): void + { + $this->cache->put($this->cache->slot(self::CALL), self::DATA); + foreach (\array_keys($this->store->entries) as $key) { + if ($key !== 'gatepost.generation') { + $this->store->set($key, ['data' => self::DATA]); + } + } + + self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + } +} From a0ab8392b739620fb7de51e59f397f153ac8ddcf Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:46:15 +0100 Subject: [PATCH 08/21] test(client): run the contract scenarios against the mock server Every client must pass the contract scenarios of the spec against the mock server, so the PHP client gives the errors, waits and outcomes of the TypeScript client. The contract job runs the mock server from the js repo with this repo's own spec, on PHP 8.1 and the newest PHP. The synchronous client never has two requests in flight, so the runner leaves out each scenario whose order is parallel. A unit test in the normal check reads the scenario files and checks that rule. Signed-off-by: MobileMage --- .github/workflows/ci.yml | 49 ++++++ README.md | 9 ++ composer.json | 2 + phpunit.xml.dist | 8 +- tests/Contract/MockServerTestCase.php | 32 ++++ tests/Contract/Outcome.php | 128 +++++++++++++++ tests/Contract/RecordingTransport.php | 70 +++++++++ tests/Contract/Scenario.php | 192 +++++++++++++++++++++++ tests/Contract/ScenarioTest.php | 209 +++++++++++++++++++++++++ tests/Unit/Client/ScenarioListTest.php | 60 +++++++ 10 files changed, 758 insertions(+), 1 deletion(-) create mode 100644 tests/Contract/MockServerTestCase.php create mode 100644 tests/Contract/Outcome.php create mode 100644 tests/Contract/RecordingTransport.php create mode 100644 tests/Contract/Scenario.php create mode 100644 tests/Contract/ScenarioTest.php create mode 100644 tests/Unit/Client/ScenarioListTest.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed24c10..a2826af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,3 +104,52 @@ jobs: require "vendor/autoload.php"; echo Gatepost\Postcode\Postcode::parse("ek 01 a03 fk 01")->value->canonical; ' | grep -x 'EK-01-A03-FK-01' + + # The client must pass each contract scenario of the spec against the mock server. The mock + # server lives in the js repo and runs on Node with no install. It reads the fixtures and the + # scenarios of this repo's own spec submodule, so only its code comes from js. + contract: + runs-on: ubuntu-24.04 + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.5'] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: true + persist-credentials: false + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: gatepost-dev/js + # A js commit that holds packages/mock-server. Move it when the mock server changes. + ref: 2ee026be7be1272f8d58a3d649722000e6335fdd + path: build/js + sparse-checkout: packages/mock-server + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + - uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: ${{ matrix.php }} + coverage: none + extensions: :intl + - run: composer install --no-interaction --no-progress + - name: Use the newest PHPUnit on the newest PHP + if: matrix.php != '8.1' + run: | + composer config --unset platform.php + composer update --no-interaction --no-progress --with-all-dependencies phpunit/phpunit + - name: Start the mock server + run: | + GATEPOST_SPEC_DIR="$PWD/spec" PORT=4010 \ + node build/js/packages/mock-server/src/main.ts > "$RUNNER_TEMP/mock.log" 2>&1 & + curl --silent --show-error --retry 10 --retry-connrefused --retry-delay 1 \ + http://127.0.0.1:4010/healthz + - run: composer contract + env: + GATEPOST_MOCK_URL: http://127.0.0.1:4010 + - if: failure() + run: cat "$RUNNER_TEMP/mock.log" diff --git a/README.md b/README.md index 3c1fbce..eed27a2 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,15 @@ composer --working-dir=tools/bc-check install --ignore-platform-req=ext-intl composer check ``` +`composer check` runs no contract scenario, because the scenarios need the mock server, which runs on Node 22.22.2 or later. CI runs them in its `contract` job. To run them yourself, clone the js repo next to this one, start its mock server with this repo's spec, and run `composer contract`: + +```sh +git clone --depth 1 https://github.com/gatepost-dev/js ../js +GATEPOST_SPEC_DIR="$PWD/spec" PORT=4010 node ../js/packages/mock-server/src/main.ts & +GATEPOST_MOCK_URL=http://127.0.0.1:4010 composer contract +kill %1 +``` + The BC check tool lists ext-intl as a requirement. Its check runs without that extension, so the install line ignores the requirement. The line also works on a machine that has ext-intl. ## Licence diff --git a/composer.json b/composer.json index affe6e3..e1ffd6a 100644 --- a/composer.json +++ b/composer.json @@ -66,6 +66,7 @@ "@tells" ], "context:check": "cmp CONTEXT.md spec/CONTEXT.md", + "contract": "phpunit --testsuite contract", "cs": "php-cs-fixer fix", "cs:check": "php-cs-fixer fix --dry-run --diff", "deps:licences": "@php scripts/check-licences.php", @@ -95,6 +96,7 @@ "changes:check": "Check that CHANGELOG.md matches the change files.", "check": "Run every check that CI runs.", "context:check": "Check that CONTEXT.md matches the copy in the spec.", + "contract": "Run the contract scenarios against the mock server at GATEPOST_MOCK_URL.", "cs": "Fix the code style.", "cs:check": "Check the code style.", "deps:licences": "Check that each locked package has a permissive licence (DEP-4).", diff --git a/phpunit.xml.dist b/phpunit.xml.dist index c8eeccb..e45fca9 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -5,6 +5,7 @@ xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="vendor/autoload.php" cacheDirectory="build/phpunit" + defaultTestSuite="unit" colors="true" executionOrder="random" failOnDeprecation="true" @@ -17,8 +18,13 @@ - + tests + tests/Contract + + + + tests/Contract diff --git a/tests/Contract/MockServerTestCase.php b/tests/Contract/MockServerTestCase.php new file mode 100644 index 0000000..281dc3b --- /dev/null +++ b/tests/Contract/MockServerTestCase.php @@ -0,0 +1,32 @@ +mockUrl = $mockUrl; + } +} diff --git a/tests/Contract/Outcome.php b/tests/Contract/Outcome.php new file mode 100644 index 0000000..2cc6aad --- /dev/null +++ b/tests/Contract/Outcome.php @@ -0,0 +1,128 @@ + + */ + public static function of(LookupResult|ReverseResult|AutocompleteResult $result): array + { + return match (true) { + $result instanceof LookupResult => self::lookup($result), + $result instanceof ReverseResult => self::reverse($result), + default => self::autocomplete($result), + }; + } + + /** + * @return array{code: string, status: ?int, apiCode: ?string, retryAfterMs: ?int} + */ + public static function error(PostcodeException $error): array + { + return [ + 'code' => $error->errorCode()->value, + 'status' => $error->status(), + 'apiCode' => $error->apiCode(), + 'retryAfterMs' => $error->retryAfterMs(), + ]; + } + + /** + * @return array + */ + private static function lookup(LookupResult $result): array + { + $address = $result->administrativeAddress; + + return [ + 'postcode' => $result->postcode->canonical, + 'valid' => $result->valid, + 'status' => $result->status, + 'levelRequested' => $result->levelRequested, + 'levelReceived' => $result->levelReceived, + 'administrativeAddress' => $address === null ? null : [ + 'stateName' => $address->stateName, + 'lgaName' => $address->lgaName, + 'localityName' => $address->localityName, + 'zone' => $address->zone, + ], + 'recentHouseAddress' => $result->recentHouseAddress, + 'buildingUseStatus' => $result->buildingUseStatus, + ]; + } + + /** + * @return array + */ + private static function reverse(ReverseResult $result): array + { + return [ + 'found' => $result->found, + 'radiusM' => self::number($result->radiusM), + 'unit' => $result->unit === null ? null : self::unit($result->unit), + 'area' => $result->area, + 'district' => $result->district, + 'state' => $result->state, + ]; + } + + /** + * @return array + */ + private static function unit(ReverseUnit $unit): array + { + return [ + 'postcode' => $unit->postcode->canonical, + 'distanceM' => self::number($unit->distanceM), + 'confidence' => $unit->confidence->value, + 'stateName' => $unit->stateName, + 'lgaName' => $unit->lgaName, + 'localityName' => $unit->localityName, + 'address' => $unit->address, + ]; + } + + /** + * @return array + */ + private static function autocomplete(AutocompleteResult $result): array + { + return [ + 'segment' => $result->segment->value, + 'suggestions' => \array_map(static fn(Suggestion $suggestion): array => [ + 'code' => $suggestion->code, + 'label' => $suggestion->label, + 'postcode' => $suggestion->postcode?->canonical, + ], $result->suggestions), + ]; + } + + // JSON writes 25 and 25.0 the same, and json_decode() reads 25 as an int, so a whole number + // becomes an int before the comparison. + private static function number(?float $metres): int|float|null + { + if ($metres === null) { + return null; + } + + return \floor($metres) === $metres ? (int) $metres : $metres; + } +} diff --git a/tests/Contract/RecordingTransport.php b/tests/Contract/RecordingTransport.php new file mode 100644 index 0000000..4ba9877 --- /dev/null +++ b/tests/Contract/RecordingTransport.php @@ -0,0 +1,70 @@ + */ + public array $requests = []; + + /** @var list The start of each attempt, in milliseconds. */ + public array $startsMs = []; + + /** @var list The end of each attempt, in milliseconds. */ + public array $endsMs = []; + + public int $maxInFlight = 0; + + private int $inFlight = 0; + + public function __construct( + private readonly ClientInterface $transport, + private readonly string $scenarioId, + private readonly string $run, + ) {} + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $this->requests[] = $request; + $marked = $request + ->withHeader('X-Scenario-Id', $this->scenarioId) + ->withHeader('X-Scenario-Run', $this->run); + ++$this->inFlight; + $this->maxInFlight = \max($this->maxInFlight, $this->inFlight); + $this->startsMs[] = \microtime(true) * 1000; + try { + return $this->transport->sendRequest($marked); + } finally { + $this->endsMs[] = \microtime(true) * 1000; + --$this->inFlight; + } + } + + /** + * Each wait, from the end of one attempt to the start of the next one. + * + * @return list + */ + public function waitsMs(): array + { + $waits = []; + foreach (\array_slice($this->startsMs, 1) as $index => $startMs) { + $waits[] = $startMs - $this->endsMs[$index]; + } + + return $waits; + } +} diff --git a/tests/Contract/Scenario.php b/tests/Contract/Scenario.php new file mode 100644 index 0000000..bfd0bc3 --- /dev/null +++ b/tests/Contract/Scenario.php @@ -0,0 +1,192 @@ + The client options of the scenario. */ + public readonly array $client; + + /** @var list> */ + public readonly array $calls; + + /** @var array */ + public readonly array $expect; + + private function __construct(string $file) + { + $text = (string) \file_get_contents($file); + $scenario = \json_decode($text, true, 512, \JSON_THROW_ON_ERROR); + $id = \is_array($scenario) ? ($scenario['id'] ?? null) : null; + if (!\is_array($scenario) || ($scenario['version'] ?? null) !== 1 || !\is_string($id)) { + throw new UnexpectedValueException(\basename($file) . ' has no version 1 and no id.'); + } + $order = $scenario['order'] ?? null; + $this->id = $id; + $this->order = \is_string($order) ? $order : null; + $this->client = self::options($scenario['client'] ?? null, $id); + $this->calls = self::calls($scenario['calls'] ?? null, $id); + $this->expect = self::fields($scenario['expect'] ?? null, $id); + } + + /** + * Each scenario file, keyed by its id. + * + * @return array + */ + public static function all(): array + { + $files = \glob(\dirname(__DIR__, 2) . '/spec/contract/*.json'); + if ($files === false || $files === []) { + throw new UnexpectedValueException('spec/contract holds no scenario. Check out spec.'); + } + $scenarios = []; + foreach ($files as $file) { + $scenario = new self($file); + $scenarios[$scenario->id] = $scenario; + } + + return $scenarios; + } + + /** + * The scenarios that start their calls at the same time. The client is synchronous, so it + * never has two requests in flight, and these scenarios cannot apply to it. + * + * @return array + */ + public static function parallel(): array + { + return \array_filter( + self::all(), + static fn(self $scenario): bool => $scenario->isParallel(), + ); + } + + /** + * Every scenario that the client can run. + * + * @return array + */ + public static function runnable(): array + { + return \array_filter( + self::all(), + static fn(self $scenario): bool => !$scenario->isParallel(), + ); + } + + public function isParallel(): bool + { + return $this->order === 'parallel'; + } + + /** + * The outcomes of the scenario, one for each call. + * + * @return list + */ + public function outcomes(): array + { + $outcomes = $this->expect['outcomes'] ?? null; + if (!\is_array($outcomes) || !\array_is_list($outcomes)) { + throw new UnexpectedValueException("{$this->id} has no list of outcomes."); + } + + return $outcomes; + } + + /** + * The number of requests that the scenario expects. + */ + public function attempts(): int + { + $attempts = $this->expect['attempts'] ?? null; + if (!\is_int($attempts)) { + throw new UnexpectedValueException("{$this->id} has no number of attempts."); + } + + return $attempts; + } + + /** + * The bounds of each wait between two attempts, or an empty list. + * + * @return list + */ + public function waits(): array + { + $waits = []; + foreach ((array) ($this->expect['waitsMs'] ?? []) as $wait) { + $min = \is_array($wait) ? ($wait['min'] ?? null) : null; + $max = \is_array($wait) ? ($wait['max'] ?? null) : null; + if (!\is_int($min) || !\is_int($max)) { + throw new UnexpectedValueException("{$this->id} has a wait with no bounds."); + } + $waits[] = ['min' => $min, 'max' => $max]; + } + + return $waits; + } + + /** + * @return array + */ + private static function options(mixed $client, string $id): array + { + $options = []; + foreach (self::fields($client, $id) as $name => $value) { + if (!\is_int($value) && !\is_string($value)) { + throw new UnexpectedValueException("{$id} has a client option that is not valid."); + } + $options[$name] = $value; + } + + return $options; + } + + /** + * @return list> + */ + private static function calls(mixed $calls, string $id): array + { + if (!\is_array($calls) || !\array_is_list($calls)) { + throw new UnexpectedValueException("{$id} has no list of calls."); + } + + return \array_map(static fn(mixed $call): array => self::fields($call, $id), $calls); + } + + /** + * @return array + */ + private static function fields(mixed $object, string $id): array + { + if (!\is_array($object)) { + throw new UnexpectedValueException("{$id} has a value that is not an object."); + } + $fields = []; + foreach ($object as $name => $value) { + $fields[(string) $name] = $value; + } + + return $fields; + } +} diff --git a/tests/Contract/ScenarioTest.php b/tests/Contract/ScenarioTest.php new file mode 100644 index 0000000..43c634b --- /dev/null +++ b/tests/Contract/ScenarioTest.php @@ -0,0 +1,209 @@ + + */ + public static function scenarios(): array + { + return \array_map( + static fn(Scenario $scenario): array => [$scenario], + Scenario::runnable(), + ); + } + + #[Test] + #[DataProvider('scenarios')] + public function reachesTheOutcomeOfTheScenario(Scenario $scenario): void + { + $option = $scenario->client['timeoutMs'] ?? null; + $timeoutMs = $option === null ? null : (int) $option; + $transport = new RecordingTransport( + new Client(['timeout' => ($timeoutMs ?? self::defaultTimeoutMs($scenario)) / 1000]), + $scenario->id, + \bin2hex(\random_bytes(8)), + ); + $client = $this->client($scenario, $transport, $timeoutMs); + + $outcomes = \array_map( + static fn(array $call): array => self::outcome($client, $call), + $scenario->calls, + ); + + self::assertSame($scenario->outcomes(), $outcomes); + self::assertCount($scenario->attempts(), $transport->requests, 'The number of attempts'); + self::assertWaits($scenario->waits(), $transport->waitsMs()); + self::assertFirstRequest($scenario, $transport); + self::assertSame( + $scenario->expect['maxInFlight'] ?? $transport->maxInFlight, + $transport->maxInFlight, + 'The requests in flight', + ); + } + + private function client( + Scenario $scenario, + RecordingTransport $transport, + ?int $timeoutMs, + ): PostcodeClient { + $options = $scenario->client; + $apiKey = $options['apiKey'] ?? null; + $cacheTtlMs = (int) ($options['cacheTtlMs'] ?? 0); + + return new PostcodeClient( + $transport, + new HttpFactory(), + apiKey: \is_string($apiKey) ? $apiKey : null, + baseUrl: $this->mockUrl, + timeoutMs: $timeoutMs, + maxRetries: (int) ($options['maxRetries'] ?? 2), + cacheTtlMs: $cacheTtlMs, + cache: $cacheTtlMs > 0 ? new ArrayCache() : null, + ); + } + + // Without the option, the client waits 15 s for autocomplete and 8 s for the other calls. + private static function defaultTimeoutMs(Scenario $scenario): int + { + foreach ($scenario->calls as $call) { + if ($call['method'] !== 'autocomplete') { + return 8000; + } + } + + return 15000; + } + + /** + * @param array $call + * + * @return array + */ + private static function outcome(PostcodeClient $client, array $call): array + { + try { + return ['result' => Outcome::of(self::make($client, $call))]; + } catch (PostcodeException $error) { + return ['error' => Outcome::error($error)]; + } + } + + /** + * @param array $call + */ + private static function make( + PostcodeClient $client, + array $call, + ): LookupResult|ReverseResult|AutocompleteResult { + $maxDistanceM = $call['maxDistanceM'] ?? null; + + return match ($call['method']) { + 'lookup' => $client->lookup(self::text($call, 'code'), self::level($call)), + 'reverse' => $client->reverse( + self::number($call, 'lat'), + self::number($call, 'lng'), + $maxDistanceM === null ? null : self::number($call, 'maxDistanceM'), + ), + 'autocomplete' => $client->autocomplete(self::text($call, 'q')), + default => throw new UnexpectedValueException('The call names no known method.'), + }; + } + + /** + * @param list $bounds + * @param list $waitsMs + */ + private static function assertWaits(array $bounds, array $waitsMs): void + { + if ($bounds === []) { + return; + } + self::assertCount(\count($bounds), $waitsMs, 'The number of waits'); + foreach ($bounds as $index => $bound) { + $wait = \sprintf('Wait %d took %.1f ms.', $index + 1, $waitsMs[$index]); + self::assertGreaterThanOrEqual($bound['min'], $waitsMs[$index], $wait); + $latest = $bound['max'] + self::LATE_TIMER_MS; + self::assertLessThanOrEqual($latest, $waitsMs[$index], $wait); + } + } + + private static function assertFirstRequest( + Scenario $scenario, + RecordingTransport $transport, + ): void { + $expected = $scenario->expect['request'] ?? null; + if (!\is_array($expected)) { + return; + } + $request = $transport->requests[0]; + \parse_str($request->getUri()->getQuery(), $query); + $apiKey = $request->hasHeader('X-API-Key') ? $request->getHeaderLine('X-API-Key') : null; + + self::assertSame($expected, [ + 'method' => $request->getMethod(), + 'path' => $request->getUri()->getPath(), + 'query' => $query, + 'apiKey' => $apiKey, + ]); + } + + /** + * @param array $call + */ + private static function text(array $call, string $name): string + { + $value = $call[$name] ?? null; + + return \is_string($value) ? $value : throw new UnexpectedValueException("No {$name}."); + } + + /** + * @param array $call + */ + private static function level(array $call): int + { + $level = $call['level'] ?? 1; + + return \is_int($level) ? $level : throw new UnexpectedValueException('No level.'); + } + + /** + * @param array $call + */ + private static function number(array $call, string $name): int|float + { + $value = $call[$name] ?? null; + + return \is_int($value) || \is_float($value) + ? $value + : throw new UnexpectedValueException("No {$name}."); + } +} diff --git a/tests/Unit/Client/ScenarioListTest.php b/tests/Unit/Client/ScenarioListTest.php new file mode 100644 index 0000000..d8fa441 --- /dev/null +++ b/tests/Unit/Client/ScenarioListTest.php @@ -0,0 +1,60 @@ +order, $scenario->id); + } + } + + #[Test] + public function leavesOutOnlyScenariosWithSeveralCalls(): void + { + foreach (Scenario::parallel() as $scenario) { + self::assertGreaterThan(1, \count($scenario->calls), $scenario->id); + } + } + + #[Test] + public function runsNoScenarioThatNeedsRequestsInFlightTogether(): void + { + foreach (Scenario::runnable() as $scenario) { + self::assertLessThanOrEqual(1, $scenario->expect['maxInFlight'] ?? 1, $scenario->id); + } + } +} From 6d96c76c91e41758bd89269be7c45e723fd2bfe5 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 03:54:36 +0100 Subject: [PATCH 09/21] fix(client): read autocomplete timeouts per call and refuse bad text The client reads a failed attempt as a timeout when it took timeoutMs or more, and as a network error when it took less. With two defaults, 8 s and 15 s, a caller who set the documented 8 s on the transport got a network error from autocomplete and three attempts. The docs now say that the transport timeout must be the longest in use, and tests pin the limits at 14999 and 15000 ms for autocomplete and 7999 and 8000 ms for a lookup. An optional clock lets the tests run with no sleep and no reflection. A JSON object that holds no field is now kept apart from an empty list, so suggestions with the value {} give an unexpected_response, as in the other clients. Text over the input limit and text that is not UTF-8 now get their own message, because the old one named a rule that the text had not broken. Signed-off-by: MobileMage --- docs/api-dump.md | 1 - src/Client/Internal/Fields.php | 20 +-- src/Client/Internal/Sender.php | 23 +++- src/Client/PostcodeClient.php | 39 ++++-- src/Client/Suggestion.php | 3 +- tests/Unit/Client/AutocompleteEdgeTest.php | 78 +++++++++++ tests/Unit/Client/AutocompleteTimeoutTest.php | 126 +++++++++++------- tests/Unit/Client/FieldsTest.php | 16 ++- tests/Unit/Client/LookupResultLenientTest.php | 9 +- 9 files changed, 241 insertions(+), 74 deletions(-) create mode 100644 tests/Unit/Client/AutocompleteEdgeTest.php diff --git a/docs/api-dump.md b/docs/api-dump.md index b15e073..eddf04d 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -54,7 +54,6 @@ final class LookupResult final class PostcodeClient { - public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', ?int $timeoutMs = null, int $maxRetries = 2, int $cacheTtlMs = 0, ?\Psr\SimpleCache\CacheInterface $cache = null); public function lookup(\Gatepost\Postcode\Postcode|string $code, int $level = 1): LookupResult; public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): ReverseResult; public function autocomplete(string $q): AutocompleteResult; diff --git a/src/Client/Internal/Fields.php b/src/Client/Internal/Fields.php index 7a1b630..b61c12c 100644 --- a/src/Client/Internal/Fields.php +++ b/src/Client/Internal/Fields.php @@ -8,6 +8,7 @@ namespace Gatepost\Postcode\Client\Internal; use Gatepost\Postcode\Client\PostcodeException; +use stdClass; /** * Reads the fields of a decoded response body (PHP-14). A reader takes only the fields that it @@ -19,8 +20,9 @@ final class Fields { /** - * A JSON object whose keys are only 0, 1, 2 and so on reads as a list, so it fails here too. - * No shape of the gateway has such keys. + * The Sender decodes a JSON object with fields to an array, and an empty JSON object to a + * stdClass, so that an empty object and an empty list stay apart. A JSON object whose keys + * are only 0, 1, 2 and so on still reads as a list. No shape of the gateway has such keys. * * @return array * @@ -28,11 +30,9 @@ final class Fields */ public static function object(mixed $value): array { - if (!\is_array($value) || ($value !== [] && \array_is_list($value))) { - throw PostcodeException::unreadable(200); - } + $fields = self::objectOrNull(['value' => $value], 'value'); - return $value; + return $fields ?? throw PostcodeException::unreadable(200); } /** @@ -45,8 +45,11 @@ public static function object(mixed $value): array public static function objectOrNull(array $fields, string $name): ?array { $value = $fields[$name] ?? null; + if ($value instanceof stdClass) { + return []; + } - return \is_array($value) && ($value === [] || !\array_is_list($value)) ? $value : null; + return \is_array($value) && $value !== [] && !\array_is_list($value) ? $value : null; } /** @@ -69,7 +72,8 @@ public static function bool(array $fields, string $name): bool * * @return list * - * @throws PostcodeException When the field is missing or is not a JSON array. + * @throws PostcodeException When the field is missing or is not a JSON array. A JSON object + * whose keys are 0, 1, 2 and so on reads as a list. */ public static function list(array $fields, string $name): array { diff --git a/src/Client/Internal/Sender.php b/src/Client/Internal/Sender.php index faf676a..7a10106 100644 --- a/src/Client/Internal/Sender.php +++ b/src/Client/Internal/Sender.php @@ -15,6 +15,7 @@ use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use stdClass; /** * Sends one call as one or more attempts, with the retries and waits of spec/client.md. A PSR-18 @@ -144,7 +145,7 @@ private static function backoffBaseMs(int $retry): int private static function dataField(ResponseInterface $response, ?int $retryAfterMs): mixed { $status = $response->getStatusCode(); - $body = \json_decode((string) $response->getBody(), true); + $body = self::decode((string) $response->getBody()); if ($status !== 200) { throw PostcodeException::fromStatus($status, self::apiCode($body), $retryAfterMs); } @@ -155,6 +156,26 @@ private static function dataField(ResponseInterface $response, ?int $retryAfterM return $body['data']; } + /** + * Decodes JSON to arrays, but keeps an empty JSON object as a stdClass. PHP reads {} and [] + * as the same empty array, and Fields needs to tell them apart. + */ + private static function decode(string $json): mixed + { + return self::arrays(\json_decode($json)); + } + + private static function arrays(mixed $value): mixed + { + if ($value instanceof stdClass) { + $fields = \get_object_vars($value); + + return $fields === [] ? $value : \array_map(self::arrays(...), $fields); + } + + return \is_array($value) ? \array_map(self::arrays(...), $value) : $value; + } + /** * The error.code of an error body. The client never fails on an unexpected error body. */ diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index 87b1676..de9bd7d 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -12,6 +12,7 @@ use Gatepost\Postcode\Client\Internal\ResultCache; use Gatepost\Postcode\Client\Internal\Sender; use Gatepost\Postcode\Client\Internal\SystemTimer; +use Gatepost\Postcode\Client\Internal\Timer; use Gatepost\Postcode\Internal\SpecData; use Gatepost\Postcode\Postcode; use Gatepost\Postcode\Precision; @@ -29,7 +30,7 @@ * * ```php * $client = new PostcodeClient( - * new \GuzzleHttp\Client(['timeout' => 8]), + * new \GuzzleHttp\Client(['timeout' => 15]), * new \GuzzleHttp\Psr7\HttpFactory(), * apiKey: \getenv('NIPOST_API_KEY') ?: null, * ); @@ -53,8 +54,13 @@ final class PostcodeClient private readonly ?ResultCache $cache; /** - * @param ClientInterface $transport A PSR-18 HTTP client. Set its timeout to - * timeoutMs, because PSR-18 has no timeout. + * @param ClientInterface $transport A PSR-18 HTTP client. PSR-18 has no timeout, + * so set the timeout of the transport to the + * longest timeout in use, which is 15 s with + * the defaults. The client reads a failed + * attempt as a timeout when it took timeoutMs + * or more, and as a network_error when it + * took less. * @param RequestFactoryInterface $requestFactory A PSR-17 factory for the requests. * @param ?string $apiKey The key for the X-API-Key header. With null, * the client sends no key, and the gateway @@ -63,12 +69,15 @@ final class PostcodeClient * @param ?int $timeoutMs The longest wait for one attempt. A failed * attempt that took this long is a timeout. * With null, it is 8000, and 15000 for - * autocomplete. + * autocomplete. Set the timeout of the + * transport to this value or more. * @param int $maxRetries The most retries after the first attempt. * @param int $cacheTtlMs How long the client keeps a result. 0 turns * the cache off. * @param ?CacheInterface $cache A PSR-16 cache for the results. The client * needs one when cacheTtlMs is above 0. + * @param ?Timer $timer The clock and the waits of the client, for + * tests. @internal Leave it null. * * @throws InvalidArgumentException When an option is out of range, the key is empty, or * cacheTtlMs is above 0 with no cache. @@ -87,6 +96,7 @@ public function __construct( int $maxRetries = 2, int $cacheTtlMs = 0, ?CacheInterface $cache = null, + ?Timer $timer = null, ) { if ($apiKey === '') { throw new InvalidArgumentException('apiKey is empty. Pass null to send no key.'); @@ -104,7 +114,7 @@ public function __construct( throw new InvalidArgumentException('cacheTtlMs is above 0. Pass a PSR-16 cache too.'); } $this->baseUrl = \rtrim($baseUrl, '/'); - $timer = new SystemTimer(); + $timer ??= new SystemTimer(); $this->sender = new Sender($transport, $timeoutMs ?? 8000, $maxRetries, $timer); $this->autocompleteSender = new Sender( $transport, @@ -205,7 +215,9 @@ public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): Re * Suggests ways to complete the segment that the user types. The client normalises the text * with Postcode::normalize(). It sends no request for text that is empty, longer than a * postcode, or holds a character other than A to Z and 0 to 9. A timeout ends the call - * without a retry, because the next keystroke replaces it. + * without a retry, because the next keystroke replaces it. The call waits 15 s by default, + * so the transport needs a timeout of 15 s or more. With a shorter transport timeout, the + * failure reads as a network_error, and the client retries it. * * ```php * $result = $client->autocomplete('fc01z'); @@ -221,8 +233,19 @@ public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): Re public function autocomplete(string $q): AutocompleteResult { // Like the other clients, refuse text over the input limit of the core before the - // normalisation, which has no limit of its own. A string that is not UTF-8 fails here too. - $typed = \preg_match(self::WITHIN_INPUT_LIMIT, $q) === 1 ? Postcode::normalize($q) : ''; + // normalisation, which has no limit of its own. preg_match gives false for a string + // that is not UTF-8. + $within = \preg_match(self::WITHIN_INPUT_LIMIT, $q); + if ($within === false) { + throw PostcodeException::invalidInput('The text is not valid UTF-8.'); + } + if ($within === 0) { + throw PostcodeException::invalidInput( + 'The text has more than ' . SpecData::MAX_INPUT_CODE_POINTS . ' characters. ' + . 'Send no request for it.', + ); + } + $typed = Postcode::normalize($q); if (\preg_match(self::AUTOCOMPLETE_TEXT, $typed) !== 1) { throw PostcodeException::invalidInput( 'The text must hold 1 to 11 letters A to Z and digits 0 to 9, after spaces and ' diff --git a/src/Client/Suggestion.php b/src/Client/Suggestion.php index 09830d3..094f187 100644 --- a/src/Client/Suggestion.php +++ b/src/Client/Suggestion.php @@ -48,7 +48,8 @@ public static function fromFields(mixed $suggestion, Precision $segment, string /** * The typed segments before the active one, followed by the code. The result counts only when - * it parses and ends in the active segment. + * it parses and ends in the active segment. A code of the wrong length can parse as another + * precision: FC01 after F is the LGA, not the state, and that gives null. */ private static function postcodeOf(string $typed, string $code, Precision $segment): ?Postcode { diff --git a/tests/Unit/Client/AutocompleteEdgeTest.php b/tests/Unit/Client/AutocompleteEdgeTest.php new file mode 100644 index 0000000..e45ee0a --- /dev/null +++ b/tests/Unit/Client/AutocompleteEdgeTest.php @@ -0,0 +1,78 @@ +autocomplete('FC' . \str_repeat(' ', 70)); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::InvalidInput, $error->errorCode()); + self::assertStringContainsString('64', $error->getMessage()); + self::assertStringNotContainsString('1 to 11', $error->getMessage()); + } + } + + #[Test] + public function saysThatTextWithBytesThatAreNotUtf8IsNotText(): void + { + $client = new PostcodeClient(new FixtureGateway(), new HttpFactory()); + + try { + $client->autocomplete("FC\xFF"); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertStringContainsString('UTF-8', $error->getMessage()); + } + } + + #[Test] + public function failsWithUnexpectedResponseForSuggestionsThatAreAJsonObject(): void + { + $body = '{"data":{"segment":"state","suggestions":{}}}'; + $transport = new ScriptedTransport([new Response(200, [], $body)]); + $client = new PostcodeClient($transport, new HttpFactory()); + + $this->expectExceptionObject(PostcodeException::unreadable(200)); + + $client->autocomplete('F'); + } + + #[Test] + public function givesNullForACodeThatEndsOutsideTheActiveSegment(): void + { + $district = ['segment' => 'district', 'suggestions' => [['code' => 'Z99ZZ']]]; + $state = ['segment' => 'state', 'suggestions' => [['code' => 'FC01']]]; + + // FC01Z99ZZ and FC01 are postcodes, but of the area and of the LGA, not of the segment. + $inDistrict = AutocompleteResult::fromResponse($district, 'FC01Z'); + $inState = AutocompleteResult::fromResponse($state, 'F'); + + self::assertNull($inDistrict->suggestions[0]->postcode); + self::assertNull($inState->suggestions[0]->postcode); + } +} diff --git a/tests/Unit/Client/AutocompleteTimeoutTest.php b/tests/Unit/Client/AutocompleteTimeoutTest.php index 56f39cc..da1587c 100644 --- a/tests/Unit/Client/AutocompleteTimeoutTest.php +++ b/tests/Unit/Client/AutocompleteTimeoutTest.php @@ -8,77 +8,111 @@ namespace Gatepost\Postcode\Tests\Unit\Client; use Gatepost\Postcode\Client\ErrorCode; -use Gatepost\Postcode\Client\Internal\Sender; use Gatepost\Postcode\Client\PostcodeClient; use Gatepost\Postcode\Client\PostcodeException; -use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Psr7\HttpFactory; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use Psr\Http\Client\ClientInterface; -use Psr\Http\Message\RequestInterface; -use Psr\Http\Message\ResponseInterface; -use ReflectionProperty; /** - * The timeout of autocomplete: 15 s by default, because a user waits for each completion, and no - * retry after a timeout, because the next keystroke replaces the call. + * The timeout of each call. A PSR-18 transport has no timeout of its own, so the client calls an + * attempt that failed after timeoutMs or more a timeout, and a failure that came sooner a + * network error. The default is 8 s, and 15 s for autocomplete, because a user waits for each + * completion. A timeout of autocomplete gets no retry, because the next keystroke replaces it. + * The fake clock moves with each scripted failure, so no test waits. */ final class AutocompleteTimeoutTest extends TestCase { - #[Test] - public function waitsFifteenSecondsByDefaultAndTheCallersTimeoutWhenGiven(): void + /** + * @return array + */ + public static function autocompleteFailures(): array { - $factory = new HttpFactory(); - - self::assertSame(15000, self::timeoutOfAutocomplete(new PostcodeClient( - new FixtureGateway(), - $factory, - ))); - self::assertSame(200, self::timeoutOfAutocomplete(new PostcodeClient( - new FixtureGateway(), - $factory, - timeoutMs: 200, - ))); + return [ + 'default, 1 ms short' => [null, 14999.0, ErrorCode::NetworkError], + 'default, on the limit' => [null, 15000.0, ErrorCode::Timeout], + 'given, 1 ms short' => [200, 199.0, ErrorCode::NetworkError], + 'given, on the limit' => [200, 200.0, ErrorCode::Timeout], + ]; } #[Test] - public function doesNotRetryATimeout(): void - { - $transport = new class implements ClientInterface { - public int $attempts = 0; - - public function sendRequest(RequestInterface $request): ResponseInterface - { - ++$this->attempts; - \usleep(30_000); - - throw new ConnectException('The connection timed out.', $request); - } - }; - $client = new PostcodeClient($transport, new HttpFactory(), timeoutMs: 20); + #[DataProvider('autocompleteFailures')] + public function autocompleteReadsAFailureByTheTimeoutOfTheCall( + ?int $timeoutMs, + float $failsAfterMs, + ErrorCode $expected, + ): void { + $timer = new FakeTimer(); + $client = new PostcodeClient( + new ScriptedTransport([$failsAfterMs], $timer), + new HttpFactory(), + timeoutMs: $timeoutMs, + maxRetries: 0, + timer: $timer, + ); try { $client->autocomplete('F'); self::fail('The call did not fail.'); } catch (PostcodeException $error) { - self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertSame($expected, $error->errorCode()); } - self::assertSame(1, $transport->attempts); } /** - * Reads the timeout of the sender that autocomplete uses. A timeout takes real time, and the - * default one is 15 s, so a test cannot wait for it. + * @return array */ - private static function timeoutOfAutocomplete(PostcodeClient $client): int + public static function lookupFailures(): array + { + return [ + '1 ms short' => [7999.0, ErrorCode::NetworkError], + 'on the limit' => [8000.0, ErrorCode::Timeout], + ]; + } + + #[Test] + #[DataProvider('lookupFailures')] + public function lookupReadsAFailureByTheDefaultOfEightSeconds( + float $failsAfterMs, + ErrorCode $expected, + ): void { + $timer = new FakeTimer(); + $client = new PostcodeClient( + new ScriptedTransport([$failsAfterMs], $timer), + new HttpFactory(), + maxRetries: 0, + timer: $timer, + ); + + try { + $client->lookup(FixtureGateway::UNIT); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame($expected, $error->errorCode()); + } + } + + #[Test] + public function doesNotRetryATimeoutOfAutocomplete(): void { - $sender = (new ReflectionProperty(PostcodeClient::class, 'autocompleteSender')) - ->getValue($client); - self::assertInstanceOf(Sender::class, $sender); - $timeout = (new ReflectionProperty(Sender::class, 'timeoutMs'))->getValue($sender); - self::assertIsInt($timeout); + $timer = new FakeTimer(); + $transport = new ScriptedTransport([15000.0], $timer); + $client = new PostcodeClient( + $transport, + new HttpFactory(), + maxRetries: 2, + timer: $timer, + ); - return $timeout; + try { + $client->autocomplete('F'); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + } + self::assertCount(1, $transport->requests); + self::assertSame([], $timer->waits); } } diff --git a/tests/Unit/Client/FieldsTest.php b/tests/Unit/Client/FieldsTest.php index 036e0f1..39ee124 100644 --- a/tests/Unit/Client/FieldsTest.php +++ b/tests/Unit/Client/FieldsTest.php @@ -20,21 +20,24 @@ final class FieldsTest extends TestCase { /** - * @return array + * @return array}> */ public static function objects(): array { return [ - 'an object with fields' => [['valid' => true]], - 'an empty object, which json_decode() reads as an empty array' => [[]], + 'an object with fields' => [['valid' => true], ['valid' => true]], + 'an empty object, which the Sender keeps as a stdClass' => [new \stdClass(), []], ]; } + /** + * @param array $expected + */ #[Test] #[DataProvider('objects')] - public function readsAJsonObject(mixed $value): void + public function readsAJsonObject(mixed $value, array $expected): void { - self::assertSame($value, Fields::object($value)); + self::assertSame($expected, Fields::object($value)); } #[Test] @@ -47,6 +50,8 @@ public function readsAnObjectFieldAsNullWhenItIsNotAnObject(): void self::assertNull(Fields::objectOrNull($fields, 'c')); self::assertNull(Fields::objectOrNull($fields, 'd')); self::assertNull(Fields::objectOrNull($fields, 'missing')); + self::assertSame([], Fields::objectOrNull(['e' => new \stdClass()], 'e')); + self::assertNull(Fields::objectOrNull(['e' => []], 'e')); } #[Test] @@ -68,6 +73,7 @@ public static function notObjects(): array { return [ 'a list' => [[1, 2]], + 'an empty list' => [[]], 'a list of one object' => [[['valid' => true]]], 'text' => ['valid'], 'null' => [null], diff --git a/tests/Unit/Client/LookupResultLenientTest.php b/tests/Unit/Client/LookupResultLenientTest.php index ee7da16..41e4123 100644 --- a/tests/Unit/Client/LookupResultLenientTest.php +++ b/tests/Unit/Client/LookupResultLenientTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; /** * The parts of a lookup response outside the closed list of spec/client.md give null when they @@ -55,7 +56,7 @@ public function givesNullForAPartOfTheWrongType(array $fields, int $level): void } /** - * @return array, ?string, ?string}> + * @return array|stdClass, ?string, ?string}> */ public static function addressNames(): array { @@ -68,18 +69,18 @@ public static function addressNames(): array null, ], 'a zone that is a number' => [[...$names, 'zone' => 5], 'S', null], - 'an empty address' => [[], null, null], + 'an empty address' => [new stdClass(), null, null], 'a state that is null' => [[...$names, 'state_name' => null], null, 'Z'], ]; } /** - * @param array $address + * @param array|stdClass $address */ #[Test] #[DataProvider('addressNames')] public function givesNullForEachNameThatIsMissingOrNotText( - array $address, + array|stdClass $address, ?string $state, ?string $zone, ): void { From 9bb4dab2260827f8de3fd5ac9514a1e653a040fc Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:00:40 +0100 Subject: [PATCH 10/21] fix(client): never let the cache fail a call that the gateway answered A cache is an extra, so a store that throws on a read or a write now costs a request and nothing more, and the caller still gets the result. A kept entry that the client cannot read counts as a miss, and the new result replaces it, so an entry from an older version or a damaged entry cannot fail every call until its TTL ends. The cache key now uses an HMAC of the request with the API key as the HMAC key, so a reader of the cache cannot test guesses of the key with a plain hash. clearCache() still passes a failure of the store to the caller, because a clear that did not happen must not look as if it did. Signed-off-by: MobileMage --- src/Client/Internal/ResultCache.php | 43 +++++--- src/Client/PostcodeClient.php | 20 ++-- tests/Unit/Client/ArrayCache.php | 15 +++ tests/Unit/Client/CacheFaultTest.php | 138 ++++++++++++++++++++++++++ tests/Unit/Client/ResultCacheTest.php | 53 +++++++--- tests/Unit/Client/StoreRefused.php | 16 +++ 6 files changed, 252 insertions(+), 33 deletions(-) create mode 100644 tests/Unit/Client/CacheFaultTest.php create mode 100644 tests/Unit/Client/StoreRefused.php diff --git a/src/Client/Internal/ResultCache.php b/src/Client/Internal/ResultCache.php index 78b6900..2870fde 100644 --- a/src/Client/Internal/ResultCache.php +++ b/src/Client/Internal/ResultCache.php @@ -8,6 +8,7 @@ namespace Gatepost\Postcode\Client\Internal; use Psr\SimpleCache\CacheInterface; +use Throwable; /** * Keeps the data of each answered call in the caller's PSR-16 cache for cacheTtlMs (SEC-3). The @@ -37,23 +38,35 @@ public function __construct( /** * The key of a call in the current generation. * - * @param string $call The method and the canonical arguments of the call, with the API key. - * The cache sees only a hash of it, so its keys hold no postcode and no - * API key. + * @param string $url The address of the request, which holds the canonical postcode and + * the level. + * @param string $apiKey The API key, or an empty string. It is the HMAC key, so the cache + * sees only a one-way hash and its keys hold no postcode and no API + * key. Two API keys never share an entry. + * + * @return ?string The key, or null when the cache failed. The call then runs with no cache. */ - public function slot(string $call): string + public function slot(string $url, string $apiKey): ?string { - $hash = \substr(\hash('sha256', $call), 0, self::HASH_LENGTH); - - return self::PREFIX . $this->generation() . '.' . $hash; + $hash = \substr(\hash_hmac('sha256', $url, $apiKey), 0, self::HASH_LENGTH); + try { + return self::PREFIX . $this->generation() . '.' . $hash; + } catch (Throwable) { + return null; + } } /** - * @return mixed The kept data, or null when nothing is kept. A result never has null data. + * @return mixed The kept data, or null when nothing is kept or the cache failed. A result + * never has null data. */ public function get(string $slot): mixed { - $entry = $this->cache->get($slot); + try { + $entry = $this->cache->get($slot); + } catch (Throwable) { + return null; + } $expiresAtMs = \is_array($entry) ? ($entry['expiresAtMs'] ?? null) : null; if (!\is_float($expiresAtMs) || $expiresAtMs <= $this->timer->nowMs()) { return null; @@ -63,18 +76,24 @@ public function get(string $slot): mixed } /** - * Keeps the data of a call that the client read without an error. + * Keeps the data of a call that the client read without an error. A cache that fails keeps + * nothing, and the caller still gets the result. */ public function put(string $slot, mixed $data): void { $entry = ['expiresAtMs' => $this->timer->nowMs() + $this->ttlMs, 'data' => $data]; // PSR-16 counts whole seconds, so the entry can outlive its TTL by up to one second. // get() checks the time in milliseconds. - $this->cache->set($slot, $entry, (int) \ceil($this->ttlMs / 1000)); + try { + $this->cache->set($slot, $entry, (int) \ceil($this->ttlMs / 1000)); + } catch (Throwable) { + // A cache is an extra. Its failure never costs the caller a good result. + } } /** - * Makes every result that this cache kept unreachable. + * Makes every result that this cache kept unreachable. A failure of the cache passes to the + * caller, because a clear that did not happen must not look as if it did. */ public function clear(): void { diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index de9bd7d..808138f 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -266,8 +266,9 @@ public function autocomplete(string $q): AutocompleteResult } /** - * Removes every result that the client kept. The caller's other cache entries stay. A request - * that started before this call keeps no result. + * Removes every result that any client of the same cache kept, also the results of a client + * with another API key. The caller's other cache entries stay. A request that started before + * this call keeps no result. A failure of the cache passes to the caller. * * ```php * $client->clearCache(); @@ -353,12 +354,19 @@ private function call( if ($cache === null) { return $read($sender->send($request, $retryTimeouts)); } - // The query holds the canonical postcode and the level. Two keys can hold different - // levels, and a cache can serve two clients, so each API key keeps its own results. - $slot = $cache->slot($url . ' ' . ($this->apiKey ?? '')); + // The URL holds the canonical postcode and the level. A cache can serve two clients, so + // each API key keeps its own results. + $slot = $cache->slot($url, $this->apiKey ?? ''); + if ($slot === null) { + return $read($sender->send($request, $retryTimeouts)); + } $kept = $cache->get($slot); if ($kept !== null) { - return $read($kept); + try { + return $read($kept); + } catch (PostcodeException) { + // An entry that this client cannot read is a miss. The new result replaces it. + } } $response = $sender->send($request, $retryTimeouts); $result = $read($response); diff --git a/tests/Unit/Client/ArrayCache.php b/tests/Unit/Client/ArrayCache.php index 1d4f6bf..a25ea7e 100644 --- a/tests/Unit/Client/ArrayCache.php +++ b/tests/Unit/Client/ArrayCache.php @@ -10,6 +10,7 @@ use DateInterval; use InvalidArgumentException; use Psr\SimpleCache\CacheInterface; +use Throwable; /** * A PSR-16 cache in memory that never expires an entry by itself. It records the TTL of each @@ -23,13 +24,27 @@ final class ArrayCache implements CacheInterface /** @var array */ public array $ttls = []; + /** A failure that get() throws, as a broken store does. */ + public ?Throwable $getFails = null; + + /** A failure that set() throws. */ + public ?Throwable $setFails = null; + public function get(string $key, mixed $default = null): mixed { + if ($this->getFails !== null) { + throw $this->getFails; + } + return \array_key_exists($key, $this->entries) ? $this->entries[$key] : $default; } public function set(string $key, mixed $value, DateInterval|int|null $ttl = null): bool { + if ($this->setFails !== null) { + throw $this->setFails; + } + $this->entries[$key] = $value; $this->ttls[$key] = $ttl; diff --git a/tests/Unit/Client/CacheFaultTest.php b/tests/Unit/Client/CacheFaultTest.php new file mode 100644 index 0000000..71399a6 --- /dev/null +++ b/tests/Unit/Client/CacheFaultTest.php @@ -0,0 +1,138 @@ + + */ + public static function failures(): array + { + return [ + 'a runtime error' => [new RuntimeException('The store is down.')], + 'a PSR-16 exception' => [new StoreRefused('The store refused it.')], + ]; + } + + #[Test] + #[DataProvider('failures')] + public function readsAStoreThatFailsOnReadAsAMiss(Throwable $failure): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $store->getFails = $failure; + $client = self::cachingClient($gateway, $store); + + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + + self::assertCount(2, $gateway->requests); + } + + #[Test] + #[DataProvider('failures')] + public function keepsTheResultWhenTheStoreFailsOnWrite(Throwable $failure): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $store->setFails = $failure; + $client = self::cachingClient($gateway, $store); + + $result = $client->reverse(9.0, 7.0); + + self::assertTrue($result->found); + self::assertCount(1, $gateway->requests); + } + + #[Test] + public function keepsTheResultWhenOnlyTheEntryWriteFails(): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $client = self::cachingClient($gateway, $store); + // The first call makes the generation key. The next call fails only on its entry. + $client->lookup(FixtureGateway::UNIT); + $store->setFails = new RuntimeException('The store is full.'); + + self::assertTrue($client->lookup(FixtureGateway::UNIT, level: 2)->valid); + } + + #[Test] + public function readsAKeptEntryThatTheClientCannotReadAsAMissAndReplacesIt(): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $client = self::cachingClient($gateway, $store); + $client->lookup(FixtureGateway::UNIT); + foreach (\array_keys($store->entries) as $key) { + $entry = $store->entries[$key]; + if (\is_array($entry) && \array_key_exists('data', $entry)) { + $entry['data'] = ['valid' => 'zzz']; + $store->entries[$key] = $entry; + } + } + + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + + self::assertCount(2, $gateway->requests); + } + + #[Test] + public function tellsTheCallerWhenClearCacheFails(): void + { + $store = new ArrayCache(); + $client = self::cachingClient(new FixtureGateway(), $store); + $store->setFails = new RuntimeException('The store is down.'); + + $this->expectException(RuntimeException::class); + + $client->clearCache(); + } + + #[Test] + public function holdsNeitherTheApiKeyNorThePostcodeInAnyKey(): void + { + $store = new ArrayCache(); + $client = self::cachingClient(new FixtureGateway(), $store); + + $client->lookup(FixtureGateway::UNIT); + + foreach (\array_keys($store->entries) as $key) { + self::assertStringNotContainsString('nipost', $key); + self::assertStringNotContainsString('Z99', $key); + } + } + + private static function cachingClient( + FixtureGateway $gateway, + ArrayCache $store, + ): PostcodeClient { + return new PostcodeClient( + $gateway, + new HttpFactory(), + apiKey: 'nipost_test_mock_l3', + cacheTtlMs: 60_000, + cache: $store, + ); + } +} diff --git a/tests/Unit/Client/ResultCacheTest.php b/tests/Unit/Client/ResultCacheTest.php index 9f5d32a..5f6fba7 100644 --- a/tests/Unit/Client/ResultCacheTest.php +++ b/tests/Unit/Client/ResultCacheTest.php @@ -20,7 +20,6 @@ final class ResultCacheTest extends TestCase { private const KEY = 'nipost_test_mock_l3'; private const URL = 'https://gateway.invalid/v1/lookup?code=FC-01-Z99-ZZ-01&level=1'; - private const CALL = self::URL . ' ' . self::KEY; private const DATA = ['postcode' => 'FC-01-Z99-ZZ-01', 'valid' => true]; private ArrayCache $store; @@ -39,14 +38,14 @@ protected function setUp(): void #[Test] public function givesBackTheDataUntilTheTtlEnds(): void { - $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->cache->put($this->slot(), self::DATA); $this->timer->advance(1499); - self::assertSame(self::DATA, $this->cache->get($this->cache->slot(self::CALL))); + self::assertSame(self::DATA, $this->cache->get($this->slot())); $this->timer->advance(1); - self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertNull($this->cache->get($this->slot())); } /** @@ -67,7 +66,7 @@ public function asksTheCacheForWholeSecondsRoundedUp(int $ttlMs, int $seconds): { $cache = new ResultCache($this->store, $ttlMs, $this->timer); - $cache->put($cache->slot(self::CALL), self::DATA); + $cache->put(self::slotOf($cache), self::DATA); $resultTtls = \array_filter($this->store->ttls, static fn($ttl): bool => $ttl !== null); self::assertSame([$seconds], \array_values($resultTtls)); @@ -76,8 +75,8 @@ public function asksTheCacheForWholeSecondsRoundedUp(int $ttlMs, int $seconds): #[Test] public function keepsEachCallUnderAKeyThatPsr16Allows(): void { - $this->cache->put($this->cache->slot(self::CALL), self::DATA); - $this->cache->put($this->cache->slot(self::CALL . ' other'), self::DATA); + $this->cache->put($this->slot(), self::DATA); + $this->cache->put($this->slot(self::URL . '&other=1'), self::DATA); $keys = \array_keys($this->store->entries); self::assertCount(3, $keys); @@ -88,49 +87,73 @@ public function keepsEachCallUnderAKeyThatPsr16Allows(): void } } + #[Test] + public function hashesTheCallWithTheApiKeyAsTheHmacKey(): void + { + $slot = $this->slot(); + + $hash = \substr(\hash_hmac('sha256', self::URL, self::KEY), 0, 40); + self::assertStringEndsWith('.' . $hash, $slot); + self::assertStringNotContainsString(self::KEY, $slot); + self::assertNotSame($slot, $this->cache->slot(self::URL, 'another_key')); + } + #[Test] public function hidesEveryKeptResultAfterClearAndKeepsTheOtherEntries(): void { $this->store->set('app.session', 'kept'); - $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->cache->put($this->slot(), self::DATA); $this->cache->clear(); - self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertNull($this->cache->get($this->slot())); self::assertSame('kept', $this->store->get('app.session')); } #[Test] public function keepsNoResultOfACallThatStartedBeforeClear(): void { - $slot = $this->cache->slot(self::CALL); + $slot = $this->slot(); $this->cache->clear(); $this->cache->put($slot, self::DATA); - self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertNull($this->cache->get($this->slot())); } #[Test] public function hidesTheOldResultsWhenTheCacheEvictsTheGeneration(): void { - $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->cache->put($this->slot(), self::DATA); $this->store->delete('gatepost.generation'); - self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertNull($this->cache->get($this->slot())); } #[Test] public function readsAnEntryOfAnotherShapeAsNothingKept(): void { - $this->cache->put($this->cache->slot(self::CALL), self::DATA); + $this->cache->put($this->slot(), self::DATA); foreach (\array_keys($this->store->entries) as $key) { if ($key !== 'gatepost.generation') { $this->store->set($key, ['data' => self::DATA]); } } - self::assertNull($this->cache->get($this->cache->slot(self::CALL))); + self::assertNull($this->cache->get($this->slot())); + } + + private function slot(string $url = self::URL): string + { + return self::slotOf($this->cache, $url); + } + + private static function slotOf(ResultCache $cache, string $url = self::URL): string + { + $slot = $cache->slot($url, self::KEY); + self::assertNotNull($slot); + + return $slot; } } diff --git a/tests/Unit/Client/StoreRefused.php b/tests/Unit/Client/StoreRefused.php new file mode 100644 index 0000000..46a45c6 --- /dev/null +++ b/tests/Unit/Client/StoreRefused.php @@ -0,0 +1,16 @@ + Date: Sat, 3 Oct 2026 04:08:50 +0100 Subject: [PATCH 11/21] docs(client): show the client in the README and run its examples DOC-3 asks that CI runs each code example of a README. The new examples call the gateway, so the contract job runs them against the mock server with a mock key, and the unit suite runs the examples that need no network. The README says that levels 2 to 5 follow NIPOST's docs, that the client is synchronous, and that a caller sets the timeout on the PSR-18 client, which has none, to the longest timeout in use. It tells callers to use a cache store with eviction, because the client sets no size bound of its own. Signed-off-by: MobileMage --- .../unreleased/Added-20261003-040232.yaml | 3 + AGENTS.md | 2 +- README.md | 88 ++++++++++++++++++- composer.json | 2 +- tests/Contract/ReadmeExampleTest.php | 57 ++++++++++++ tests/Readme.php | 76 ++++++++++++++++ tests/Unit/ReadmeTest.php | 68 ++++---------- 7 files changed, 239 insertions(+), 57 deletions(-) create mode 100644 .changes/unreleased/Added-20261003-040232.yaml create mode 100644 tests/Contract/ReadmeExampleTest.php create mode 100644 tests/Readme.php diff --git a/.changes/unreleased/Added-20261003-040232.yaml b/.changes/unreleased/Added-20261003-040232.yaml new file mode 100644 index 0000000..fb42e55 --- /dev/null +++ b/.changes/unreleased/Added-20261003-040232.yaml @@ -0,0 +1,3 @@ +kind: Added +body: 'Add the client of NIPOST''s gateway: lookup, reverse and autocomplete, from spec 0.2.0.' +time: 2026-10-03T04:02:32.482746+01:00 diff --git a/AGENTS.md b/AGENTS.md index d125f32..1b42119 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # AGENTS.md -This repo is part of Gatepost, unofficial open-source developer tools for Nigeria's National Digital Postcode. This repo holds the PHP SDK, `gatepost/postcode`. Today it has the core. +This repo is part of Gatepost, unofficial open-source developer tools for Nigeria's National Digital Postcode. This repo holds the PHP SDK, `gatepost/postcode`. Today it has the core and the client. ## Read first diff --git a/README.md b/README.md index eed27a2..6c2aab2 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ # gatepost/postcode -Parse, check and format Nigeria's digital postcodes in PHP, with no network access. +Parse, check and format Nigeria's digital postcodes in PHP, and query NIPOST's gateway for them. > Unofficial. Not made or endorsed by NIPOST. @@ -46,6 +46,7 @@ if ($result->isOk()) { - Shortens a code to its area, district, LGA or state, and checks whether one code contains another. - Hides the unit of a code for logs. - Runs without the intl and mbstring extensions. +- Queries NIPOST's gateway with your own API key: what it knows about a postcode, the postcode at a GPS fix, and suggestions while a user types. ### Limit the input @@ -63,16 +64,97 @@ echo Postcode::parse(str_pad($code, 65))->error?->code->value, "\n"; // bad_leng `normalize` has no input limit, so call `parse` for text from an untrusted source. +## Call the gateway + +`PostcodeClient` calls NIPOST's gateway with your own API key. The client needs a PSR-18 HTTP client and a PSR-17 request factory, such as Guzzle 7 (`composer require guzzlehttp/guzzle`). Keep the key on a server, because it is a secret. + +The client is synchronous. Each call blocks until it has a result or throws a `PostcodeException`. It has no parallel calls and no cancel. The contract scenarios that start calls in parallel do not apply to it, and the contract suite leaves them out. + +### Check a postcode with the gateway + +```php + 15]), + new HttpFactory(), + apiKey: getenv('NIPOST_API_KEY') ?: null, +); + +try { + $result = $client->lookup('FC-01-Z99-ZZ-01'); + echo $result->valid ? 'valid' : 'not found', "\n"; +} catch (PostcodeException $error) { + echo $error->errorCode()->value, "\n"; +} +``` + +The examples use `FC-01-Z99-ZZ-01`, a synthetic postcode, and show the answers of Gatepost's mock server, which CI runs them against. The real gateway does not know this postcode. + +`lookup` checks the code with `Postcode::parse()` first, and sends no request for a code that fails. A postcode that the gateway does not know is a result with `valid` set to false, not an error. + +A key holds a lookup level, and `lookup` sends level 1 when you give none. Pass `level: 2` to `lookup` for a higher level. A level above the grant of the key fails with `ErrorCode::Forbidden`. Levels 2 to 5 use credits. NIPOST's docs say that level 2 adds the names of the places that hold the postcode and a recent house address, and level 3 adds what the building is used for. Gatepost has seen only level 1 responses from the gateway. The fields of levels 2 to 5 are documented, not observed: they follow NIPOST's docs, and no Gatepost test has seen them from the gateway yet. `levelReceived` comes from the fields of the response, because the gateway does not say which level it sent. + +### Find a place and complete a postcode + +`reverse($lat, $lng)` finds the postcode at a point. An optional third argument is the radius in metres, from 0 to 250. `found` can be true while `unit` is null: the gateway then found an area, but no unit within the radius. `area`, `district` and `state` are the text that the gateway sent. + +`autocomplete($text)` takes the text that a user typed. It names the segment that the user is typing, and gives the gateway's values for it. Each value comes with the partial postcode that it completes, when the text and the value make one. + +```php + 15]), + new HttpFactory(), + apiKey: getenv('NIPOST_API_KEY') ?: null, +); + +$place = $client->reverse(9.0, 7.0); +echo $place->unit?->postcode->canonical, "\n"; + +$typing = $client->autocomplete('fc 01 z'); +echo $typing->segment->value, "\n"; +``` + +Each call refuses bad input before it sends a request, and throws `ErrorCode::InvalidInput`. This covers a level outside 1 to 5, a latitude, a longitude or a radius outside its range, and `autocomplete` text that is empty, has more than 11 letters and digits, or holds another character. + +### Handle errors and timeouts + +Each failed call throws `PostcodeException`. `errorCode()` gives one of the cases of `ErrorCode`, such as `ErrorCode::RateLimited`. `status()` gives the HTTP status, `apiCode()` gives the gateway's own error code, and `retryAfterMs()` gives the wait that a 429 asked for. `ErrorCode::UnexpectedResponse` means that the gateway answered 200 with a body that the client cannot read. None of the messages of the client holds the API key. Show the user a message for the error code, not the message of the exception, which is for developers. + +The client tries a call again after a 502, 503 or 504, a failed connection or a timeout, at most twice by default (`maxRetries`). It waits 500 ms before the first retry and twice as long before the next, plus a random part of up to 250 ms. After a 429, it waits only when `Retry-After` asks for 10 seconds or less. A timeout of `autocomplete` gets no retry, because the next keystroke replaces the call. + +PSR-18 has no timeout, so set the timeout on your HTTP client, and make it the longest timeout in use. `autocomplete` waits 15 seconds for one attempt, and the other calls wait 8 seconds, so the examples set 15. With a shorter timeout on the transport, a slow call fails before the client counts it as a timeout. The client then reads the failure as `ErrorCode::NetworkError` and tries it again. If you pass `timeoutMs`, it applies to every call, and the transport needs that value or more. The client reports a failed attempt that took `timeoutMs` or more as `ErrorCode::Timeout`. + +### Keep results + +The client keeps no result by default. To keep results, pass `cacheTtlMs` and a PSR-16 `cache`, such as the cache of your framework. An identical call within that time gets the kept result with no request. The client never keeps an error. + +The client sets no size bound of its own, so use a store with eviction, such as Redis or APCu with a size limit. A store with no bound grows with each different postcode that you look up. The cache keys hold a keyed hash of the call, never the API key or the postcode in the clear. Two clients with the same API key and the same cache share their results. + +A cache that fails never fails a call. The client reads a failed read as a miss, and skips a failed write. `clearCache()` is the exception. It removes every result that the clients of the same cache kept, also for another API key, and leaves the other entries of the cache alone. When the store fails, `clearCache()` throws the failure, so that you do not read old results after you asked to remove them. + ## Requirements | Requirement | Version | |---|---| | PHP | 8.1 or later | +| HTTP, for the client | a PSR-18 client and a PSR-17 request factory, such as Guzzle 7 | | Gatepost spec | 0.2.0 | ## Docs -The Gatepost docs site will hold the guide and the API reference. Until it is live, each public method of `Postcode` has a doc comment with an example. +The Gatepost docs site will hold the guide and the API reference. Until it is live, each public method of `Postcode` and `PostcodeClient` has a doc comment with an example. ## Support @@ -91,7 +173,7 @@ composer --working-dir=tools/bc-check install --ignore-platform-req=ext-intl composer check ``` -`composer check` runs no contract scenario, because the scenarios need the mock server, which runs on Node 22.22.2 or later. CI runs them in its `contract` job. To run them yourself, clone the js repo next to this one, start its mock server with this repo's spec, and run `composer contract`: +`composer check` runs no contract scenario and no example of the client, because they need the mock server, which runs on Node 22.22.2 or later. CI runs them in its `contract` job. To run them yourself, clone the js repo next to this one, start its mock server with this repo's spec, and run `composer contract`: ```sh git clone --depth 1 https://github.com/gatepost-dev/js ../js diff --git a/composer.json b/composer.json index e1ffd6a..7e86907 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "gatepost/postcode", - "description": "Unofficial. Not made or endorsed by NIPOST. Parse Nigerian postcodes offline.", + "description": "Unofficial. Not made or endorsed by NIPOST. Read and query Nigerian postcodes.", "license": "Apache-2.0", "type": "library", "require": { diff --git a/tests/Contract/ReadmeExampleTest.php b/tests/Contract/ReadmeExampleTest.php new file mode 100644 index 0000000..5b7e0ff --- /dev/null +++ b/tests/Contract/ReadmeExampleTest.php @@ -0,0 +1,57 @@ + + */ + public static function examples(): array + { + return [ + 'the check of a postcode' => ['Check a postcode with the gateway', "valid\n"], + 'the search for a place' => [ + 'Find a place and complete a postcode', + "FC-01-Z99-ZZ-01\ndistrict\n", + ], + ]; + } + + #[Test] + #[DataProvider('examples')] + public function runsEachClientExampleAgainstTheMockServer( + string $heading, + string $printed, + ): void { + $example = Readme::phpExample(Readme::section($heading)); + self::assertSame(1, \substr_count($example, self::FACTORY_LINE)); + $baseUrl = ' baseUrl: ' . \var_export($this->mockUrl, true) . ",\n"; + \putenv('NIPOST_API_KEY=nipost_test_mock_l1'); + $this->expectOutputString($printed); + + try { + Readme::run(\str_replace(self::FACTORY_LINE, self::FACTORY_LINE . $baseUrl, $example)); + } finally { + \putenv('NIPOST_API_KEY'); + } + } +} diff --git a/tests/Readme.php b/tests/Readme.php new file mode 100644 index 0000000..211dff0 --- /dev/null +++ b/tests/Readme.php @@ -0,0 +1,76 @@ +expectOutputString($printed); - // A closure keeps the variables of the example away from the variables of the test. - (static function () use ($file): void { - require $file; - })(); - } finally { - \unlink($file); - } + $this->expectOutputString($printed); + + Readme::run(Readme::phpExample(Readme::section($heading))); } /** @@ -78,22 +68,24 @@ public function readsWhichVersionsAConstraintAllows( self::assertSame($allowed, self::constraintAllows($constraint, $version)); } + // The example of the client calls the gateway, so the contract suite runs it against the + // mock server. #[Test] - public function listsEachPhpExampleOfTheReadmeInTheTest(): void + public function listsEachPhpExampleOfTheReadmeInATest(): void { - $examples = \preg_match_all('/^```php$/m', self::repoFile('README.md')); + $examples = \preg_match_all('/^```php$/m', Readme::text()); self::assertSame( - \count(self::examples()), + \count(self::examples()) + \count(ReadmeExampleTest::examples()), $examples, - 'Add each PHP example of the README to examples(), so that the test runs it.', + 'Add each PHP example of the README to examples() of a test, so that the test runs it.', ); } #[Test] public function statesTheInputLimitOfTheSpec(): void { - $section = self::section('Limit the input'); + $section = Readme::section('Limit the input'); self::assertSame(1, \preg_match('/at most (\d+) code points/', $section, $limit)); self::assertSame(SpecData::MAX_INPUT_CODE_POINTS, (int) $limit[1]); @@ -102,10 +94,12 @@ public function statesTheInputLimitOfTheSpec(): void #[Test] public function installsWithAConstraintThatAllowsTheNewestRelease(): void { - $found = \preg_match('/^## (\S+) - /m', self::repoFile('CHANGELOG.md'), $newest); + $changelog = \file_get_contents(\dirname(__DIR__, 2) . '/CHANGELOG.md'); + self::assertIsString($changelog); + $found = \preg_match('/^## (\S+) - /m', $changelog, $newest); self::assertSame(1, $found, 'The changelog has no release.'); $pattern = '/^composer require gatepost\/postcode(?::(\S+))?$/m'; - $found = \preg_match($pattern, self::section('Install'), $command); + $found = \preg_match($pattern, Readme::section('Install'), $command); self::assertSame(1, $found, 'The Install section has no composer require line.'); $constraint = $command[1] ?? '*'; @@ -115,14 +109,6 @@ public function installsWithAConstraintThatAllowsTheNewestRelease(): void ); } - private static function repoFile(string $name): string - { - $contents = \file_get_contents(\dirname(__DIR__, 2) . '/' . $name); - self::assertIsString($contents); - - return $contents; - } - // Composer installs a pre-release only when the constraint names its stability, such as @alpha. // The version must also lie in the range of the constraint. private static function constraintAllows(string $constraint, string $version): bool @@ -135,26 +121,4 @@ private static function stabilityFlag(string $version): string { return \preg_match('/-(alpha|beta|rc)\b/i', $version, $label) === 1 ? '@' . $label[1] : ''; } - - /** - * The text of a section, from its heading to the next heading. - */ - private static function section(string $heading): string - { - $pattern = '/^#{2,3} ' . \preg_quote($heading, '/') . '\n(.*?)(?=^#|\z)/ms'; - $found = \preg_match($pattern, self::repoFile('README.md'), $section); - - self::assertSame(1, $found, "The README has no section {$heading}."); - - return $section[1]; - } - - private static function phpExample(string $section): string - { - $found = \preg_match('/^```php\n(.*?)^```$/ms', $section, $example); - - self::assertSame(1, $found, 'The section has no PHP example.'); - - return $example[1]; - } } From 9be7c635f6c5ebe1c6dcedb4c7d6258050469032 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:45:30 +0100 Subject: [PATCH 12/21] docs(client): tell callers to pass the transport timeout as timeoutMs A PSR-18 transport holds one timeout, so the client uses one timeoutMs for every call. The README said to set the transport timeout to 15 s and pass no timeoutMs. A hanging gateway then made each lookup attempt last 15 s, which the client read as a timeout, and a call lasted about 31 s. The README, the transport doc and the decision record now say to pass the same value as the transport timeout. The README examples do so. Two tests pin the setup: one reads the examples, and one runs a hanging gateway against the fake clock. Signed-off-by: MobileMage --- README.md | 4 +- ...03-the-client-depends-on-psr-interfaces.md | 2 +- src/Client/PostcodeClient.php | 22 +++-- .../Unit/Client/TransportTimeoutSetupTest.php | 97 +++++++++++++++++++ tests/Unit/ReadmeTest.php | 24 +++++ 5 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 tests/Unit/Client/TransportTimeoutSetupTest.php diff --git a/README.md b/README.md index 6c2aab2..9961624 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,7 @@ $client = new PostcodeClient( new Client(['timeout' => 15]), new HttpFactory(), apiKey: getenv('NIPOST_API_KEY') ?: null, + timeoutMs: 15000, ); try { @@ -117,6 +118,7 @@ $client = new PostcodeClient( new Client(['timeout' => 15]), new HttpFactory(), apiKey: getenv('NIPOST_API_KEY') ?: null, + timeoutMs: 15000, ); $place = $client->reverse(9.0, 7.0); @@ -134,7 +136,7 @@ Each failed call throws `PostcodeException`. `errorCode()` gives one of the case The client tries a call again after a 502, 503 or 504, a failed connection or a timeout, at most twice by default (`maxRetries`). It waits 500 ms before the first retry and twice as long before the next, plus a random part of up to 250 ms. After a 429, it waits only when `Retry-After` asks for 10 seconds or less. A timeout of `autocomplete` gets no retry, because the next keystroke replaces the call. -PSR-18 has no timeout, so set the timeout on your HTTP client, and make it the longest timeout in use. `autocomplete` waits 15 seconds for one attempt, and the other calls wait 8 seconds, so the examples set 15. With a shorter timeout on the transport, a slow call fails before the client counts it as a timeout. The client then reads the failure as `ErrorCode::NetworkError` and tries it again. If you pass `timeoutMs`, it applies to every call, and the transport needs that value or more. The client reports a failed attempt that took `timeoutMs` or more as `ErrorCode::Timeout`. +PSR-18 has no timeout, so the client cannot stop a slow attempt. Set the timeout on your HTTP client, and pass the same value as `timeoutMs`, as the examples do with 15 seconds. The client applies `timeoutMs` to every call, because one transport holds one timeout. 15 seconds is long enough for `autocomplete`, whose default wait is 15 seconds. The other calls wait 8 seconds by default, and `timeoutMs` replaces that value. The client reports a failed attempt that took `timeoutMs` or more as `ErrorCode::Timeout`, and a failure that came sooner as `ErrorCode::NetworkError`. If the two values differ, the client reads a slow failure with the wrong code, and tries it again when it should not. ### Keep results diff --git a/docs/adr/0003-the-client-depends-on-psr-interfaces.md b/docs/adr/0003-the-client-depends-on-psr-interfaces.md index 9137351..95adf18 100644 --- a/docs/adr/0003-the-client-depends-on-psr-interfaces.md +++ b/docs/adr/0003-the-client-depends-on-psr-interfaces.md @@ -8,4 +8,4 @@ Guzzle is a dev dependency only. The contract tests use it as the transport. The constraints allow PSR-7 1.1 and 2.0, and PSR-16 2.0 and 3.0, because older WordPress hosts and frameworks still pin the earlier major versions. The client calls only methods that both majors have. -PSR-18 has no timeout. The caller sets the timeout on the HTTP client, and passes the same value as `timeoutMs`. The client times each attempt, and it reports a failed attempt that took `timeoutMs` or more as `timeout`. +PSR-18 has no timeout, and one transport holds one timeout. The caller sets the timeout on the HTTP client, and passes the same value as `timeoutMs`. The client uses that value for every call, also for autocomplete, so 15 s covers all calls. It times each attempt, and it reports a failed attempt that took `timeoutMs` or more as `timeout`. A client that needs a shorter timeout for lookups can take a second transport for autocomplete later. That change adds an option and breaks nothing. diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index 808138f..1431012 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -33,6 +33,7 @@ * new \GuzzleHttp\Client(['timeout' => 15]), * new \GuzzleHttp\Psr7\HttpFactory(), * apiKey: \getenv('NIPOST_API_KEY') ?: null, + * timeoutMs: 15000, * ); * ``` */ @@ -55,9 +56,9 @@ final class PostcodeClient /** * @param ClientInterface $transport A PSR-18 HTTP client. PSR-18 has no timeout, - * so set the timeout of the transport to the - * longest timeout in use, which is 15 s with - * the defaults. The client reads a failed + * so set the timeout of the transport and pass + * the same value as timeoutMs. 15 s covers + * autocomplete. The client reads a failed * attempt as a timeout when it took timeoutMs * or more, and as a network_error when it * took less. @@ -66,11 +67,11 @@ final class PostcodeClient * the client sends no key, and the gateway * answers 401. * @param string $baseUrl The gateway's address. - * @param ?int $timeoutMs The longest wait for one attempt. A failed - * attempt that took this long is a timeout. - * With null, it is 8000, and 15000 for - * autocomplete. Set the timeout of the - * transport to this value or more. + * @param ?int $timeoutMs The longest wait for one attempt, for + * every call. A failed attempt that took this + * long is a timeout. With null, it is 8000, + * and 15000 for autocomplete. Pass the timeout + * of the transport. * @param int $maxRetries The most retries after the first attempt. * @param int $cacheTtlMs How long the client keeps a result. 0 turns * the cache off. @@ -216,8 +217,9 @@ public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): Re * with Postcode::normalize(). It sends no request for text that is empty, longer than a * postcode, or holds a character other than A to Z and 0 to 9. A timeout ends the call * without a retry, because the next keystroke replaces it. The call waits 15 s by default, - * so the transport needs a timeout of 15 s or more. With a shorter transport timeout, the - * failure reads as a network_error, and the client retries it. + * so set the timeout of the transport to 15 s and pass the same timeoutMs. With a + * shorter transport timeout, the failure reads as a network_error, and the client retries + * it. * * ```php * $result = $client->autocomplete('fc01z'); diff --git a/tests/Unit/Client/TransportTimeoutSetupTest.php b/tests/Unit/Client/TransportTimeoutSetupTest.php new file mode 100644 index 0000000..d3d8952 --- /dev/null +++ b/tests/Unit/Client/TransportTimeoutSetupTest.php @@ -0,0 +1,97 @@ +timer = new FakeTimer(); + } + + #[Test] + public function endsAHangingLookupWithATimeoutAfterThreeAttempts(): void + { + $transport = $this->hangingTransport(3); + + $error = $this->failure( + $transport, + static fn(PostcodeClient $client): mixed => $client->lookup(FixtureGateway::UNIT), + ); + + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertCount(3, $transport->requests); + // Three attempts of 15 s, and the waits of 500 and 1000 ms, which jitter does not move. + self::assertSame([500, 1000], $this->timer->waits); + self::assertEqualsWithDelta(46_500.0, $this->elapsedMs(), 0.001); + } + + #[Test] + public function endsAHangingAutocompleteWithATimeoutAfterOneAttempt(): void + { + $transport = $this->hangingTransport(1); + + $error = $this->failure( + $transport, + static fn(PostcodeClient $client): mixed => $client->autocomplete('F'), + ); + + self::assertSame(ErrorCode::Timeout, $error->errorCode()); + self::assertCount(1, $transport->requests); + self::assertSame([], $this->timer->waits); + } + + private function hangingTransport(int $attempts): ScriptedTransport + { + return new ScriptedTransport( + \array_fill(0, $attempts, (float) self::TRANSPORT_TIMEOUT_MS), + $this->timer, + ); + } + + private function elapsedMs(): float + { + return $this->timer->nowMs() - FakeTimer::START_MS; + } + + /** + * @param callable(PostcodeClient): mixed $call + */ + private function failure(ScriptedTransport $transport, callable $call): PostcodeException + { + $client = new PostcodeClient( + $transport, + new HttpFactory(), + timeoutMs: self::TRANSPORT_TIMEOUT_MS, + timer: $this->timer, + ); + try { + $call($client); + } catch (PostcodeException $error) { + return $error; + } + + self::fail('The call did not fail.'); + } +} diff --git a/tests/Unit/ReadmeTest.php b/tests/Unit/ReadmeTest.php index 38f8dc5..a6f0860 100644 --- a/tests/Unit/ReadmeTest.php +++ b/tests/Unit/ReadmeTest.php @@ -82,6 +82,30 @@ public function listsEachPhpExampleOfTheReadmeInATest(): void ); } + /** + * @return array + */ + public static function clientExampleSections(): array + { + return [ + 'the check of a postcode' => ['Check a postcode with the gateway'], + 'the search for a place' => ['Find a place and complete a postcode'], + ]; + } + + // One PSR-18 transport holds one timeout. The client reads a failed attempt by timeoutMs, so + // the example must pass the timeout of its transport as timeoutMs too. + #[Test] + #[DataProvider('clientExampleSections')] + public function givesTheTransportTimeoutAndTimeoutMsTheSameValue(string $heading): void + { + $example = Readme::phpExample(Readme::section($heading)); + + self::assertSame(1, \preg_match("/'timeout' => (\d+)/", $example, $transport)); + self::assertSame(1, \preg_match('/timeoutMs: (\d+),/', $example, $client)); + self::assertSame((int) $transport[1] * 1000, (int) $client[1]); + } + #[Test] public function statesTheInputLimitOfTheSpec(): void { From 745889485c3a3ca209a2fbfaedb5fe22c60206eb Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:46:19 +0100 Subject: [PATCH 13/21] fix(client): send a plain ampersand between the query parameters http_build_query() reads the ini value arg_separator.output when it gets no separator. A host that sets it to & made every reverse call lose its second parameter, and every lookup send a parameter named amp;level. The client now passes the separator and the encoding itself. The client also sends Accept: application/json, as the TypeScript client does. Signed-off-by: MobileMage --- src/Client/PostcodeClient.php | 9 +++- tests/Unit/Client/LookupRequestTest.php | 1 + tests/Unit/Client/QuerySeparatorTest.php | 62 ++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/Client/QuerySeparatorTest.php diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index 1431012..7775a09 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -327,7 +327,11 @@ private static function decimal(string $name, float $number, int $max, ?int $min */ private function url(string $path, array $query): string { - return $this->baseUrl . $path . '?' . \http_build_query($query); + // An explicit separator: a host can set arg_separator.output to &, and the gateway + // then reads no second parameter. + $text = \http_build_query($query, '', '&', \PHP_QUERY_RFC3986); + + return $this->baseUrl . $path . '?' . $text; } /** @@ -348,7 +352,8 @@ private function call( Closure $read, bool $retryTimeouts, ): mixed { - $request = $this->requestFactory->createRequest('GET', $url); + $request = $this->requestFactory->createRequest('GET', $url) + ->withHeader('Accept', 'application/json'); if ($this->apiKey !== null) { $request = $request->withHeader('X-API-Key', $this->apiKey); } diff --git a/tests/Unit/Client/LookupRequestTest.php b/tests/Unit/Client/LookupRequestTest.php index d3b7174..cf5f33f 100644 --- a/tests/Unit/Client/LookupRequestTest.php +++ b/tests/Unit/Client/LookupRequestTest.php @@ -37,6 +37,7 @@ public function sendsTheCanonicalCodeAndTheLevelWithTheKey(): void (string) $request->getUri(), ); self::assertSame([self::KEY], $request->getHeader('X-API-Key')); + self::assertSame(['application/json'], $request->getHeader('Accept')); } #[Test] diff --git a/tests/Unit/Client/QuerySeparatorTest.php b/tests/Unit/Client/QuerySeparatorTest.php new file mode 100644 index 0000000..241ccda --- /dev/null +++ b/tests/Unit/Client/QuerySeparatorTest.php @@ -0,0 +1,62 @@ +separator = \ini_set('arg_separator.output', '&'); + } + + protected function tearDown(): void + { + if ($this->separator !== false) { + \ini_set('arg_separator.output', $this->separator); + } + } + + #[Test] + public function joinsTheParametersOfALookupWithAnAmpersand(): void + { + $gateway = new ScriptedTransport([FixtureGateway::fixture('lookup/valid-level-2')]); + $client = new PostcodeClient($gateway, new HttpFactory()); + + $client->lookup(FixtureGateway::UNIT, level: 2); + + self::assertSame( + 'code=FC-01-Z99-ZZ-01&level=2', + $gateway->requests[0]->getUri()->getQuery(), + ); + } + + #[Test] + public function joinsTheParametersOfAReverseCallWithAnAmpersand(): void + { + $gateway = new FixtureGateway(); + $client = new PostcodeClient($gateway, new HttpFactory()); + + $client->reverse(9.0, 7.0, 25.0); + + self::assertSame( + 'lat=9&lng=7&max_distance_m=25', + $gateway->requests[0]->getUri()->getQuery(), + ); + } +} From c281f937c5652aba8abc4150d0cd41c62807bd07 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:46:51 +0100 Subject: [PATCH 14/21] fix(repo): list the client options in the API dump again The dump left out a symbol when its doc comment held the text @internal anywhere. The doc of the timer parameter held it, so the dump lost the whole constructor of PostcodeClient, and a rename of an option did not change the dump. The dump now reads only a tag at the start of a doc line. The timer parameter doc no longer holds the word, and the dump shows the constructor again. Signed-off-by: MobileMage --- docs/api-dump.md | 1 + scripts/dump-api-symbols.php | 4 +++- src/Client/PostcodeClient.php | 4 ++-- tests/Fixtures/ApiDump/Noted.php | 16 ++++++++++++++++ tests/Fixtures/ApiDump/expected.md | 5 +++++ 5 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 tests/Fixtures/ApiDump/Noted.php diff --git a/docs/api-dump.md b/docs/api-dump.md index eddf04d..ce4859c 100644 --- a/docs/api-dump.md +++ b/docs/api-dump.md @@ -54,6 +54,7 @@ final class LookupResult final class PostcodeClient { + public function __construct(\Psr\Http\Client\ClientInterface $transport, \Psr\Http\Message\RequestFactoryInterface $requestFactory, ?string $apiKey = null, string $baseUrl = 'https://api.postcode.gov.ng', ?int $timeoutMs = null, int $maxRetries = 2, int $cacheTtlMs = 0, ?\Psr\SimpleCache\CacheInterface $cache = null, ?Internal\Timer $timer = null); public function lookup(\Gatepost\Postcode\Postcode|string $code, int $level = 1): LookupResult; public function reverse(float $lat, float $lng, ?float $maxDistanceM = null): ReverseResult; public function autocomplete(string $q): AutocompleteResult; diff --git a/scripts/dump-api-symbols.php b/scripts/dump-api-symbols.php index 6a93a03..17afd70 100644 --- a/scripts/dump-api-symbols.php +++ b/scripts/dump-api-symbols.php @@ -111,9 +111,11 @@ function unwritable(string $what, mixed $value): RuntimeException } // The doc comments of the package say @internal for a public symbol that only the package calls. +// Only a tag counts: the word in the text of a doc, such as a parameter, hides nothing. function isInternal(string|false $docComment): bool { - return $docComment !== false && \str_contains($docComment, '@internal'); + return $docComment !== false + && \preg_match('/^[ \t]*(?:\/\*\*|\*)?[ \t]*@internal\b/m', $docComment) === 1; } /** diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index 7775a09..61ea1c0 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -77,8 +77,8 @@ final class PostcodeClient * the cache off. * @param ?CacheInterface $cache A PSR-16 cache for the results. The client * needs one when cacheTtlMs is above 0. - * @param ?Timer $timer The clock and the waits of the client, for - * tests. @internal Leave it null. + * @param ?Timer $timer The clock and the waits of the client. + * Only the tests pass it. Leave it null. * * @throws InvalidArgumentException When an option is out of range, the key is empty, or * cacheTtlMs is above 0 with no cache. diff --git a/tests/Fixtures/ApiDump/Noted.php b/tests/Fixtures/ApiDump/Noted.php new file mode 100644 index 0000000..e3d0740 --- /dev/null +++ b/tests/Fixtures/ApiDump/Noted.php @@ -0,0 +1,16 @@ + Date: Sat, 3 Oct 2026 04:47:05 +0100 Subject: [PATCH 15/21] fix(repo): stop the mutation run from crashing on an escaped mutant The decimal writer stripped trailing zeros that its own search never leaves, so the call could not change any result. Infection's console reporter crashed on the mutant that removed it, because it put a style tag right after the backslash of the native call name. The command then exited 1, and the weekly mutation workflow would fail on its first run. The dead call is gone. The mutation script also passes --show-mutations=0, so that a later mutant of that kind cannot crash the reporter. The text log still lists each escaped mutant. Signed-off-by: MobileMage --- composer.json | 2 +- src/Client/Internal/Decimal.php | 3 ++- tests/Unit/Client/DecimalTest.php | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 7e86907..d2d716a 100644 --- a/composer.json +++ b/composer.json @@ -75,7 +75,7 @@ "licences": "uvx --with charset-normalizer==3.5.2 reuse@6.2.0 lint", "mutation": [ "@putenv XDEBUG_MODE=coverage", - "tools/infection/vendor/bin/infection --threads=max" + "tools/infection/vendor/bin/infection --threads=max --show-mutations=0" ], "phpmd": "@php -d error_reporting=24575 vendor/bin/phpmd scripts,src,tests text phpmd.xml", "phpstan": "phpstan analyse --no-progress", diff --git a/src/Client/Internal/Decimal.php b/src/Client/Internal/Decimal.php index 9a40491..2e9063c 100644 --- a/src/Client/Internal/Decimal.php +++ b/src/Client/Internal/Decimal.php @@ -47,7 +47,8 @@ private static function plain(string $scientific): string { [$mantissa, $exponent] = \explode('e', $scientific); $sign = \str_starts_with($mantissa, '-') ? '-' : ''; - $digits = \rtrim(\str_replace(['-', '.'], '', $mantissa), '0'); + // The search stops at the fewest digits that read back, so the last digit is not 0. + $digits = \str_replace(['-', '.'], '', $mantissa); // The count of digits before the decimal point. $whole = (int) $exponent + 1; if ($whole <= 0) { diff --git a/tests/Unit/Client/DecimalTest.php b/tests/Unit/Client/DecimalTest.php index 08fdbf9..8eb0a25 100644 --- a/tests/Unit/Client/DecimalTest.php +++ b/tests/Unit/Client/DecimalTest.php @@ -24,6 +24,7 @@ final class DecimalTest extends TestCase public static function numbers(): array { return [ + 'one' => [1.0, '1'], 'a whole number' => [9.0, '9'], 'a whole number with zeros' => [250.0, '250'], 'a negative whole number' => [-90.0, '-90'], From 88064365d61f35fea8e96564f800ec9d98b75fbd Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:47:54 +0100 Subject: [PATCH 16/21] fix(client): read all three HTTP date forms of Retry-After RFC 9110 says that a recipient must accept IMF-fixdate, RFC 850 and asctime dates. The client read only the first, so a 429 with an RFC 850 or asctime date gave no wait. The format that PHP uses to read a weekday also moves the date to that weekday, so a date with a wrong weekday gave a wait of days. The client now reads the three forms, with the same shape rules as the TypeScript client, and it counts a date whose weekday does not match as no header. Signed-off-by: MobileMage --- src/Client/Internal/Sender.php | 50 +++++++++++++++++++---- tests/Unit/Client/SenderRateLimitTest.php | 34 +++++++++++++++ 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/Client/Internal/Sender.php b/src/Client/Internal/Sender.php index 7a10106..283eeb1 100644 --- a/src/Client/Internal/Sender.php +++ b/src/Client/Internal/Sender.php @@ -30,7 +30,28 @@ final class Sender private const MAX_JITTER_MS = 250; private const LONGEST_RETRY_AFTER_MS = 10_000; private const RETRIED_STATUSES = [502, 503, 504]; - private const HTTP_DATE = 'D, d M Y H:i:s \G\M\T'; + // The three HTTP date forms of RFC 9110: IMF-fixdate, RFC 850 and asctime. Each entry holds a + // pattern that splits the weekday from the rest, the format of the rest, and the format that + // writes the weekday back. The format of createFromFormat() would move a date to the weekday + // that it reads, so the client reads the date without its weekday and compares the weekdays. + private const HTTP_DATES = [ + [ + '/\A([A-Z][a-z]{2}), (\d\d [A-Z][a-z]{2} \d{4} \d\d:\d\d:\d\d GMT)\z/', + 'd M Y H:i:s \G\M\T', + 'D', + ], + [ + '/\A([A-Z][a-z]+day), (\d\d-[A-Z][a-z]{2}-\d\d \d\d:\d\d:\d\d GMT)\z/', + 'd-M-y H:i:s \G\M\T', + 'l', + ], + [ + // A day of one digit has a space before it, so two spaces follow the month. + '/\A([A-Z][a-z]{2}) ([A-Z][a-z]{2} (?: \d|\d\d) \d\d:\d\d:\d\d \d{4})\z/', + 'M j H:i:s Y', + 'D', + ], + ]; public function __construct( private readonly ClientInterface $transport, @@ -189,8 +210,8 @@ private static function apiCode(mixed $body): ?string /** * The wait that the Retry-After header of a 429, 502, 503 or 504 asks for, as seconds or as - * an HTTP date. A header in another form counts as no header, and so does a date in the past - * or a date that does not exist. + * an HTTP date. A header in another form counts as no header, and so does a date in the past, + * a date that does not exist and a date with the wrong weekday. */ private function retryAfterMs(ResponseInterface $response): ?int { @@ -203,14 +224,29 @@ private function retryAfterMs(ResponseInterface $response): ?int if (\preg_match('/\A\d{1,12}\z/', $value) === 1) { return (int) $value * 1000; } - $utc = new DateTimeZone('UTC'); - $date = DateTimeImmutable::createFromFormat(self::HTTP_DATE, $value, $utc); - $problems = DateTimeImmutable::getLastErrors(); - if ($date === false || ($problems !== false && $problems['warning_count'] > 0)) { + $date = self::httpDate($value); + if ($date === null) { return null; } $waitMs = (int) \ceil($date->getTimestamp() * 1000 - $this->timer->nowMs()); return $waitMs < 0 ? null : $waitMs; } + + private static function httpDate(string $value): ?DateTimeImmutable + { + foreach (self::HTTP_DATES as [$shape, $format, $weekdayFormat]) { + if (\preg_match($shape, $value, $parts) !== 1) { + continue; + } + $rest = \preg_replace('/ +/', ' ', $parts[2]) ?? $parts[2]; + $date = DateTimeImmutable::createFromFormat($format, $rest, new DateTimeZone('UTC')); + $problems = DateTimeImmutable::getLastErrors(); + $isDate = $date !== false && ($problems === false || $problems['warning_count'] === 0); + + return $isDate && $date->format($weekdayFormat) === $parts[1] ? $date : null; + } + + return null; + } } diff --git a/tests/Unit/Client/SenderRateLimitTest.php b/tests/Unit/Client/SenderRateLimitTest.php index ce0fc2c..7eb3950 100644 --- a/tests/Unit/Client/SenderRateLimitTest.php +++ b/tests/Unit/Client/SenderRateLimitTest.php @@ -26,6 +26,8 @@ public static function shortRetryAfters(): array 'one second' => ['1', 1000], 'ten seconds, the longest that the client waits' => ['10', 10_000], 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], + 'an RFC 850 date three seconds ahead' => ['Monday, 12-Oct-26 09:00:03 GMT', 3000], + 'an asctime date three seconds ahead' => ['Mon Oct 12 09:00:03 2026', 3000], 'an HTTP date in the current second' => ['Mon, 12 Oct 2026 09:00:00 GMT', 0], ]; } @@ -62,6 +64,38 @@ public static function finalRateLimits(): array ['Retry-After' => 'Sat, 31 Feb 2027 09:00:03 GMT'], null, ], + 'an asctime date with a padded day' => [ + ['Retry-After' => 'Tue Nov 3 09:00:03 2026'], + 1_900_803_000, + ], + 'a date with the wrong weekday' => [ + ['Retry-After' => 'Fri, 12 Oct 2026 09:00:03 GMT'], + null, + ], + 'an RFC 850 date with the wrong weekday' => [ + ['Retry-After' => 'Friday, 12-Oct-26 09:00:03 GMT'], + null, + ], + 'an asctime date with the wrong weekday' => [ + ['Retry-After' => 'Fri Oct 12 09:00:03 2026'], + null, + ], + 'a date with a lower case month' => [ + ['Retry-After' => 'Mon, 12 oct 2026 09:00:03 GMT'], + null, + ], + 'a date with a numeric zone' => [ + ['Retry-After' => 'Mon, 12 Oct 2026 09:00:03 +0000'], + null, + ], + 'an asctime date with a zone' => [ + ['Retry-After' => 'Mon Oct 12 09:00:03 2026 GMT'], + null, + ], + 'a date with text after it' => [ + ['Retry-After' => 'Mon, 12 Oct 2026 09:00:03 GMT and more'], + null, + ], 'a number of ten digits' => [['Retry-After' => '1000000000'], 1_000_000_000_000], ]; } From 307740c96524f8071b181e0d6fe31f3558908227 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:50:11 +0100 Subject: [PATCH 17/21] refactor(client): drop a dead fallback in the HTTP date reader The reader collapsed spaces with preg_replace() and kept the original text for a null result. The pattern is valid, so the result is never null, and the fallback could not run. Only the asctime form can hold two spaces in a row, so a plain replace of two spaces is enough. Signed-off-by: MobileMage --- src/Client/Internal/Sender.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Client/Internal/Sender.php b/src/Client/Internal/Sender.php index 283eeb1..7cb6ac6 100644 --- a/src/Client/Internal/Sender.php +++ b/src/Client/Internal/Sender.php @@ -239,7 +239,8 @@ private static function httpDate(string $value): ?DateTimeImmutable if (\preg_match($shape, $value, $parts) !== 1) { continue; } - $rest = \preg_replace('/ +/', ' ', $parts[2]) ?? $parts[2]; + // Only the asctime form can hold two spaces in a row, before a day of one digit. + $rest = \str_replace(' ', ' ', $parts[2]); $date = DateTimeImmutable::createFromFormat($format, $rest, new DateTimeZone('UTC')); $problems = DateTimeImmutable::getLastErrors(); $isDate = $date !== false && ($problems === false || $problems['warning_count'] === 0); From c43cdba40b65ddcc23f17b204ae34c43a6dca90c Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:50:17 +0100 Subject: [PATCH 18/21] test(client): pin retry, cache and message rules that no test covered A reverse call that times out must retry. A client with no maxRetries must retry twice. A lookup response with the fields of levels 3 and 4 must read as level 4. A store that fails only when the client reads an entry must cost a request and nothing more. No test failed when one of these rules changed. The tests now fail for each of those changes. They also pin the 429 wait message at its rounding edges, the two autocomplete input messages and the decimal form of 1. The 429 messages are made inside the test body, because the mutation tool changed the code only after a data provider had made them. Signed-off-by: MobileMage --- tests/Unit/Client/ArrayCache.php | 6 ++ tests/Unit/Client/AutocompleteEdgeTest.php | 25 ++++++ tests/Unit/Client/CacheFaultTest.php | 16 ++++ tests/Unit/Client/LookupResultTest.php | 12 +++ tests/Unit/Client/PostcodeExceptionTest.php | 30 +++++++ tests/Unit/Client/RetryByCallTest.php | 88 +++++++++++++++++++++ tests/Unit/Client/SenderRateLimitTest.php | 12 +++ 7 files changed, 189 insertions(+) create mode 100644 tests/Unit/Client/RetryByCallTest.php diff --git a/tests/Unit/Client/ArrayCache.php b/tests/Unit/Client/ArrayCache.php index a25ea7e..66dc223 100644 --- a/tests/Unit/Client/ArrayCache.php +++ b/tests/Unit/Client/ArrayCache.php @@ -27,6 +27,9 @@ final class ArrayCache implements CacheInterface /** A failure that get() throws, as a broken store does. */ public ?Throwable $getFails = null; + /** A failure that get() throws for each key except the generation key of the client. */ + public ?Throwable $entryGetFails = null; + /** A failure that set() throws. */ public ?Throwable $setFails = null; @@ -35,6 +38,9 @@ public function get(string $key, mixed $default = null): mixed if ($this->getFails !== null) { throw $this->getFails; } + if ($this->entryGetFails !== null && $key !== 'gatepost.generation') { + throw $this->entryGetFails; + } return \array_key_exists($key, $this->entries) ? $this->entries[$key] : $default; } diff --git a/tests/Unit/Client/AutocompleteEdgeTest.php b/tests/Unit/Client/AutocompleteEdgeTest.php index e45ee0a..38475f0 100644 --- a/tests/Unit/Client/AutocompleteEdgeTest.php +++ b/tests/Unit/Client/AutocompleteEdgeTest.php @@ -37,6 +37,31 @@ public function saysThatTextOverTheInputLimitIsTooLong(): void } } + #[Test] + public function saysWhatToDoWithTextOverTheInputLimit(): void + { + $client = new PostcodeClient(new FixtureGateway(), new HttpFactory()); + + $this->expectExceptionObject(PostcodeException::invalidInput( + 'The text has more than 64 characters. Send no request for it.', + )); + + $client->autocomplete('FC' . \str_repeat(' ', 70)); + } + + #[Test] + public function saysWhatToDoWithTextThatIsNotOneToElevenLettersAndDigits(): void + { + $client = new PostcodeClient(new FixtureGateway(), new HttpFactory()); + + $this->expectExceptionObject(PostcodeException::invalidInput( + 'The text must hold 1 to 11 letters A to Z and digits 0 to 9, after spaces and ' + . 'hyphens are removed. Send no request until the user types one.', + )); + + $client->autocomplete('FC!'); + } + #[Test] public function saysThatTextWithBytesThatAreNotUtf8IsNotText(): void { diff --git a/tests/Unit/Client/CacheFaultTest.php b/tests/Unit/Client/CacheFaultTest.php index 71399a6..eeb60bc 100644 --- a/tests/Unit/Client/CacheFaultTest.php +++ b/tests/Unit/Client/CacheFaultTest.php @@ -48,6 +48,22 @@ public function readsAStoreThatFailsOnReadAsAMiss(Throwable $failure): void self::assertCount(2, $gateway->requests); } + #[Test] + #[DataProvider('failures')] + public function readsAStoreThatFailsOnlyOnTheReadOfAnEntryAsAMiss(Throwable $failure): void + { + $gateway = new FixtureGateway(); + $store = new ArrayCache(); + $client = self::cachingClient($gateway, $store); + // The first call makes the generation key. The store then fails on each entry read. + $client->lookup(FixtureGateway::UNIT); + $store->entryGetFails = $failure; + + self::assertTrue($client->lookup(FixtureGateway::UNIT)->valid); + + self::assertCount(2, $gateway->requests); + } + #[Test] #[DataProvider('failures')] public function keepsTheResultWhenTheStoreFailsOnWrite(Throwable $failure): void diff --git a/tests/Unit/Client/LookupResultTest.php b/tests/Unit/Client/LookupResultTest.php index 2ec510b..fc1b251 100644 --- a/tests/Unit/Client/LookupResultTest.php +++ b/tests/Unit/Client/LookupResultTest.php @@ -78,6 +78,18 @@ public static function levelFields(): array 'a building use status' => [['building_use_status' => 'residential'], 3], 'other building information' => [['other_building_info' => ['floors' => 2]], 4], 'a point geometry' => [['point_geometry' => ['type' => 'Point']], 5], + 'the fields of levels 2 and 3' => [ + ['recent_house_address' => ['recent' => 'SYNTHETIC'], 'building_use_status' => 'x'], + 3, + ], + 'the fields of levels 3 and 4' => [ + ['building_use_status' => 'residential', 'other_building_info' => ['floors' => 2]], + 4, + ], + 'the fields of levels 4 and 5' => [ + ['other_building_info' => ['floors' => 2], 'point_geometry' => ['type' => 'Point']], + 5, + ], 'a field of level 2 that is null' => [['administrative_address' => null], 1], ]; } diff --git a/tests/Unit/Client/PostcodeExceptionTest.php b/tests/Unit/Client/PostcodeExceptionTest.php index 18bb154..52ac33c 100644 --- a/tests/Unit/Client/PostcodeExceptionTest.php +++ b/tests/Unit/Client/PostcodeExceptionTest.php @@ -130,6 +130,36 @@ public function saysWhatHappenedAndWhatToDo(PostcodeException $error, string $me self::assertSame(0, $error->getCode()); } + /** + * @return array + */ + public static function retryAfterWaits(): array + { + return [ + 'zero' => [0, 'Wait, then try again.'], + 'one second' => [1000, 'Wait 1 s, then try again.'], + 'one millisecond over one second' => [1001, 'Wait 2 s, then try again.'], + 'one millisecond under one second' => [999, 'Wait 1 s, then try again.'], + 'a fraction of a second' => [11_400, 'Wait 12 s, then try again.'], + ]; + } + + // The error is made inside the test, not in a data provider, so a mutation test that changes + // the message code also changes the error that this test reads. + #[Test] + #[DataProvider('retryAfterWaits')] + public function tellsTheUserHowLongToWaitAfterATooManyRequestsError( + int $retryAfterMs, + string $wait, + ): void { + $error = PostcodeException::fromStatus(429, 'rate_limited', $retryAfterMs); + + self::assertSame( + 'The gateway refused the request because the key sent too many (HTTP 429). ' . $wait, + $error->getMessage(), + ); + } + #[Test] public function keepsTheStatusOfAnUnreadableResponse(): void { diff --git a/tests/Unit/Client/RetryByCallTest.php b/tests/Unit/Client/RetryByCallTest.php new file mode 100644 index 0000000..3142f27 --- /dev/null +++ b/tests/Unit/Client/RetryByCallTest.php @@ -0,0 +1,88 @@ + + */ + public static function callsThatRetryATimeout(): array + { + return [ + 'lookup' => [ + static fn(PostcodeClient $client): mixed => $client->lookup(FixtureGateway::UNIT), + 'lookup/valid-level-1', + ], + 'reverse' => [ + static fn(PostcodeClient $client): mixed => $client->reverse(9.0, 7.0), + 'reverse/unit', + ], + ]; + } + + /** + * @param Closure(PostcodeClient): mixed $call + */ + #[Test] + #[DataProvider('callsThatRetryATimeout')] + public function retriesATimeoutOnceAndKeepsTheLaterAnswer(Closure $call, string $fixture): void + { + $timer = new FakeTimer(); + $transport = new ScriptedTransport( + [8000.0, FixtureGateway::fixture($fixture)], + $timer, + ); + $client = new PostcodeClient( + $transport, + new HttpFactory(), + timeoutMs: 8000, + timer: $timer, + ); + + $call($client); + + self::assertCount(2, $transport->requests); + self::assertSame([500], $timer->waits); + } + + #[Test] + public function retriesTwiceWhenTheCallerSetsNoMaxRetries(): void + { + $timer = new FakeTimer(); + $transport = new ScriptedTransport([ + ScriptedTransport::error(503, 'unavailable'), + ScriptedTransport::error(503, 'unavailable'), + ScriptedTransport::error(503, 'unavailable'), + ]); + $client = new PostcodeClient($transport, new HttpFactory(), timer: $timer); + + try { + $client->lookup(FixtureGateway::UNIT); + self::fail('The call did not fail.'); + } catch (PostcodeException $error) { + self::assertSame(ErrorCode::ServerError, $error->errorCode()); + } + self::assertCount(3, $transport->requests); + self::assertSame([500, 1000], $timer->waits); + } +} diff --git a/tests/Unit/Client/SenderRateLimitTest.php b/tests/Unit/Client/SenderRateLimitTest.php index 7eb3950..8e2a40b 100644 --- a/tests/Unit/Client/SenderRateLimitTest.php +++ b/tests/Unit/Client/SenderRateLimitTest.php @@ -80,6 +80,18 @@ public static function finalRateLimits(): array ['Retry-After' => 'Fri Oct 12 09:00:03 2026'], null, ], + 'a date that rolls over, with the weekday of the new date' => [ + ['Retry-After' => 'Wed, 31 Feb 2027 09:00:03 GMT'], + null, + ], + 'a date with a minute of 60' => [ + ['Retry-After' => 'Mon, 12 Oct 2026 09:60:03 GMT'], + null, + ], + 'a date with a month that does not exist' => [ + ['Retry-After' => 'Mon, 12 Foo 2026 09:00:03 GMT'], + null, + ], 'a date with a lower case month' => [ ['Retry-After' => 'Mon, 12 oct 2026 09:00:03 GMT'], null, From cbe479dafd4bb1e2f84959ccbcf20536e626446b Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 04:53:50 +0100 Subject: [PATCH 19/21] docs(client): correct the README on Retry-After, the cache and the key retryAfterMs() holds the wait of a 502, 503 or 504 too, and the README named only the 429. It also left out that a 5xx uses a valid Retry-After in place of its own wait, that the cache hash covers the base URL, and that the Node range of the mock server skips Node 23. The exception of a failed connection holds the failure of the transport, which can hold the request with the X-API-Key header. The README now says not to log it with its objects. Two doc comments lose a broken wrap and a note that a caller of the constructor cannot use. Signed-off-by: MobileMage --- README.md | 8 ++++---- src/Client/PostcodeClient.php | 4 +--- src/Client/PostcodeException.php | 5 ++--- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9961624..14e2123 100644 --- a/README.md +++ b/README.md @@ -132,9 +132,9 @@ Each call refuses bad input before it sends a request, and throws `ErrorCode::In ### Handle errors and timeouts -Each failed call throws `PostcodeException`. `errorCode()` gives one of the cases of `ErrorCode`, such as `ErrorCode::RateLimited`. `status()` gives the HTTP status, `apiCode()` gives the gateway's own error code, and `retryAfterMs()` gives the wait that a 429 asked for. `ErrorCode::UnexpectedResponse` means that the gateway answered 200 with a body that the client cannot read. None of the messages of the client holds the API key. Show the user a message for the error code, not the message of the exception, which is for developers. +Each failed call throws `PostcodeException`. `errorCode()` gives one of the cases of `ErrorCode`, such as `ErrorCode::RateLimited`. `status()` gives the HTTP status, `apiCode()` gives the gateway's own error code, and `retryAfterMs()` gives the wait that the `Retry-After` header of a 429, 502, 503 or 504 asked for. `ErrorCode::UnexpectedResponse` means that the gateway answered 200 with a body that the client cannot read. None of the messages of the client holds the API key. Show the user a message for the error code, not the message of the exception, which is for developers. The exception of a failed connection or timeout holds the failure of your transport as `getPrevious()`. That failure can hold the request with its `X-API-Key` header, so do not log it with its objects. -The client tries a call again after a 502, 503 or 504, a failed connection or a timeout, at most twice by default (`maxRetries`). It waits 500 ms before the first retry and twice as long before the next, plus a random part of up to 250 ms. After a 429, it waits only when `Retry-After` asks for 10 seconds or less. A timeout of `autocomplete` gets no retry, because the next keystroke replaces the call. +The client tries a call again after a 502, 503 or 504, a failed connection or a timeout, at most twice by default (`maxRetries`). It waits 500 ms before the first retry and twice as long before the next, plus a random part of up to 250 ms. After a 429, it waits only when `Retry-After` asks for 10 seconds or less. After a 502, 503 or 504, it waits for a valid `Retry-After` of 10 seconds or less in place of its own wait. `Retry-After` can hold seconds or an HTTP date in any of the three forms of RFC 9110. A timeout of `autocomplete` gets no retry, because the next keystroke replaces the call. PSR-18 has no timeout, so the client cannot stop a slow attempt. Set the timeout on your HTTP client, and pass the same value as `timeoutMs`, as the examples do with 15 seconds. The client applies `timeoutMs` to every call, because one transport holds one timeout. 15 seconds is long enough for `autocomplete`, whose default wait is 15 seconds. The other calls wait 8 seconds by default, and `timeoutMs` replaces that value. The client reports a failed attempt that took `timeoutMs` or more as `ErrorCode::Timeout`, and a failure that came sooner as `ErrorCode::NetworkError`. If the two values differ, the client reads a slow failure with the wrong code, and tries it again when it should not. @@ -142,7 +142,7 @@ PSR-18 has no timeout, so the client cannot stop a slow attempt. Set the timeout The client keeps no result by default. To keep results, pass `cacheTtlMs` and a PSR-16 `cache`, such as the cache of your framework. An identical call within that time gets the kept result with no request. The client never keeps an error. -The client sets no size bound of its own, so use a store with eviction, such as Redis or APCu with a size limit. A store with no bound grows with each different postcode that you look up. The cache keys hold a keyed hash of the call, never the API key or the postcode in the clear. Two clients with the same API key and the same cache share their results. +The client sets no size bound of its own, so use a store with eviction, such as Redis or APCu with a size limit. A store with no bound grows with each different postcode that you look up. The cache keys hold a keyed hash of the call, never the API key or the postcode in the clear. Two clients with the same API key, the same base URL and the same cache share their results. A cache that fails never fails a call. The client reads a failed read as a miss, and skips a failed write. `clearCache()` is the exception. It removes every result that the clients of the same cache kept, also for another API key, and leaves the other entries of the cache alone. When the store fails, `clearCache()` throws the failure, so that you do not read old results after you asked to remove them. @@ -175,7 +175,7 @@ composer --working-dir=tools/bc-check install --ignore-platform-req=ext-intl composer check ``` -`composer check` runs no contract scenario and no example of the client, because they need the mock server, which runs on Node 22.22.2 or later. CI runs them in its `contract` job. To run them yourself, clone the js repo next to this one, start its mock server with this repo's spec, and run `composer contract`: +`composer check` runs no contract scenario and no example of the client, because they need the mock server, which runs on Node 22.22.2 or later in the 22 line, or on Node 24.15.0 or later. CI runs them in its `contract` job. To run them yourself, clone the js repo next to this one, start its mock server with this repo's spec, and run `composer contract`: ```sh git clone --depth 1 https://github.com/gatepost-dev/js ../js diff --git a/src/Client/PostcodeClient.php b/src/Client/PostcodeClient.php index 61ea1c0..cbf7fd1 100644 --- a/src/Client/PostcodeClient.php +++ b/src/Client/PostcodeClient.php @@ -54,6 +54,7 @@ final class PostcodeClient private readonly ?ResultCache $cache; + // Callers name each option, so the long parameter list does not cost them any clarity. /** * @param ClientInterface $transport A PSR-18 HTTP client. PSR-18 has no timeout, * so set the timeout of the transport and pass @@ -83,9 +84,6 @@ final class PostcodeClient * @throws InvalidArgumentException When an option is out of range, the key is empty, or * cacheTtlMs is above 0 with no cache. * - * Callers name each option, and TELL-3 counts no named or defaulted parameter, so PHPMD's - * count of parameters does not apply here. - * * @SuppressWarnings("PHPMD.ExcessiveParameterList") */ public function __construct( diff --git a/src/Client/PostcodeException.php b/src/Client/PostcodeException.php index da3bc86..6b1acbc 100644 --- a/src/Client/PostcodeException.php +++ b/src/Client/PostcodeException.php @@ -115,9 +115,8 @@ public function apiCode(): ?string } /** - * The wait in milliseconds that a valid Retry-After header of a 429, 502, - * 503 or 504 asked for, or null. Tell - * the user when to try again. + * The wait in milliseconds that a valid Retry-After header of a 429, 502, 503 or 504 asked + * for, or null. Tell the user when to try again. */ public function retryAfterMs(): ?int { From 8f5489ffb12bc791c04780853863fe2a7c2520e2 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 05:04:37 +0100 Subject: [PATCH 20/21] fix(client): ignore the day name of a Retry-After date RFC 9110 does not require a recipient to check the day name of an HTTP date, and the TypeScript client ignores it. Commit 8806436 made the PHP client count a date with a wrong day name as no header. The two clients then gave different waits for the same header. The client still reads the day name in each of the three date forms, but it does not compare it. A date with a wrong day name is valid and gives the wait of its date. The tests that expected no wait now expect the wait of the date. Signed-off-by: MobileMage --- src/Client/Internal/Sender.php | 25 ++++++++++------------- tests/Unit/Client/SenderRateLimitTest.php | 15 +++----------- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/Client/Internal/Sender.php b/src/Client/Internal/Sender.php index 7cb6ac6..2c0d543 100644 --- a/src/Client/Internal/Sender.php +++ b/src/Client/Internal/Sender.php @@ -31,25 +31,22 @@ final class Sender private const LONGEST_RETRY_AFTER_MS = 10_000; private const RETRIED_STATUSES = [502, 503, 504]; // The three HTTP date forms of RFC 9110: IMF-fixdate, RFC 850 and asctime. Each entry holds a - // pattern that splits the weekday from the rest, the format of the rest, and the format that - // writes the weekday back. The format of createFromFormat() would move a date to the weekday - // that it reads, so the client reads the date without its weekday and compares the weekdays. + // pattern and the format of the date in it. The pattern reads the day name but the client does + // not check it, as RFC 9110 does not require it. The format of createFromFormat() would move a + // date to the day name that it reads, so the format has no day name. private const HTTP_DATES = [ [ - '/\A([A-Z][a-z]{2}), (\d\d [A-Z][a-z]{2} \d{4} \d\d:\d\d:\d\d GMT)\z/', + '/\A[A-Z][a-z]{2}, (\d\d [A-Z][a-z]{2} \d{4} \d\d:\d\d:\d\d GMT)\z/', 'd M Y H:i:s \G\M\T', - 'D', ], [ - '/\A([A-Z][a-z]+day), (\d\d-[A-Z][a-z]{2}-\d\d \d\d:\d\d:\d\d GMT)\z/', + '/\A[A-Z][a-z]+day, (\d\d-[A-Z][a-z]{2}-\d\d \d\d:\d\d:\d\d GMT)\z/', 'd-M-y H:i:s \G\M\T', - 'l', ], [ // A day of one digit has a space before it, so two spaces follow the month. - '/\A([A-Z][a-z]{2}) ([A-Z][a-z]{2} (?: \d|\d\d) \d\d:\d\d:\d\d \d{4})\z/', + '/\A[A-Z][a-z]{2} ([A-Z][a-z]{2} (?: \d|\d\d) \d\d:\d\d:\d\d \d{4})\z/', 'M j H:i:s Y', - 'D', ], ]; @@ -210,8 +207,8 @@ private static function apiCode(mixed $body): ?string /** * The wait that the Retry-After header of a 429, 502, 503 or 504 asks for, as seconds or as - * an HTTP date. A header in another form counts as no header, and so does a date in the past, - * a date that does not exist and a date with the wrong weekday. + * an HTTP date. A header in another form counts as no header, and so does a date in the past + * and a date that does not exist. The day name of a date is not checked. */ private function retryAfterMs(ResponseInterface $response): ?int { @@ -235,17 +232,17 @@ private function retryAfterMs(ResponseInterface $response): ?int private static function httpDate(string $value): ?DateTimeImmutable { - foreach (self::HTTP_DATES as [$shape, $format, $weekdayFormat]) { + foreach (self::HTTP_DATES as [$shape, $format]) { if (\preg_match($shape, $value, $parts) !== 1) { continue; } // Only the asctime form can hold two spaces in a row, before a day of one digit. - $rest = \str_replace(' ', ' ', $parts[2]); + $rest = \str_replace(' ', ' ', $parts[1]); $date = DateTimeImmutable::createFromFormat($format, $rest, new DateTimeZone('UTC')); $problems = DateTimeImmutable::getLastErrors(); $isDate = $date !== false && ($problems === false || $problems['warning_count'] === 0); - return $isDate && $date->format($weekdayFormat) === $parts[1] ? $date : null; + return $isDate ? $date : null; } return null; diff --git a/tests/Unit/Client/SenderRateLimitTest.php b/tests/Unit/Client/SenderRateLimitTest.php index 8e2a40b..934f015 100644 --- a/tests/Unit/Client/SenderRateLimitTest.php +++ b/tests/Unit/Client/SenderRateLimitTest.php @@ -28,6 +28,9 @@ public static function shortRetryAfters(): array 'an HTTP date three seconds ahead' => ['Mon, 12 Oct 2026 09:00:03 GMT', 3000], 'an RFC 850 date three seconds ahead' => ['Monday, 12-Oct-26 09:00:03 GMT', 3000], 'an asctime date three seconds ahead' => ['Mon Oct 12 09:00:03 2026', 3000], + 'an HTTP date with the wrong weekday' => ['Fri, 12 Oct 2026 09:00:03 GMT', 3000], + 'an RFC 850 date with the wrong weekday' => ['Friday, 12-Oct-26 09:00:03 GMT', 3000], + 'an asctime date with the wrong weekday' => ['Fri Oct 12 09:00:03 2026', 3000], 'an HTTP date in the current second' => ['Mon, 12 Oct 2026 09:00:00 GMT', 0], ]; } @@ -68,18 +71,6 @@ public static function finalRateLimits(): array ['Retry-After' => 'Tue Nov 3 09:00:03 2026'], 1_900_803_000, ], - 'a date with the wrong weekday' => [ - ['Retry-After' => 'Fri, 12 Oct 2026 09:00:03 GMT'], - null, - ], - 'an RFC 850 date with the wrong weekday' => [ - ['Retry-After' => 'Friday, 12-Oct-26 09:00:03 GMT'], - null, - ], - 'an asctime date with the wrong weekday' => [ - ['Retry-After' => 'Fri Oct 12 09:00:03 2026'], - null, - ], 'a date that rolls over, with the weekday of the new date' => [ ['Retry-After' => 'Wed, 31 Feb 2027 09:00:03 GMT'], null, From a1a982bf013b5d255a020193765eb53d0fa6d556 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Sat, 3 Oct 2026 12:05:54 +0100 Subject: [PATCH 21/21] chore(repo): use spec v0.2.0 and the mock server on js main The spec and the mock server are merged. The spec submodule now names the spec release v0.2.0, and the contract job takes the mock server from the js commit on main, which holds the same server. Signed-off-by: MobileMage --- .github/workflows/ci.yml | 2 +- spec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2826af..f881bce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,7 +124,7 @@ jobs: with: repository: gatepost-dev/js # A js commit that holds packages/mock-server. Move it when the mock server changes. - ref: 2ee026be7be1272f8d58a3d649722000e6335fdd + ref: c94adef64c7d3640ddc20f53a27ec2f6fba96601 path: build/js sparse-checkout: packages/mock-server persist-credentials: false diff --git a/spec b/spec index a4f601d..359db01 160000 --- a/spec +++ b/spec @@ -1 +1 @@ -Subproject commit a4f601d0c58240318a4cce68302574c8bb021cae +Subproject commit 359db01aa97f2d5b183db431d14dbccb793af9f3