Merge pull request #78 from swisnl/bugfix/issue-77

Properly handle relations without data
swisnl · Aug 5, 2020 · 7ff218f · 7ff218f
2 parents a3b3fc9 + 89c59fa
commit 7ff218f
Show file tree

Hide file tree

Showing 13 changed files with 245 additions and 73 deletions.
diff --git a/src/Concerns/HasRelations.php b/src/Concerns/HasRelations.php
@@ -141,14 +141,14 @@ public function getRelationValue(string $name): ?DataInterface
     /**
      * Set the specific relationship on the model.
      *
-     * @param string                                             $name
-     * @param \Swis\JsonApi\Client\Interfaces\DataInterface|null $data
-     * @param \Swis\JsonApi\Client\Links|null                    $links
-     * @param \Swis\JsonApi\Client\Meta|null                     $meta
+     * @param string                                                   $name
+     * @param \Swis\JsonApi\Client\Interfaces\DataInterface|false|null $data
+     * @param \Swis\JsonApi\Client\Links|null                          $links
+     * @param \Swis\JsonApi\Client\Meta|null                           $meta
      *
      * @return static
      */
-    public function setRelation(string $name, DataInterface $data = null, Links $links = null, Meta $meta = null)
+    public function setRelation(string $name, $data = false, Links $links = null, Meta $meta = null)
     {
         $method = Str::camel($name);
         if (method_exists($this, $method)) {
@@ -160,8 +160,11 @@ public function setRelation(string $name, DataInterface $data = null, Links $lin
             $relationObject = $this->morphTo($name);
         }
 
-        if ($data !== null) {
-            $relationObject->associate($data);
+        if ($data !== false) {
+            $relationObject->dissociate();
+            if ($data !== null) {
+                $relationObject->associate($data);
+            }
         }
         $relationObject->setLinks($links);
         $relationObject->setMeta($meta);

diff --git a/src/Interfaces/ItemInterface.php b/src/Interfaces/ItemInterface.php
@@ -127,14 +127,14 @@ public function getAvailableRelations(): array;
     /**
      * Set the specific relationship in the model.
      *
-     * @param string                                             $relation
-     * @param \Swis\JsonApi\Client\Interfaces\DataInterface|null $value
-     * @param \Swis\JsonApi\Client\Links|null                    $links
-     * @param \Swis\JsonApi\Client\Meta|null                     $meta
+     * @param string                                                   $relation
+     * @param \Swis\JsonApi\Client\Interfaces\DataInterface|false|null $value
+     * @param \Swis\JsonApi\Client\Links|null                          $links
+     * @param \Swis\JsonApi\Client\Meta|null                           $meta
      *
      * @return static
      */
-    public function setRelation(string $relation, DataInterface $value = null, Links $links = null, Meta $meta = null);
+    public function setRelation(string $relation, $value = false, Links $links = null, Meta $meta = null);
 
     /**
      * @return \Swis\JsonApi\Client\Interfaces\OneRelationInterface[]|\Swis\JsonApi\Client\Interfaces\ManyRelationInterface[]

diff --git a/src/Item.php b/src/Item.php
@@ -130,28 +130,37 @@ public function getRelationships(): array
         $relationships = [];
 
         foreach ($this->getRelations() as $name => $relation) {
-            if ($relation instanceof OneRelationInterface) {
-                $relationships[$name] = ['data' => null];
+            if ($relation->hasIncluded()) {
+                if ($relation instanceof OneRelationInterface) {
+                    $relationships[$name]['data'] = null;
 
-                if ($relation->getIncluded() !== null) {
-                    $relationships[$name] = [
-                        'data' => [
+                    if ($relation->getIncluded() !== null) {
+                        $relationships[$name]['data'] = [
                             'type' => $relation->getIncluded()->getType(),
                             'id' => $relation->getIncluded()->getId(),
-                        ],
-                    ];
-                }
-            } elseif ($relation instanceof ManyRelationInterface) {
-                $relationships[$name]['data'] = [];
+                        ];
+                    }
+                } elseif ($relation instanceof ManyRelationInterface) {
+                    $relationships[$name]['data'] = [];
 
-                foreach ($relation->getIncluded() as $item) {
-                    $relationships[$name]['data'][] =
-                        [
+                    foreach ($relation->getIncluded() as $item) {
+                        $relationships[$name]['data'][] = [
                             'type' => $item->getType(),
                             'id' => $item->getId(),
                         ];
+                    }
                 }
             }
+
+            $links = $relation->getLinks();
+            if ($links !== null) {
+                $relationships[$name]['links'] = $links->toArray();
+            }
+
+            $meta = $relation->getMeta();
+            if ($meta !== null) {
+                $relationships[$name]['meta'] = $meta->toArray();
+            }
         }
 
         return $relationships;

diff --git a/src/Parsers/ItemParser.php b/src/Parsers/ItemParser.php
@@ -135,9 +135,12 @@ private function setRelations(ItemInterface $item, $data): void
                 throw new ValidationException('Relationship object MUST contain at least one of the following properties: `links`, `data`, `meta`.');
             }
 
-            $value = null;
-            if (property_exists($relationship, 'data') && $relationship->data !== null) {
-                $value = $this->parseRelationshipData($relationship->data);
+            $value = false;
+            if (property_exists($relationship, 'data')) {
+                $value = null;
+                if ($relationship->data !== null) {
+                    $value = $this->parseRelationshipData($relationship->data);
+                }
             }
 
             $links = null;

diff --git a/src/Relations/AbstractManyRelation.php b/src/Relations/AbstractManyRelation.php
@@ -6,7 +6,7 @@
 use Swis\JsonApi\Client\Interfaces\ManyRelationInterface;
 
 /**
- * @property \Swis\JsonApi\Client\Collection|null $included
+ * @property \Swis\JsonApi\Client\Collection|false|null $included
  */
 abstract class AbstractManyRelation extends AbstractRelation implements ManyRelationInterface
 {
@@ -22,14 +22,6 @@ public function associate(Collection $included)
         return $this;
     }
 
-    /**
-     * @return bool
-     */
-    public function hasIncluded(): bool
-    {
-        return $this->getIncluded()->isNotEmpty();
-    }
-
     /**
      * @return \Swis\JsonApi\Client\Collection
      */

diff --git a/src/Relations/AbstractOneRelation.php b/src/Relations/AbstractOneRelation.php
@@ -6,7 +6,7 @@
 use Swis\JsonApi\Client\Interfaces\OneRelationInterface;
 
 /**
- * @property \Swis\JsonApi\Client\Interfaces\ItemInterface|null $included
+ * @property \Swis\JsonApi\Client\Interfaces\ItemInterface|false|null $included
  */
 abstract class AbstractOneRelation extends AbstractRelation implements OneRelationInterface
 {
@@ -27,14 +27,6 @@ public function associate(ItemInterface $included)
      */
     public function getIncluded(): ? ItemInterface
     {
-        return $this->included;
-    }
-
-    /**
-     * @return bool
-     */
-    public function hasIncluded(): bool
-    {
-        return null !== $this->getIncluded();
+        return $this->included ?: null;
     }
 }
diff --git a/src/Relations/AbstractRelation.php b/src/Relations/AbstractRelation.php
@@ -11,9 +11,9 @@ abstract class AbstractRelation
     use HasMeta;
 
     /**
-     * @var \Swis\JsonApi\Client\Interfaces\DataInterface|null
+     * @var \Swis\JsonApi\Client\Interfaces\DataInterface|false|null
      */
-    protected $included;
+    protected $included = false;
 
     /**
      * @var bool
@@ -30,6 +30,14 @@ public function dissociate()
         return $this;
     }
 
+    /**
+     * @return bool
+     */
+    public function hasIncluded(): bool
+    {
+        return $this->included !== false;
+    }
+
     /**
      * @param bool $omitIncluded
      *

diff --git a/tests/Concerns/HasRelationsTest.php b/tests/Concerns/HasRelationsTest.php
@@ -29,6 +29,7 @@ public function it_can_get_and_set_an_item_as_relation()
 
         $relation = $mock->getRelation('foo');
         $this->assertInstanceOf(MorphToRelation::class, $relation);
+        $this->assertTrue($relation->hasIncluded());
         $this->assertSame($data, $relation->getIncluded());
     }
 
@@ -45,9 +46,41 @@ public function it_can_get_and_set_a_collection_as_relation()
 
         $relation = $mock->getRelation('foo');
         $this->assertInstanceOf(MorphToManyRelation::class, $relation);
+        $this->assertTrue($relation->hasIncluded());
         $this->assertSame($data, $relation->getIncluded());
     }
 
+    /**
+     * @test
+     */
+    public function it_can_get_and_set_null_as_relation()
+    {
+        /** @var \PHPUnit\Framework\MockObject\MockObject&\Swis\JsonApi\Client\Concerns\HasRelations $mock */
+        $mock = $this->getMockForTrait(HasRelations::class);
+
+        $mock->setRelation('foo', null);
+
+        $relation = $mock->getRelation('foo');
+        $this->assertInstanceOf(MorphToRelation::class, $relation);
+        $this->assertTrue($relation->hasIncluded());
+        $this->assertNull($relation->getIncluded());
+    }
+
+    /**
+     * @test
+     */
+    public function it_does_not_set_false_as_relation()
+    {
+        /** @var \PHPUnit\Framework\MockObject\MockObject&\Swis\JsonApi\Client\Concerns\HasRelations $mock */
+        $mock = $this->getMockForTrait(HasRelations::class);
+
+        $mock->setRelation('foo', false);
+
+        $relation = $mock->getRelation('foo');
+        $this->assertInstanceOf(MorphToRelation::class, $relation);
+        $this->assertFalse($relation->hasIncluded());
+    }
+
     /**
      * @test
      */

diff --git a/tests/DocumentClientTest.php b/tests/DocumentClientTest.php
@@ -18,7 +18,7 @@ class DocumentClientTest extends AbstractTest
     public function the_base_url_can_be_changed_after_instantiation()
     {
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ClientInterface $client */
-        $client = $this->getMockBuilder(ClientInterface::class)->getMock();
+        $client = $this->createMock(ClientInterface::class);
 
         $client->expects($this->once())
             ->method('getBaseUri')
@@ -29,7 +29,7 @@ public function the_base_url_can_be_changed_after_instantiation()
             ->with('http://www.test-changed.com');
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ResponseParserInterface $parser */
-        $parser = $this->getMockBuilder(ResponseParserInterface::class)->getMock();
+        $parser = $this->createMock(ResponseParserInterface::class);
 
         $documentClient = new DocumentClient($client, $parser);
 
@@ -46,15 +46,15 @@ public function it_builds_a_get_request()
         $document = new Document();
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ClientInterface $client */
-        $client = $this->getMockBuilder(ClientInterface::class)->getMock();
+        $client = $this->createMock(ClientInterface::class);
 
         $client->expects($this->once())
             ->method('get')
             ->with('/test/1', ['X-Foo' => 'bar'])
             ->willReturn($response);
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ResponseParserInterface $parser */
-        $parser = $this->getMockBuilder(ResponseParserInterface::class)->getMock();
+        $parser = $this->createMock(ResponseParserInterface::class);
 
         $parser->expects($this->once())
             ->method('parse')
@@ -77,15 +77,15 @@ public function it_builds_a_delete_request()
         $document = new Document();
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ClientInterface $client */
-        $client = $this->getMockBuilder(ClientInterface::class)->getMock();
+        $client = $this->createMock(ClientInterface::class);
 
         $client->expects($this->once())
             ->method('delete')
             ->with('/test/1', ['X-Foo' => 'bar'])
             ->willReturn($response);
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ResponseParserInterface $parser */
-        $parser = $this->getMockBuilder(ResponseParserInterface::class)->getMock();
+        $parser = $this->createMock(ResponseParserInterface::class);
 
         $parser->expects($this->once())
             ->method('parse')
@@ -110,15 +110,15 @@ public function it_builds_a_patch_request()
         $itemDocument->setData((new Item())->setType('test')->setId('1'));
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ClientInterface $client */
-        $client = $this->getMockBuilder(ClientInterface::class)->getMock();
+        $client = $this->createMock(ClientInterface::class);
 
         $client->expects($this->once())
             ->method('patch')
             ->with('/test/1', '{"data":{"type":"test","id":"1"}}', ['X-Foo' => 'bar'])
             ->willReturn($response);
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ResponseParserInterface $parser */
-        $parser = $this->getMockBuilder(ResponseParserInterface::class)->getMock();
+        $parser = $this->createMock(ResponseParserInterface::class);
 
         $parser->expects($this->once())
             ->method('parse')
@@ -143,15 +143,15 @@ public function it_builds_a_post_request()
         $itemDocument->setData((new Item())->setType('test'));
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ClientInterface $client */
-        $client = $this->getMockBuilder(ClientInterface::class)->getMock();
+        $client = $this->createMock(ClientInterface::class);
 
         $client->expects($this->once())
             ->method('post')
             ->with('/test/1', '{"data":{"type":"test"}}', ['X-Foo' => 'bar'])
             ->willReturn($response);
 
         /** @var \PHPUnit\Framework\MockObject\MockObject|\Swis\JsonApi\Client\Interfaces\ResponseParserInterface $parser */
-        $parser = $this->getMockBuilder(ResponseParserInterface::class)->getMock();
+        $parser = $this->createMock(ResponseParserInterface::class);
 
         $parser->expects($this->once())
             ->method('parse')