From 9b84d72697f76a590be17ada0d1c103ac4d4908a Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 17 Jul 2017 22:35:32 +0200 Subject: [PATCH 1/7] [FrameworkBundle] fix ValidatorCacheWarmer: use serializing ArrayAdapter --- .../FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index 81291d772fbf0..0abd39df48fef 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -62,7 +62,7 @@ public function warmUp($cacheDir) } $adapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); - $arrayPool = new ArrayAdapter(0, false); + $arrayPool = new ArrayAdapter(0); $loaders = $this->validatorBuilder->getLoaders(); $metadataFactory = new LazyLoadingMetadataFactory(new LoaderChain($loaders), new Psr6Cache($arrayPool)); @@ -86,7 +86,10 @@ public function warmUp($cacheDir) spl_autoload_unregister(array($adapter, 'throwOnRequiredClass')); } - $values = $arrayPool->getValues(); + // the ArrayAdapter stores the values serialized as the MetaData objects + // are mutated inside LazyLoadingMetadataFactory after being written into the cache + // so here we un-serialize the values first + $values = array_map(function ($val) { return unserialize($val); }, $arrayPool->getValues()); $adapter->warmUp(array_filter($values)); foreach ($values as $k => $v) { From 8753b065df79b4c68ead78b5ba9d87d842e6837d Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 19:51:31 +0200 Subject: [PATCH 2/7] also apply for other cache warmers + fix array_filter --- .../AbstractPhpFileCacheWarmer.php | 77 +++++++++++++++++ .../CacheWarmer/AnnotationsCacheWarmer.php | 84 +++++++------------ .../CacheWarmer/SerializerCacheWarmer.php | 68 +++++---------- .../CacheWarmer/ValidatorCacheWarmer.php | 73 +++++----------- .../CacheWarmer/ValidatorCacheWarmerTest.php | 3 +- 5 files changed, 150 insertions(+), 155 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php new file mode 100644 index 0000000000000..049b16f9011e2 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -0,0 +1,77 @@ + + * + * 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; + +abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface +{ + protected $phpArrayFile; + protected $fallbackPool; + + /** + * @param string $phpArrayFile The PHP file where metadata are cached + * @param CacheItemPoolInterface $fallbackPool The pool where runtime-discovered metadata are cached + */ + public function __construct($phpArrayFile, CacheItemPoolInterface $fallbackPool) + { + $this->phpArrayFile = $phpArrayFile; + if (!$fallbackPool instanceof AdapterInterface) { + $fallbackPool = new ProxyAdapter($fallbackPool); + } + $this->fallbackPool = $fallbackPool; + } + + /** + * {@inheritdoc} + */ + public function warmUp($cacheDir) + { + $phpArrayAdapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); + $arrayAdapter = new ArrayAdapter(); + + spl_autoload_register(array($phpArrayAdapter, 'throwOnRequiredClass')); + try { + if (false === $this->doWarmUp($cacheDir, $phpArrayAdapter, $arrayAdapter)) { + return; + } + } finally { + spl_autoload_unregister(array($phpArrayAdapter, 'throwOnRequiredClass')); + } + + // the ArrayAdapter stores the values serialized + // to avoid mutation of the data after it was written to the cache + // so here we un-serialize the values first + $values = array_map(function ($val) { return unserialize($val); }, array_filter($arrayAdapter->getValues())); + $phpArrayAdapter->warmUp($values); + + foreach ($values as $k => $v) { + $item = $this->fallbackPool->getItem($k); + $this->fallbackPool->saveDeferred($item->set($v)); + } + $this->fallbackPool->commit(); + } + + /** + * @param string $cacheDir + * @param PhpArrayAdapter $phpArrayAdapter + * @param ArrayAdapter $arrayAdapter + * + * @return bool|void false if there is nothing to warm-up + */ + abstract protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter); +} diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php index a6fb4ed095d2b..3434e3c319c19 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -15,12 +15,9 @@ use Doctrine\Common\Annotations\CachedReader; use Doctrine\Common\Annotations\Reader; 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\Cache\DoctrineProvider; -use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; /** * Warms up annotation caches for classes found in composer's autoload class map @@ -28,11 +25,9 @@ * * @author Titouan Galopin */ -class AnnotationsCacheWarmer implements CacheWarmerInterface +class AnnotationsCacheWarmer extends AbstractPhpFileCacheWarmer { private $annotationReader; - private $phpArrayFile; - private $fallbackPool; /** * @param Reader $annotationReader @@ -41,71 +36,50 @@ class AnnotationsCacheWarmer implements CacheWarmerInterface */ public function __construct(Reader $annotationReader, $phpArrayFile, CacheItemPoolInterface $fallbackPool) { + parent::__construct($phpArrayFile, $fallbackPool); $this->annotationReader = $annotationReader; - $this->phpArrayFile = $phpArrayFile; - if (!$fallbackPool instanceof AdapterInterface) { - $fallbackPool = new ProxyAdapter($fallbackPool); - } - $this->fallbackPool = $fallbackPool; } /** * {@inheritdoc} */ - public function warmUp($cacheDir) + public function isOptional() + { + return true; + } + + /** + * {@inheritdoc} + */ + protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) { - $adapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); $annotatedClassPatterns = $cacheDir.'/annotations.map'; if (!is_file($annotatedClassPatterns)) { - $adapter->warmUp(array()); + $phpArrayAdapter->warmUp(array()); - return; + return false; } $annotatedClasses = include $annotatedClassPatterns; - - $arrayPool = new ArrayAdapter(0, false); - $reader = new CachedReader($this->annotationReader, new DoctrineProvider($arrayPool)); - - spl_autoload_register(array($adapter, 'throwOnRequiredClass')); - try { - foreach ($annotatedClasses as $class) { - try { - $this->readAllComponents($reader, $class); - } catch (\ReflectionException $e) { - // ignore failing reflection - } catch (AnnotationException $e) { - /* - * Ignore any AnnotationException to not break the cache warming process if an Annotation is badly - * configured or could not be found / read / etc. - * - * In particular cases, an Annotation in your code can be used and defined only for a specific - * environment but is always added to the annotations.map file by some Symfony default behaviors, - * and you always end up with a not found Annotation. - */ - } + $reader = new CachedReader($this->annotationReader, new DoctrineProvider($arrayAdapter)); + + foreach ($annotatedClasses as $class) { + try { + $this->readAllComponents($reader, $class); + } catch (\ReflectionException $e) { + // ignore failing reflection + } catch (AnnotationException $e) { + /* + * Ignore any AnnotationException to not break the cache warming process if an Annotation is badly + * configured or could not be found / read / etc. + * + * In particular cases, an Annotation in your code can be used and defined only for a specific + * environment but is always added to the annotations.map file by some Symfony default behaviors, + * and you always end up with a not found Annotation. + */ } - } finally { - spl_autoload_unregister(array($adapter, 'throwOnRequiredClass')); } - - $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; } private function readAllComponents(Reader $reader, $class) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php index c017f51268b3d..6a111d415b308 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -13,11 +13,8 @@ use Doctrine\Common\Annotations\AnnotationException; 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; @@ -30,11 +27,9 @@ * * @author Titouan Galopin */ -class SerializerCacheWarmer implements CacheWarmerInterface +class SerializerCacheWarmer extends AbstractPhpFileCacheWarmer { private $loaders; - private $phpArrayFile; - private $fallbackPool; /** * @param LoaderInterface[] $loaders The serializer metadata loaders @@ -43,61 +38,40 @@ class SerializerCacheWarmer implements CacheWarmerInterface */ public function __construct(array $loaders, $phpArrayFile, CacheItemPoolInterface $fallbackPool) { + parent::__construct($phpArrayFile, $fallbackPool); $this->loaders = $loaders; - $this->phpArrayFile = $phpArrayFile; - if (!$fallbackPool instanceof AdapterInterface) { - $fallbackPool = new ProxyAdapter($fallbackPool); - } - $this->fallbackPool = $fallbackPool; } /** * {@inheritdoc} */ - public function warmUp($cacheDir) + public function isOptional() + { + return true; + } + + /** + * {@inheritdoc} + */ + protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) { if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { - return; + return false; } - $adapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); - $arrayPool = new ArrayAdapter(0, false); + $metadataFactory = new CacheClassMetadataFactory(new ClassMetadataFactory(new LoaderChain($this->loaders)), $arrayAdapter); - $metadataFactory = new CacheClassMetadataFactory(new ClassMetadataFactory(new LoaderChain($this->loaders)), $arrayPool); - - spl_autoload_register(array($adapter, 'throwOnRequiredClass')); - try { - foreach ($this->extractSupportedLoaders($this->loaders) as $loader) { - foreach ($loader->getMappedClasses() as $mappedClass) { - try { - $metadataFactory->getMetadataFor($mappedClass); - } catch (\ReflectionException $e) { - // ignore failing reflection - } catch (AnnotationException $e) { - // ignore failing annotations - } + foreach ($this->extractSupportedLoaders($this->loaders) as $loader) { + foreach ($loader->getMappedClasses() as $mappedClass) { + try { + $metadataFactory->getMetadataFor($mappedClass); + } catch (\ReflectionException $e) { + // ignore failing reflection + } catch (AnnotationException $e) { + // ignore failing annotations } } - } finally { - spl_autoload_unregister(array($adapter, 'throwOnRequiredClass')); } - - $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; } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index 0abd39df48fef..5c0532cc4bb18 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -13,11 +13,8 @@ use Doctrine\Common\Annotations\AnnotationException; 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\Validator\Mapping\Cache\Psr6Cache; use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory; use Symfony\Component\Validator\Mapping\Loader\LoaderChain; @@ -31,11 +28,9 @@ * * @author Titouan Galopin */ -class ValidatorCacheWarmer implements CacheWarmerInterface +class ValidatorCacheWarmer extends AbstractPhpFileCacheWarmer { private $validatorBuilder; - private $phpArrayFile; - private $fallbackPool; /** * @param ValidatorBuilderInterface $validatorBuilder @@ -44,67 +39,43 @@ class ValidatorCacheWarmer implements CacheWarmerInterface */ public function __construct(ValidatorBuilderInterface $validatorBuilder, $phpArrayFile, CacheItemPoolInterface $fallbackPool) { + parent::__construct($phpArrayFile, $fallbackPool); $this->validatorBuilder = $validatorBuilder; - $this->phpArrayFile = $phpArrayFile; - if (!$fallbackPool instanceof AdapterInterface) { - $fallbackPool = new ProxyAdapter($fallbackPool); - } - $this->fallbackPool = $fallbackPool; } /** * {@inheritdoc} */ - public function warmUp($cacheDir) + public function isOptional() + { + return true; + } + + /** + * {@inheritdoc} + */ + protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) { if (!method_exists($this->validatorBuilder, 'getLoaders')) { - return; + return false; } - $adapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); - $arrayPool = new ArrayAdapter(0); - $loaders = $this->validatorBuilder->getLoaders(); - $metadataFactory = new LazyLoadingMetadataFactory(new LoaderChain($loaders), new Psr6Cache($arrayPool)); + $metadataFactory = new LazyLoadingMetadataFactory(new LoaderChain($loaders), new Psr6Cache($arrayAdapter)); - spl_autoload_register(array($adapter, 'throwOnRequiredClass')); - try { - foreach ($this->extractSupportedLoaders($loaders) as $loader) { - foreach ($loader->getMappedClasses() as $mappedClass) { - try { - if ($metadataFactory->hasMetadataFor($mappedClass)) { - $metadataFactory->getMetadataFor($mappedClass); - } - } catch (\ReflectionException $e) { - // ignore failing reflection - } catch (AnnotationException $e) { - // ignore failing annotations + foreach ($this->extractSupportedLoaders($loaders) as $loader) { + foreach ($loader->getMappedClasses() as $mappedClass) { + try { + if ($metadataFactory->hasMetadataFor($mappedClass)) { + $metadataFactory->getMetadataFor($mappedClass); } + } catch (\ReflectionException $e) { + // ignore failing reflection + } catch (AnnotationException $e) { + // ignore failing annotations } } - } finally { - spl_autoload_unregister(array($adapter, 'throwOnRequiredClass')); } - - // the ArrayAdapter stores the values serialized as the MetaData objects - // are mutated inside LazyLoadingMetadataFactory after being written into the cache - // so here we un-serialize the values first - $values = array_map(function ($val) { return unserialize($val); }, $arrayPool->getValues()); - $adapter->warmUp(array_filter($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; } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php index 23b4732afcb3a..9613007468773 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php @@ -79,9 +79,8 @@ public function testWarmUpWithAnnotations() $values = $fallbackPool->getValues(); $this->assertInternalType('array', $values); - $this->assertCount(2, $values); + $this->assertCount(1, $values); $this->assertArrayHasKey('Symfony.Bundle.FrameworkBundle.Tests.Fixtures.Validation.Category', $values); - $this->assertArrayHasKey('Symfony.Bundle.FrameworkBundle.Tests.Fixtures.Validation.SubCategory', $values); } public function testWarmUpWithoutLoader() From 71b41f7c1eff5f5a8ecc970904ae5770e5113b44 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 22:50:17 +0200 Subject: [PATCH 3/7] review changes --- .../AbstractPhpFileCacheWarmer.php | 20 +++++++++++++++++-- .../CacheWarmer/AnnotationsCacheWarmer.php | 8 -------- .../CacheWarmer/SerializerCacheWarmer.php | 8 -------- .../CacheWarmer/ValidatorCacheWarmer.php | 14 ++++++------- .../CacheWarmer/ValidatorCacheWarmerTest.php | 3 ++- 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index 049b16f9011e2..db11b5684754f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -18,6 +18,9 @@ use Symfony\Component\Cache\Adapter\ProxyAdapter; use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; +/** + * @internal This class is meant for internal use only. + */ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface { protected $phpArrayFile; @@ -36,6 +39,14 @@ public function __construct($phpArrayFile, CacheItemPoolInterface $fallbackPool) $this->fallbackPool = $fallbackPool; } + /** + * {@inheritdoc} + */ + public function isOptional() + { + return true; + } + /** * {@inheritdoc} */ @@ -56,8 +67,8 @@ public function warmUp($cacheDir) // the ArrayAdapter stores the values serialized // to avoid mutation of the data after it was written to the cache // so here we un-serialize the values first - $values = array_map(function ($val) { return unserialize($val); }, array_filter($arrayAdapter->getValues())); - $phpArrayAdapter->warmUp($values); + $values = array_map(function ($val) { return null !== $val ? unserialize($val) : null; }, $arrayAdapter->getValues()); + $this->warmUpPhpArrayAdapter($phpArrayAdapter, $values); foreach ($values as $k => $v) { $item = $this->fallbackPool->getItem($k); @@ -66,6 +77,11 @@ public function warmUp($cacheDir) $this->fallbackPool->commit(); } + protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array $values) + { + $phpArrayAdapter->warmUp($values); + } + /** * @param string $cacheDir * @param PhpArrayAdapter $phpArrayAdapter diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php index 3434e3c319c19..40077c97f3ba5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -40,14 +40,6 @@ public function __construct(Reader $annotationReader, $phpArrayFile, CacheItemPo $this->annotationReader = $annotationReader; } - /** - * {@inheritdoc} - */ - public function isOptional() - { - return true; - } - /** * {@inheritdoc} */ diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php index 6a111d415b308..3a5c6decd2553 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -42,14 +42,6 @@ public function __construct(array $loaders, $phpArrayFile, CacheItemPoolInterfac $this->loaders = $loaders; } - /** - * {@inheritdoc} - */ - public function isOptional() - { - return true; - } - /** * {@inheritdoc} */ diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index 5c0532cc4bb18..69ab0ab551cd2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -43,14 +43,6 @@ public function __construct(ValidatorBuilderInterface $validatorBuilder, $phpArr $this->validatorBuilder = $validatorBuilder; } - /** - * {@inheritdoc} - */ - public function isOptional() - { - return true; - } - /** * {@inheritdoc} */ @@ -78,6 +70,12 @@ protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAd } } + protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array $values) + { + // make sure we don't cache null values + parent::warmUpPhpArrayAdapter($phpArrayAdapter, array_filter($values)); + } + /** * @param LoaderInterface[] $loaders * diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php index 9613007468773..23b4732afcb3a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php @@ -79,8 +79,9 @@ public function testWarmUpWithAnnotations() $values = $fallbackPool->getValues(); $this->assertInternalType('array', $values); - $this->assertCount(1, $values); + $this->assertCount(2, $values); $this->assertArrayHasKey('Symfony.Bundle.FrameworkBundle.Tests.Fixtures.Validation.Category', $values); + $this->assertArrayHasKey('Symfony.Bundle.FrameworkBundle.Tests.Fixtures.Validation.SubCategory', $values); } public function testWarmUpWithoutLoader() From 6bd0398eeb2783e41af6f1f1ab1cd8b0a07b1e7f Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 22:54:44 +0200 Subject: [PATCH 4/7] remove comment --- .../FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index db11b5684754f..7f29be4f61b1c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -19,7 +19,7 @@ use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; /** - * @internal This class is meant for internal use only. + * @internal */ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface { From 44ffbf5074068296a6264dc23128a70577d5e250 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 22:55:51 +0200 Subject: [PATCH 5/7] make members private --- .../CacheWarmer/AbstractPhpFileCacheWarmer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index 7f29be4f61b1c..556a16d9878b8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -23,8 +23,8 @@ */ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface { - protected $phpArrayFile; - protected $fallbackPool; + private $phpArrayFile; + private $fallbackPool; /** * @param string $phpArrayFile The PHP file where metadata are cached From 8ab2ad62482d4899f3faa8d8e36f8cfe3f507f3b Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 23:09:35 +0200 Subject: [PATCH 6/7] simplify --- .../CacheWarmer/AbstractPhpFileCacheWarmer.php | 13 ++++--------- .../CacheWarmer/AnnotationsCacheWarmer.php | 7 ++----- .../CacheWarmer/SerializerCacheWarmer.php | 5 ++--- .../CacheWarmer/ValidatorCacheWarmer.php | 4 ++-- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index 556a16d9878b8..892f30b46c437 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -57,9 +57,7 @@ public function warmUp($cacheDir) spl_autoload_register(array($phpArrayAdapter, 'throwOnRequiredClass')); try { - if (false === $this->doWarmUp($cacheDir, $phpArrayAdapter, $arrayAdapter)) { - return; - } + $this->doWarmUp($cacheDir, $arrayAdapter); } finally { spl_autoload_unregister(array($phpArrayAdapter, 'throwOnRequiredClass')); } @@ -83,11 +81,8 @@ protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array } /** - * @param string $cacheDir - * @param PhpArrayAdapter $phpArrayAdapter - * @param ArrayAdapter $arrayAdapter - * - * @return bool|void false if there is nothing to warm-up + * @param string $cacheDir + * @param ArrayAdapter $arrayAdapter */ - abstract protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter); + abstract protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter); } diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php index 40077c97f3ba5..4b36cfa12b1da 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -16,7 +16,6 @@ use Doctrine\Common\Annotations\Reader; use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Adapter\PhpArrayAdapter; use Symfony\Component\Cache\DoctrineProvider; /** @@ -43,14 +42,12 @@ public function __construct(Reader $annotationReader, $phpArrayFile, CacheItemPo /** * {@inheritdoc} */ - protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) + protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) { $annotatedClassPatterns = $cacheDir.'/annotations.map'; if (!is_file($annotatedClassPatterns)) { - $phpArrayAdapter->warmUp(array()); - - return false; + return; } $annotatedClasses = include $annotatedClassPatterns; diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php index 3a5c6decd2553..bd2b0adddad3a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -14,7 +14,6 @@ use Doctrine\Common\Annotations\AnnotationException; use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Adapter\PhpArrayAdapter; use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory; use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory; use Symfony\Component\Serializer\Mapping\Loader\LoaderChain; @@ -45,10 +44,10 @@ public function __construct(array $loaders, $phpArrayFile, CacheItemPoolInterfac /** * {@inheritdoc} */ - protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) + protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) { if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { - return false; + return; } $metadataFactory = new CacheClassMetadataFactory(new ClassMetadataFactory(new LoaderChain($this->loaders)), $arrayAdapter); diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index 69ab0ab551cd2..e42fe5e00a4dd 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -46,10 +46,10 @@ public function __construct(ValidatorBuilderInterface $validatorBuilder, $phpArr /** * {@inheritdoc} */ - protected function doWarmUp($cacheDir, PhpArrayAdapter $phpArrayAdapter, ArrayAdapter $arrayAdapter) + protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) { if (!method_exists($this->validatorBuilder, 'getLoaders')) { - return false; + return; } $loaders = $this->validatorBuilder->getLoaders(); From 73abab8a36d52e4e5fa30808d2c8323402042dad Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 18 Jul 2017 23:17:54 +0200 Subject: [PATCH 7/7] more review feedback --- .../CacheWarmer/AbstractPhpFileCacheWarmer.php | 14 +++++++++----- .../CacheWarmer/AnnotationsCacheWarmer.php | 4 +++- .../CacheWarmer/SerializerCacheWarmer.php | 4 +++- .../CacheWarmer/ValidatorCacheWarmer.php | 4 +++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index 892f30b46c437..25801a7829c91 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -52,21 +52,23 @@ public function isOptional() */ public function warmUp($cacheDir) { - $phpArrayAdapter = new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool); $arrayAdapter = new ArrayAdapter(); - spl_autoload_register(array($phpArrayAdapter, 'throwOnRequiredClass')); + spl_autoload_register(array(PhpArrayAdapter::class, 'throwOnRequiredClass')); try { - $this->doWarmUp($cacheDir, $arrayAdapter); + if (!$this->doWarmUp($cacheDir, $arrayAdapter)) { + return; + } } finally { - spl_autoload_unregister(array($phpArrayAdapter, 'throwOnRequiredClass')); + spl_autoload_unregister(array(PhpArrayAdapter::class, 'throwOnRequiredClass')); } // the ArrayAdapter stores the values serialized // to avoid mutation of the data after it was written to the cache // so here we un-serialize the values first $values = array_map(function ($val) { return null !== $val ? unserialize($val) : null; }, $arrayAdapter->getValues()); - $this->warmUpPhpArrayAdapter($phpArrayAdapter, $values); + + $this->warmUpPhpArrayAdapter(new PhpArrayAdapter($this->phpArrayFile, $this->fallbackPool), $values); foreach ($values as $k => $v) { $item = $this->fallbackPool->getItem($k); @@ -83,6 +85,8 @@ protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array /** * @param string $cacheDir * @param ArrayAdapter $arrayAdapter + * + * @return bool false if there is nothing to warm-up */ abstract protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter); } diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php index 4b36cfa12b1da..4026b53bd7740 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -47,7 +47,7 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) $annotatedClassPatterns = $cacheDir.'/annotations.map'; if (!is_file($annotatedClassPatterns)) { - return; + return true; } $annotatedClasses = include $annotatedClassPatterns; @@ -69,6 +69,8 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) */ } } + + return true; } private function readAllComponents(Reader $reader, $class) diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php index bd2b0adddad3a..75cc2bcf9b384 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -47,7 +47,7 @@ public function __construct(array $loaders, $phpArrayFile, CacheItemPoolInterfac protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) { if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) { - return; + return false; } $metadataFactory = new CacheClassMetadataFactory(new ClassMetadataFactory(new LoaderChain($this->loaders)), $arrayAdapter); @@ -63,6 +63,8 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) } } } + + return true; } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index e42fe5e00a4dd..20f76cb4fdd73 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -49,7 +49,7 @@ public function __construct(ValidatorBuilderInterface $validatorBuilder, $phpArr protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) { if (!method_exists($this->validatorBuilder, 'getLoaders')) { - return; + return false; } $loaders = $this->validatorBuilder->getLoaders(); @@ -68,6 +68,8 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) } } } + + return true; } protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array $values)