test for null return values

src/Entities/Field.php

@@ -53,9 +53,11 @@ class Field extends Entity
      */
     public function persist(): ?bool
     {
-        return $this->endpoint?->update(
+        return !is_null($this->getId()) ?
+            $this->endpoint?->update(
                 $this->getId(),
-            $this->getName()
+                $this->getName() ?? ''
-        );
+            ) :
+            null;
     }
 }

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();
+        $tags?->clear();
-        $this->set(SubscriberEndpoint::TAGS, $tags->toArray());
+        $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);
+        $tags?->removeElement($tagId);
-        $this->set(SubscriberEndpoint::TAGS, $tags->toArray());
+        $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,11 +317,12 @@ class Subscriber extends Entity
      */
     public function persist(): ?bool
     {
+        if (!is_null($this->getId())) {
             $return = $this->endpoint?->update(
                 $this->getId(),
                 $this->getFields()->toArray(),
-            $this->getEmailAddress(),
+                $this->getEmailAddress() ?? '',
-            $this->getSmsPhone()
+                $this->getSmsPhone() ?? ''
             );
 
             $this->persistTags();
@@ -329,6 +330,9 @@ class Subscriber extends Entity
             return $return;
         }
 
+        return null;
+    }
+
     /**
      * @throws CommunicationException
      */
@@ -338,20 +342,31 @@ 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) {
+                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)) {
+            if (!is_null($this->getEmailAddress())) {
                 $this->endpoint->tag($this->getEmailAddress(), $addTags);
             }
         }
+    }
 
     // missing getters (return is timestamp list)
 

src/Entities/Subscription.php

@@ -81,9 +81,11 @@ class Subscription extends Entity
      */
     public function persist(): ?bool
     {
-        return $this->endpoint?->update(
+        return !is_null($this->getListId()) ?
+            $this->endpoint?->update(
                 $this->getListId(),
-            $this->getName()
+                $this->getName() ?? ''
-        );
+            ) :
+            null;
     }
 }

src/Entities/Tag.php

@@ -65,10 +65,12 @@ class Tag extends Entity
      */
     public function persist(): ?bool
     {
-        return $this->endpoint?->update(
+        return !is_null($this->getId()) ?
+            $this->endpoint?->update(
                 $this->getId(),
-            $this->getName(),
+                $this->getName() ?? '',
-            $this->getText()
+                $this->getText() ?? ''
-        );
+            ) :
+            null;
     }
 }

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([
+        $tagList = is_null($currentTags) ?
-            "12494453",
+            null :
-            "12494463",
+            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 'existing tag' => [["12494453", "12494463"], '12494463', true];
-        yield 'missing tag' => ['12495463', false];
+        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'));
+        is_null($expectedTagList) ?
-        $this->assertContains('78546214', $this->callMethod($this->entity, 'getTags'));
+            $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'));
+        is_null($expectedTags) ?
-        $this->assertContains('12494463', $this->callMethod($this->entity, 'getTags'));
+            $this->assertNull($this->callMethod($entityMock, 'getTags')) :
-        $this->assertNotContains('12494453', $this->callMethod($this->entity, '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 endpoint'    => [true, 'fixture', self::once(), self::once(), true];
-        yield 'has no endpoint'    => [false, self::never(), null];
+        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([
+        $entityMock->method('getTags')->willReturn(
-            "12494453",
+            is_null($currentTagList) ? null : new ArrayCollection($currentTagList)
-            "12494463",
+        );
-        ]));
 
         $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 removed' => [true, self::once(), ["12494453", "12494463"], ["12494453"], self::once(), self::never()];
-        yield 'has endpoint, tag added' => [true, self::once(), ["12494453","12494463","12494464"], self::never(), self::once()];
+        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"], self::never(), self::never()];
+        yield 'has endpoint, taglist equals' => [true, self::once(), ["12494453", "12494463"], ["12494453","12494463"], self::never(), self::never()];
-        yield 'has no endpoint'    => [false, self::never(), null, 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()];
     }
 
     /**

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 endpoint'        => [true, 'fixture', self::once(), true];
-        yield 'has no endpoint'    => [false, self::never(), null];
+        yield 'has no endpoint'     => [false, 'fixture', self::never(), null];
+        yield 'has endpoint, no id' => [true, null, self::never(), null];
     }
 }