Skip to content

Commit 758b2d5

Browse files
nicolas-grekasnicolas-grekas
committed
bug #32516 [FrameworkBundle][Config] Ignore exceptions thrown during reflection classes autoload (fancyweb)
This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle][Config] Ignore exceptions thrown during reflection classes autoload | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#32499 with PHP 7.3+ | License | MIT | Doc PR | - The behavior when an exception is thrown in a class loader changed in PHP 7.3 (cf https://3v4l.org/OQPk9). That means that the `throwOnRequiredClass` trick that is done in the parent class of these cache warmers (`AbstractPhpFileCacheWarmer`) does not work anymore with PHP7.3+. Commits ------- dbd9b75d86 [FrameworkBundle][Config] Ignore exeptions thrown during reflection classes autoload
2 parents 701dd0f + 65a911b commit 758b2d5

File tree

10 files changed

+196
-21
lines changed

10 files changed

+196
-21
lines changed

‎CacheWarmer/AbstractPhpFileCacheWarmer.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Cache\Adapter\ArrayAdapter;
1717
use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
1818
use Symfony\Component\Cache\Adapter\ProxyAdapter;
19+
use Symfony\Component\Config\Resource\ClassExistenceResource;
1920
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
2021

2122
/**
@@ -54,13 +55,13 @@ public function warmUp($cacheDir)
5455
{
5556
$arrayAdapter = new ArrayAdapter();
5657

57-
spl_autoload_register([PhpArrayAdapter::class, 'throwOnRequiredClass']);
58+
spl_autoload_register([ClassExistenceResource::class, 'throwOnRequiredClass']);
5859
try {
5960
if (!$this->doWarmUp($cacheDir, $arrayAdapter)) {
6061
return;
6162
}
6263
} finally {
63-
spl_autoload_unregister([PhpArrayAdapter::class, 'throwOnRequiredClass']);
64+
spl_autoload_unregister([ClassExistenceResource::class, 'throwOnRequiredClass']);
6465
}
6566

6667
// the ArrayAdapter stores the values serialized
@@ -82,6 +83,17 @@ protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array
8283
$phpArrayAdapter->warmUp($values);
8384
}
8485

86+
/**
87+
* @internal
88+
*/
89+
final protected function ignoreAutoloadException($class, \Exception $exception)
90+
{
91+
try {
92+
ClassExistenceResource::throwOnRequiredClass($class, $exception);
93+
} catch (\ReflectionException $e) {
94+
}
95+
}
96+
8597
/**
8698
* @param string $cacheDir
8799
* @param ArrayAdapter $arrayAdapter

‎CacheWarmer/AnnotationsCacheWarmer.php

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,8 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
6464
}
6565
try {
6666
$this->readAllComponents($reader, $class);
67-
} catch (\ReflectionException $e) {
68-
// ignore failing reflection
69-
} catch (AnnotationException $e) {
70-
/*
71-
* Ignore any AnnotationException to not break the cache warming process if an Annotation is badly
72-
* configured or could not be found / read / etc.
73-
*
74-
* In particular cases, an Annotation in your code can be used and defined only for a specific
75-
* environment but is always added to the annotations.map file by some Symfony default behaviors,
76-
* and you always end up with a not found Annotation.
77-
*/
67+
} catch (\Exception $e) {
68+
$this->ignoreAutoloadException($class, $e);
7869
}
7970
}
8071

