From 3a8e0da65e054da4976bf22d91f3b28984d85096 Mon Sep 17 00:00:00 2001
From: Krishan Koenig <krishan.koenig@googlemail.com>
Date: Fri, 7 Feb 2025 13:29:11 +0100
Subject: [PATCH] refactor: resources are required to always provide a
 response, responses are only hydrated if body is not empty

---
 src/Contracts/IsResponseAware.php             |  4 ++--
 .../SubscriptionEndpointCollection.php        |  4 ++--
 src/Fake/MockResponse.php                     |  7 ++----
 src/Http/Middleware/Hydrate.php               |  2 +-
 src/Traits/HasResponse.php                    | 10 +++------
 .../ProfileMethodEndpointCollectionTest.php   | 12 +++++-----
 .../SubscriptionEndpointCollectionTest.php    |  2 +-
 .../CancelPaymentRefundRequestTest.php        |  2 +-
 .../Requests/DeleteProfileRequestTest.php     |  2 +-
 .../Requests/RevokeMandateRequestTest.php     |  2 +-
 tests/MollieApiClientTest.php                 | 18 +++++++++++++++
 tests/Resources/BaseCollectionTest.php        | 13 +++++++++--
 tests/Resources/CursorCollectionTest.php      | 22 +++++++++++++++++++
 tests/Resources/LazyCollectionTest.php        |  5 +++++
 tests/Resources/MandateCollectionTest.php     |  5 +++++
 15 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/src/Contracts/IsResponseAware.php b/src/Contracts/IsResponseAware.php
index 97c2d482..d77955a5 100644
--- a/src/Contracts/IsResponseAware.php
+++ b/src/Contracts/IsResponseAware.php
@@ -6,7 +6,7 @@
 
 interface IsResponseAware extends ViableResponse
 {
-    public function getResponse(): ?Response;
+    public function getResponse(): Response;
 
-    public function setResponse(?Response $response): self;
+    public function setResponse(Response $response): self;
 }
