From 279e778cd687942d30ec6f3ba0d6f2c8a53b0a6b Mon Sep 17 00:00:00 2001 From: Dennis van der Vliet Date: Fri, 6 Jan 2023 10:22:03 +0100 Subject: [PATCH 1/7] Use MessageReponse mapper instead of Message when calling list endpoint The Message mapper is missing the `id` and `href` properties since that mapper is used to map requests. We were still using the Message mapper in the list endpoint which resulted in those properties being removed from the response. --- src/MessageBird/Resources/Base.php | 12 ++-- tests/Integration/Contacts/ContactTest.php | 4 +- tests/Integration/Messages/MessagesTest.php | 74 ++++++++++++++++++++- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/MessageBird/Resources/Base.php b/src/MessageBird/Resources/Base.php index 7186b9e1..a2ffb366 100644 --- a/src/MessageBird/Resources/Base.php +++ b/src/MessageBird/Resources/Base.php @@ -186,10 +186,14 @@ public function getList(?array $parameters = []) foreach ($items as $item) { /** @psalm-suppress UndefinedClass */ - $object = new $objectName($this->httpClient); - - $message = $object->loadFromArray($item); - $baseList->items[] = $message; + if ($this->responseObject) { + $baseList->items[] = $this->responseObject->loadFromStdclass($item); + } else { + $object = new $objectName($this->httpClient); + + $message = $object->loadFromArray($item); + $baseList->items[] = $message; + } } return $baseList; diff --git a/tests/Integration/Contacts/ContactTest.php b/tests/Integration/Contacts/ContactTest.php index 88f6024a..3966129d 100644 --- a/tests/Integration/Contacts/ContactTest.php +++ b/tests/Integration/Contacts/ContactTest.php @@ -8,6 +8,8 @@ use MessageBird\Objects\Contact; use MessageBird\Objects\Group; use MessageBird\Objects\Message; +use MessageBird\Objects\MessageResponse; + use Tests\Integration\BaseTest; class ContactTest extends BaseTest @@ -155,7 +157,7 @@ public function testContactGetMessages(): void $messages = $this->client->contacts->getMessages("contact_id"); foreach ($messages->items as $message) { - self::assertInstanceOf(Message::class, $message); + self::assertInstanceOf(MessageResponse::class, $message); } } } diff --git a/tests/Integration/Messages/MessagesTest.php b/tests/Integration/Messages/MessagesTest.php index b84bbc07..ccd81e9d 100644 --- a/tests/Integration/Messages/MessagesTest.php +++ b/tests/Integration/Messages/MessagesTest.php @@ -100,14 +100,84 @@ public function testFlashSmsMessage(): void public function testListMessage(): void { - $this->expectException(ServerException::class); + $this->mockClient->expects(self::atLeastOnce())->method('performHttpRequest')->willReturn([ + 200, + '', + '{ + "offset": 0, + "limit": 10, + "count": 10, + "totalCount": 16, + "links": { + "first": "https://rest.messagebird.com/messages/?offset=0&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", + "previous": null, + "next": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", + "last": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00" + }, + "items": [ + { + "id": "7d8451f8", + "href": "https://rest.messagebird.com/messages/7d8451f8", + "direction": "mt", + "type": "sms", + "originator": "+31612345678", + "body": "Hi there", + "reference": null, + "validity": null, + "gateway": 10, + "typeDetails": { + "verify": true + }, + "datacoding": "plain", + "mclass": 1, + "scheduledDatetime": null, + "createdDatetime": "2022-10-12T18:46:22+00:00", + "recipients": { + "totalCount": 1, + "totalSentCount": 1, + "totalDeliveredCount": 1, + "totalDeliveryFailedCount": 0, + "items": [ + { + "recipient": 31612345678, + "originator": null, + "status": "delivered", + "statusDatetime": "2022-10-12T18:46:24+00:00", + "recipientCountry": "Netherlands", + "recipientCountryPrefix": 31, + "recipientOperator": "KPN", + "messageLength": 20, + "statusErrorCode": null, + "statusReason": "successfully delivered", + "price": { + "amount": 0.076, + "currency": "EUR" + }, + "mccmnc": "20408", + "mcc": "204", + "mnc": "08", + "messagePartCount": 1 + } + ] + } + } + ] +}', + ]); + $this->mockClient->expects(self::once())->method('performHttpRequest')->with( "GET", 'messages', ['offset' => 100, 'limit' => 30], null ); - $this->client->messages->getList(['offset' => 100, 'limit' => 30]); + + + $messageList = $this->client->messages->getList(['offset' => 100, 'limit' => 30]); + + self::assertSame('7d8451f8', $messageList->items[0]->id); + self::assertSame('https://rest.messagebird.com/messages/7d8451f8', $messageList->items[0]->href); + } public function testReadMessage(): void From 00c4c7549f3a56956bc3b291aa3b93e5737adda0 Mon Sep 17 00:00:00 2001 From: Dennis van der Vliet Date: Fri, 6 Jan 2023 10:23:10 +0100 Subject: [PATCH 2/7] Small cleanup --- src/MessageBird/Resources/Base.php | 2 +- tests/Integration/Contacts/ContactTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/MessageBird/Resources/Base.php b/src/MessageBird/Resources/Base.php index a2ffb366..9cce2f27 100644 --- a/src/MessageBird/Resources/Base.php +++ b/src/MessageBird/Resources/Base.php @@ -185,10 +185,10 @@ public function getList(?array $parameters = []) } foreach ($items as $item) { - /** @psalm-suppress UndefinedClass */ if ($this->responseObject) { $baseList->items[] = $this->responseObject->loadFromStdclass($item); } else { + /** @psalm-suppress UndefinedClass */ $object = new $objectName($this->httpClient); $message = $object->loadFromArray($item); diff --git a/tests/Integration/Contacts/ContactTest.php b/tests/Integration/Contacts/ContactTest.php index 3966129d..c63cb395 100644 --- a/tests/Integration/Contacts/ContactTest.php +++ b/tests/Integration/Contacts/ContactTest.php @@ -7,7 +7,6 @@ use MessageBird\Objects\BaseList; use MessageBird\Objects\Contact; use MessageBird\Objects\Group; -use MessageBird\Objects\Message; use MessageBird\Objects\MessageResponse; use Tests\Integration\BaseTest; From fd09058d2b693c31a24f497b3ebe64b707d4ad81 Mon Sep 17 00:00:00 2001 From: Dennis van der Vliet Date: Fri, 6 Jan 2023 10:32:14 +0100 Subject: [PATCH 3/7] Use newer checkout action --- .github/workflows/psalm.yml | 2 +- .github/workflows/tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml index 60915c4c..a93f882d 100644 --- a/.github/workflows/psalm.yml +++ b/.github/workflows/psalm.yml @@ -13,7 +13,7 @@ jobs: # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Psalm – Static Analysis for PHP uses: docker://vimeo/psalm-github-actions:4.18.1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ad84aef5..ddbb56b0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -18,7 +18,7 @@ jobs: steps: - name: Checkout the code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Setup PHP uses: shivammathur/setup-php@v2 From ec92248c77e7ee8ec7e80c85c0a3cf13261555b1 Mon Sep 17 00:00:00 2001 From: Dennis van der Vliet Date: Fri, 6 Jan 2023 21:00:16 +0100 Subject: [PATCH 4/7] Add additional test and move JSON to file for readability --- tests/Integration/Messages/MessagesTest.php | 64 +--------- .../Messages/get-list-response.json | 116 ++++++++++++++++++ 2 files changed, 120 insertions(+), 60 deletions(-) create mode 100644 tests/Integration/Messages/get-list-response.json diff --git a/tests/Integration/Messages/MessagesTest.php b/tests/Integration/Messages/MessagesTest.php index ccd81e9d..f1a0439f 100644 --- a/tests/Integration/Messages/MessagesTest.php +++ b/tests/Integration/Messages/MessagesTest.php @@ -103,66 +103,7 @@ public function testListMessage(): void $this->mockClient->expects(self::atLeastOnce())->method('performHttpRequest')->willReturn([ 200, '', - '{ - "offset": 0, - "limit": 10, - "count": 10, - "totalCount": 16, - "links": { - "first": "https://rest.messagebird.com/messages/?offset=0&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", - "previous": null, - "next": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", - "last": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00" - }, - "items": [ - { - "id": "7d8451f8", - "href": "https://rest.messagebird.com/messages/7d8451f8", - "direction": "mt", - "type": "sms", - "originator": "+31612345678", - "body": "Hi there", - "reference": null, - "validity": null, - "gateway": 10, - "typeDetails": { - "verify": true - }, - "datacoding": "plain", - "mclass": 1, - "scheduledDatetime": null, - "createdDatetime": "2022-10-12T18:46:22+00:00", - "recipients": { - "totalCount": 1, - "totalSentCount": 1, - "totalDeliveredCount": 1, - "totalDeliveryFailedCount": 0, - "items": [ - { - "recipient": 31612345678, - "originator": null, - "status": "delivered", - "statusDatetime": "2022-10-12T18:46:24+00:00", - "recipientCountry": "Netherlands", - "recipientCountryPrefix": 31, - "recipientOperator": "KPN", - "messageLength": 20, - "statusErrorCode": null, - "statusReason": "successfully delivered", - "price": { - "amount": 0.076, - "currency": "EUR" - }, - "mccmnc": "20408", - "mcc": "204", - "mnc": "08", - "messagePartCount": 1 - } - ] - } - } - ] -}', + file_get_contents(__DIR__ . '/get-list-response.json') ]); $this->mockClient->expects(self::once())->method('performHttpRequest')->with( @@ -178,6 +119,9 @@ public function testListMessage(): void self::assertSame('7d8451f8', $messageList->items[0]->id); self::assertSame('https://rest.messagebird.com/messages/7d8451f8', $messageList->items[0]->href); + self::assertSame('7d8451f8', $messageList->items[1]->id); + self::assertSame('https://rest.messagebird.com/messages/7d8451f8', $messageList->items[1]->href); + } public function testReadMessage(): void diff --git a/tests/Integration/Messages/get-list-response.json b/tests/Integration/Messages/get-list-response.json new file mode 100644 index 00000000..6607efda --- /dev/null +++ b/tests/Integration/Messages/get-list-response.json @@ -0,0 +1,116 @@ +{ + "offset": 0, + "limit": 10, + "count": 10, + "totalCount": 16, + "links": + { + "first": "https://rest.messagebird.com/messages/?offset=0&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", + "previous": null, + "next": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00", + "last": "https://rest.messagebird.com/messages/?offset=10&limit=10&recipient=31653474496&from=2021-03-16T00%3A00%3A00%2B00%3A00" + }, + "items": + [ + { + "id": "7d8451f8", + "href": "https://rest.messagebird.com/messages/7d8451f8", + "direction": "mt", + "type": "sms", + "originator": "+31612345678", + "body": "Hi there", + "reference": null, + "validity": null, + "gateway": 10, + "typeDetails": + { + "verify": true + }, + "datacoding": "plain", + "mclass": 1, + "scheduledDatetime": null, + "createdDatetime": "2022-10-12T18:46:22+00:00", + "recipients": + { + "totalCount": 1, + "totalSentCount": 1, + "totalDeliveredCount": 1, + "totalDeliveryFailedCount": 0, + "items": + [ + { + "recipient": 31612345678, + "originator": null, + "status": "delivered", + "statusDatetime": "2022-10-12T18:46:24+00:00", + "recipientCountry": "Netherlands", + "recipientCountryPrefix": 31, + "recipientOperator": "KPN", + "messageLength": 20, + "statusErrorCode": null, + "statusReason": "successfully delivered", + "price": + { + "amount": 0.076, + "currency": "EUR" + }, + "mccmnc": "20408", + "mcc": "204", + "mnc": "08", + "messagePartCount": 1 + } + ] + } + }, + { + "id": "7d8451f8", + "href": "https://rest.messagebird.com/messages/7d8451f8", + "direction": "mt", + "type": "sms", + "originator": "+31612345678", + "body": "Hi there", + "reference": null, + "validity": null, + "gateway": 10, + "typeDetails": + { + "verify": true + }, + "datacoding": "plain", + "mclass": 1, + "scheduledDatetime": null, + "createdDatetime": "2022-10-12T18:46:22+00:00", + "recipients": + { + "totalCount": 1, + "totalSentCount": 1, + "totalDeliveredCount": 1, + "totalDeliveryFailedCount": 0, + "items": + [ + { + "recipient": 31612345678, + "originator": null, + "status": "delivered", + "statusDatetime": "2022-10-12T18:46:24+00:00", + "recipientCountry": "Netherlands", + "recipientCountryPrefix": 31, + "recipientOperator": "KPN", + "messageLength": 20, + "statusErrorCode": null, + "statusReason": "successfully delivered", + "price": + { + "amount": 0.076, + "currency": "EUR" + }, + "mccmnc": "20408", + "mcc": "204", + "mnc": "08", + "messagePartCount": 1 + } + ] + } + } + ] +} From 841ecf4eeb10cb325778549a6e610eec02e34cf3 Mon Sep 17 00:00:00 2001 From: Dennis van der Vliet Date: Fri, 6 Jan 2023 21:06:14 +0100 Subject: [PATCH 5/7] Drop build step for 7.3 and reduce number of builds --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ddbb56b0..bb0edc46 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,8 +11,8 @@ jobs: strategy: fail-fast: true matrix: - php: [ '7.3', '7.4', '8.0', '8.1' ] - stability: [ prefer-lowest, prefer-stable ] + php: [ '7.4', '8.0', '8.1', '8.2' ] + stability: [ prefer-stable ] name: PHP ${{ matrix.php }} - ${{ matrix.stability }} From 4b6d6f080e77989590cd0461712d16752d294187 Mon Sep 17 00:00:00 2001 From: Alexandru Bucur Date: Fri, 6 Jan 2023 23:24:55 +0100 Subject: [PATCH 6/7] make sure we have unique responses in the new test --- tests/Integration/Messages/MessagesTest.php | 4 ++-- tests/Integration/Messages/get-list-response.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Integration/Messages/MessagesTest.php b/tests/Integration/Messages/MessagesTest.php index f1a0439f..f336c3ce 100644 --- a/tests/Integration/Messages/MessagesTest.php +++ b/tests/Integration/Messages/MessagesTest.php @@ -119,8 +119,8 @@ public function testListMessage(): void self::assertSame('7d8451f8', $messageList->items[0]->id); self::assertSame('https://rest.messagebird.com/messages/7d8451f8', $messageList->items[0]->href); - self::assertSame('7d8451f8', $messageList->items[1]->id); - self::assertSame('https://rest.messagebird.com/messages/7d8451f8', $messageList->items[1]->href); + self::assertSame('7d8451f9', $messageList->items[1]->id); + self::assertSame('https://rest.messagebird.com/messages/7d8451f9', $messageList->items[1]->href); } diff --git a/tests/Integration/Messages/get-list-response.json b/tests/Integration/Messages/get-list-response.json index 6607efda..48a679ce 100644 --- a/tests/Integration/Messages/get-list-response.json +++ b/tests/Integration/Messages/get-list-response.json @@ -63,8 +63,8 @@ } }, { - "id": "7d8451f8", - "href": "https://rest.messagebird.com/messages/7d8451f8", + "id": "7d8451f9", + "href": "https://rest.messagebird.com/messages/7d8451f9", "direction": "mt", "type": "sms", "originator": "+31612345678", From bbd3d90e958c71c042bc835fc2f47be098d5d42d Mon Sep 17 00:00:00 2001 From: Alexandru Bucur Date: Fri, 6 Jan 2023 23:29:07 +0100 Subject: [PATCH 7/7] clone reference to the response object --- src/MessageBird/Resources/Base.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/MessageBird/Resources/Base.php b/src/MessageBird/Resources/Base.php index 9cce2f27..4ad4ceaf 100644 --- a/src/MessageBird/Resources/Base.php +++ b/src/MessageBird/Resources/Base.php @@ -186,7 +186,8 @@ public function getList(?array $parameters = []) foreach ($items as $item) { if ($this->responseObject) { - $baseList->items[] = $this->responseObject->loadFromStdclass($item); + $responseObject = clone $this->responseObject; + $baseList->items[] = $responseObject->loadFromStdclass($item); } else { /** @psalm-suppress UndefinedClass */ $object = new $objectName($this->httpClient);