-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Serializer groups support #13107
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
Changes from all commits
35d0ce8
290d50b
3ee6fba
3d39c1f
01c8366
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
|
||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||
use Symfony\Component\DependencyInjection\Definition; | ||
use Symfony\Component\DependencyInjection\DefinitionDecorator; | ||
use Symfony\Component\DependencyInjection\Exception\LogicException; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
|
@@ -29,6 +30,7 @@ | |
* | ||
* @author Fabien Potencier <fabien@symfony.com> | ||
* @author Jeremy Mikola <jmikola@gmail.com> | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
class FrameworkExtension extends Extension | ||
{ | ||
|
@@ -121,11 +123,10 @@ public function load(array $configs, ContainerBuilder $container) | |
} | ||
|
||
$this->registerAnnotationsConfiguration($config['annotations'], $container, $loader); | ||
|
||
$this->registerPropertyAccessConfiguration($config['property_access'], $container); | ||
|
||
if (isset($config['serializer']) && $config['serializer']['enabled']) { | ||
$loader->load('serializer.xml'); | ||
if (isset($config['serializer'])) { | ||
$this->registerSerializerConfiguration($config['serializer'], $container, $loader); | ||
} | ||
|
||
$loader->load('debug_prod.xml'); | ||
|
@@ -866,6 +867,86 @@ private function registerSecurityCsrfConfiguration(array $config, ContainerBuild | |
$loader->load('security_csrf.xml'); | ||
} | ||
|
||
/** | ||
* Loads the serializer configuration. | ||
* | ||
* @param array $config A serializer configuration array | ||
* @param ContainerBuilder $container A ContainerBuilder instance | ||
* @param XmlFileLoader $loader An XmlFileLoader instance | ||
*/ | ||
private function registerSerializerConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader) | ||
{ | ||
if (!$config['enabled']) { | ||
return; | ||
} | ||
|
||
$loader->load('serializer.xml'); | ||
$chainLoader = $container->getDefinition('serializer.mapping.chain_loader'); | ||
|
||
$serializerLoaders = array(); | ||
if (isset($config['enable_annotations']) && $config['enable_annotations']) { | ||
$annotationLoader = new Definition( | ||
'Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader', | ||
array(new Reference('annotation_reader')) | ||
); | ||
$annotationLoader->setPublic(false); | ||
|
||
$serializerLoaders[] = $annotationLoader; | ||
} | ||
|
||
$bundles = $container->getParameter('kernel.bundles'); | ||
foreach ($bundles as $bundle) { | ||
$reflection = new \ReflectionClass($bundle); | ||
$dirname = dirname($reflection->getFilename()); | ||
|
||
if (is_file($file = $dirname.'/Resources/config/serialization.xml')) { | ||
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader', array(realpath($file))); | ||
$definition->setPublic(false); | ||
|
||
$serializerLoaders[] = $definition; | ||
$container->addResource(new FileResource($file)); | ||
} | ||
|
||
if (is_file($file = $dirname.'/Resources/config/serialization.yml')) { | ||
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader', array(realpath($file))); | ||
$definition->setPublic(false); | ||
|
||
$serializerLoaders[] = $definition; | ||
$container->addResource(new FileResource($file)); | ||
} | ||
|
||
if (is_dir($dir = $dirname.'/Resources/config/serialization')) { | ||
foreach (Finder::create()->files()->in($dir)->name('*.xml') as $file) { | ||
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader', array($file->getRealpath())); | ||
$definition->setPublic(false); | ||
|
||
$serializerLoaders[] = $definition; | ||
} | ||
foreach (Finder::create()->files()->in($dir)->name('*.yml') as $file) { | ||
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader', array($file->getRealpath())); | ||
$definition->setPublic(false); | ||
|
||
$serializerLoaders[] = $definition; | ||
} | ||
|
||
$container->addResource(new DirectoryResource($dir)); | ||
} | ||
} | ||
|
||
$chainLoader->replaceArgument(0, $serializerLoaders); | ||
|
||
if (isset($config['cache']) && $config['cache']) { | ||
$container->setParameter( | ||
'serializer.mapping.cache.prefix', | ||
'serializer_'.hash('sha256', $container->getParameter('kernel.root_dir')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
); | ||
|
||
$container->getDefinition('serializer.mapping.class_metadata_factory')->replaceArgument( | ||
1, new Reference($config['cache']) | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Returns the base path for the XSD files. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,11 +17,31 @@ | |
</service> | ||
|
||
<!-- Normalizer --> | ||
<service id="serializer.normalizer.get_set_method" class="Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer" public="false"> | ||
<service id="serializer.normalizer.object" class="Symfony\Component\Serializer\Normalizer\ObjectNormalizer" public="false"> | ||
<argument type="service" id="serializer.mapping.class_metadata_factory" /> | ||
|
||
<!-- Run after all custom serializers --> | ||
<tag name="serializer.normalizer" priority="-1000" /> | ||
</service> | ||
|
||
<!-- Loader --> | ||
<service id="serializer.mapping.chain_loader" class="Symfony\Component\Serializer\Mapping\Loader\LoaderChain" public="false"> | ||
<argument type="collection" /> | ||
</service> | ||
|
||
<!-- Class Metadata Factory --> | ||
<service id="serializer.mapping.class_metadata_factory" class="Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory" public="false"> | ||
<argument type="service" id="serializer.mapping.chain_loader" /> | ||
<argument>null</argument> | ||
</service> | ||
|
||
<!-- Cache --> | ||
<service id="serializer.mapping.cache.apc" class="Doctrine\Common\Cache\ApcCache" public="false"> | ||
<call method="setNamespace"> | ||
<argument>%serializer.mapping.cache.prefix%</argument> | ||
</call> | ||
</service> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. 👍 |
||
|
||
<!-- Encoders --> | ||
<service id="serializer.encoder.xml" class="%serializer.encoder.xml.class%" public="false" > | ||
<tag name="serializer.encoder" /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JMSSerializerBundle
which is probably the most used use theserializer
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabpot:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @webmozart @fabpot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, I'd prefer to keep what we have in other parts of the framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Validator component just got the ability to read the configuration from a directory (see #13855). Why not adapt this approach here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer @xabbuh. I had not seen this PR.
Support for
Resources/config/serialization/*[.xml|.yml]
added.