From ae9d1aa26012f7aaf465d0c666205f2af4e06ec3 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Fri, 9 Aug 2019 14:25:21 +0200 Subject: [PATCH 1/2] improve naming strategy handing by wrapping always custom naming strategies with the custom serialized name strategy --- DependencyInjection/Configuration.php | 1 + .../JMSSerializerExtension.php | 12 +++- Resources/config/services.xml | 5 +- Resources/doc/configuration.rst | 3 +- .../NamingStrategyTest.php | 60 +++++++++++++++++-- 5 files changed, 69 insertions(+), 12 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index f6b08b85..0e76de9d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -117,6 +117,7 @@ private function addSerializersSection(NodeBuilder $builder) ->end() ->children() ->scalarNode('id')->cannotBeEmpty()->end() + ->booleanNode('allow_custom_serialized_name')->defaultValue(true)->end() ->scalarNode('separator')->defaultValue('_')->end() ->booleanNode('lower_case')->defaultTrue()->end() ->end() diff --git a/DependencyInjection/JMSSerializerExtension.php b/DependencyInjection/JMSSerializerExtension.php index 4bfd3c31..45310adc 100644 --- a/DependencyInjection/JMSSerializerExtension.php +++ b/DependencyInjection/JMSSerializerExtension.php @@ -5,6 +5,7 @@ use JMS\Serializer\EventDispatcher\EventSubscriberInterface; use JMS\Serializer\Exception\RuntimeException; use JMS\Serializer\Handler\SubscribingHandlerInterface; +use JMS\Serializer\Naming\SerializedNameAnnotationStrategy; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -54,8 +55,14 @@ public function loadInternal(array $config, ContainerBuilder $container) ->addArgument($config['property_naming']['separator']) ->addArgument($config['property_naming']['lower_case']); - if (!empty($config['property_naming']['id'])) { - $container->setAlias('jms_serializer.naming_strategy', $config['property_naming']['id']); + $namingStrategyServiceId = $config['property_naming']['id'] ?? 'jms_serializer.camel_case_naming_strategy'; + $container->setAlias('jms_serializer.naming_strategy', $namingStrategyServiceId); + + if (!empty($config['property_naming']['allow_custom_serialized_name'])) { + $container + ->register($namingStrategyServiceId . '.decorated', SerializedNameAnnotationStrategy::class) + ->setDecoratedService($namingStrategyServiceId) + ->addArgument(new Reference($namingStrategyServiceId. '.decorated.inner')); } $bundles = $container->getParameter('kernel.bundles'); @@ -72,7 +79,6 @@ public function loadInternal(array $config, ContainerBuilder $container) $container ->getDefinition('jms_serializer.accessor_strategy.default') ->setArgument(0, new Reference($config['expression_evaluator']['id'])); - } else { $container->removeDefinition('jms_serializer.expression_evaluator'); } diff --git a/Resources/config/services.xml b/Resources/config/services.xml index d44a5c3a..c958fe2a 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -112,11 +112,8 @@ - - - - + diff --git a/Resources/doc/configuration.rst b/Resources/doc/configuration.rst index cc6fb293..332ccd73 100644 --- a/Resources/doc/configuration.rst +++ b/Resources/doc/configuration.rst @@ -182,7 +182,8 @@ values: fallback_strategy: "null" # possible values ("null" | "exception" | "fallback") property_naming: - id: ~ + id: ~ # service id for a custom naming strategy to use, separator and lower_case will be ignored + allow_custom_serialized_name: true # consider metadata custom name definitions (@SerializedName as example) separator: _ lower_case: true diff --git a/Tests/DependencyInjection/NamingStrategyTest.php b/Tests/DependencyInjection/NamingStrategyTest.php index 39f0631f..2c204fde 100644 --- a/Tests/DependencyInjection/NamingStrategyTest.php +++ b/Tests/DependencyInjection/NamingStrategyTest.php @@ -2,11 +2,14 @@ namespace JMS\SerializerBundle\Tests\DependencyInjection; +use Doctrine\Common\Annotations\AnnotationReader; use JMS\Serializer\Metadata\PropertyMetadata; use JMS\Serializer\Naming\PropertyNamingStrategyInterface; +use JMS\Serializer\Naming\SerializedNameAnnotationStrategy; use JMS\SerializerBundle\DependencyInjection\JMSSerializerExtension; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; class NamingStrategyTest extends TestCase { @@ -24,21 +27,70 @@ private function getContainer(array $configs = array()) $container->setParameter('kernel.cache_dir', sys_get_temp_dir() . '/serializer'); $container->setParameter('kernel.bundles', array()); + $container->setDefinition("annotation_reader", new Definition(AnnotationReader::class)); + $container->setDefinition("custom_naming_strategy", new Definition(CustomNamingStrategy::class)); + $loader->load(['jms_serializer' => $configs], $container); + + // set public only for testing + $container->getAlias('jms_serializer.naming_strategy')->setPublic(true); + + $container->compile(); return $container; } - public function testCustomNamingStrategy() + public function testDefaultNamingStrategy() + { + $container = $this->getContainer(); + + $strategy = $container->get('jms_serializer.naming_strategy'); + $this->assertInstanceOf(SerializedNameAnnotationStrategy::class, $strategy); + + $property = new PropertyMetadata(\stdClass::class, 'propOne'); + self::assertSame('prop_one', $strategy->translateName($property)); + + $property = new PropertyMetadata(\stdClass::class, 'propOne'); + $property->serializedName = 'abc'; + self::assertSame('abc', $strategy->translateName($property)); + } + + public function testCustomNamingStrategyIsWrappedBySerializedNameAnnotationStrategy() + { + $container = $this->getContainer(array( + 'property_naming' => array( + 'id' => 'custom_naming_strategy', + ) + )); + + $strategy = $container->get('jms_serializer.naming_strategy'); + $this->assertInstanceOf(SerializedNameAnnotationStrategy::class, $strategy); + + $property = new PropertyMetadata(\stdClass::class, 'prop1'); + self::assertSame('foo', $strategy->translateName($property)); + + $property = new PropertyMetadata(\stdClass::class, 'prop1'); + $property->serializedName = 'abc'; + self::assertSame('abc', $strategy->translateName($property)); + } + + public function testCustomNamingStrategyWrapCanBeDisabled() { $container = $this->getContainer(array( 'property_naming' => array( 'id' => 'custom_naming_strategy', + 'allow_custom_serialized_name' => false, ) )); - $customNamingStrategy = new CustomNamingStrategy(); - $container->set("custom_naming_strategy", $customNamingStrategy); - $this->assertSame($customNamingStrategy, $container->get('jms_serializer.naming_strategy')); + $strategy = $container->get('jms_serializer.naming_strategy'); + $this->assertInstanceOf(CustomNamingStrategy::class, $strategy); + + $property = new PropertyMetadata(\stdClass::class, 'prop1'); + self::assertSame('foo', $strategy->translateName($property)); + + $property = new PropertyMetadata(\stdClass::class, 'prop1'); + $property->serializedName = 'abc'; + self::assertSame('foo', $strategy->translateName($property)); } } From 3e3f6e68d9e470212aca41bd05c82a48b81ae7c1 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Fri, 9 Aug 2019 14:55:21 +0200 Subject: [PATCH 2/2] fully BC implementation --- DependencyInjection/Configuration.php | 2 +- DependencyInjection/JMSSerializerExtension.php | 4 ++-- Resources/config/services.xml | 5 ++++- Resources/doc/configuration.rst | 2 +- Tests/DependencyInjection/NamingStrategyTest.php | 4 ++-- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 0e76de9d..73c3ba99 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -117,7 +117,7 @@ private function addSerializersSection(NodeBuilder $builder) ->end() ->children() ->scalarNode('id')->cannotBeEmpty()->end() - ->booleanNode('allow_custom_serialized_name')->defaultValue(true)->end() + ->booleanNode('allow_custom_serialized_name')->defaultValue(false)->end() ->scalarNode('separator')->defaultValue('_')->end() ->booleanNode('lower_case')->defaultTrue()->end() ->end() diff --git a/DependencyInjection/JMSSerializerExtension.php b/DependencyInjection/JMSSerializerExtension.php index 45310adc..70fa8878 100644 --- a/DependencyInjection/JMSSerializerExtension.php +++ b/DependencyInjection/JMSSerializerExtension.php @@ -55,10 +55,10 @@ public function loadInternal(array $config, ContainerBuilder $container) ->addArgument($config['property_naming']['separator']) ->addArgument($config['property_naming']['lower_case']); - $namingStrategyServiceId = $config['property_naming']['id'] ?? 'jms_serializer.camel_case_naming_strategy'; + $namingStrategyServiceId = $config['property_naming']['id'] ?? 'jms_serializer.serialized_name_annotation_strategy'; $container->setAlias('jms_serializer.naming_strategy', $namingStrategyServiceId); - if (!empty($config['property_naming']['allow_custom_serialized_name'])) { + if (!empty($config['property_naming']['allow_custom_serialized_name']) && $namingStrategyServiceId !== 'jms_serializer.serialized_name_annotation_strategy') { $container ->register($namingStrategyServiceId . '.decorated', SerializedNameAnnotationStrategy::class) ->setDecoratedService($namingStrategyServiceId) diff --git a/Resources/config/services.xml b/Resources/config/services.xml index c958fe2a..d44a5c3a 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -112,8 +112,11 @@ + + + - + diff --git a/Resources/doc/configuration.rst b/Resources/doc/configuration.rst index 332ccd73..49930481 100644 --- a/Resources/doc/configuration.rst +++ b/Resources/doc/configuration.rst @@ -183,7 +183,7 @@ values: property_naming: id: ~ # service id for a custom naming strategy to use, separator and lower_case will be ignored - allow_custom_serialized_name: true # consider metadata custom name definitions (@SerializedName as example) + allow_custom_serialized_name: false # consider metadata custom name definitions (@SerializedName as example), suggested to true, false as default for BC reasons separator: _ lower_case: true diff --git a/Tests/DependencyInjection/NamingStrategyTest.php b/Tests/DependencyInjection/NamingStrategyTest.php index 2c204fde..a0ed06d8 100644 --- a/Tests/DependencyInjection/NamingStrategyTest.php +++ b/Tests/DependencyInjection/NamingStrategyTest.php @@ -59,6 +59,7 @@ public function testCustomNamingStrategyIsWrappedBySerializedNameAnnotationStrat $container = $this->getContainer(array( 'property_naming' => array( 'id' => 'custom_naming_strategy', + 'allow_custom_serialized_name' => true ) )); @@ -73,12 +74,11 @@ public function testCustomNamingStrategyIsWrappedBySerializedNameAnnotationStrat self::assertSame('abc', $strategy->translateName($property)); } - public function testCustomNamingStrategyWrapCanBeDisabled() + public function testCustomNamingStrategyWrapIsDisabledByDefault() { $container = $this->getContainer(array( 'property_naming' => array( 'id' => 'custom_naming_strategy', - 'allow_custom_serialized_name' => false, ) ));