From 1ea3941210b3e659c207a416b44264a2d50c93f6 Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Thu, 16 Jan 2025 14:17:55 +0100 Subject: [PATCH] test for null return values --- src/Entities/Field.php | 10 ++- src/Entities/Subscriber.php | 55 +++++++----- src/Entities/Subscription.php | 10 ++- src/Entities/Tag.php | 12 +-- tests/unit/Entities/SubscriberTest.php | 102 +++++++++++++++-------- tests/unit/Entities/SubscriptionTest.php | 10 ++- 6 files changed, 129 insertions(+), 70 deletions(-) diff --git a/src/Entities/Field.php b/src/Entities/Field.php index 60c737f..9608ab7 100644 --- a/src/Entities/Field.php +++ b/src/Entities/Field.php @@ -53,9 +53,11 @@ class Field extends Entity */ public function persist(): ?bool { - return $this->endpoint?->update( - $this->getId(), - $this->getName() - ); + return !is_null($this->getId()) ? + $this->endpoint?->update( + $this->getId(), + $this->getName() ?? '' + ) : + null; } } diff --git a/src/Entities/Subscriber.php b/src/Entities/Subscriber.php index 2283453..9a5c16c 100644 --- a/src/Entities/Subscriber.php +++ b/src/Entities/Subscriber.php @@ -175,7 +175,7 @@ class Subscriber extends Entity // use persist method to send to Klicktipp } - protected function getFieldLongName(string $fieldId): ?string + protected function getFieldLongName(string $fieldId): string { return str_starts_with($fieldId, 'field') ? trim($fieldId) : 'field'.trim($fieldId); } @@ -193,15 +193,15 @@ class Subscriber extends Entity public function clearTags(): void { $tags = $this->getTags(); - $tags->clear(); - $this->set(SubscriberEndpoint::TAGS, $tags->toArray()); + $tags?->clear(); + $this->set(SubscriberEndpoint::TAGS, $tags?->toArray()); // use persist method to send to Klicktipp } public function addTag(string $tagId): void { - $tags = $this->getTags(); + $tags = $this->getTags() ?: new ArrayCollection(); $tags->add($tagId); $this->set(SubscriberEndpoint::TAGS, $tags->toArray()); @@ -211,8 +211,8 @@ class Subscriber extends Entity public function removeTag(string $tagId): void { $tags = $this->getTags(); - $tags->removeElement($tagId); - $this->set(SubscriberEndpoint::TAGS, $tags->toArray()); + $tags?->removeElement($tagId); + $this->set(SubscriberEndpoint::TAGS, $tags?->toArray()); // use persist method to send to Klicktipp } @@ -232,7 +232,7 @@ class Subscriber extends Entity public function getManualTagTime(string $tagId): ?DateTime { - return $this->getDateTimeFromValue($this->getManualTags()->get($tagId)); + return $this->getDateTimeFromValue($this->getManualTags()?->get($tagId)); } public function getSmartTags(): ?ArrayCollection @@ -317,16 +317,20 @@ class Subscriber extends Entity */ public function persist(): ?bool { - $return = $this->endpoint?->update( - $this->getId(), - $this->getFields()->toArray(), - $this->getEmailAddress(), - $this->getSmsPhone() - ); + if (!is_null($this->getId())) { + $return = $this->endpoint?->update( + $this->getId(), + $this->getFields()->toArray(), + $this->getEmailAddress() ?? '', + $this->getSmsPhone() ?? '' + ); - $this->persistTags(); + $this->persistTags(); - return $return; + return $return; + } + + return null; } /** @@ -338,18 +342,29 @@ class Subscriber extends Entity return; } - $currentTags = $this->endpoint->getEntity($this->getId())->getTags(); + $currentTags = $this->endpoint->getEntity($this->getId() ?? '')->getTags(); + + $removeTags = array_diff( + $currentTags?->toArray() ?? [], + $this->getTags()?->toArray() ?? [] + ); - $removeTags = array_diff($currentTags->toArray(), $this->getTags()->toArray()); if (count($removeTags)) { foreach ($removeTags as $removeTag) { - $this->endpoint->untag($this->getEmailAddress(), $removeTag); + if (!is_null($this->getEmailAddress())) { + $this->endpoint->untag($this->getEmailAddress(), $removeTag); + } } } - $addTags = array_diff($this->getTags()->toArray(), $currentTags->toArray()); + $addTags = array_diff( + $this->getTags()?->toArray() ?? [], + $currentTags?->toArray() ?? [] + ); if (count($addTags)) { - $this->endpoint->tag($this->getEmailAddress(), $addTags); + if (!is_null($this->getEmailAddress())) { + $this->endpoint->tag($this->getEmailAddress(), $addTags); + } } } diff --git a/src/Entities/Subscription.php b/src/Entities/Subscription.php index fc8577c..9f1e3f1 100644 --- a/src/Entities/Subscription.php +++ b/src/Entities/Subscription.php @@ -81,9 +81,11 @@ class Subscription extends Entity */ public function persist(): ?bool { - return $this->endpoint?->update( - $this->getListId(), - $this->getName() - ); + return !is_null($this->getListId()) ? + $this->endpoint?->update( + $this->getListId(), + $this->getName() ?? '' + ) : + null; } } diff --git a/src/Entities/Tag.php b/src/Entities/Tag.php index 9b3a252..f0ee888 100644 --- a/src/Entities/Tag.php +++ b/src/Entities/Tag.php @@ -65,10 +65,12 @@ class Tag extends Entity */ public function persist(): ?bool { - return $this->endpoint?->update( - $this->getId(), - $this->getName(), - $this->getText() - ); + return !is_null($this->getId()) ? + $this->endpoint?->update( + $this->getId(), + $this->getName() ?? '', + $this->getText() ?? '' + ) : + null; } } diff --git a/tests/unit/Entities/SubscriberTest.php b/tests/unit/Entities/SubscriberTest.php index 864ad9c..3e7c27e 100644 --- a/tests/unit/Entities/SubscriberTest.php +++ b/tests/unit/Entities/SubscriberTest.php @@ -706,12 +706,11 @@ class SubscriberTest extends TestCase * @covers \D3\KlicktippPhpClient\Entities\Subscriber::isTagSet * @dataProvider isTagSetDataProvider */ - public function testIsTagSet(string $searchTagId, bool $expected): void + public function testIsTagSet(?array $currentTags, string $searchTagId, bool $expected): void { - $tagList = new ArrayCollection([ - "12494453", - "12494463", - ]); + $tagList = is_null($currentTags) ? + null : + new ArrayCollection($currentTags); $sut = $this->getMockBuilder(Subscriber::class) ->onlyMethods(['getTags']) @@ -730,8 +729,9 @@ class SubscriberTest extends TestCase public static function isTagSetDataProvider(): Generator { - yield 'existing tag' => ['12494463', true]; - yield 'missing tag' => ['12495463', false]; + yield 'existing tag' => [["12494453", "12494463"], '12494463', true]; + yield 'missing tag' => [["12494453", "12494463"], '12495463', false]; + yield 'null tag' => [null, '12495463', false]; } /** @@ -739,15 +739,27 @@ class SubscriberTest extends TestCase * @return void * @covers \D3\KlicktippPhpClient\Entities\Subscriber::clearTags * @throws ReflectionException + * @dataProvider clearTagsDataProvider */ - public function testClearTags(): void + public function testClearTags(?array $currentTagList, ?array $expectedTags): void { + $entityMock = new Subscriber([SubscriberEndpoint::TAGS => $currentTagList]); + $this->callMethod( - $this->entity, + $entityMock, 'clearTags' ); - $this->assertCount(0, $this->callMethod($this->entity, 'getTags')); + is_null($expectedTags) ? + $this->assertNull($this->callMethod($entityMock, 'getTags')) : + $this->assertEquals(new ArrayCollection($expectedTags), $this->callMethod($entityMock, 'getTags')); + } + + public static function clearTagsDataProvider(): Generator + { + yield 'tag list exists' => [["12494453","12494463"], []]; + yield 'tag list empty' => [[], []]; + yield 'tag list null' => [null, null]; } /** @@ -755,36 +767,56 @@ class SubscriberTest extends TestCase * @return void * @covers \D3\KlicktippPhpClient\Entities\Subscriber::addTag * @throws ReflectionException + * @dataProvider addTagDataProvider */ - public function testAddTag(): void + public function testAddTag(?array $currentTagList, ?array $expectedTagList): void { + $entityMock = new Subscriber([SubscriberEndpoint::TAGS => $currentTagList]); + $this->callMethod( - $this->entity, + $entityMock, 'addTag', ['78546214'] ); - $this->assertCount(3, $this->callMethod($this->entity, 'getTags')); - $this->assertContains('78546214', $this->callMethod($this->entity, 'getTags')); + is_null($expectedTagList) ? + $this->assertNull($this->callMethod($entityMock, 'getTags')) : + $this->assertEquals(new ArrayCollection($expectedTagList), $this->callMethod($entityMock, 'getTags')); + } + + public static function addTagDataProvider(): Generator + { + yield 'tag list exists' => [["12494453","12494463"], ["12494453","12494463", "78546214"]]; + yield 'tag list empty' => [[], ["78546214"]]; + yield 'tag list null' => [null, ["78546214"]]; } /** * @test - * @return void - * @covers \D3\KlicktippPhpClient\Entities\Subscriber::removeTag * @throws ReflectionException + * @covers \D3\KlicktippPhpClient\Entities\Subscriber::removeTag + * @dataProvider removeTagDataProvider */ - public function testRemoveTag(): void + public function testRemoveTag(?array $currentTagList, ?array $expectedTags): void { + $entityMock = new Subscriber([SubscriberEndpoint::TAGS => $currentTagList]); + $this->callMethod( - $this->entity, + $entityMock, 'removeTag', ['12494453'] ); - $this->assertCount(1, $this->callMethod($this->entity, 'getTags')); - $this->assertContains('12494463', $this->callMethod($this->entity, 'getTags')); - $this->assertNotContains('12494453', $this->callMethod($this->entity, 'getTags')); + is_null($expectedTags) ? + $this->assertNull($this->callMethod($entityMock, 'getTags')) : + $this->assertEquals(new ArrayCollection($expectedTags), $this->callMethod($entityMock, 'getTags')); + } + + public static function removeTagDataProvider(): Generator + { + yield 'tag list exists' => [["12494453","12494463"], [1 => "12494463"]]; + yield 'tag list empty' => [[], []]; + yield 'tag list null' => [null, null]; } /** @@ -795,7 +827,9 @@ class SubscriberTest extends TestCase */ public function testPersist( bool $endpointSet, + ?string $currentId, InvokedCount $endpointInvocation, + InvokedCount $persistTagsInvocation, ?bool $expectedReturn ): void { $endpointMock = $this->getMockBuilder(SubscriberEndpoint::class) @@ -805,10 +839,10 @@ class SubscriberTest extends TestCase $endpointMock->expects($endpointInvocation)->method('update')->willReturn(true); $sut = $this->getMockBuilder(Subscriber::class) - ->setConstructorArgs([[SubscriberEndpoint::ID => 'foo'], $endpointSet ? $endpointMock : null]) + ->setConstructorArgs([[SubscriberEndpoint::ID => $currentId], $endpointSet ? $endpointMock : null]) ->onlyMethods(['persistTags']) ->getMock(); - $sut->expects($this->once())->method('persistTags'); + $sut->expects($persistTagsInvocation)->method('persistTags'); $this->assertSame( $expectedReturn, @@ -821,8 +855,9 @@ class SubscriberTest extends TestCase public static function persistDataProvider(): Generator { - yield 'has endpoint' => [true, self::once(), true]; - yield 'has no endpoint' => [false, self::never(), null]; + yield 'has endpoint' => [true, 'fixture', self::once(), self::once(), true]; + yield 'has no endpoint' => [false, 'fixture', self::never(), self::once(), null]; + yield 'has endpoint, no id' => [true, null, self::never(), self::never(), null]; } /** @@ -834,6 +869,7 @@ class SubscriberTest extends TestCase public function testPersistTags( bool $endpointSet, InvokedCount $endpointInvocation, + ?array $currentTagList, ?array $newTagList, InvokedCount $removeTagInvocation, InvokedCount $setTagInvocation @@ -842,10 +878,9 @@ class SubscriberTest extends TestCase ->disableOriginalConstructor() ->onlyMethods(['getTags']) ->getMock(); - $entityMock->method('getTags')->willReturn(new ArrayCollection([ - "12494453", - "12494463", - ])); + $entityMock->method('getTags')->willReturn( + is_null($currentTagList) ? null : new ArrayCollection($currentTagList) + ); $endpointMock = $this->getMockBuilder(SubscriberEndpoint::class) ->disableOriginalConstructor() @@ -868,10 +903,11 @@ class SubscriberTest extends TestCase public static function persistTagsDataProvider(): Generator { - yield 'has endpoint, tag removed' => [true, self::once(), ["12494453"], self::once(), self::never()]; - yield 'has endpoint, tag added' => [true, self::once(), ["12494453","12494463","12494464"], self::never(), self::once()]; - yield 'has endpoint, taglist equals' => [true, self::once(), ["12494453","12494463"], self::never(), self::never()]; - yield 'has no endpoint' => [false, self::never(), null, self::never(), self::never()]; + yield 'has endpoint, tag removed' => [true, self::once(), ["12494453", "12494463"], ["12494453"], self::once(), self::never()]; + yield 'has endpoint, tag added' => [true, self::once(), ["12494453", "12494463"], ["12494453","12494463","12494464"], self::never(), self::once()]; + yield 'has endpoint, taglist equals' => [true, self::once(), ["12494453", "12494463"], ["12494453","12494463"], self::never(), self::never()]; + yield 'has no endpoint' => [false, self::never(), ["12494453", "12494463"], null, self::never(), self::never()]; + yield 'has endpoint, taglist null' => [true, self::once(), null, null, self::never(), self::never()]; } /** diff --git a/tests/unit/Entities/SubscriptionTest.php b/tests/unit/Entities/SubscriptionTest.php index 19bdbc9..f27a489 100644 --- a/tests/unit/Entities/SubscriptionTest.php +++ b/tests/unit/Entities/SubscriptionTest.php @@ -205,6 +205,7 @@ class SubscriptionTest extends TestCase */ public function testPersist( bool $endpointSet, + ?string $id, InvokedCount $endpointInvocation, ?bool $expectedReturn ): void { @@ -213,12 +214,12 @@ class SubscriptionTest extends TestCase ->onlyMethods(['update']) ->getMock(); $endpointMock->expects($endpointInvocation)->method('update')->with( - $this->identicalTo('foo'), + $this->identicalTo($id), $this->identicalTo('bar'), )->willReturn(true); $sut = new Subscription( - [SubscriptionEndpoint::LISTID => 'foo', SubscriptionEndpoint::NAME => 'bar'], + [SubscriptionEndpoint::LISTID => $id, SubscriptionEndpoint::NAME => 'bar'], $endpointSet ? $endpointMock : null ); @@ -233,7 +234,8 @@ class SubscriptionTest extends TestCase public static function persistDataProvider(): Generator { - yield 'has endpoint' => [true, self::once(), true]; - yield 'has no endpoint' => [false, self::never(), null]; + yield 'has endpoint' => [true, 'fixture', self::once(), true]; + yield 'has no endpoint' => [false, 'fixture', self::never(), null]; + yield 'has endpoint, no id' => [true, null, self::never(), null]; } }