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

Commit f3f6b58

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.4' into 4.3
* 3.4: [FrameworkBundle][Config] Ignore exeptions thrown during reflection classes autoload
2 parents 1aba480 + 48859fd commit f3f6b58

File tree

12 files changed

+244
-32
lines changed
  • composer.json
  • Component

    • 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, $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
    @@ -62,10 +62,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
    6262
    if ($metadataFactory->hasMetadataFor($mappedClass)) {
    6363
    $metadataFactory->getMetadataFor($mappedClass);
    6464
    }
    65-
    } catch (\ReflectionException $e) {
    66-
    // ignore failing reflection
    6765
    } catch (AnnotationException $e) {
    6866
    // ignore failing annotations
    67+
    } catch (\Exception $e) {
    68+
    $this->ignoreAutoloadException($mappedClass, $e);
    6969
    }
    7070
    }
    7171
    }

    ‎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(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.3",
    22-
    "symfony/config": "~4.2",
    22+
    "symfony/config": "^4.3.4",
    2323
    "symfony/debug": "~4.0",
    2424
    "symfony/dependency-injection": "^4.3",
    2525
    "symfony/http-foundation": "^4.3",

    ‎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