Skip to content

Commit 9775a97

Browse files
soyukasoyuka
committed
Deep object update consistency
1 parent dd105c0 commit 9775a97

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
@@ -577,6 +577,10 @@ protected function getAttributeValue($object, $attribute, $format = null, array
577577
$attributeValue = null;
578578
}
579579

580+
if ($context['api_denormalize'] ?? false) {
581+
return $attributeValue;
582+
}
583+
580584
$type = $propertyMetadata->getType();
581585

582586
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()) {
@@ -101,9 +105,10 @@ public function createFromRequest(Request $request, bool $normalization, array $
101105
return $context;
102106
}
103107

108+
// 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
104109
foreach ($resourceMetadata->getItemOperations() as $operation) {
105110
if ('PATCH' === ($operation['method'] ?? '') && \in_array('application/merge-patch+json', $operation['input_formats']['json'] ?? [], true)) {
106-
$context['skip_null_values'] = true;
111+
$context[AbstractItemNormalizer::SKIP_NULL_VALUES] = true;
107112

108113
break;
109114
}

‎tests/Behat/DoctrineContext.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\NetworkPathDummy as NetworkPathDummyDocument;
6262
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\NetworkPathRelationDummy as NetworkPathRelationDummyDocument;
6363
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Order as OrderDocument;
64+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\PatchDummyRelation as PatchDummyRelationDocument;
6465
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Person as PersonDocument;
6566
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\PersonToPet as PersonToPetDocument;
6667
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Pet as PetDocument;
@@ -128,6 +129,7 @@
128129
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\NetworkPathDummy;
129130
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\NetworkPathRelationDummy;
130131
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Order;
132+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PatchDummyRelation;
131133
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Person;
132134
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PersonToPet;
133135
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Pet;
@@ -1658,6 +1660,19 @@ public function thereIsAnInitializeInput(int $id)
16581660
$this->manager->flush();
16591661
}
16601662

1663+
/**
1664+
* @Given there is a PatchDummyRelation
1665+
*/
1666+
public function thereIsAPatchDummyRelation()
1667+
{
1668+
$dummy = $this->buildPatchDummyRelation();
1669+
$related = $this->buildRelatedDummy();
1670+
$dummy->setRelated($related);
1671+
$this->manager->persist($related);
1672+
$this->manager->persist($dummy);
1673+
$this->manager->flush();
1674+
}
1675+
16611676
private function isOrm(): bool
16621677
{
16631678
return null !== $this->schemaTool;
@@ -2091,4 +2106,12 @@ private function buildInitializeInput()
20912106
{
20922107
return $this->isOrm() ? new InitializeInput() : new InitializeInputDocument();
20932108
}
2109+
2110+
/**
2111+
* @return PatchDummyRelation|PatchDummyRelationDocument
2112+
*/
2113+
private function buildPatchDummyRelation()
2114+
{
2115+
return $this->isOrm() ? new PatchDummyRelation() : new PatchDummyRelationDocument();
2116+
}
20942117
}

‎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, 'iri_only' => false];
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, 'iri_only' => false];
101+
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));
88102
}
89103

90104
public function testThrowExceptionOnInvalidRequest()

0 commit comments

Comments
 (0)