diff --git a/src/EndpointCollection/SubscriptionEndpointCollection.php b/src/EndpointCollection/SubscriptionEndpointCollection.php
index eceec0c7..fb804d68 100644
--- a/src/EndpointCollection/SubscriptionEndpointCollection.php
+++ b/src/EndpointCollection/SubscriptionEndpointCollection.php
@@ -90,7 +90,7 @@ public function update(string $customerId, string $subscriptionId, array $payloa
      *
      * @throws RequestException
      */
-    public function cancelFor(Customer $customer, string $subscriptionId, bool $testmode = false): ?Subscription
+    public function cancelFor(Customer $customer, string $subscriptionId, bool $testmode = false): Subscription
     {
         return $this->cancelForId($customer->id, $subscriptionId, $testmode);
     }
@@ -100,7 +100,7 @@ public function cancelFor(Customer $customer, string $subscriptionId, bool $test
      *
      * @throws RequestException
      */
-    public function cancelForId(string $customerId, string $subscriptionId, bool $testmode = false): ?Subscription
+    public function cancelForId(string $customerId, string $subscriptionId, bool $testmode = false): Subscription
     {
         return $this->send((new CancelSubscriptionRequest($customerId, $subscriptionId))->test($testmode));
     }
diff --git a/src/Fake/MockResponse.php b/src/Fake/MockResponse.php
index dedcfb2c..1fa9dcf2 100644
--- a/src/Fake/MockResponse.php
+++ b/src/Fake/MockResponse.php
@@ -59,12 +59,9 @@ public static function created($body = [], string $resourceKey = ''): self
         return new self($body, 201, $resourceKey);
     }
 
-    /**
-     * @param  string|array  $body
-     */
-    public static function noContent($body = [], string $resourceKey = ''): self
+    public static function noContent(string $resourceKey = ''): self
     {
-        return new self($body, 204, $resourceKey);
+        return new self('', 204, $resourceKey);
     }
 
     public static function notFound(string $resourceKey = ''): self
diff --git a/src/Http/Middleware/Hydrate.php b/src/Http/Middleware/Hydrate.php
index 749a1c2a..5ff600c4 100644
--- a/src/Http/Middleware/Hydrate.php
+++ b/src/Http/Middleware/Hydrate.php
@@ -13,7 +13,7 @@ public function __invoke(Response $response)
     {
         $request = $response->getRequest();
 
-        if ($request instanceof ResourceHydratableRequest && $request->isHydratable()) {
+        if (!$response->isEmpty() &&$request instanceof ResourceHydratableRequest && $request->isHydratable()) {
             return (new ResourceHydrator)->hydrate($request, $response);
         }
 
diff --git a/src/Traits/HasResponse.php b/src/Traits/HasResponse.php
index d85735f7..4869b886 100644
--- a/src/Traits/HasResponse.php
+++ b/src/Traits/HasResponse.php
@@ -8,14 +8,14 @@
 
 trait HasResponse
 {
-    protected ?Response $response = null;
+    protected Response $response;
 
-    public function getResponse(): ?Response
+    public function getResponse(): Response
     {
         return $this->response;
     }
 
-    public function setResponse(?Response $response): self
+    public function setResponse(Response $response): self
     {
         $this->response = $response;
 
@@ -24,10 +24,6 @@ public function setResponse(?Response $response): self
 
     public function getPendingRequest(): PendingRequest
     {
-        if (! $this->response) {
-            throw new LogicException('Response is not set');
-        }
-
         return $this->response->getPendingRequest();
     }
 }
diff --git a/tests/EndpointCollection/ProfileMethodEndpointCollectionTest.php b/tests/EndpointCollection/ProfileMethodEndpointCollectionTest.php
index c05701ee..130b5b28 100644
--- a/tests/EndpointCollection/ProfileMethodEndpointCollectionTest.php
+++ b/tests/EndpointCollection/ProfileMethodEndpointCollectionTest.php
@@ -4,8 +4,8 @@
 
 use Mollie\Api\Fake\MockMollieClient;
 use Mollie\Api\Fake\MockResponse;
-use Mollie\Api\Http\Requests\DisableProfileMethodRequest;
-use Mollie\Api\Http\Requests\EnableProfileMethodRequest;
+use Mollie\Api\Http\Requests\DisableMethodRequest;
+use Mollie\Api\Http\Requests\EnableMethodRequest;
 use Mollie\Api\Resources\Method;
 use PHPUnit\Framework\TestCase;
 
@@ -15,7 +15,7 @@ class ProfileMethodEndpointCollectionTest extends TestCase
     public function enable_for_id()
     {
         $client = new MockMollieClient([
-            EnableProfileMethodRequest::class => MockResponse::ok('method', 'ideal'),
+            EnableMethodRequest::class => MockResponse::ok('method', 'ideal'),
         ]);
 
         /** @var Method $method */
@@ -28,7 +28,7 @@ public function enable_for_id()
     public function enable()
     {
         $client = new MockMollieClient([
-            EnableProfileMethodRequest::class => MockResponse::ok('method', 'ideal'),
+            EnableMethodRequest::class => MockResponse::ok('method', 'ideal'),
         ]);
 
         /** @var Method $method */
@@ -41,7 +41,7 @@ public function enable()
     public function disable_for_id()
     {
         $client = new MockMollieClient([
-            DisableProfileMethodRequest::class => MockResponse::noContent(),
+            DisableMethodRequest::class => MockResponse::noContent(),
         ]);
 
         $client->profileMethods->disableForId('pfl_v9hTwCvYqw', 'ideal');
@@ -54,7 +54,7 @@ public function disable_for_id()
     public function disable()
     {
         $client = new MockMollieClient([
-            DisableProfileMethodRequest::class => MockResponse::noContent(),
+            DisableMethodRequest::class => MockResponse::noContent(),
         ]);
 
         $client->profileMethods->disable('ideal');
diff --git a/tests/EndpointCollection/SubscriptionEndpointCollectionTest.php b/tests/EndpointCollection/SubscriptionEndpointCollectionTest.php
index 31af7539..07e61935 100644
--- a/tests/EndpointCollection/SubscriptionEndpointCollectionTest.php
+++ b/tests/EndpointCollection/SubscriptionEndpointCollectionTest.php
@@ -84,7 +84,7 @@ public function update_for()
     public function cancel_for()
     {
         $client = new MockMollieClient([
-            CancelSubscriptionRequest::class => MockResponse::noContent(),
+            CancelSubscriptionRequest::class => MockResponse::ok('subscription'),
         ]);
 
         $customer = new Customer($client);
diff --git a/tests/Http/Requests/CancelPaymentRefundRequestTest.php b/tests/Http/Requests/CancelPaymentRefundRequestTest.php
index 800705af..1551a602 100644
--- a/tests/Http/Requests/CancelPaymentRefundRequestTest.php
+++ b/tests/Http/Requests/CancelPaymentRefundRequestTest.php
@@ -14,7 +14,7 @@ class CancelPaymentRefundRequestTest extends TestCase
     public function it_can_cancel_payment_refund()
     {
         $client = new MockMollieClient([
-            CancelPaymentRefundRequest::class => MockResponse::noContent(''),
+            CancelPaymentRefundRequest::class => MockResponse::noContent(),
         ]);
 
         $paymentId = 'tr_7UhSN1zuXS';
diff --git a/tests/Http/Requests/DeleteProfileRequestTest.php b/tests/Http/Requests/DeleteProfileRequestTest.php
index f4f25b20..87d9da54 100644
--- a/tests/Http/Requests/DeleteProfileRequestTest.php
+++ b/tests/Http/Requests/DeleteProfileRequestTest.php
@@ -14,7 +14,7 @@ class DeleteProfileRequestTest extends TestCase
     public function it_can_delete_profile()
     {
         $client = new MockMollieClient([
-            DeleteProfileRequest::class => MockResponse::noContent(''),
+            DeleteProfileRequest::class => MockResponse::noContent(),
         ]);
 
         $profileId = 'pfl_v9hTwCvYqw';
diff --git a/tests/Http/Requests/RevokeMandateRequestTest.php b/tests/Http/Requests/RevokeMandateRequestTest.php
index d6729c45..23135bd7 100644
--- a/tests/Http/Requests/RevokeMandateRequestTest.php
+++ b/tests/Http/Requests/RevokeMandateRequestTest.php
@@ -14,7 +14,7 @@ class RevokeMandateRequestTest extends TestCase
     public function it_can_revoke_mandate()
     {
         $client = new MockMollieClient([
-            RevokeMandateRequest::class => MockResponse::noContent(''),
+            RevokeMandateRequest::class => MockResponse::noContent(),
         ]);
 
         $customerId = 'cst_kEn1PlbGa';
diff --git a/tests/MollieApiClientTest.php b/tests/MollieApiClientTest.php
index ee56e626..571cd0f9 100644
--- a/tests/MollieApiClientTest.php
+++ b/tests/MollieApiClientTest.php
@@ -15,6 +15,7 @@
 use Mollie\Api\Http\PendingRequest;
 use Mollie\Api\Http\Request;
 use Mollie\Api\Http\Requests\CreatePaymentRequest;
+use Mollie\Api\Http\Requests\DynamicPostRequest;
 use Mollie\Api\Http\Requests\UpdatePaymentRequest;
 use Mollie\Api\Http\Response;
 use Mollie\Api\Idempotency\FakeIdempotencyKeyGenerator;
@@ -496,6 +497,23 @@ public function empty_or_null_payload_parameters_are_not_added_to_the_request()
 
         $client->send($request);
     }
+
+    /** @test */
+    public function a_response_with_empty_body_is_not_hydrated()
+    {
+        $client = new MockMollieClient([
+            DynamicPostRequest::class => MockResponse::noContent(),
+        ]);
+
+        $request = new DynamicPostRequest('dummy');
+
+        $request->setHydratableResource(new WrapperResource(DummyResourceWrapper::class));
+
+        /** @var Response $response */
+        $response = $client->send($request);
+
+        $this->assertInstanceOf(Response::class, $response);
+    }
 }
 
 class DummyResourceWrapper extends ResourceWrapper
diff --git a/tests/Resources/BaseCollectionTest.php b/tests/Resources/BaseCollectionTest.php
index 7aeaa827..4997a2f9 100644
--- a/tests/Resources/BaseCollectionTest.php
+++ b/tests/Resources/BaseCollectionTest.php
@@ -3,6 +3,7 @@
 namespace Tests\Resources;
 
 use Mollie\Api\Contracts\Connector;
+use Mollie\Api\Http\Response;
 use Mollie\Api\Resources\BaseCollection;
 use PHPUnit\Framework\TestCase;
 use stdClass;
@@ -11,9 +12,12 @@ class BaseCollectionTest extends TestCase
 {
     private Connector $connectorMock;
 
+    private Response $response;
+
     protected function setUp(): void
     {
         $this->connectorMock = $this->createMock(Connector::class);
+        $this->response = $this->createMock(Response::class);
     }
 
     /** @test */
@@ -25,6 +29,8 @@ public function constructor_initializes_collection_properly()
 
         $collection = new TestCollection($this->connectorMock, $items, $links);
 
+        $collection->setResponse($this->response);
+
         $this->assertCount(2, $collection);
         $this->assertSame($items, $collection->getArrayCopy());
         $this->assertSame($links, $collection->_links);
@@ -40,6 +46,7 @@ public function contains_returns_true_when_item_exists()
         ];
 
         $collection = new TestCollection($this->connectorMock, $items);
+        $collection->setResponse($this->response);
 
         $this->assertTrue($collection->contains(fn ($item) => $item === 'banana'));
     }
@@ -54,7 +61,7 @@ public function contains_returns_false_when_item_does_not_exist()
         ];
 
         $collection = new TestCollection($this->connectorMock, $items);
-
+        $collection->setResponse($this->response);
         $this->assertFalse($collection->contains(fn ($item) => $item === 'grape'));
     }
 
@@ -68,6 +75,7 @@ public function filter_returns_filtered_collection()
         ];
 
         $collection = new TestCollection($this->connectorMock, $items);
+        $collection->setResponse($this->response);
         $filtered = $collection->filter(fn ($item) => $item !== 'banana');
 
         $this->assertCount(2, $filtered);
@@ -84,6 +92,7 @@ public function filter_returns_filtered_collection()
     public function first_returns_first_item()
     {
         $collection = new TestCollection($this->connectorMock, ['item1', 'item2']);
+        $collection->setResponse($this->response);
         $this->assertEquals('item1', $collection->first());
     }
 
@@ -94,7 +103,7 @@ public function first_where_returns_first_item_where_condition_is_true()
             ['id' => 'item1'],
             ['id' => 'item2'],
         ]);
-
+        $collection->setResponse($this->response);
         $this->assertEquals(['id' => 'item2'], $collection->firstWhere('id', 'item2'));
         $this->assertEquals(['id' => 'item1'], $collection->firstWhere(fn ($item) => $item['id'] === 'item1'));
     }
diff --git a/tests/Resources/CursorCollectionTest.php b/tests/Resources/CursorCollectionTest.php
index 4c7b2f9c..20f477af 100644
--- a/tests/Resources/CursorCollectionTest.php
+++ b/tests/Resources/CursorCollectionTest.php
@@ -6,6 +6,7 @@
 use Mollie\Api\Fake\MockResponse;
 use Mollie\Api\Fake\SequenceMockResponse;
 use Mollie\Api\Http\Requests\DynamicGetRequest;
+use Mollie\Api\Http\Response;
 use Mollie\Api\Resources\LazyCollection;
 use Mollie\Api\Resources\PaymentCollection;
 use PHPUnit\Framework\TestCase;
@@ -13,6 +14,15 @@
 
 class CursorCollectionTest extends TestCase
 {
+    private Response $response;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->response = $this->createMock(Response::class);
+    }
+
     /** @test */
     public function can_get_next_collection_result_when_next_link_is_available()
     {
@@ -30,6 +40,8 @@ public function can_get_next_collection_result_when_next_link_is_available()
             ])
         );
 
+        $collection->setResponse($this->response);
+
         $this->assertTrue($collection->hasNext());
 
         $nextPage = $collection->next();
@@ -47,6 +59,8 @@ public function test_will_return_null_if_no_next_result_is_available()
             (object) []
         );
 
