8000 Merge branch '4.3' into 4.4 · symfony/symfony@f4f2fda · GitHub
[go: up one dir, main page]
Skip to content

Commit f4f2fda

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.3' into 4.4
* 4.3: [FrameworkBundle][Config] Ignore exeptions thrown during reflection classes autoload
2 parents 38ff264 + f3f6b58 commit f4f2fda

File tree

12 files changed

+244
-32
lines changed

12 files changed

+244
-32
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Cache\Adapter\ArrayAdapter;
1515
use Symfony\Component\Cache\Adapter\NullAdapter;
1616
use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
17+
use Symfony\Component\Config\Resource\ClassExistenceResource;
1718
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
1819

1920
/**
@@ -46,13 +47,13 @@ public function warmUp($cacheDir)
4647
{
4748
$arrayAdapter = new ArrayAdapter();
4849

49-
spl_autoload_register([PhpArrayAdapter::class, 'throwOnRequiredClass']);
50+
spl_autoload_register([ClassExistenceResource::class, 'throwOnRequiredClass']);
5051
try {
5152
if (!$this->doWarmUp($cacheDir, $arrayAdapter)) {
5253
return;
5354
}
5455
} finally {
55-
spl_autoload_unregister([PhpArrayAdapter::class, 'throwOnRequiredClass']);
56+
spl_autoload_unregister([ClassExistenceResource::class, 'throwOnRequiredClass']);
5657
}
5758

5859
// the ArrayAdapter stores the values serialized
@@ -68,6 +69,17 @@ protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array
6869
$phpArrayAdapter->warmUp($values);
6970
}
7071

72+
/**
73+
* @internal
74+
*/
75+
final protected function ignoreAutoloadException($class, \Exception $exception)
76+
{
77+
try {
78+
ClassExistenceResource::throwOnRequiredClass($class, $exception);
79+
} catch (\ReflectionException $e) {
80+
}
81+
}
82+
7183
/**
7284
* @param string $cacheDir
7385
* @param ArrayAdapter $arrayAdapter

‎src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php

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

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

9396
foreach ($reflectionClass->getMethods() as $reflectionMethod) {
94-
$reader->getMethodAnnotations($reflectionMethod);
97+
try {
98+
$reader->getMethodAnnotations($reflectionMethod);
99+
} catch (AnnotationException $e) {
100+
}
95101
}
96102

97103
foreach ($reflectionClass->getProperties() as $reflectionProperty) {
98-
$reader->getPropertyAnnotations($reflectionProperty);
104+
try {
105+
$reader->getPropertyAnnotations($reflectionProperty);
106+
} catch (AnnotationException $e) {
107+
}
99108
}
100109
}
101110
}

‎src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
5858
foreach ($loader->getMappedClasses() as $mappedClass) {
5959
try {
6060
$metadataFactory->getMetadataFor($mappedClass);
61-
} catch (\ReflectionException $e) {
62-
// ignore failing reflection
6361
} catch (AnnotationException $e) {
6462
// ignore failing annotations
63+
} catch (\Exception $e) {
64+
$this->ignoreAutoloadException($mappedClass, $e);
6565
}
6666
}
6767
}

‎src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
6767
if ($metadataFactory->hasMetadataFor($mappedClass)) {
6868
$metadataFactory->getMetadataFor($mappedClass);
6969
}
70-
} catch (\ReflectionException $e) {
71-
// ignore failing reflection
7270
} catch (AnnotationException $e) {
7371
// ignore failing annotations
72+
} catch (\Exception $e) {
73+
$this->ignoreAutoloadException($mappedClass, $e);
7474
}
7575
}
7676
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php

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

75+
/**
76+
* Test that the cache warming process is not broken if a class loader
77+
* throws an exception (on class / file not found for example).
78+
*/
79+
public function testClassAutoloadException()
80+
{
81+
$this->assertFalse(class_exists($annotatedClass = 'C\C\C', false));
82+
83+
file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
84+
$warmer = new AnnotationsCacheWarmer(new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
85+
86+
spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
87+
if ($class === $annotatedClass) {
88+
throw new \DomainException('This exception should be caught by the warmer.');
89+
}
90+
}, true, true);
91+
92+
$warmer->warmUp($this->cacheDir);
93+
94+
spl_autoload_unregister($classLoader);
95+
}
96+
97+
/**
98+
* Test that the cache warming process is broken if a class loader throws an
99+
* exception but that is unrelated to the class load.
100+
*/
101+
public function testClassAutoloadExceptionWithUnrelatedException()
102+
{
103+
$this->expectException(\DomainException::class);
104+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
105+
106+
$this->assertFalse(class_exists($annotatedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_AnnotationsCacheWarmerTest', false));
107+
108+
file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
109+
$warmer = new AnnotationsCacheWarmer(< F438 span class=pl-k>new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
110+
111+
spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
112+
if ($class === $annotatedClass) {
113+
eval('class '.$annotatedClass.'{}');
114+
throw new \DomainException('This exception should not be caught by the warmer.');
115+
}
116+
}, true, true);
117+
118+
$warmer->warmUp($this->cacheDir);
119+
120+
spl_autoload_unregister($classLoader);
121+
}
122+
75123
/**
76124
* @return MockObject|Reader
77125
*/

‎src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,58 @@ public function testWarmUpWithoutLoader()
6060

6161
$this->assertFileExists($file);
6262
}
63+
64+
/**
65+
* Test that the cache warming process is not broken if a class loader
66+
* throws an exception (on class / file not found for example).
67+
*/
68+
public function testClassAutoloadException()
69+
{
70+
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
71+
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
72+
}
73+
74+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
75+
76+
$warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
77+
78+
spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
79+
if ($class === $mappedClass) {
80+
throw new \DomainException('This exception should be caught by the warmer.');
81+
}
82+
}, true, true);
83+
84+
$warmer->warmUp('foo');
85+
86+
spl_autoload_unregister($classLoader);
87+
}
88+
89+
/**
90+
* Test that the cache warming process is broken if a class loader throws an
91+
* exception but that is unrelated to the class load.
92+
*/
93+
public function testClassAutoloadExceptionWithUnrelatedException()
94+
{
95+
$this->expectException(\DomainException::class);
96+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
97+
98+
if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
99+
$this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
100+
}
101+
102+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
103+
104+
$warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
105+
106+
spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
107+
if ($class === $mappedClass) {
108+
eval('class '.$mappedClass.'{}');
109+
throw new \DomainException('This exception should not be caught by the warmer.');
110+
}
111+
}, true, true);
112+
113+
$warmer->warmUp('foo');
114+
115+
spl_autoload_unregister($classLoader);
116+
}
63117
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,54 @@ public function testWarmUpWithoutLoader()
8181

