From e5014d834deef32f2c8ab13ee785337d38b69e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Fri, 31 Jan 2020 18:09:24 +0100 Subject: [PATCH] Fix a bug preventing to serialize validator's payload --- .../DependencyInjection/Configuration.php | 2 +- ...tractConstraintViolationListNormalizer.php | 12 +++++++---- .../ApiPlatformExtensionTest.php | 2 +- .../DependencyInjection/ConfigurationTest.php | 2 +- .../ConstraintViolationNormalizerTest.php | 21 ++++++++++++++----- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index b49c43b28ed..01ab400629b 100644 --- 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 index 03a3052835a..d977595e470 100644 --- 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 index c378d7897d7..77980595527 100644 --- 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 index a891c8724e2..6166ca4de6d 100644 --- 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 index f381974a33e..15604c75bf7 100644 --- 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,9 +73,6 @@ public function testNormalize() [ 'propertyPath' => '_d', 'message' => 'a', - 'payload' => [ - 'severity' => 'warning', - ], ], [ 'propertyPath' => '_4', @@ -80,6 +80,17 @@ public function testNormalize() ], ], ]; + 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, []]; + } }