[FrameworkBundle][Serializer] Fix a deprecation triggered by the ClassMetadataFactory #18630
Conversation
|
The tests failure seems unrelated. |
| @@ -513,7 +513,7 @@ private function addSerializerSection(ArrayNodeDefinition $rootNode) | |||
| ->canBeEnabled() | |||
| ->children() | |||
| ->booleanNode('enable_annotations')->defaultFalse()->end() | |||
| ->scalarNode('cache')->defaultValue('serializer.mapping.cache.symfony')->end() | |||
| ->scalarNode('cache')->defaultValue('cache.pool.serializer')->end() | |||
There was a problem hiding this comment.
this is a BC break to me, isn't it?
There was a problem hiding this comment.
If this is merged in 3.1 where it was introduced, then I think it isn't.
|
@Ener-Getick please, don't remove any of the rows of the PR table. In your table we miss the |
|
@javiereguiluz I thought it was enough to target the right branch, added ☺ |
| { | ||
| public function process(ContainerBuilder $container) | ||
| { | ||
| if (!$container->hasAlias('serializer.mapping.cache') && !$container->hasDefinition('serializer.mapping.cache')) { |
There was a problem hiding this comment.
You can just use $container->has('serializer.mapping.cache').
|
Can't we always use the |
|
Understood, this is for 3.1 :)
|
|
If we don't want to deprecate the cache configuration key, then I'd suggest doing what @xabbuh suggests, and do it at runtime using a static method as factory added to |
|
I agree with @nicolas-grekas. Now that we have the |
|
@nicolas-grekas @xabbuh then the default value introduced in #18561 should be removed right ? |
|
Yep, can you do that in this PR, as a first commit? |
|
@nicolas-grekas yes, I'm gonna split my commit |
| @@ -982,7 +982,9 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder | |||
|
|
|||
| $chainLoader->replaceArgument(0, $serializerLoaders); | |||
|
|
|||
| if (!$container->getParameter('kernel.debug')) { | |||
| if (isset($config['cache']) && $config['cache']) { | |||
| @trigger_error('Using the option "framework.serializer.cache" is deprecated since version 3.1. This option will be removed in 4.0. You can configure a cache pool called "serializer" under "framework.cache" instead.', E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
What about:
The "framework.serializer.cache" option is deprecated since Symfony 3.1 and will be removed in 4.0. You can configure a cache pool called "serializer" under "framework.cache.pools" instead.
205f808 to
0d7ecc7
Compare
…ion for serializer" This reverts commit 4f0b8be.
60d94c6 to
f44cb14
Compare
|
Rebased and updated. I also added notes in the upgrading files to inform about this new deprecation. |
| $this->assertFalse($container->hasDefinition('serializer.mapping.cache_class_metadata_factory')); | ||
| } | ||
|
|
||
| public function testDeprecatedSerializerCacheOption() |
There was a problem hiding this comment.
Should be tagged @group legacy so that we don't forget removing it when preparing 4.0
| @@ -0,0 +1,8 @@ | |||
| <?php | |||
|
|
|||
| $container->loadFromExtension('framework', array( | |||
There was a problem hiding this comment.
I suggest adding the "legacy" word in these fixtures so we can easily find them when preparing 4.0
There was a problem hiding this comment.
👍, renamed to serializer_legacy_cache
|
👍 with only one wonder: should the UPGRADE files tell about the serializer pool, and/or tell about |
|
I didn't think about |
|
|
||
| ```yaml | ||
| framework: | ||
| serializer: ~ |
There was a problem hiding this comment.
this level should be removed, cache is a direct child of framework
There was a problem hiding this comment.
no it should not be removed. If you remove the serializer key entirely, it disables the serializer in FrameworkBundle
There was a problem hiding this comment.
|
Thank you @Ener-Getick. |
…by the ClassMetadataFactory (Ener-Getick) This PR was merged into the 3.1-dev branch. Discussion ---------- [FrameworkBundle][Serializer] Fix a deprecation triggered by the ClassMetadataFactory | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT Without apparent reasons, [FOSRestBundle's tests fail](https://travis-ci.org/FriendsOfSymfony/FOSRestBundle/jobs/124384888) since #18561. ``` Passing a Doctrine Cache instance as 2nd parameter of the "Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory" constructor is deprecated. This parameter will be removed in Symfony 4.0. Use the "Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory" class instead: 6x 1x in ErrorWithTemplatingFormatTest::testSerializeExceptionHtml from FOS\RestBundle\Tests\Functional 1x in SerializerErrorTest::testSerializeExceptionJson from FOS\RestBundle\Tests\Functional 1x in SerializerErrorTest::testSerializeExceptionJsonWithoutDebug from FOS\RestBundle\Tests\Functional 1x in SerializerErrorTest::testSerializeExceptionXml from FOS\RestBundle\Tests\Functional 1x in SerializerErrorTest::testSerializeInvalidFormJson from FOS\RestBundle\Tests\Functional 1x in SerializerErrorTest::testSerializeInvalidFormXml from FOS\RestBundle\Tests\Functional ``` We don't use cache in our tests but some of them are not in ``debug`` mode (will change soon) so the cache is automatically used. This PR fixes this deprecation by detecting if the cache used by the serializer is psr6 compliant or not (if it is, then it replaces the default metadata factory by an instance of the new class ``CacheClassMetadataFactory``, otherwise the second parameter of the ``ClassMetadataFactory`` is used). Commits ------- 15579d5 [FrameworkBundle] Deprecate framework.serializer.cache eccbffb [Serializer] Improve a deprecation message 96e418a Revert "[FrameworkBundle] Fallback to default cache system in production for serializer"
… regressions (Ener-Getick) This PR was merged into the 3.1-dev branch. Discussion ---------- [FrameworkBundle] Fix a typo and add a test to prevent new regressions | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This fixes a typo introduced in #18630 and adds a new functional test to avoid most regressions in service definitions in the future when features are only available in non-debug mode. cc @nicolas-grekas Commits ------- 61872ce Fix a typo and add a check to prevent regressions
…r-Getick) This PR was merged into the 3.1-dev branch. Discussion ---------- Do not use validator.cache and serializer.cache anymore ``framework.serializer.cache`` has been deprecated in symfony/symfony#18630 in favor of the new caching system. see symfony/symfony#18630 Commits ------- e7a2085 Use the new cache system configuration
Without apparent reasons, FOSRestBundle's tests fail since #18561.
We don't use cache in our tests but some of them are not in
debugmode (will change soon) so the cache is automatically used.This PR fixes this deprecation by detecting if the cache used by the serializer is psr6 compliant or not (if it is, then it replaces the default metadata factory by an instance of the new class
CacheClassMetadataFactory, otherwise the second parameter of theClassMetadataFactoryis used).