Patch deep object update consistency

soyuka · Nov 26, 2020 · 41cfbb7 · 41cfbb7
1 parent 6a56b62
commit 41cfbb7
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 17 deletions.
diff --git a/features/main/relation.feature b/features/main/relation.feature
@@ -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
           }
         }
       }
@@ -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"
+      }
     }
     """
 
@@ -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"
+      }
     }
     """
 
@@ -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"
+      }
     }
     """
 
@@ -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"
+      }
+    }
+    """
diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php
@@ -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 (

diff --git a/src/Serializer/SerializerContextBuilder.php b/src/Serializer/SerializerContextBuilder.php
@@ -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()) {
@@ -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;
             }

diff --git a/tests/Fixtures/TestBundle/Entity/RelationEmbedder.php b/tests/Fixtures/TestBundle/Entity/RelationEmbedder.php
@@ -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

diff --git a/tests/Serializer/SerializerContextBuilderTest.php b/tests/Serializer/SerializerContextBuilderTest.php
@@ -78,7 +78,7 @@ public function testCreateFromRequest()
 
         $request = Request::create('/foos', 'PUT');
         $request->attributes->replace(['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'put', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
-        $expected = ['bar' => 'baz', 'collection_operation_name' => 'put', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => true, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null];
+        $expected = ['bar' => 'baz', 'collection_operation_name' => 'put', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => true, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'deep_object_to_populate' => true];
         $this->assertEquals($expected, $this->builder->createFromRequest($request, false));
 
         $request = Request::create('/bars/1/foos');