Skip to content

Commit 8608271

Browse files
soyukasoyuka
committed
Deep object update consistency
1 parent 6a56b62 commit 8608271

File tree

7 files changed

+195
-2
lines changed

7 files changed

+195
-2
lines changed

‎features/main/patch.feature

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,34 @@ Feature: Sending PATCH requets
2828
{"name": null}
2929
"""
3030
Then the JSON node "name" should not exist
31+
32+
@createSchema
33+
Scenario: Patch the relation
34+
Given there is a PatchDummyRelation
35+
When I add "Content-Type" header equal to "application/merge-patch+json"
36+
And I send a "PATCH" request to "/patch_dummy_relations/1" with body:
37+
"""
38+
{
39+
"related": {
40+
"symfony": "A new name"
41+
}
42+
}
43+
"""
44+
Then print last JSON response
45+
Then the response status code should be 200
46+
And the response should be in JSON
47+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
48+
And the JSON should be equal to:
49+
"""
50+
{
51+
"@context": "/contexts/PatchDummyRelation",
52+
"@id": "/patch_dummy_relations/1",
53+
"@type": "PatchDummyRelation",
54+
"related": {
55+
"@id": "/related_dummies/1",
56+
"@type": "https://schema.org/Product",
57+
"id": 1,
58+
"symfony": "A new name"
59+
}
60+
}
61+
"""

‎src/Serializer/AbstractItemNormalizer.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,10 @@ protected function getAttributeValue($object, $attribute, $format = null, array
527527
$attributeValue = null;
528528
}
529529

530+
if ($context['api_denormalize'] ?? false) {
531+
return $attributeValue;
532+
}
533+
530534
$type = $propertyMetadata->getType();
531535

532536
if (

‎src/Serializer/SerializerContextBuilder.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ public function createFromRequest(Request $request, bool $normalization, array $
6363

6464
if (!$normalization) {
6565
if (!isset($context['api_allow_update'])) {
66-
$context['api_allow_update'] = \in_array($request->getMethod(), ['PUT', 'PATCH'], true);
66+
$context['api_allow_update'] = \in_array($method = $request->getMethod(), ['PUT', 'PATCH'], true);
67+
68+
if ($context['api_allow_update'] && 'PATCH' === $method) {
69+
$context[AbstractItemNormalizer::DEEP_OBJECT_TO_POPULATE] = $context[AbstractItemNormalizer::DEEP_OBJECT_TO_POPULATE] ?? true;
70+
}
6771
}
6872

6973
if ('csv' === $request->getContentType()) {
@@ -100,9 +104,10 @@ public function createFromRequest(Request $request, bool $normalization, array $
100104
return $context;
101105
}
102106

107+
// TODO: We should always use `skip_null_values` but changing this would be a BC break, for now use it only when `merge-patch+json` is activated on a Resource
103108
foreach ($resourceMetadata->getItemOperations() as $operation) {
104109
if ('PATCH' === ($operation['method'] ?? '') && \in_array('application/merge-patch+json', $operation['input_formats']['json'] ?? [], true)) {
105-
$context['skip_null_values'] = true;
110+
$context[AbstractItemNormalizer::SKIP_NULL_VALUES] = true;
106111

107112
break;
108113
}

‎tests/Behat/DoctrineContext.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Greeting as GreetingDocument;
5555
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\MaxDepthDummy as MaxDepthDummyDocument;
5656
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Order as OrderDocument;
57+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\PatchDummyRelation as PatchDummyRelationDocument;
5758
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Person as PersonDocument;
5859
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\PersonToPet as PersonToPetDocument;
5960
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Pet as PetDocument;
@@ -113,6 +114,7 @@
113114
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\InternalUser;
114115
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\MaxDepthDummy;
115116
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Order;
117+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PatchDummyRelation;
116118
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Person;
117119
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PersonToPet;
118120
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Pet;
@@ -1520,6 +1522,19 @@ public function thereAreConvertedOwnerObjects(int $nb)
15201522
$this->manager->flush();
15211523
}
15221524

1525+
/**
1526+
* @Given there is a PatchDummyRelation
1527+
*/
1528+
public function thereIsAPatchDummyRelation()
1529+
{
1530+
$dummy = $this->buildPatchDummyRelation();
1531+
$related = $this->buildRelatedDummy();
1532+
$dummy->setRelated($related);
1533+
$this->manager->persist($related);
1534+
$this->manager->persist($dummy);
1535+
$this->manager->flush();
1536+
}
1537+
15231538
private function isOrm(): bool
15241539
{
15251540
return null !== $this->schemaTool;
@@ -1897,4 +1912,12 @@ private function buildConvertedRelated()
18971912
{
18981913
return $this->isOrm() ? new ConvertedRelated() : new ConvertedRelatedDocument();
18991914
}
1915+
1916+
/**
1917+
* @return PatchDummyRelation|PatchDummyRelationDocument
1918+
*/
1919+
private function buildPatchDummyRelation()
1920+
{
1921+
return $this->isOrm() ? new PatchDummyRelation() : new PatchDummyRelationDocument();
1922+
}
19001923
}

‎tests/Fixtures/TestBundle/Document/PatchDummyRelation.php

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Document;
15+
16+
use ApiPlatform\Core\Annotation\ApiResource;
17+
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
18+
use Symfony\Component\Serializer\Annotation\Groups;
19+
20+
/**
21+
* @author Kévin Dunglas <dunglas@gmail.com>
22+
*
23+
* @ApiResource(
24+
* attributes={
25+
* "normalization_context"={"groups"={"chicago"}},
26+
* "denormalization_context"={"groups"={"chicago"}},
27+
* },
28+
* itemOperations={
29+
* "get",
30+
* "patch"={"input_formats"={"json"={"application/merge-patch+json"}, "jsonapi"}}
31+
* }
32+
* )
33+
* @ODM\Document
34+
*/
35+
class PatchDummyRelation
36+
{
37+
/**
38+
* @ODM\Id(strategy="INCREMENT", type="integer")
39+
*/
40+
public $id;
41+
42+
/**
43+
* @ODM\ReferenceOne(targetDocument=RelatedDummy::class)
44+
* @Groups({"chicago"})
45+
*/
46+
protected $related;
47+
48+
public function getRelated()
49+
{
50+
return $this->related;
51+
}
52+
53+
public function setRelated(RelatedDummy $relatedDummy)
54+
{
55+
$this->related = $relatedDummy;
56+
}
57+
}

‎tests/Fixtures/TestBundle/Entity/PatchDummyRelation.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity;
15+
16+
use ApiPlatform\Core\Annotation\ApiResource;
17+
use Doctrine\ORM\Mapping as ORM;
18+
use Symfony\Component\Serializer\Annotation\Groups;
19+
20+
/**
21+
* @author Kévin Dunglas <dunglas@gmail.com>
22+
*
23+
* @ApiResource(
24+
* attributes={
25+
* "normalization_context"={"groups"={"chicago"}},
26+
* "denormalization_context"={"groups"={"chicago"}},
27+
* },
28+
* itemOperations={
29+
* "get",
30+
* "patch"={"input_formats"={"json"={"application/merge-patch+json"}, "jsonapi"}}
31+
* }
32+
* )
33+
* @ORM\Entity
34+
*/
35+
class PatchDummyRelation
36+
{
37+
/**
38+
* @ORM\Id
39+
* @ORM\Column(type="integer")
40+
* @ORM\GeneratedValue(strategy="AUTO")
41+
*/
42+
public $id;
43+
44+
/**
45+
* @ORM\ManyToOne(targetEntity="RelatedDummy")
46+
* @Groups({"chicago"})
47+
*/
48+
protected $related;
49+
50+
public function getRelated()
51+
{
52+
return $this->related;
53+
}
54+
55+
public function setRelated(RelatedDummy $relatedDummy)
56+
{
57+
$this->related = $relatedDummy;
58+
}
59+
}

‎tests/Serializer/SerializerContextBuilderTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,17 @@ protected function setUp(): void
4848
]
4949
);
5050

51+
$resourceMetadataWithPatch = new ResourceMetadata(
52+
null,
53+
null,
54+
null,
55+
['patch' => ['method' => 'PATCH', 'input_formats' => ['json' => ['application/merge-patch+json']]]],
56+
[]
57+
);
58+
5159
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
5260
$resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata);
61+
$resourceMetadataFactoryProphecy->create('FooWithPatch')->willReturn($resourceMetadataWithPatch);
5362

5463
$this->builder = new SerializerContextBuilder($resourceMetadataFactoryProphecy->reveal());
5564
}
@@ -85,6 +94,11 @@ public function testCreateFromRequest()
8594
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
8695
$expected = ['bar' => 'baz', 'subresource_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/bars/1/foos', 'operation_type' => 'subresource', 'api_allow_update' => false, 'uri' => 'http://localhost/bars/1/foos', 'output' => null, 'input' => null];
8796
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));
97+
98+
$request = Request::create('/foowithpatch/1', 'PATCH');
99+
$request->attributes->replace(['_api_resource_class' => 'FooWithPatch', '_api_item_operation_name' => 'patch', '_api_format' => 'json', '_api_mime_type' => 'application/json']);
100+
$expected = ['item_operation_name' => 'patch', 'resource_class' => 'FooWithPatch', 'request_uri' => '/foowithpatch/1', 'operation_type' => 'item', 'api_allow_update' => true, 'uri' => 'http://localhost/foowithpatch/1', 'output' => null, 'input' => null, 'deep_object_to_populate' => true, 'skip_null_values' => true];
101+
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));
88102
}
89103

90104
public function testThrowExceptionOnInvalidRequest()

0 commit comments

Comments
 (0)