+        $collection->setResponse($this->response);
+
         $this->assertFalse($collection->hasNext());
         $this->assertNull($collection->next());
     }
@@ -67,6 +81,8 @@ public function test_can_get_previous_collection_result_when_previous_link_is_av
             ])
         );
 
+        $collection->setResponse($this->response);
+
         $this->assertTrue($collection->hasPrevious());
 
         $previousPage = $collection->previous();
@@ -84,6 +100,8 @@ public function test_will_return_null_if_no_previous_result_is_available()
             (object) []
         );
 
+        $collection->setResponse($this->response);
+
         $this->assertFalse($collection->hasPrevious());
         $this->assertNull($collection->previous());
     }
@@ -98,6 +116,8 @@ public function test_auto_paginator_returns_lazy_collection()
             (object) []
         );
 
+        $collection->setResponse($this->response);
+
         $this->assertInstanceOf(LazyCollection::class, $collection->getAutoIterator());
     }
 
@@ -121,6 +141,8 @@ public function test_auto_paginator_can_handle_consecutive_calls()
             ])
         );
 
+        $collection->setResponse($this->response);
+
         $paymentIds = [];
         foreach ($collection->getAutoIterator() as $payment) {
             $paymentIds[] = $payment->id;
diff --git a/tests/Resources/LazyCollectionTest.php b/tests/Resources/LazyCollectionTest.php
index d7bf9c16..a4cb2e38 100644
--- a/tests/Resources/LazyCollectionTest.php
+++ b/tests/Resources/LazyCollectionTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Resources;
 
+use Mollie\Api\Http\Response;
 use Mollie\Api\Resources\LazyCollection;
 use PHPUnit\Framework\TestCase;
 
@@ -21,6 +22,10 @@ protected function setUp(): void
             yield 2;
             yield 3;
         });
+
+        $response = $this->createMock(Response::class);
+
+        $this->collection->setResponse($response);
     }
 
     /** @test */
diff --git a/tests/Resources/MandateCollectionTest.php b/tests/Resources/MandateCollectionTest.php
index 824478ad..0869acd6 100644
--- a/tests/Resources/MandateCollectionTest.php
+++ b/tests/Resources/MandateCollectionTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Resources;
 
+use Mollie\Api\Http\Response;
 use Mollie\Api\MollieApiClient;
 use Mollie\Api\Resources\Mandate;
 use Mollie\Api\Resources\MandateCollection;
@@ -33,6 +34,10 @@ public function test_where_status()
             $this->getMandateWithStatus(MandateStatus::PENDING),
         ]);
 
+        $response = $this->createMock(Response::class);
+
+        $collection->setResponse($response);
+
         $valid = $collection->whereStatus(MandateStatus::VALID);
         $invalid = $collection->whereStatus(MandateStatus::INVALID);
         $pending = $collection->whereStatus(MandateStatus::PENDING);