Fix a bug preventing to serialize validator's payload (#3375)

* Fix a bug preventing to serialize validator's payload * Review
api-platform · Apr 21, 2020 · 5dd51a0 · 5dd51a0
1 parent 311678d
commit 5dd51a0
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 10 deletions.
diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php
@@ -94,7 +94,7 @@ public function getConfigTreeBuilder()
                 ->arrayNode('validator')
                     ->addDefaultsIfNotSet()
                     ->children()
-                        ->variableNode('serialize_payload_fields')->defaultValue([])->info('Enable the serialization of payload fields when a validation error is thrown.')->end()
+                        ->variableNode('serialize_payload_fields')->defaultValue([])->info('Set to null to serialize all payload fields when a validation error is thrown, or set the fields you want to include explicitly.')->end()
                     ->end()
                 ->end()
                 ->arrayNode('eager_loading')

diff --git a/src/Serializer/AbstractConstraintViolationListNormalizer.php b/src/Serializer/AbstractConstraintViolationListNormalizer.php
@@ -35,7 +35,7 @@ abstract class AbstractConstraintViolationListNormalizer implements NormalizerIn
     public function __construct(array $serializePayloadFields = null, NameConverterInterface $nameConverter = null)
     {
         $this->nameConverter = $nameConverter;
-        $this->serializePayloadFields = $serializePayloadFields;
+        $this->serializePayloadFields = null === $serializePayloadFields ? null : array_flip($serializePayloadFields);
     }
 
     /**
@@ -66,10 +66,14 @@ protected function getMessagesAndViolations(ConstraintViolationListInterface $co
             ];
 
             $constraint = $violation->getConstraint();
-            if ($this->serializePayloadFields && $constraint && $constraint->payload) {
+            if (
+                [] !== $this->serializePayloadFields &&
+                $constraint &&
+                $constraint->payload &&
                 // If some fields are whitelisted, only them are added
-                $payloadFields = null === $this->serializePayloadFields ? $constraint->payload : array_intersect_key($constraint->payload, array_flip($this->serializePayloadFields));
-                $payloadFields && $violationData['payload'] = $payloadFields;
+                $payloadFields = null === $this->serializePayloadFields ? $constraint->payload : array_intersect_key($constraint->payload, $this->serializePayloadFields)
+            ) {
+                $violationData['payload'] = $payloadFields;
             }
 
             $violations[] = $violationData;

diff --git a/tests/Hydra/Serializer/ConstraintViolationNormalizerTest.php b/tests/Hydra/Serializer/ConstraintViolationNormalizerTest.php
@@ -40,7 +40,10 @@ public function testSupportNormalization()
         $this->assertTrue($normalizer->hasCacheableSupportsMethod());
     }
 
-    public function testNormalize()
+    /**
+     * @dataProvider payloadFieldsProvider
+     */
+    public function testNormalize(?array $fields, array $result)
     {
         $urlGeneratorProphecy = $this->prophesize(UrlGeneratorInterface::class);
         $nameConverterProphecy = $this->prophesize(NameConverterInterface::class);
@@ -50,7 +53,7 @@ public function testNormalize()
             return '_'.$args[0];
         });
 
-        $normalizer = new ConstraintViolationListNormalizer($urlGeneratorProphecy->reveal(), ['severity', 'anotherField1'], $nameConverterProphecy->reveal());
+        $normalizer = new ConstraintViolationListNormalizer($urlGeneratorProphecy->reveal(), $fields, $nameConverterProphecy->reveal());
 
         // Note : we use NotNull constraint and not Constraint class because Constraint is abstract
         $constraint = new NotNull();
@@ -70,16 +73,24 @@ public function testNormalize()
                 [
                     'propertyPath' => '_d',
                     'message' => 'a',
-                    'payload' => [
-                        'severity' => 'warning',
-                    ],
                 ],
                 [
                     'propertyPath' => '_4',
                     'message' => '1',
                 ],
             ],
         ];
+        if ([] !== $result) {
+            $expected['violations'][0]['payload'] = $result;
+        }
+
         $this->assertEquals($expected, $normalizer->normalize($list));
     }
+
+    public function payloadFieldsProvider(): iterable
+    {
+        yield [['severity', 'anotherField1'], ['severity' => 'warning']];
+        yield [null, ['severity' => 'warning', 'anotherField2' => 'aValue']];
+        yield [[], []];
+    }
 }