Skip to content

Allow serializer context configuration #28927

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

Allow serializer context configuration #28927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,12 @@ private function addSerializerSection(ArrayNodeDefinition $rootNode)
->end()
->end()
->end()
->arrayNode('default_context')
->normalizeKeys(false)
->useAttributeAsKey('name')
->defaultValue([])
->prototype('variable')->end()
->end()
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't allow for things like the circular_reference_handler. I mean, you can set it, but there's no way to set this to a service, for example - none of the handler stuff will work. We need to check for the handler options explicitly in FrameworkExtension and convert them to References. In the docs, I guess we'll need to document these handlers as services that should use the __invoke() method.

Copy link
Member

@dunglas dunglas Nov 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe could be also do something more generic: check if a service corresponding to a given string exists, if it's the case, convert it to a reference. WDYT?

->end()
->end()
->end()
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,8 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
$defaultContext += ['max_depth_handler' => new Reference($config['max_depth_handler'])];
$container->getDefinition('serializer.normalizer.object')->replaceArgument(6, $defaultContext);
}

$container->setParameter('serializer.default_context', $config['default_context'] ?? []);
}

private function registerPropertyInfoConfiguration(ContainerBuilder $container, XmlFileLoader $loader)
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@
<xsd:complexType name="serializer">
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element name="mapping" type="file_mapping" />
<xsd:element name="default-context" type="metadata" minOccurs="0" maxOccurs="1" />
</xsd:choice>
<xsd:attribute name="enabled" type="xsd:boolean" />
<xsd:attribute name="enable-annotations" type="xsd:boolean" />
Expand Down
15 changes: 12 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@

<!-- Normalizer -->
<service id="serializer.normalizer.constraint_violation_list" class="Symfony\Component\Serializer\Normalizer\ConstraintViolationListNormalizer">
<argument type="collection" />
<argument type="service" id="serializer.name_converter.metadata_aware" />
<argument>%serializer.default_context%</argument>
<!-- Run before serializer.normalizer.object -->
<tag name="serializer.normalizer" priority="-915" />
</service>
Expand All @@ -44,6 +43,7 @@
</service>

<service id="serializer.normalizer.dateinterval" class="Symfony\Component\Serializer\Normalizer\DateIntervalNormalizer">
<argument>%serializer.default_context%</argument>
<!-- Run before serializer.normalizer.object -->
<tag name="serializer.normalizer" priority="-915" />
</service>
Expand All @@ -55,11 +55,15 @@
</service>

<service id="serializer.normalizer.datetime" class="Symfony\Component\Serializer\Normalizer\DateTimeNormalizer">
<argument>%serializer.default_context%</argument>
<!-- Run before serializer.normalizer.object -->
<tag name="serializer.normalizer" priority="-910" />
</service>

<service id="serializer.normalizer.json_serializable" class="Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer">
<argument>null</argument>
<argument>null</argument>
<argument>%serializer.default_context%</argument>
<!-- Run before serializer.normalizer.object -->
<tag name="serializer.normalizer" priority="-900" />
</service>
Expand All @@ -77,7 +81,7 @@
<argument type="service" id="property_info" on-invalid="ignore" />
<argument type="service" id="serializer.mapping.class_discriminator_resolver" on-invalid="ignore" />
<argument>null</argument>
<argument type="collection" />
<argument>%serializer.default_context%</argument>

<!-- Run after all custom normalizers -->
<tag name="serializer.normalizer" priority="-1000" />
Expand Down Expand Up @@ -122,6 +126,7 @@

<!-- Encoders -->
<service id="serializer.encoder.xml" class="Symfony\Component\Serializer\Encoder\XmlEncoder">
<argument>%serializer.default_context%</argument>
<tag name="serializer.encoder" />
</service>

Expand All @@ -130,10 +135,14 @@
</service>

<service id="serializer.encoder.yaml" class="Symfony\Component\Serializer\Encoder\YamlEncoder">
<argument>null</argument>
<argument>null</argument>
<argument>%serializer.default_context%</argument>
<tag name="serializer.encoder" />
</service>

<service id="serializer.encoder.csv" class="Symfony\Component\Serializer\Encoder\CsvEncoder">
<argument>%serializer.default_context%</argument>
<tag name="serializer.encoder" />
</service>

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ protected static function getBundleDefaultConfig()
'enabled' => true,
],
'serializer' => [
'default_context' => [],
'enabled' => !class_exists(FullStack::class),
'enable_annotations' => !class_exists(FullStack::class),
'mapping' => ['paths' => []],
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
'name_converter' => 'serializer.name_converter.camel_case_to_snake_case',
'circular_reference_handler' => 'my.circular.reference.handler',
'max_depth_handler' => 'my.max.depth.handler',
'default_context' => ['enable_max_depth' => true],
],
'property_info' => true,
'ide' => 'file%%link%%format',
Expand Down
6 changes: 5 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@
</framework:translator>
<framework:validation enabled="true" />
<framework:annotations cache="file" debug="true" file-cache-dir="%kernel.cache_dir%/annotations" />
<framework:serializer enabled="true" enable-annotations="true" name-converter="serializer.name_converter.camel_case_to_snake_case" circular-reference-handler="my.circular.reference.handler" max-depth-handler="my.max.depth.handler" />
<framework:serializer enabled="true" enable-annotations="true" name-converter="serializer.name_converter.camel_case_to_snake_case" circular-reference-handler="my.circular.reference.handler" max-depth-handler="my.max.depth.handler">
<framework:default-context>
<framework:enable_max_depth>true</framework:enable_max_depth>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this key allow any "array-like" structure? Shouldn't the XSD be updated accordingly?

