Skip to content

Commit 5f28f3c

Browse files
committed
[Bugfix] Prevent relations being treated as attributes
Closes #447
1 parent 6f27b8e commit 5f28f3c

File tree

5 files changed

+49
-13
lines changed

5 files changed

+49
-13
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ Amend query method signature on validated request to match Laravel request signa
1414
### Fixed
1515
- [#445](https://github.com/cloudcreativity/laravel-json-api/issues/445)
1616
Allow resource identifier to be zero.
17+
- [#447](https://github.com/cloudcreativity/laravel-json-api/issues/447)
18+
Ensure deserialized attributes do not include relations if the relation has been sent as an attribute.
1719

1820
## [1.5.0] - 2019-10-14
1921

src/Document/ResourceObject.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -717,10 +717,6 @@ private function fieldNames(): Collection
717717
*/
718718
private function normalize(): void
719719
{
720-
if (collect($this->attributes)->intersectByKeys($this->relationships)->isNotEmpty()) {
721-
throw new \LogicException('Attributes and relationships cannot have the same field names.');
722-
}
723-
724720
$this->fieldValues = $this->fieldValues();
725721
$this->fieldNames = $this->fieldNames();
726722
}

src/Eloquent/Concerns/DeserializesAttributes.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ protected function modelKeyForField($field, $model)
103103
*/
104104
protected function deserializeAttributes($attributes, $record)
105105
{
106-
return collect($attributes)->reject(function ($v, $field) use ($record) {
107-
return $this->isNotFillable($field, $record);
106+
return collect($attributes)->filter(function ($v, $field) use ($record) {
107+
return $this->isFillableAttribute($field, $record);
108108
})->mapWithKeys(function ($value, $field) use ($record) {
109109
$key = $this->modelKeyForField($field, $record);
110110

@@ -169,4 +169,16 @@ protected function isDateAttribute($field, $record)
169169
return in_array($field, $this->dates, true);
170170
}
171171

172+
/**
173+
* Is the field a fillable attribute?
174+
*
175+
* @param string $field
176+
* @param mixed $record
177+
* @return bool
178+
*/
179+
protected function isFillableAttribute($field, $record)
180+
{
181+
return $this->isFillable($field, $record) && !$this->isRelation($field);
182+
}
183+
172184
}

tests/lib/Integration/Eloquent/ResourceTest.php

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public function testCreate()
166166
'author' => [
167167
'data' => [
168168
'type' => 'users',
169-
'id' => (string) $model->author_id,
169+
'id' => (string) $model->author->getRouteKey(),
170170
],
171171
],
172172
],
@@ -182,7 +182,7 @@ public function testCreate()
182182
'title' => $model->title,
183183
'slug' => $model->slug,
184184
'content' => $model->content,
185-
'author_id' => $model->author_id,
185+
'author_id' => $model->author->getKey(),
186186
]);
187187
}
188188

@@ -455,6 +455,32 @@ public function testUpdateWithUnrecognisedRelationship()
455455
]);
456456
}
457457

458+
/**
459+
* The client sends an unexpected attribute with the same name as a
460+
* relationship.
461+
*/
462+
public function testUpdateWithRelationshipAsAttribute()
463+
{
464+
$post = factory(Post::class)->create();
465+
466+
$data = [
467+
'type' => 'posts',
468+
'id' => (string) $post->getRouteKey(),
469+
'attributes' => [
470+
'title' => 'Hello World',
471+
'author' => 'foobar',
472+
],
473+
];
474+
475+
$this->doUpdate($data)->assertStatus(200);
476+
477+
$this->assertDatabaseHas('posts', [
478+
'id' => $post->getKey(),
479+
'title' => 'Hello World',
480+
'author_id' => $post->author_id,
481+
]);
482+
}
483+
458484
/**
459485
* Laravel conversion middleware e.g. trim strings, works.
460486
*

tests/lib/Unit/Document/ResourceObjectTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,16 +182,16 @@ public function testGetWithDefault(): void
182182

183183
/**
184184
* Fields share a common namespace, so if there is a duplicate field
185-
* name in the attributes and relationships, we expect an exception as the resource
186-
* is not valid.
185+
* name in the attributes and relationships, there is a collision.
186+
* We expect the relationship to be returned.
187187
*/
188188
public function testDuplicateFields(): void
189189
{
190190
$this->values['attributes']['author'] = null;
191191

192-
$this->expectException(\LogicException::class);
193-
$this->expectExceptionMessage('same field names');
194-
ResourceObject::create($this->values);
192+
$resource = ResourceObject::create($this->values);
193+
194+
$this->assertSame($this->values['relationships']['author']['data'], $resource['author']);
195195
}
196196

197197
public function testCannotSetOffset(): void

0 commit comments

Comments
 (0)