8282
$this->assertFileExists($file);
8383
}
84+
85+
/**
86+
* Test that the cache warming process is not broken if a class loader
87+
* throws an exception (on class / file not found for example).
88+
*/
89+
public function testClassAutoloadException()
90+
{
91+
$this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest', false));
92+
93+
$validatorBuilder = new ValidatorBuilder();
94+
$validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/does_not_exist.yaml');
95+
$warmer = new ValidatorCacheWarmer($validatorBuilder, tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
96+
97+
spl_autoload_register($classloader = function ($class) use ($mappedClass) {
98+
if ($class === $mappedClass) {
99+
throw new \DomainException('This exception should be caught by the warmer.');
100+
}
101+
}, true, true);
102+
103+
$warmer->warmUp('foo');
104+
105+
spl_autoload_unregister($classloader);
106+
}
107+
108+
/**
109+
* Test that the cache warming process is broken if a class loader throws an
110+
* exception but that is unrelated to the class load.
111+
*/
112+
public function testClassAutoloadExceptionWithUnrelatedException()
113+
{
114+
$this->expectException(\DomainException::class);
115+
$this->expectExceptionMessage('This exception should not be caught by the warmer.');
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+
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+
}
84134
}

‎src/Symfony/Bundle/FrameworkBundle/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: ~

