Skip to content

[Serializer] Add name_converter flag and use an array of name converters #35374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADE-5.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,8 @@ Routing
-------

* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.

Serializer
----------

* Deprecated passing a name converter directly to the second argument of the constructor of `AbstractNormalizer`, pass an array of name converters instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? it looks fine to me

Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@

<service id="serializer.normalizer.object" class="Symfony\Component\Serializer\Normalizer\ObjectNormalizer">
<argument type="service" id="serializer.mapping.class_metadata_factory" />
<argument type="service" id="serializer.name_converter.metadata_aware" />
<argument type="collection">
<argument type="service" id="serializer.name_converter.metadata_aware" />
<argument type="service" id="serializer.name_converter.camel_case_to_snake_case" />
</argument>
<argument type="service" id="serializer.property_accessor" />
<argument type="service" id="property_info" on-invalid="ignore" />
<argument type="service" id="serializer.mapping.class_discriminator_resolver" on-invalid="ignore" />
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ CHANGELOG
-----

* added support for scalar values denormalization
* changed the name converter constructor argument in normalizers to an array of name converters
* added `AbstractNormalizer::NAME_CONVERTER` constant to set the name converter to use in context

5.0.0
-----
Expand Down
36 changes: 33 additions & 3 deletions src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ abstract class AbstractNormalizer implements NormalizerInterface, DenormalizerIn
*/
public const IGNORED_ATTRIBUTES = 'ignored_attributes';

/**
* The name converter to use when (de)normalizing a property.
*
* It has to correspond of a class of the given name converters.
*
* If not specified or if it does not exist, the first name converter will be used if available.
*/
public const NAME_CONVERTER = 'name_converter';

/**
* @internal
*/
Expand All @@ -130,16 +139,30 @@ abstract class AbstractNormalizer implements NormalizerInterface, DenormalizerIn

/**
* @var NameConverterInterface|null
*
* @deprecated since Symfony 5.1, use $nameConverters instead.
*/
protected $nameConverter;

/**
* @var array<string, NameConverterInterface>
*/
protected $nameConverters = [];

/**
* Sets the {@link ClassMetadataFactoryInterface} to use.
*/
public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, array $defaultContext = [])
public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, /* array */ $nameConverter/*s*/ = [], array $defaultContext = [])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's possible to change or deprecate the name of a constructor argument since it could be used by DI with a named argument.