@@ -84,14 +75,32 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
8475
private function readAllComponents(Reader $reader, $class)
8576
{
8677
$reflectionClass = new \ReflectionClass($class);
87-
$reader->getClassAnnotations($reflectionClass);
78+
79+
try {
80+
$reader->getClassAnnotations($reflectionClass);
81+
} catch (AnnotationException $e) {
82+
/*
83+
* Ignore any AnnotationException to not break the cache warming process if an Annotation is badly
84+
* configured or could not be found / read / etc.
85+
*
86+
* In particular cases, an Annotation in your code can be used and defined only for a specific
87+
* environment but is always added to the annotations.map file by some Symfony default behaviors,
88+
* and you always end up with a not found Annotation.
89+
*/
90+
}
8891

8992
foreach ($reflectionClass->getMethods() as $reflectionMethod) {
90-
$reader->getMethodAnnotations($reflectionMethod);
93+
try {
94+
$reader->getMethodAnnotations($reflectionMethod);
95+
} catch (AnnotationException $e) {
96+
}
9197
}
9298

9399
foreach ($reflectionClass->getProperties() as $reflectionProperty) {
94-
$reader->getPropertyAnnotations($reflectionProperty);
100+
try {
101+
$reader->getPropertyAnnotations($reflectionProperty);
102+
} catch (AnnotationException $e) {
103+
}
95104
}
96105
}
97106
}

‎CacheWarmer/SerializerCacheWarmer.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
5656
foreach ($loader->getMappedClasses() as $mappedClass) {
5757
try {
5858
$metadataFactory->getMetadataFor($mappedClass);
59-
} catch (\ReflectionException $e) {
60-
// ignore failing reflection
6159
} catch (AnnotationException $e) {
6260
// ignore failing annotations
61+
} catch (\Exception $e) {
62+
$this->ignoreAutoloadException($mappedClass, $e);
6363
}
6464
}
6565
}

‎CacheWarmer/ValidatorCacheWarmer.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
6161
if ($metadataFactory->hasMetadataFor($mappedClass)) {
6262
$metadataFactory->getMetadataFor($mappedClass);
6363
}
64-
} catch (\ReflectionException $e) {
65-
// ignore failing reflection
6664
} catch (AnnotationException $e) {
6765
// ignore failing annotations
66+
} catch (\Exception $e) {
67+
$this->ignoreAutoloadException($mappedClass, $e);
6868
}
6969
}
7070
}

‎Tests/CacheWarmer/AnnotationsCacheWarmerTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,54 @@ public function testAnnotationsCacheWarmerWithDebugEnabled()
8686
$reader->getPropertyAnnotations($refClass->getProperty('cacheDir'));
8787
}
8888

89+
/**
90+
* Test that the cache warming process is not broken if a class loader
91+
* throws an exception (on class / file not found for example).
92+
*/
93+
public function testClassAutoloadException()
94+
{
95+
$this->assertFalse(class_exists($annotatedClass = 'C\C\C', false));
96+
97+
file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
98+
$warmer = new AnnotationsCacheWarmer(new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
99+
100+
spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
101+
if ($class === $annotatedClass) {
102+
throw new \DomainException('This exception should be caught by the warmer.');
103+
}
104+
}, true, true);
105+
106+
$warmer->warmUp($this->cacheDir);
107+
108+
spl_autoload_unregister($classLoader);
109+
}
110+
111+
/**
112+
* Test that the cache warming process is broken if a class loader throws an
113+
* exception but that is unrelated to the class load.
114+
*/
115+
public function testClassAutoloadExceptionWithUnrelatedException()
116+
{
117+
$this->expectException(\DomainException::class);
118+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
119+
120+
$this->assertFalse(class_exists($annotatedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_AnnotationsCacheWarmerTest', false));
121+
122+
file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
123+
$warmer = new AnnotationsCacheWarmer(new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
124+
125+
spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
126+
if ($class === $annotatedClass) {
127+
eval('class '.$annotatedClass.'{}');
128+
throw new \DomainException('This exception should not be caught by the warmer.');
129+
}
130+
}, true, true);
131+
132+
$warmer->warmUp($this->cacheDir);
133+
134+
spl_autoload_unregister($classLoader);
135+
}
136+
89137
/**
90138
* @return MockObject|Reader
91139
*/

‎Tests/CacheWarmer/SerializerCacheWarmerTest.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,58 @@ public function testWarmUpWithoutLoader()
7777
$this->assertIsArray($values);
7878
$this->assertCount(0, $values);
7979
}
80+
81+
/**
82+
* Test that the cache warming process is not broken if a class loader
83+
* throws an exception (on class / file not found for example).
84+
*/
85+
public function testClassAutoloadException()
86+
{
87+
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
88+
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
89+
}
90+
91+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
92+
93+
$warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
94+
95+
spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
96+
if ($class === $mappedClass) {
97+
throw new \DomainException('This exception should be caught by the warmer.');
98+
}
99+
}, true, true);
100+
101+
$warmer->warmUp('foo');
102+
103+
spl_autoload_unregister($classLoader);
104+
}
105+
106+
/**
107+
* Test that the cache warming process is broken if a class loader throws an
108+
* exception but that is unrelated to the class load.
109+
*/
110+
public function testClassAutoloadExceptionWithUnrelatedException()
111+
{
112+
$this->expectException(\DomainException::class);
113+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
114+
115+
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
116+
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
117+
}
118+
119+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
120+
121+
$warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
122+
123+
spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
124+
if ($class === $mappedClass) {
125+
eval('class '.$mappedClass.'{}');
126+
throw new \DomainException('This exception should not be caught by the warmer.');
127+
}
128+
}, true, true);
129+
130+
$warmer->warmUp('foo');
131+
132+
spl_autoload_unregister($classLoader);
133+
}
80134
}

