From 0737ba47620a23695c069b2ab59a43abe82b2d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 6 Aug 2026 11:10:27 +0200 Subject: [PATCH] Close crucial test gaps found by a suite audit; fix doc slips New tests, driven by cross-referencing the suite against Infection's escaped mutants: - MappingException on a 2xx body that fails DTO mapping (the SDK's known gotcha), asserting the request context and MappingError chain - both supported date formats (Z suffix and fractional seconds) - dates were previously entirely untested - MalformedResponseException when a 2xx body is valid JSON but not an array (only the invalid-JSON branch was covered) - a 202 Accepted operation response maps to Payment (matches the live-verified async behavior) - host-pinning guard edges: uppercase scheme/host and an explicit :443 default port are allowed - Content-Type stamping: absent on GET, a preset value is preserved - a 3xx row in the status-code provider - the non-payment callback test now asserts the guard's message, so removing the type guard can no longer hide behind the shape-mismatch path Also removes the dead $allowEmpty parameter of Client::decodeJson() (never passed true) and fixes two doc slips: updatePayment()'s docblock said PUT while the SDK sends PATCH, and the README's ValidationException example used a 3-char order id that would nowadays fail the 4-20 length rule instead of the duplicate check it narrates. The README now also states that unconditionally-required fields are required constructor arguments. Mutation score: MSI 72% -> 80%, covered MSI 76% -> 82%. --- README.md | 6 +- src/Client/Client.php | 6 +- src/Client/Endpoint/PaymentsEndpoint.php | 2 +- tests/Callback/CallbackTest.php | 3 + tests/Client/ClientTest.php | 61 +++++++++++++++++++ .../Client/Endpoint/PaymentsEndpointTest.php | 48 +++++++++++++++ 6 files changed, 119 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 903af97..0719363 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,10 @@ echo $payment->state; // "initial" echo $payment->state()?->name; // PaymentState enum (or null for an unknown value) ``` +Fields the Quickpay API unconditionally requires (verified against the live API) are required +constructor arguments — `orderId` and `currency` here, `amount` on links and operations. Every other +field is optional and simply omitted from the request JSON when unset. + ### Payment link flow (redirect the customer to the payment window) The recommended way to take a payment is to create the payment, create a link for it, then redirect @@ -211,7 +215,7 @@ use Setono\Quickpay\Exception\QuickpayException; use Setono\Quickpay\Exception\ValidationException; try { - $client->payments()->create(new CreatePaymentRequest(orderId: 'dup', currency: 'DKK')); + $client->payments()->create(new CreatePaymentRequest(orderId: 'dup-order-1', currency: 'DKK')); } catch (ValidationException $e) { $e->getMessageText(); // Quickpay's "message" $e->getErrorCode(); // Quickpay's "error_code" diff --git a/src/Client/Client.php b/src/Client/Client.php index 4f8eefb..94d8f70 100644 --- a/src/Client/Client.php +++ b/src/Client/Client.php @@ -308,14 +308,10 @@ private static function camelToSnake(string $key): string * * @throws MalformedResponseException if the body is not valid JSON or does not decode to an array */ - private static function decodeJson(RequestInterface $request, ResponseInterface $response, bool $allowEmpty = false): array + private static function decodeJson(RequestInterface $request, ResponseInterface $response): array { $body = (string) $response->getBody(); - if ($allowEmpty && '' === trim($body)) { - return []; - } - // Strip query + fragment so consumer-supplied secrets don't land in exception messages. $sanitizedUri = $request->getUri()->withQuery('')->withFragment(''); $context = sprintf(' [%s %s]', $request->getMethod(), (string) $sanitizedUri); diff --git a/src/Client/Endpoint/PaymentsEndpoint.php b/src/Client/Endpoint/PaymentsEndpoint.php index 48589e2..ff67e33 100644 --- a/src/Client/Endpoint/PaymentsEndpoint.php +++ b/src/Client/Endpoint/PaymentsEndpoint.php @@ -40,7 +40,7 @@ public function create(CreatePaymentRequest $request): Payment } /** - * PUT `/payments/{id}`. + * PATCH `/payments/{id}`. */ public function updatePayment(int $id, UpdatePaymentRequest $request): Payment { diff --git a/tests/Callback/CallbackTest.php b/tests/Callback/CallbackTest.php index 1a02e49..8bf9527 100644 --- a/tests/Callback/CallbackTest.php +++ b/tests/Callback/CallbackTest.php @@ -176,7 +176,10 @@ public function it_does_not_map_a_non_payment_resource_to_a_payment(): void self::assertFalse($callback->isPayment()); self::assertSame(['id' => 42, 'state' => 'active'], $callback->toArray()); + // Assert the MESSAGE too: without it, removing the type guard would still end in an + // InvalidCallbackException via the shape-mismatch path and the guard would go untested. $this->expectException(InvalidCallbackException::class); + $this->expectExceptionMessage('not a payment'); $callback->payment(); } diff --git a/tests/Client/ClientTest.php b/tests/Client/ClientTest.php index 3d0cc53..a8a85dd 100644 --- a/tests/Client/ClientTest.php +++ b/tests/Client/ClientTest.php @@ -4,6 +4,7 @@ namespace Setono\Quickpay\Client; +use Nyholm\Psr7\Factory\Psr17Factory; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Setono\Quickpay\Exception\ConflictException; @@ -139,6 +140,66 @@ public static function statusCodeProvider(): iterable yield '500' => [500, InternalServerErrorException::class]; yield '503' => [503, InternalServerErrorException::class]; yield '418' => [418, UnexpectedStatusCodeException::class]; + yield '302' => [302, UnexpectedStatusCodeException::class]; + } + + #[Test] + public function it_throws_when_the_body_is_valid_json_but_not_an_array(): void + { + $http = (new ScriptedHttpClient())->on(self::BASE . '/ping', '"pong"'); + + $this->expectException(MalformedResponseException::class); + $this->expectExceptionMessage('Expected decoded response body to be an array but got string'); + + $this->client($http)->ping(); + } + + #[Test] + public function it_does_not_set_a_content_type_on_get_requests(): void + { + $http = (new ScriptedHttpClient())->on(self::BASE . '/ping', self::fixture('ping.json')); + + $this->client($http)->ping(); + + self::assertFalse($http->sentRequests[0]->hasHeader('Content-Type')); + } + + #[Test] + public function it_preserves_a_preset_content_type(): void + { + $http = (new ScriptedHttpClient())->on(self::BASE . '/ping', self::fixture('ping.json')); + + $request = (new Psr17Factory())->createRequest('POST', self::BASE . '/ping') + ->withHeader('Content-Type', 'application/custom+json'); + $this->client($http)->request($request); + + self::assertSame('application/custom+json', $http->sentRequests[0]->getHeaderLine('Content-Type')); + } + + #[Test] + public function it_allows_an_absolute_url_regardless_of_casing(): void + { + // RFC 3986 hosts are case-insensitive — the host-pinning guard must not reject the + // Quickpay host just because it is written in upper case, and the port guard must resolve + // the default port from the lowercased SCHEME too. (The PSR-7 implementation then + // normalizes scheme + host to lower case and drops the default port on the wire.) + $http = (new ScriptedHttpClient())->on(self::BASE . '/ping', self::fixture('ping.json')); + + $this->client($http)->get('HTTPS://API.QUICKPAY.NET:443/ping'); + + self::assertSame(self::BASE . '/ping', (string) $http->sentRequests[0]->getUri()); + } + + #[Test] + public function it_allows_an_explicit_default_port_on_the_api_host(): void + { + // An explicit :443 matches the https default, so the port guard must not reject it. (The + // PSR-7 implementation then drops the redundant default port on the wire.) + $http = (new ScriptedHttpClient())->on(self::BASE . '/ping', self::fixture('ping.json')); + + $this->client($http)->get(self::BASE . ':443/ping'); + + self::assertSame(self::BASE . '/ping', (string) $http->sentRequests[0]->getUri()); } #[Test] diff --git a/tests/Client/Endpoint/PaymentsEndpointTest.php b/tests/Client/Endpoint/PaymentsEndpointTest.php index f601c34..52a7070 100644 --- a/tests/Client/Endpoint/PaymentsEndpointTest.php +++ b/tests/Client/Endpoint/PaymentsEndpointTest.php @@ -4,8 +4,10 @@ namespace Setono\Quickpay\Client\Endpoint; +use CuyZ\Valinor\Mapper\MappingError; use PHPUnit\Framework\Attributes\Test; use Setono\Quickpay\Enum\PaymentState; +use Setono\Quickpay\Exception\MappingException; use Setono\Quickpay\QuickpayTestCase; use Setono\Quickpay\Request\Payment\AuthorizePaymentRequest; use Setono\Quickpay\Request\Payment\BasketItem; @@ -259,6 +261,52 @@ public function it_overrides_the_client_wide_synchronized_default_per_call(): vo self::assertSame(self::BASE . '/payments/1234/capture', (string) $http->sentRequests[0]->getUri()); } + #[Test] + public function it_throws_a_mapping_exception_when_a_2xx_body_does_not_fit_the_dto(): void + { + // Valinor is strict: a single mis-typed field fails the WHOLE resource mapping (the `$raw` + // fallback only protects fields the SDK does not type). `id` cannot cast to int here. + $http = (new ScriptedHttpClient())->on( + self::BASE . '/payments/1234', + '{"id":"nope","order_id":"o-1","currency":"DKK","state":"new","merchant_id":1}', + ); + + try { + $this->client($http)->payments()->getById(1234); + self::fail('Expected a MappingException.'); + } catch (MappingException $e) { + self::assertStringContainsString('Could not map response body to', $e->getMessage()); + self::assertStringContainsString('[GET https://api.quickpay.net/payments/1234]', $e->getMessage()); + self::assertInstanceOf(MappingError::class, $e->getPrevious()); + } + } + + #[Test] + public function it_maps_both_supported_date_formats(): void + { + $http = (new ScriptedHttpClient())->on( + self::BASE . '/payments/1234', + '{"id":1234,"order_id":"o-1","currency":"DKK","state":"new","merchant_id":1,' + . '"created_at":"2018-10-17T13:25:44Z","updated_at":"2018-10-17T13:25:44.557Z"}', + ); + + $payment = $this->client($http)->payments()->getById(1234); + + self::assertSame('2018-10-17 13:25:44 +00:00', $payment->createdAt?->format('Y-m-d H:i:s P')); + self::assertSame('2018-10-17 13:25:44.557000', $payment->updatedAt?->format('Y-m-d H:i:s.u')); + } + + #[Test] + public function it_maps_a_202_accepted_operation_response(): void + { + // Async operations answer 202 Accepted; the body is still the full payment resource. + $http = (new ScriptedHttpClient())->on(self::BASE . '/payments/1234/capture', self::fixture('payment.json'), 202); + + $payment = $this->client($http)->payments()->capture(1234, new CaptureRequest(1000)); + + self::assertSame(1234, $payment->id); + } + #[Test] public function it_lists_payments(): void {