</framework:default-context>
</framework:serializer>
<framework:property-info />
</framework:config>
</container>
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ framework:
name_converter: serializer.name_converter.camel_case_to_snake_case
circular_reference_handler: my.circular.reference.handler
max_depth_handler: my.max.depth.handler
default_context:
enable_max_depth: true
property_info: ~
ide: file%%link%%format
request:
Expand Down
5 changes: 2 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,9 +1047,8 @@ public function testSerializerEnabled()
$this->assertNull($container->getDefinition('serializer.mapping.class_metadata_factory')->getArgument(1));
$this->assertEquals(new Reference('serializer.name_converter.camel_case_to_snake_case'), $container->getDefinition('serializer.name_converter.metadata_aware')->getArgument(1));
$this->assertEquals(new Reference('property_info', ContainerBuilder::IGNORE_ON_INVALID_REFERENCE), $container->getDefinition('serializer.normalizer.object')->getArgument(3));
$this->assertArrayHasKey('circular_reference_handler', $container->getDefinition('serializer.normalizer.object')->getArgument(6));
$this->assertArrayHasKey('max_depth_handler', $container->getDefinition('serializer.normalizer.object')->getArgument(6));
$this->assertEquals($container->getDefinition('serializer.normalizer.object')->getArgument(6)['max_depth_handler'], new Reference('my.max.depth.handler'));
$this->assertEquals(['setCircularReferenceHandler', [new Reference('my.circular.reference.handler')]], $container->getDefinition('serializer.normalizer.object')->getMethodCalls()[0]);
$this->assertEquals(['setMaxDepthHandler', [new Reference('my.max.depth.handler')]], $container->getDefinition('serializer.normalizer.object')->getMethodCalls()[1]);
}

public function testRegisterSerializerExtractor()
Expand Down
28 changes: 28 additions & 0 deletions src/Symfony/Component/Serializer/DependencyInjection/SerializerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,39 @@ public function process(ContainerBuilder $container)

$serializerDefinition = $container->getDefinition($this->serializerService);
$serializerDefinition->replaceArgument(0, $normalizers);
$this->addDefaultContextParameter($normalizers, $container);

if (!$encoders = $this->findAndSortTaggedServices($this->encoderTag, $container)) {
throw new RuntimeException(sprintf('You must tag at least one service as "%s" to use the "%s" service.', $this->encoderTag, $this->serializerService));
}

$serializerDefinition->replaceArgument(1, $encoders);
$this->addDefaultContextParameter($encoders, $container);
}

private function addDefaultContextParameter($services, $container)
{
foreach ($services as $service) {
$definition = $container->getDefinition($service);
if (!$definition->isAutowired()) {
continue;
}

if (null === $class = $definition->getClass()) {
continue;
}

$reflection = new \ReflectionClass($class);

if (null === $constructor = $reflection->getConstructor()) {
continue;
}

foreach ($constructor->getParameters() as $arg) {
if ('defaultContext' === $arg->name) {
$definition->setArgument('$defaultContext', '%serializer.default_context%');
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't love this level of magic. I'd prefer:

A) Just documenting the parameter and allowing people to wire it up manually
B) Creating a DefaultContext service that holds the config and passing that around. For the existing classes that are expecting an array, that's fine - we can pass the array to those classes, but make this service available.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer B) because it will work with projects providing extra context options such as API Platform, and doing it manually is tedious.

What we could do:

  • Introduce a new ContextInterface (that can be extended)
  • Allow all parameters that currently accept an array to also accept this new interface, and deprecacate passing an array
  • Use it both for the "default context" and the context you can pass to methods (it means that this class MUST be safely serializable)

Copy link
Member

Choose a reason for hiding this comment

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

Other better option: create a context builder that returns an array. Then it will be consistent with the builder introduced in HttpClient.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussion about this in #30888

}
}
14 changes: 14 additions & 0 deletions src/Symfony/Component/Serializer/Tests/DependencyInjection/SerializerPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Serializer\DependencyInjection\SerializerPass;
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;

/**
* Tests for the SerializerPass class.
Expand Down Expand Up @@ -68,4 +69,17 @@ public function testServicesAreOrderedAccordingToPriority()
$this->assertEquals($expected, $definition->getArgument(0));
$this->assertEquals($expected, $definition->getArgument(1));
}

public function testServiceHasDefaultContextParameterArgument()
{
$container = new ContainerBuilder();

$definition = $container->register('serializer')->setClass(ObjectNormalizer::class)->setArguments([null, null, null, null, null, null, null])->addTag('serializer.normalizer')->addTag('serializer.encoder');
$definition->setAutowired(true);

$serializerPass = new SerializerPass();
$serializerPass->process($container);

$this->assertEquals('%serializer.default_context%', $definition->getArgument('$defaultContext'));
}
}