‎Tests/CacheWarmer/ValidatorCacheWarmerTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,54 @@ public function testWarmUpWithoutLoader()
107107
$this->assertIsArray($values);
108108
$this->assertCount(0, $values);
109109
}
110+
111+
/**
112+
* Test that the cache warming process is not broken if a class loader
113+
* throws an exception (on class / file not found for example).
114+
*/
115+
public function testClassAutoloadException()
116+
{
117+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest', false));
118+
119+
$validatorBuilder = new ValidatorBuilder();
120+
$validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/does_not_exist.yaml');
121+
$warmer = new ValidatorCacheWarmer($validatorBuilder, tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
122+
123+
spl_autoload_register($classloader = function ($class) use ($mappedClass) {
124+
if ($class === $mappedClass) {
125+
throw new \DomainException('This exception should be caught by the warmer.');
126+
}
127+
}, true, true);
128+
129+
$warmer->warmUp('foo');
130+
131+
spl_autoload_unregister($classloader);
132+
}
133+
134+
/**
135+
* Test that the cache warming process is broken if a class loader throws an
136+
* exception but that is unrelated to the class load.
137+
*/
138+
public function testClassAutoloadExceptionWithUnrelatedException()
139+
{
140+
$this->expectException(\DomainException::class);
141+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
142+
143+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest', false));
144+
145+
$validatorBuilder = new ValidatorBuilder();
146+
$validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/does_not_exist.yaml');
147+
$warmer = new ValidatorCacheWarmer($validatorBuilder, tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
148+
149+
spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
150+
if ($class === $mappedClass) {
151+
eval('class '.$mappedClass.'{}');
152+
throw new \DomainException('This exception should not be caught by the warmer.');
153+
}
154+
}, true, true);
155+
156+
$warmer->warmUp('foo');
157+
158+
spl_autoload_unregister($classLoader);
159+
}
110160
}

‎Tests/Fixtures/Serialization/Resources/does_not_exist.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest: ~

‎Tests/Fixtures/Validation/Resources/does_not_exist.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest: ~

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"symfony/cache": "~3.4|~4.0",
2222
"symfony/class-loader": "~3.2",
2323
"symfony/dependency-injection": "^3.4.24|^4.2.5",
24-
"symfony/config": "~3.4|~4.0",
24+
"symfony/config": "^3.4.31|^4.3.4",
2525
"symfony/debug": "~2.8|~3.0|~4.0",
2626
"symfony/event-dispatcher": "~3.4|~4.0",
2727
"symfony/http-foundation": "^3.3.11|~4.0",

0 commit comments

Comments
 (0)