‎src/Symfony/Bundle/FrameworkBundle/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: ~

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"php": "^7.1.3",
2020
"ext-xml": "*",
2121
"symfony/cache": "^4.4|^5.0",
22-
"symfony/config": "^4.2|^5.0",
22+
"symfony/config": "^4.3.4|^5.0",
2323
"symfony/dependency-injection": "^4.4|^5.0",
2424
"symfony/error-renderer": "^4.4|^5.0",
2525
"symfony/http-foundation": "^4.3|^5.0",

‎src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ private function generateItems(array $keys): \Generator
288288
/**
289289
* @throws \ReflectionException When $class is not found and is required
290290
*
291-
* @internal
291+
* @internal to be removed in Symfony 5.0
292292
*/
293293
public static function throwOnRequiredClass($class)
294294
{

‎src/Symfony/Component/Config/Resource/ClassExistenceResource.php

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,14 @@ public function isFresh($timestamp)
7676

7777
try {
7878
$exists = class_exists($this->resource) || interface_exists($this->resource, false) || trait_exists($this->resource, false);
79-
} catch (\ReflectionException $e) {
80-
if (0 >= $timestamp) {
81-
unset(self::$existsCache[1][$this->resource]);
82-
throw $e;
79+
} catch (\Exception $e) {
80+
try {
81+
self::throwOnRequiredClass($this->resource, $e);
82+
} catch (\ReflectionException $e) {
83+
if (0 >= $timestamp) {
84+
unset(self::$existsCache[1][$this->resource]);
85+
throw $e;
86+
}
8387
}
8488
} finally {
8589
self::$autoloadedClass = $autoloadedClass;
@@ -109,24 +113,57 @@ public function __sleep(): array
109113
}
110114

111115
/**
112-
* @throws \ReflectionException When $class is not found and is required
116+
* Throws a reflection exception when the passed class does not exist but is required.
117+
*
118+
* A class is considered "not required" when it's loaded as part of a "class_exists" or similar check.
119+
*
120+
* This function can be used as an autoload function to throw a reflection
121+
* exception if the class was not found by previous autoload functions.
122+
*
123+
* A previous exception can be passed. In this case, the class is considered as being
124+
* required totally, so if it doesn't exist, a reflection exception is always thrown.
125+
* If it exists, the previous exception is rethrown.
126+
*
127+
* @throws \ReflectionException
113128
*
114129
* @internal
115130
*/
116-
public static function throwOnRequiredClass($class)
131+
public static function throwOnRequiredClass($class, \Exception $previous = null)
117132
{
118-
if (self::$autoloadedClass === $class) {
133+
// If the passed class is the resource being checked, we shouldn't throw.
134+
if (null === $previous && self::$autoloadedClass === $class) {
135+
return;
136+
}
137+
138+
if (class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false)) {
139+
if (null !== $previous) {
140+
throw $previous;
141+
}
142+
119143
return;
120144
}
121-
$e = new \ReflectionException("Class $class not found");
145+
146+
if ($previous instanceof \ReflectionException) {
147+
throw $previous;
148+
}
149+
150+
$e = new \ReflectionException("Class $class not found", 0, $previous);
151+
152+
if (null !== $previous) {
153+
throw $e;
154+
}
155+
122156
$trace = $e->getTrace();
123157
$autoloadFrame = [
124158
'function' => 'spl_autoload_call',
125159
'args' => [$class],
126160
];
127-
$i = 1 + array_search($autoloadFrame, $trace, true);
128161

129-
if (isset($trace[$i]['function']) && !isset($trace[$i]['class'])) {
162+
if (false === $i = array_search($autoloadFrame, $trace, true)) {
163+
throw $e;
164+
}
165+
166+
if (isset($trace[++$i]['function']) && !isset($trace[$i]['class'])) {
130167
switch ($trace[$i]['function']) {
131168
case 'get_class_methods':
132169
case 'get_class_vars':

0 commit comments

Comments
 (0)
0