{
$this->classMetadataFactory = $classMetadataFactory;
$this->nameConverter = $nameConverter;
if (!\is_array($nameConverter)) {
@trigger_error(sprintf('The 2nd constructor argument of the class "%s" should be an array of name converters, using a name converter directly or null is deprecated since Symfony 5.1.', __CLASS__), E_USER_DEPRECATED);
$this->nameConverter = $nameConverter;
} else {
foreach ($nameConverter as $nameConverterInstance) {
$this->nameConverters[\get_class($nameConverterInstance)] = $nameConverterInstance;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a good idea to use the FQCN for referencing a name converter?
We could also use the service id.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, it's not: it prevents passing two differently configured instances of the same class
instead, why not pass a service locator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use a service locator too.
We would keep the name converter and we would add one more argument, am I right?
Or maybe we can replace the name converter with the service locator. I'm not sure to like it since it means not having a clear dependency between the normalizer and the name converter and having to use a default name converter service id.

}
}
$this->defaultContext = array_merge($this->defaultContext, $defaultContext);

if (isset($this->defaultContext[self::CALLBACKS])) {
Expand Down Expand Up @@ -347,7 +370,14 @@ protected function instantiateObject(array &$data, string $class, array &$contex
$params = [];
foreach ($constructorParameters as $constructorParameter) {
$paramName = $constructorParameter->name;
$key = $this->nameConverter ? $this->nameConverter->normalize($paramName, $class, $format, $context) : $paramName;
$key = $paramName;
if (\count($this->nameConverters) > 0) {
$nameConverter = $this->nameConverters[$context[static::NAME_CONVERTER] ?? null] ?? reset($this->nameConverters);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the given name converter in context doesn't correspond to one of the name converters, the first name converter is used instead.
The user is not warned, but I think it's the right approach. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an exception should be thrown instead to me
having a service locator would naturally throw a service-bot-found exception btw.

$key = $nameConverter->normalize($paramName, $class, $format, $context);
} elseif ($this->nameConverter) {
@trigger_error(sprintf('Using the "nameConverter" property of the class "%s" is deprecated since Symfony 5.1, use the "nameConverters" property instead.', __CLASS__), E_USER_DEPRECATED);
$key = $this->nameConverter->normalize($paramName, $class, $format, $context);
}

$allowed = false === $allowedAttributes || \in_array($paramName, $allowedAttributes);
$ignored = !$this->isAllowedAttribute($class, $paramName, $format, $context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorFromClassMetadata;
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorResolverInterface;
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;

/**
* Base class for a normalizer dealing with objects.
Expand Down Expand Up @@ -101,7 +100,7 @@ abstract class AbstractObjectNormalizer extends AbstractNormalizer
*/
protected $classDiscriminatorResolver;

public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, callable $objectClassResolver = null, array $defaultContext = [])
public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, /* array */ $nameConverter/*s*/ = [], PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, callable $objectClassResolver = null, array $defaultContext = [])
{
parent::__construct($classMetadataFactory, $nameConverter, $defaultContext);

Expand Down Expand Up @@ -309,7 +308,11 @@ public function denormalize($data, string $type, string $format = null, array $c
$resolvedClass = $this->objectClassResolver ? ($this->objectClassResolver)($object) : \get_class($object);

foreach ($normalizedData as $attribute => $value) {
if ($this->nameConverter) {
if (\count($this->nameConverters) > 0) {
$nameConverter = $this->nameConverters[$context[static::NAME_CONVERTER] ?? null] ?? reset($this->nameConverters);
$attribute = $nameConverter->denormalize($attribute, $resolvedClass, $format, $context);
} elseif ($this->nameConverter) {
@trigger_error(sprintf('Using the "nameConverter" property of the class "%s" is deprecated since Symfony 5.1, use the "nameConverters" property instead.', __CLASS__), E_USER_DEPRECATED);
$attribute = $this->nameConverter->denormalize($attribute, $resolvedClass, $format, $context);
}

Expand Down Expand Up @@ -505,7 +508,11 @@ private function updateData(array $data, string $attribute, $attributeValue, str
return $data;
}

if ($this->nameConverter) {
if (\count($this->nameConverters) > 0) {
$nameConverter = $this->nameConverters[$context[static::NAME_CONVERTER] ?? null] ?? reset($this->nameConverters);
$attribute = $nameConverter->normalize($attribute, $class, $format, $context);
} elseif ($this->nameConverter) {
@trigger_error(sprintf('Using the "nameConverter" property of the class "%s" is deprecated since Symfony 5.1, use the "nameConverters" property instead.', __CLASS__), E_USER_DEPRECATED);
$attribute = $this->nameConverter->normalize($attribute, $class, $format, $context);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use Symfony\Component\Serializer\Mapping\AttributeMetadata;
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorResolverInterface;
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;

/**
* Converts between objects and arrays using the PropertyAccess component.
Expand All @@ -34,7 +33,7 @@ class ObjectNormalizer extends AbstractObjectNormalizer

private $objectClassResolver;

public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, PropertyAccessorInterface $propertyAccessor = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, callable $objectClassResolver = null, array $defaultContext = [])
public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, /* array */ $nameConverter/*s*/ = [], PropertyAccessorInterface $propertyAccessor = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, callable $objectClassResolver = null, array $defaultContext = [])
{
if (!class_exists(PropertyAccess::class)) {
throw new LogicException('The ObjectNormalizer class requires the "PropertyAccess" component. Install "symfony/property-access" to use it.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function testPropertyPhpDoc($class)
}
EOF;
$serializer = new Serializer([
new ObjectNormalizer(null, null, null, new PhpDocExtractor()),
new ObjectNormalizer(null, [], null, new PhpDocExtractor()),
new ArrayDenormalizer(),
], ['json' => new JsonEncoder()]);
//WHEN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Symfony\Component\Serializer\Mapping\AttributeMetadata;
use Symfony\Component\Serializer\Mapping\ClassMetadata;
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\Normalizer\PropertyNormalizer;
Expand Down Expand Up @@ -134,4 +135,13 @@ public function testObjectWithVariadicConstructorTypedArguments()
$this->assertInstanceOf(Dummy::class, $foo);
}
}

/**
* @group legacy
* @expectedDeprecation The 2nd constructor argument of the class "Symfony\Component\Serializer\Normalizer\AbstractNormalizer" should be an array of name converters, using a name converter directly or null is deprecated since Symfony 5.1.
*/
public function testConstructNameConverter()
{
new AbstractNormalizerDummy($this->classMetadata, new CamelCaseToSnakeCaseNameConverter());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private function getDenormalizerForDummyCollection()
null
));

$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, null, $extractor);
$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, [], $extractor);
$arrayDenormalizer = new ArrayDenormalizerDummy();
$serializer = new SerializerCollectionDummy([$arrayDenormalizer, $denormalizer]);
$arrayDenormalizer->setSerializer($serializer);
Expand Down Expand Up @@ -183,7 +183,7 @@ private function getDenormalizerForStringCollection()
null
));

$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, null, $extractor);
$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, [], $extractor);
$arrayDenormalizer = new ArrayDenormalizerDummy();
$serializer = new SerializerCollectionDummy([$arrayDenormalizer, $denormalizer]);
$arrayDenormalizer->setSerializer($serializer);
Expand Down Expand Up @@ -219,7 +219,7 @@ public function hasMetadataFor($value): bool
};

$discriminatorResolver = new ClassDiscriminatorFromClassMetadata($loaderMock);
$normalizer = new AbstractObjectNormalizerDummy($factory, null, new PhpDocExtractor(), $discriminatorResolver);
$normalizer = new AbstractObjectNormalizerDummy($factory, [], new PhpDocExtractor(), $discriminatorResolver);
$serializer = new Serializer([$normalizer]);
$normalizer->setSerializer($serializer);
$normalizedData = $normalizer->denormalize(['foo' => 'foo', 'baz' => 'baz', 'quux' => ['value' => 'quux'], 'type' => 'second'], AbstractDummy::class);
Expand Down
Loading