Fix a bug preventing to serialize validator's payload

api-platform · Jan 31, 2020 · e5014d8 · e5014d8
1 parent bf1f930
commit e5014d8
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 12 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')->defaultNull()->info('Enable the serialization of payload fields when a validation error is thrown.')->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 (
+                null !== $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 = [] === $this->serializePayloadFields ? $constraint->payload : array_intersect_key($constraint->payload, $this->serializePayloadFields)
+            ) {
+                $violationData['payload'] = $payloadFields;
             }
 
             $violations[] = $violationData;

diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php
@@ -1112,7 +1112,7 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo
             'api_platform.graphql.graphiql.enabled' => true,
             'api_platform.graphql.graphql_playground.enabled' => true,
             'api_platform.resource_class_directories' => Argument::type('array'),
-            'api_platform.validator.serialize_payload_fields' => [],
+            'api_platform.validator.serialize_payload_fields' => null,
             'api_platform.elasticsearch.enabled' => false,
         ];
 

diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ConfigurationTest.php
@@ -104,7 +104,7 @@ private function runDefaultConfigTests(array $doctrineIntegrationsToLoad = ['orm
             'default_operation_path_resolver' => 'api_platform.operation_path_resolver.underscore',
             'path_segment_name_generator' => 'api_platform.path_segment_name_generator.underscore',
             'validator' => [
-                'serialize_payload_fields' => [],
+                'serialize_payload_fields' => null,
             ],
             'name_converter' => null,
             'enable_fos_user' => true,

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 [[], ['severity' => 'warning', 'anotherField2' => 'aValue']];
+        yield [null, []];
+    }
 }