Skip to content

Commit 4f8d6b3

Browse files
committed
Handle absent relation data different then empty relation data
If the relation does not have data (i.e. only meta or links), we should not init the relation as if the data is empty (i.e. null or an empty array).
1 parent a3b3fc9 commit 4f8d6b3

File tree

9 files changed

+92
-40
lines changed

9 files changed

+92
-40
lines changed

src/Concerns/HasRelations.php

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,14 @@ public function getRelationValue(string $name): ?DataInterface
141141
/**
142142
* Set the specific relationship on the model.
143143
*
144-
* @param string $name
145-
* @param \Swis\JsonApi\Client\Interfaces\DataInterface|null $data
146-
* @param \Swis\JsonApi\Client\Links|null $links
147-
* @param \Swis\JsonApi\Client\Meta|null $meta
144+
* @param string $name
145+
* @param \Swis\JsonApi\Client\Interfaces\DataInterface|false|null $data
146+
* @param \Swis\JsonApi\Client\Links|null $links
147+
* @param \Swis\JsonApi\Client\Meta|null $meta
148148
*
149149
* @return static
150150
*/
151-
public function setRelation(string $name, DataInterface $data = null, Links $links = null, Meta $meta = null)
151+
public function setRelation(string $name, $data = false, Links $links = null, Meta $meta = null)
152152
{
153153
$method = Str::camel($name);
154154
if (method_exists($this, $method)) {
@@ -160,8 +160,11 @@ public function setRelation(string $name, DataInterface $data = null, Links $lin
160160
$relationObject = $this->morphTo($name);
161161
}
162162

163-
if ($data !== null) {
164-
$relationObject->associate($data);
163+
if ($data !== false) {
164+
$relationObject->dissociate();
165+
if ($data !== null) {
166+
$relationObject->associate($data);
167+
}
165168
}
166169
$relationObject->setLinks($links);
167170
$relationObject->setMeta($meta);

src/Interfaces/ItemInterface.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,14 @@ public function getAvailableRelations(): array;
127127
/**
128128
* Set the specific relationship in the model.
129129
*
130-
* @param string $relation
131-
* @param \Swis\JsonApi\Client\Interfaces\DataInterface|null $value
132-
* @param \Swis\JsonApi\Client\Links|null $links
133-
* @param \Swis\JsonApi\Client\Meta|null $meta
130+
* @param string $relation
131+
* @param \Swis\JsonApi\Client\Interfaces\DataInterface|false|null $value
132+
* @param \Swis\JsonApi\Client\Links|null $links
133+
* @param \Swis\JsonApi\Client\Meta|null $meta
134134
*
135135
* @return static
136136
*/
137-
public function setRelation(string $relation, DataInterface $value = null, Links $links = null, Meta $meta = null);
137+
public function setRelation(string $relation, $value = false, Links $links = null, Meta $meta = null);
138138

139139
/**
140140
* @return \Swis\JsonApi\Client\Interfaces\OneRelationInterface[]|\Swis\JsonApi\Client\Interfaces\ManyRelationInterface[]

src/Parsers/ItemParser.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,12 @@ private function setRelations(ItemInterface $item, $data): void
135135
throw new ValidationException('Relationship object MUST contain at least one of the following properties: `links`, `data`, `meta`.');
136136
}
137137

138-
$value = null;
139-
if (property_exists($relationship, 'data') && $relationship->data !== null) {
140-
$value = $this->parseRelationshipData($relationship->data);
138+
$value = false;
139+
if (property_exists($relationship, 'data')) {
140+
$value = null;
141+
if ($relationship->data !== null) {
142+
$value = $this->parseRelationshipData($relationship->data);
143+
}
141144
}
142145

143146
$links = null;

src/Relations/AbstractManyRelation.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use Swis\JsonApi\Client\Interfaces\ManyRelationInterface;
77

88
/**
9-
* @property \Swis\JsonApi\Client\Collection|null $included
9+
* @property \Swis\JsonApi\Client\Collection|false|null $included
1010
*/
1111
abstract class AbstractManyRelation extends AbstractRelation implements ManyRelationInterface
1212
{
@@ -22,14 +22,6 @@ public function associate(Collection $included)
2222
return $this;
2323
}
2424

25-
/**
26-
* @return bool
27-
*/
28-
public function hasIncluded(): bool
29-
{
30-
return $this->getIncluded()->isNotEmpty();
31-
}
32-
3325
/**
3426
* @return \Swis\JsonApi\Client\Collection
3527
*/

src/Relations/AbstractOneRelation.php

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use Swis\JsonApi\Client\Interfaces\OneRelationInterface;
77

88
/**
9-
* @property \Swis\JsonApi\Client\Interfaces\ItemInterface|null $included
9+
* @property \Swis\JsonApi\Client\Interfaces\ItemInterface|false|null $included
1010
*/
1111
abstract class AbstractOneRelation extends AbstractRelation implements OneRelationInterface
1212
{
@@ -27,14 +27,6 @@ public function associate(ItemInterface $included)
2727
*/
2828
public function getIncluded(): ? ItemInterface
2929
{
30-
return $this->included;
31-
}
32-
33-
/**
34-
* @return bool
35-
*/
36-
public function hasIncluded(): bool
37-
{
38-
return null !== $this->getIncluded();
30+
return $this->included ?: null;
3931
}
4032
}

src/Relations/AbstractRelation.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ abstract class AbstractRelation
1111
use HasMeta;
1212

1313
/**
14-
* @var \Swis\JsonApi\Client\Interfaces\DataInterface|null
14+
* @var \Swis\JsonApi\Client\Interfaces\DataInterface|false|null
1515
*/
16-
protected $included;
16+
protected $included = false;
1717

1818
/**
1919
* @var bool
@@ -30,6 +30,14 @@ public function dissociate()
3030
return $this;
3131
}
3232

33+
/**
34+
* @return bool
35+
*/
36+
public function hasIncluded(): bool
37+
{
38+
return $this->included !== false;
39+
}
40+
3341
/**
3442
* @param bool $omitIncluded
3543
*

tests/Concerns/HasRelationsTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public function it_can_get_and_set_an_item_as_relation()
2929

3030
$relation = $mock->getRelation('foo');
3131
$this->assertInstanceOf(MorphToRelation::class, $relation);
32+
$this->assertTrue($relation->hasIncluded());
3233
$this->assertSame($data, $relation->getIncluded());
3334
}
3435

@@ -45,9 +46,41 @@ public function it_can_get_and_set_a_collection_as_relation()
4546

4647
$relation = $mock->getRelation('foo');
4748
$this->assertInstanceOf(MorphToManyRelation::class, $relation);
49+
$this->assertTrue($relation->hasIncluded());
4850
$this->assertSame($data, $relation->getIncluded());
4951
}
5052

53+
/**
54+
* @test
55+
*/
56+
public function it_can_get_and_set_null_as_relation()
57+
{
58+
/** @var \PHPUnit\Framework\MockObject\MockObject&\Swis\JsonApi\Client\Concerns\HasRelations $mock */
59+
$mock = $this->getMockForTrait(HasRelations::class);
60+
61+
$mock->setRelation('foo', null);
62+
63+
$relation = $mock->getRelation('foo');
64+
$this->assertInstanceOf(MorphToRelation::class, $relation);
65+
$this->assertTrue($relation->hasIncluded());
66+
$this->assertNull($relation->getIncluded());
67+
}
68+
69+
/**
70+
* @test
71+
*/
72+
public function it_does_not_set_false_as_relation()
73+
{
74+
/** @var \PHPUnit\Framework\MockObject\MockObject&\Swis\JsonApi\Client\Concerns\HasRelations $mock */
75+
$mock = $this->getMockForTrait(HasRelations::class);
76+
77+
$mock->setRelation('foo', false);
78+
79+
$relation = $mock->getRelation('foo');
80+
$this->assertInstanceOf(MorphToRelation::class, $relation);
81+
$this->assertFalse($relation->hasIncluded());
82+
}
83+
5184
/**
5285
* @test
5386
*/

tests/Parsers/ItemParserTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ public function it_parses_an_empty_has_one_relationship()
490490
static::assertInstanceOf(HasOneRelation::class, $item->getRelation('empty'));
491491
static::assertNull($item->getRelation('empty')->getLinks());
492492
static::assertNull($item->getRelation('empty')->getMeta());
493+
static::assertTrue($item->getRelation('empty')->hasIncluded());
493494
static::assertNull($item->getRelation('empty')->getIncluded());
494495
}
495496

@@ -601,6 +602,21 @@ public function it_parses_an_unknown_plural_relation_as_morph_to_many()
601602
static::assertCount(3, $item->getRelation('morphmany')->getIncluded());
602603
}
603604

605+
/**
606+
* @test
607+
*/
608+
public function it_does_not_set_data_when_there_is_no_data_present()
609+
{
610+
$parser = $this->getItemParser();
611+
612+
$item = $parser->parse($this->getJsonApiItemMock('master', '1'));
613+
614+
static::assertInstanceOf(MorphToRelation::class, $item->getRelation('nodata'));
615+
static::assertInstanceOf(Links::class, $item->getRelation('nodata')->getLinks());
616+
static::assertInstanceOf(Meta::class, $item->getRelation('nodata')->getMeta());
617+
static::assertFalse($item->getRelation('nodata')->hasIncluded());
618+
}
619+
604620
/**
605621
* @test
606622
*/
@@ -753,6 +769,14 @@ private function getJsonApiItemMock($type, $id)
753769
'empty' => [
754770
'data' => null,
755771
],
772+
'nodata' => [
773+
'links' => [
774+
'self' => 'http://example.com/'.$type.'/'.$id.'/relationships/nodata',
775+
],
776+
'meta' => [
777+
'foo' => 'bar',
778+
],
779+
],
756780
],
757781
'links' => [
758782
'self' => 'http://example.com/master/1',

tests/Relations/AbstractManyRelationTest.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,8 @@ public function it_can_sort_the_included()
7878
$mock = $this->getMockForAbstractClass(AbstractManyRelation::class);
7979
/** @var \PHPUnit\Framework\MockObject\MockObject&\Swis\JsonApi\Client\Collection $collectionMock */
8080
$collectionMock = $this->getMockBuilder(Collection::class)
81-
->setMethods(['isNotEmpty', 'sortBy'])
81+
->onlyMethods(['sortBy'])
8282
->getMock();
83-
$collectionMock->expects($this->once())
84-
->method('isNotEmpty')
85-
->willReturn(true);
8683
$collectionMock->expects($this->once())
8784
->method('sortBy')
8885
->with('foo', SORT_NATURAL, true);

0 commit comments

Comments
 (0)