[FrameworkBundle] Serializer groups support #13107

dunglas · 2014-12-25T03:25:04Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
License	MIT
Doc PR	not yet

This PR enables serializer groups in the full stack framework (including XML, YAML, annotations and caching).

dunglas · 2014-12-25T10:28:08Z

This PR is now ready to be reviewed.

Tobion · 2014-12-26T01:20:07Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -19,6 +19,7 @@
 * FrameworkExtension configuration structure.
 *
 * @author Jeremy Mikola <jmikola@gmail.com>
+ * @author Kévin Dunglas <dunglas@gmail.com>


IMO 4 lines do not qualify for that.

Ok I'll remove it.

dunglas · 2015-03-03T15:08:52Z

Made ObjectNormalizer the default normalizer. #13257 must be merged before this one.

dunglas · 2015-03-03T17:59:31Z

ping @symfony/deciders

fabpot · 2015-03-09T06:48:33Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+            $reflection = new \ReflectionClass($bundle);
+            $dirname = dirname($reflection->getFilename());
+
+            if (is_file($file = $dirname.'/Resources/config/serialization.xml')) {


Do we have other examples in Symfony where such a "convention" is harcoded? I know it's not the case for the routing, but probably done the same way for the translator.

JMSSerializerBundle which is probably the most used use the serializer directory name for configs (see conf) , but it can be changed for any bundle manually. I think we should look for resource existence before searching for a default one.

I'm not saying that we should make it configurable. As a matter of fact, I think there is no need to make it configurable, I just wanted to be sure that we behave like other parts of the framework.

@fabpot:

Same for the validator (this code is based on the validator code): https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L786-L794

For translation it's the same approach but directory based instead of file based: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L670-L679

The point is to have a coherent behavior for both components (Validation, Translation, Routing...). If we allow to create a file by class (why not, I personally use annotations), it must be the same at least for validation mappings.

ping @webmozart @fabpot

For this PR, I'd prefer to keep what we have in other parts of the framework.

The Validator component just got the ability to read the configuration from a directory (see #13855). Why not adapt this approach here too?

Thanks for the pointer @xabbuh. I had not seen this PR.
Support for Resources/config/serialization/*[.xml|.yml] added.

dunglas · 2015-03-10T10:47:11Z

Rebased.

jakzal · 2015-03-10T11:15:10Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+        if (isset($config['cache']) && $config['cache'] !== 'none') {
+            $container->setParameter(
+                'serializer.mapping.cache.prefix',
+                'serializer_'.hash('sha256', $container->getParameter('kernel.root_dir'))


Could this cause issues if cache is warmed up in one place, and later moved to another directory? I know we not necessarily support such case, but I'd rather avoid adding one more obstacle here.

Probably but this is already the case for the validator: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L749

If we change this behavior, we must change it everywhere in another PR.

…rialization/ sub-dir

weaverryan · 2015-03-12T11:02:05Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml

+            <call method="setNamespace">
+                <argument>%serializer.mapping.cache.prefix%</argument>
+            </call>
+        </service>


For the record, though it still looks a little odd to me to have this service here that may not be used, I do like it. As I mentioned before, it's private, so will be removed if it's not being used. And I think it ends up being quite usable. 👍

dunglas · 2015-03-16T17:54:47Z

Is this PR ok to be merged?

fabpot · 2015-03-18T18:12:41Z

Thank you @dunglas.

…glas) This PR was merged into the 2.7 branch. Discussion ---------- [FrameworkBundle] Fix regression introduced by #13107 | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a I've introduced a bug in 83b56f6. An error will be throw if the serializer is enabled but no cache is set. This is the fix. Commits ------- efadac0 [FrameworkBundle] Fix regression introduced by #13107

This PR was squashed before being merged into the 2.7 branch (closes #764). Discussion ---------- Serializer groups support | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT Enable serializer groups (related to symfony/symfony#13107). Commits ------- 3561a7c Serializer groups support

dunglas mentioned this pull request Dec 25, 2014

Serializer groups support symfony/symfony-standard#764

Closed

dunglas force-pushed the framework_serializer_groups branch 3 times, most recently from 9de8a0f to 47012cf Compare December 25, 2014 03:53

dunglas changed the title ~~[FrameworkBundle] Serializer groups support~~ [WIP] [FrameworkBundle] Serializer groups support Dec 25, 2014

dunglas force-pushed the framework_serializer_groups branch 3 times, most recently from 475e6ce to 4ecb81e Compare December 25, 2014 10:23

dunglas changed the title ~~[WIP] [FrameworkBundle] Serializer groups support~~ [FrameworkBundle] Serializer groups support Dec 25, 2014

dunglas mentioned this pull request Dec 25, 2014

[FrameworkBundle] Optimize validator registration #13111

Merged

Tobion reviewed Dec 26, 2014
View reviewed changes

dunglas force-pushed the framework_serializer_groups branch from 4ecb81e to 72bfc01 Compare December 26, 2014 08:05

This was referenced Dec 28, 2014

[Serializer] Name Converter symfony/symfony-docs#4692

Merged

Add a book entry for serialization symfony/symfony-docs#4698

Closed

fabpot added the FrameworkBundle label Jan 2, 2015

fabpot force-pushed the 2.7 branch from 77b6cca to dcfc338 Compare January 8, 2015 13:06

dunglas force-pushed the framework_serializer_groups branch from 72bfc01 to 106fe6f Compare March 3, 2015 15:08

fabpot reviewed Mar 9, 2015
View reviewed changes

dunglas added 2 commits March 10, 2015 11:44

[FrameworkBundle] Serializer groups support

35d0ce8

[FrameworkBundle] Make ObjectNormalizer default

290d50b

dunglas force-pushed the framework_serializer_groups branch from 106fe6f to 290d50b Compare March 10, 2015 10:46

jakzal reviewed Mar 10, 2015
View reviewed changes

dunglas added 2 commits March 10, 2015 20:51

[FrameworkBundle] Read serialization mapping form Resources/config/se…

3ee6fba

…rialization/ sub-dir

[FrameworkBundle] Serializer: remove none value support

3d39c1f

[FrameworkBundle] Serializer: support service name for cache

01c8366

weaverryan reviewed Mar 12, 2015
View reviewed changes

fabpot closed this in 74ec895 Mar 18, 2015

dunglas added a commit to dunglas/symfony that referenced this pull request Mar 18, 2015

[Serializer] Fix regression introduced by symfony#13107

53ea971

dunglas added a commit to dunglas/symfony that referenced this pull request Mar 18, 2015

[FrameworkBundle] Fix regression introduced by symfony#13107

efadac0

dunglas deleted the framework_serializer_groups branch December 5, 2015 09:02

wouterj mentioned this pull request Apr 17, 2016

[FrameworkBundle][Serializer] Fix APC cache service name #18567

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Serializer groups support #13107

[FrameworkBundle] Serializer groups support #13107

dunglas commented Dec 25, 2014

dunglas commented Dec 25, 2014

Tobion Dec 26, 2014

dunglas Dec 26, 2014

dunglas commented Mar 3, 2015

dunglas commented Mar 3, 2015

fabpot Mar 9, 2015

Pierstoval Mar 9, 2015

fabpot Mar 9, 2015

dunglas Mar 9, 2015

fabpot Mar 9, 2015

dunglas Mar 10, 2015

dunglas Mar 10, 2015

fabpot Mar 10, 2015

xabbuh Mar 10, 2015

dunglas Mar 10, 2015

dunglas commented Mar 10, 2015

jakzal Mar 10, 2015

dunglas Mar 10, 2015

weaverryan Mar 12, 2015

dunglas commented Mar 16, 2015

fabpot commented Mar 18, 2015

[FrameworkBundle] Serializer groups support #13107

[FrameworkBundle] Serializer groups support #13107

Conversation

dunglas commented Dec 25, 2014

dunglas commented Dec 25, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Mar 3, 2015

dunglas commented Mar 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Mar 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Mar 16, 2015

fabpot commented Mar 18, 2015