-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507
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
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 |
---|---|---|
@@ -0,0 +1,110 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bundle\FrameworkBundle\CacheWarmer; | ||
|
||
use Psr\Cache\CacheItemPoolInterface; | ||
use Symfony\Component\Cache\Adapter\AdapterInterface; | ||
use Symfony\Component\Cache\Adapter\ArrayAdapter; | ||
use Symfony\Component\Cache\Adapter\PhpArrayAdapter; | ||
use Symfony\Component\Cache\Adapter\ProxyAdapter; | ||
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; | ||
use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory; | ||
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory; | ||
use Symfony\Component\Serializer\Mapping\Loader\LoaderChain; | ||
use Symfony\Component\Serializer\Mapping\Loader\LoaderInterface; | ||
use Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader; | ||
use Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader; | ||
|
||
/** | ||
* Warms up XML and YAML serializer metadata. | ||
* | ||
* @author Titouan Galopin <galopintitouan@gmail.com> | ||
*/ | ||
class SerializerCacheWarmer implements CacheWarmerInterface | ||
{ | ||
private $loaders; | ||
private $phpArrayFile; | ||
private $fallbackPool; | ||
|
||
/** | ||
* @param LoaderInterface[] $loaders The serializer metadata loaders. | ||
* @param string $phpArrayFile The PHP file where metadata are cached. | ||
* @param CacheItemPoolInterface $fallbackPool The pool where runtime-discovered metadata are cached. | ||
*/ | ||
public function __construct(array $loaders, $phpArrayFile, CacheItemPoolInterface $fallbackPool) | ||
{ | ||
$this->loaders = $loaders; | ||
$this->phpArrayFile = $phpArrayFile; | ||
if (!$fallbackPool instanceof AdapterInterface) { | ||
$fallbackPool = new ProxyAdapter($fallbackPool); | ||
} | ||
$this->fallbackPool = $fallbackPool; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function warmUp($cacheDir) | ||
{ | ||
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { | ||
return; | ||
} | ||
|
||
$adapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); | ||
$arrayPool = new ArrayAdapter(0, false); | ||
|
||
$metadataFactory = new CacheClassMetadataFactory(new ClassMetadataFactory(new LoaderChain($this->loaders)), $arrayPool); | ||
|
||
foreach ($this->extractSupportedLoaders($this->loaders) as $loader) { | ||
foreach ($loader->getMappedClasses() as $mappedClass) { | ||
$metadataFactory->getMetadataFor($mappedClass); | ||
} | ||
} | ||
|
||
$values = $arrayPool->getValues(); | ||
$adapter->warmUp($values); | ||
|
||
foreach ($values as $k => $v) { | ||
$item = $this->fallbackPool->getItem($k); | ||
$this->fallbackPool->saveDeferred($item->set($v)); | ||
} | ||
$this->fallbackPool->commit(); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function isOptional() | ||
{ | ||
return true; | ||
} | ||
|
||
/** | ||
* @param LoaderInterface[] $loaders | ||
* | ||
* @return XmlFileLoader[]|YamlFileLoader[] | ||
*/ | ||
private function extractSupportedLoaders(array $loaders) | ||
{ | ||
$supportedLoaders = array(); | ||
|
||
foreach ($loaders as $loader) { | ||
if ($loader instanceof XmlFileLoader || $loader instanceof YamlFileLoader) { | ||
$supportedLoaders[] = $loader; | ||
} elseif ($loader instanceof LoaderChain) { | ||
$supportedLoaders = array_merge($supportedLoaders, $this->extractSupportedLoaders($loader->getDelegatedLoaders())); | ||
} | ||
} | ||
|
||
return $supportedLoaders; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bundle\FrameworkBundle\Tests\CacheWarmer; | ||
|
||
use Symfony\Bundle\FrameworkBundle\CacheWarmer\SerializerCacheWarmer; | ||
use Symfony\Bundle\FrameworkBundle\Tests\TestCase; | ||
use Symfony\Component\Cache\Adapter\ArrayAdapter; | ||
use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory; | ||
use Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader; | ||
use Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader; | ||
|
||
class SerializerCacheWarmerTest extends TestCase | ||
{ | ||
public function testWarmUp() | ||
{ | ||
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { | ||
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.'); | ||
} | ||
|
||
$loaders = array( | ||
new XmlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/person.xml'), | ||
new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/author.yml'), | ||
); | ||
|
||
$file = sys_get_temp_dir().'/cache-serializer.php'; | ||
@unlink($file); | ||
|
||
$fallbackPool = new ArrayAdapter(); | ||
|
||
$warmer = new SerializerCacheWarmer($loaders, $file, $fallbackPool); | ||
$warmer->warmUp(dirname($file)); | ||
|
||
$this->assertFileExists($file); | ||
|
||
$values = require $file; | ||
|
||
$this->assertInternalType('array', $values); | ||
$this->assertCount(2, $values); | ||
$this->assertArrayHasKey('Symfony_Bundle_FrameworkBundle_Tests_Fixtures_Serialization_Person', $values); | ||
$this->assertArrayHasKey('Symfony_Bundle_FrameworkBundle_Tests_Fixtures_Serialization_Author', $values); | ||
|
||
$values = $fallbackPool->getValues(); | ||
|
||
$this->assertInternalType('array', $values); | ||
$this->assertCount(2, $values); | ||
$this->assertArrayHasKey('Symfony_Bundle_FrameworkBundle_Tests_Fixtures_Serialization_Person', $values); | ||
$this->assertArrayHasKey('Symfony_Bundle_FrameworkBundle_Tests_Fixtures_Serialization_Author', $values); | ||
} | ||
|
||
public function testWarmUpWithoutLoader() | ||
{ | ||
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { | ||
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.'); | ||
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. same |
||
} | ||
|
||
$file = sys_get_temp_dir().'/cache-serializer-without-loader.php'; | ||
@unlink($file); | ||
|
||
$fallbackPool = new ArrayAdapter(); | ||
|
||
$warmer = new SerializerCacheWarmer(array(), $file, $fallbackPool); | ||
$warmer->warmUp(dirname($file)); | ||
|
||
$this->assertFileExists($file); | ||
|
||
$values = require $file; | ||
|
||
$this->assertInternalType('array', $values); | ||
$this->assertCount(0, $values); | ||
|
||
$values = $fallbackPool->getValues(); | ||
|
||
$this->assertInternalType('array', $values); | ||
$this->assertCount(0, $values); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,10 +21,14 @@ | |
use Symfony\Component\Cache\Adapter\ProxyAdapter; | ||
use Symfony\Component\Cache\Adapter\RedisAdapter; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Definition; | ||
use Symfony\Component\DependencyInjection\DefinitionDecorator; | ||
use Symfony\Component\DependencyInjection\Loader\ClosureLoader; | ||
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory; | ||
use Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader; | ||
use Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader; | ||
use Symfony\Component\Serializer\Normalizer\DataUriNormalizer; | ||
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer; | ||
use Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer; | ||
|
@@ -542,8 +546,16 @@ public function testObjectNormalizerRegistered() | |
|
||
public function testSerializerCacheActivated() | ||
{ | ||
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { | ||
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.'); | ||
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. same 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. Updated |
||
} | ||
|
||
$container = $this->createContainerFromFile('serializer_enabled'); | ||
|
||
$this->assertTrue($container->hasDefinition('serializer.mapping.cache_class_metadata_factory')); | ||
|
||
$cache = $container->getDefinition('serializer.mapping.cache_class_metadata_factory')->getArgument(1); | ||
$this->assertEquals(new Reference('serializer.mapping.cache.symfony'), $cache); | ||
} | ||
|
||
public function testSerializerCacheDisabled() | ||
|
@@ -562,7 +574,10 @@ public function testDeprecatedSerializerCacheOption() | |
$container = $this->createContainerFromFile('serializer_legacy_cache', array('kernel.debug' => true, 'kernel.container_class' => __CLASS__)); | ||
|
||
$this->assertFalse($container->hasDefinition('serializer.mapping.cache_class_metadata_factory')); | ||
$this->assertEquals(new Reference('foo'), $container->getDefinition('serializer.mapping.class_metadata_factory')->getArgument(1)); | ||
$this->assertTrue($container->hasDefinition('serializer.mapping.class_metadata_factory')); | ||
|
||
$cache = $container->getDefinition('serializer.mapping.class_metadata_factory')->getArgument(1); | ||
$this->assertEquals(new Reference('foo'), $cache); | ||
}); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serialization; | ||
|
||
class Author | ||
{ | ||
public $gender; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serialization; | ||
|
||
class Person | ||
{ | ||
public $gender; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serialization\Author: | ||
attributes: | ||
gender: | ||
groups: ['group1', 'group2'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?xml version="1.0" ?> | ||
<serializer xmlns="http://symfony.com/schema/dic/serializer-mapping" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://symfony.com/schema/dic/serializer-mapping | ||
http://symfony.com/schema/dic/serializer-mapping/serializer-mapping-1.0.xsd" | ||
> | ||
<class name="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serialization\Person"> | ||
<attribute name="gender"> | ||
<group>group1</group> | ||
<group>group2</group> | ||
</attribute> | ||
</class> | ||
</serializer> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,12 +36,11 @@ class XmlFileLoader extends FileLoader | |
public function loadClassMetadata(ClassMetadataInterface $classMetadata) | ||
{ | ||
if (null === $this->classes) { | ||
$this->classes = array(); | ||
$xml = $this->parseFile($this->file); | ||
$this->classes = $this->getClassesFromXml(); | ||
} | ||
|
||
foreach ($xml->class as $class) { | ||
$this->classes[(string) $class['name']] = $class; | ||
} | ||
if (!$this->classes) { | ||
return false; | ||
} | ||
|
||
$attributesMetadata = $classMetadata->getAttributesMetadata(); | ||
|
@@ -74,6 +73,20 @@ public function loadClassMetadata(ClassMetadataInterface $classMetadata) | |
return false; | ||
} | ||
|
||
/** | ||
* Return the names of the classes mapped in this file. | ||
* | ||
* @return string[] The classes names | ||
*/ | ||
public function getMappedClasses() | ||
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. What about introducing an interface ? 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. I thought about it and I wasn't sure. I thought it may be a bit overkill just for a single method. 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. I don't think so, it may be useful in userland code. But if you prefer we can wait that someone of the core team gives his opinion :) 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. I think the best option is to do it like that for the moment and add it if someone to express the need. Creating an interface is a quite important choice as we will have to maintain it for BC. 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. Agree, we can always add the interface later. |
||
{ | ||
if (null === $this->classes) { | ||
$this->classes = $this->getClassesFromXml(); | ||
} | ||
|
||
return array_keys($this->classes); | ||
} | ||
|
||
/** | ||
* Parses a XML File. | ||
* | ||
|
@@ -93,4 +106,16 @@ private function parseFile($file) | |
|
||
return simplexml_import_dom($dom); | ||
} | ||
|
||
private function getClassesFromXml() | ||
{ | ||
$xml = $this->parseFile($this->file); | ||
$classes = array(); | ||
|
||
foreach ($xml->class as $class) { | ||
$classes[(string) $class['name']] = $class; | ||
} | ||
|
||
return $classes; | ||
} | ||
} |
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 guess this is only there in case we are not on a php7 env ?
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.
Exactly :) .
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 think you should add a note specifiing that this should be removed once symfony will only support
php >= 7.0
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.
This is not planned and not done anywhere else, and I think when this will happen we will have a lot of work to adapt the whole codebase anyway. I don't think it has much added value, don't you think?
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.
It should make the transition easier but your argument is true as well so let's not do anything it is just a fallback anyway :)