Skip to content

[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

Merged
merged 1 commit into from
Sep 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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) {
Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly :) .

Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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 :)

$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
Expand Up @@ -1067,6 +1067,7 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
}

$chainLoader->replaceArgument(0, $serializerLoaders);
$container->getDefinition('serializer.mapping.cache_warmer')->replaceArgument(0, $serializerLoaders);

if (isset($config['cache']) && $config['cache']) {
@trigger_error('The "framework.serializer.cache" option is deprecated since Symfony 3.1 and will be removed in 4.0. Configure the "cache.serializer" service under "framework.cache.pools" instead.', E_USER_DEPRECATED);
Expand All @@ -1079,12 +1080,12 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
$container->getDefinition('serializer.mapping.class_metadata_factory')->replaceArgument(
1, new Reference($config['cache'])
);
} elseif (!$container->getParameter('kernel.debug')) {
} elseif (!$container->getParameter('kernel.debug') && class_exists(CacheClassMetadataFactory::class)) {
$cacheMetadataFactory = new Definition(
CacheClassMetadataFactory::class,
array(
new Reference('serializer.mapping.cache_class_metadata_factory.inner'),
new Reference('cache.serializer'),
new Reference('serializer.mapping.cache.symfony'),
)
);
$cacheMetadataFactory->setPublic(false);
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<parameters>
<parameter key="serializer.mapping.cache.file">%kernel.cache_dir%/serialization.php</parameter>
<parameter key="serializer.mapping.cache.prefix" />
</parameters>

Expand Down Expand Up @@ -39,6 +40,19 @@
</service>

<!-- Cache -->
<service id="serializer.mapping.cache_warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\SerializerCacheWarmer" public="false">
<argument type="collection" /><!-- Loaders injected by the extension -->
<argument>%serializer.mapping.cache.file%</argument>
<argument type="service" id="cache.serializer" />
<tag name="kernel.cache_warmer" />
</service>

<service id="serializer.mapping.cache.symfony" class="Symfony\Component\Cache\Adapter\PhpArrayAdapter">
<factory class="Symfony\Component\Cache\Adapter\PhpArrayAdapter" method="create" />
<argument>%serializer.mapping.cache.file%</argument>
<argument type="service" id="cache.serializer" />
</service>

<service id="serializer.mapping.cache.doctrine.apc" class="Doctrine\Common\Cache\ApcCache" public="false">
<call method="setNamespace">
<argument>%serializer.mapping.cache.prefix%</argument>
Expand Down
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.');
Copy link
Member

Choose a reason for hiding this comment

The 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
Expand Up @@ -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;
Expand Down Expand Up @@ -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.');
Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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()
Expand All @@ -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);
});
}

Expand Down
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>
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"symfony/form": "~2.8|~3.0",
"symfony/expression-language": "~2.8|~3.0",
"symfony/process": "~2.8|~3.0",
"symfony/serializer": "~2.8|^3.0",
"symfony/serializer": "~2.8|~3.0",
"symfony/validator": "~3.1",
"symfony/yaml": "~3.2",
"symfony/property-info": "~2.8|~3.0",
Expand Down
35 changes: 30 additions & 5 deletions src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

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

What about introducing an interface ?

Copy link
Contributor Author

@tgalopin tgalopin Aug 2, 2016

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 :)

Copy link
Contributor Author

@tgalopin tgalopin Aug 2, 2016

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.
*
Expand All @@ -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;
}
}
Loading