Skip to content

Commit

Permalink
Patch deep object update consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
@soyuka
soyuka committed Nov 26, 2020
1 parent 6a56b62 commit ac40273
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 32 deletions.
24 changes: 8 additions & 16 deletions features/json/relation.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ Feature: JSON relations support
"anotherRelated": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "laravel",
"thirdLevel": null
},
"related": null
"symfony": "laravel"
}
}
"""

Expand All @@ -82,10 +80,8 @@ Feature: JSON relations support
"anotherRelated": {
"@id": "/related_dummies/2",
"@type": "https://schema.org/Product",
"symfony": "laravel2",
"thirdLevel": null
},
"related": null
"symfony": "laravel2"
}
}
"""

Expand Down Expand Up @@ -113,10 +109,8 @@ Feature: JSON relations support
"anotherRelated": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "API Platform",
"thirdLevel": null
},
"related": null
"symfony": "API Platform"
}
}
"""

Expand Down Expand Up @@ -144,10 +138,8 @@ Feature: JSON relations support
"anotherRelated": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "API Platform 2",
"thirdLevel": null
},
"related": null
"symfony": "API Platform 2"
}
}
"""

Expand Down
77 changes: 62 additions & 15 deletions features/main/relation.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,16 +262,14 @@ Feature: Relations support
"@id": "/relation_embedders/1",
"@type": "RelationEmbedder",
"krondstadt": "Krondstadt",
"anotherRelated": null,
"related": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "symfony",
"thirdLevel": {
"@id": "/third_levels/1",
"@type": "ThirdLevel",
"level": 3,
"fourthLevel": null
"level": 3
}
}
}
Expand Down Expand Up @@ -300,10 +298,8 @@ Feature: Relations support
"anotherRelated": {
"@id": "/related_dummies/2",
"@type": "https://schema.org/Product",
"symfony": "laravel",
"thirdLevel": null
},
"related": null
"symfony": "laravel"
}
}
"""

Expand All @@ -330,10 +326,8 @@ Feature: Relations support
"anotherRelated": {
"@id": "/related_dummies/3",
"@type": "https://schema.org/Product",
"symfony": "laravel2",
"thirdLevel": null
},
"related": null
"symfony": "laravel2"
}
}
"""

Expand Down Expand Up @@ -389,10 +383,8 @@ Feature: Relations support
"anotherRelated": {
"@id": "/related_dummies/2",
"@type": "https://schema.org/Product",
"symfony": "API Platform",
"thirdLevel": null
},
"related": null
"symfony": "API Platform"
}
}
"""

Expand Down Expand Up @@ -545,3 +537,58 @@ Feature: Relations support
]
}
"""

@createSchema
Scenario: Patch the relation
When I add "Content-Type" header equal to "application/ld+json"
And I send a "POST" request to "/relation_embedders" with body:
"""
{
"anotherRelated": {
"symfony": "laravel"
}
}
"""
Then the response status code should be 201
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be equal to:
"""
{
"@context": "/contexts/RelationEmbedder",
"@id": "/relation_embedders/1",
"@type": "RelationEmbedder",
"krondstadt": "Krondstadt",
"anotherRelated": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "laravel"
}
}
"""
Then I add "Content-Type" header equal to "application/merge-patch+json"
And I send a "PATCH" request to "/relation_embedders/1" with body:
"""
{
"anotherRelated": {
"symfony": "laravel2"
}
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be equal to:
"""
{
"@context": "/contexts/RelationEmbedder",
"@id": "/relation_embedders/1",
"@type": "RelationEmbedder",
"krondstadt": "Krondstadt",
"anotherRelated": {
"@id": "/related_dummies/1",
"@type": "https://schema.org/Product",
"symfony": "laravel2"
}
}
"""
4 changes: 4 additions & 0 deletions src/Serializer/AbstractItemNormalizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ protected function getAttributeValue($object, $attribute, $format = null, array
$attributeValue = null;
}

if ($context['api_denormalize'] ?? false) {
return $attributeValue;
}

$type = $propertyMetadata->getType();

if (
Expand Down
7 changes: 6 additions & 1 deletion src/Serializer/SerializerContextBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ public function createFromRequest(Request $request, bool $normalization, array $
if (!$normalization) {
if (!isset($context['api_allow_update'])) {
$context['api_allow_update'] = \in_array($request->getMethod(), ['PUT', 'PATCH'], true);

if ($context['api_allow_update'] && 'PATCH' === $request->getMethod()) {
$context[AbstractItemNormalizer::DEEP_OBJECT_TO_POPULATE] = $context[AbstractItemNormalizer::DEEP_OBJECT_TO_POPULATE] ?? true;
}
}

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

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

break;
}
Expand Down
1 change: 1 addition & 0 deletions tests/Fixtures/TestBundle/Entity/RelationEmbedder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
* "custom_get"={"route_name"="relation_embedded.custom_get", "method"="GET"},
* "custom1"={"path"="/api/custom-call/{id}", "method"="GET"},
* "custom2"={"path"="/api/custom-call/{id}", "method"="PUT"},
* "patch"={"input_formats"={"json"={"application/merge-patch+json"}, "jsonapi"}}
* }
* )
* @ORM\Entity
Expand Down
14 changes: 14 additions & 0 deletions tests/Serializer/SerializerContextBuilderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,17 @@ protected function setUp(): void
]
);

$resourceMetadataWithPatch = new ResourceMetadata(
null,
null,
null,
['patch' => ['method' => 'PATCH', 'input_formats' => ['json'=> ['application/merge-patch+json']]]],
[]
);

$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
$resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata);
$resourceMetadataFactoryProphecy->create('FooWithPatch')->willReturn($resourceMetadataWithPatch);

$this->builder = new SerializerContextBuilder($resourceMetadataFactoryProphecy->reveal());
}
Expand Down Expand Up @@ -85,6 +94,11 @@ public function testCreateFromRequest()
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$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];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/foowithpatch/1', 'PATCH');
$request->attributes->replace(['_api_resource_class' => 'FooWithPatch', '_api_item_operation_name' => 'patch', '_api_format' => 'json', '_api_mime_type' => 'application/json']);
$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];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));
}

public function testThrowExceptionOnInvalidRequest()
Expand Down

0 comments on commit ac40273

Please sign in to comment.