[DI] Only rebuild autowiring cache when actually needed #18144
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| <?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\Component\DependencyInjection\Config; | ||
|
|
||
| use Symfony\Component\Config\Resource\SelfCheckingResourceInterface; | ||
| use Symfony\Component\DependencyInjection\Compiler\AutowirePass; | ||
|
|
||
| class AutowireServiceResource implements SelfCheckingResourceInterface, \Serializable | ||
| { | ||
| private $class; | ||
| private $filePath; | ||
| private $autowiringMetadata = array(); | ||
|
|
||
| public function __construct($class, $path, array $autowiringMetadata) | ||
| { | ||
| $this->class = $class; | ||
| $this->filePath = $path; | ||
| $this->autowiringMetadata = $autowiringMetadata; | ||
| } | ||
|
|
||
| public function isFresh($timestamp) | ||
| { | ||
| if (!file_exists($this->filePath)) { | ||
| return false; | ||
| } | ||
|
|
||
| // has the file *not* been modified? Definitely fresh | ||
| if (@filemtime($this->filePath) <= $timestamp) { | ||
| return true; | ||
| } | ||
|
|
||
| try { | ||
| $reflectionClass = new \ReflectionClass($this->class); | ||
| } catch (\ReflectionException $e) { | ||
| // the class does not exist anymore! | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| return AutowirePass::createResourceForClass($reflectionClass); | ||
|
There was a problem hiding this comment. Failure looks expected here |
||
| } | ||
|
|
||
| public function __toString() | ||
| { | ||
| return 'service.autowire.'.$this->class; | ||
| } | ||
|
|
||
| public function serialize() | ||
| { | ||
| return serialize(array($this->class, $this->filePath, $this->autowiringMetadata)); | ||
| } | ||
|
|
||
| public function unserialize($serialized) | ||
| { | ||
| list($this->class, $this->filePath, $this->autowiringMetadata) = unserialize($serialized); | ||
| } | ||
|
|
||
| /** | ||
| * @deprecated Implemented for compatibility with Symfony 2.8 | ||
| */ | ||
| public function getResource() | ||
| { | ||
| return $this->filePath; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -413,6 +413,39 @@ public function testOptionalScalarArgsNotPassedIfLast() | |
| $definition->getArguments() | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @dataProvider getCreateResourceTests | ||
| */ | ||
| public function testCreateResourceForClass($className, $isEqual) | ||
| { | ||
| $startingResource = AutowirePass::createResourceForClass( | ||
|
There was a problem hiding this comment. |
||
| new \ReflectionClass(__NAMESPACE__.'\ClassForResource') | ||
| ); | ||
| $newResource = AutowirePass::createResourceForClass( | ||
|
There was a problem hiding this comment. |
||
| new \ReflectionClass(__NAMESPACE__.'\\'.$className) | ||
| ); | ||
|
|
||
| // hack so the objects don't differ by the class name | ||
| $startingReflObject = new \ReflectionObject($startingResource); | ||
| $reflProp = $startingReflObject->getProperty('class'); | ||
| $reflProp->setAccessible(true); | ||
| $reflProp->setValue($startingResource, __NAMESPACE__.'\\'.$className); | ||
|
|
||
| if ($isEqual) { | ||
| $this->assertEquals($startingResource, $newResource); | ||
| } else { | ||
|
There was a problem hiding this comment. |
||
| $this->assertNotEquals($startingResource, $newResource); | ||
| } | ||
| } | ||
|
|
||
| public function getCreateResourceTests() | ||
| { | ||
| return array( | ||
| ['IdenticalClassResource', true], | ||
| ['ClassChangedConstructorArgs', false], | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class Foo | ||
|
|
@@ -562,3 +595,26 @@ public function __construct(A $a, $foo = 'default_val', Lille $lille) | |
| { | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Classes used for testing createResourceForClass | ||
| */ | ||
| class ClassForResource | ||
| { | ||
| public function __construct($foo, Bar $bar = null) | ||
|
There was a problem hiding this comment.
|
||
| { | ||
| } | ||
|
|
||
| public function setBar(Bar $bar) | ||
|
There was a problem hiding this comment.
|
||
| { | ||
| } | ||
| } | ||
| class IdenticalClassResource extends ClassForResource | ||
| { | ||
| } | ||
| class ClassChangedConstructorArgs extends ClassForResource | ||
| { | ||
| public function __construct($foo, Bar $bar, $baz) | ||
| { | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| <?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\Component\DependencyInjection\Tests\Config; | ||
|
|
||
| use Symfony\Component\DependencyInjection\Compiler\AutowirePass; | ||
| use Symfony\Component\DependencyInjection\Config\AutowireServiceResource; | ||
|
|
||
| class AutowireServiceResourceTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| /** | ||
| * @var AutowireServiceResource | ||
| */ | ||
| private $resource; | ||
| private $file; | ||
| private $class; | ||
| private $time; | ||
|
|
||
| protected function setUp() | ||
| { | ||
| $this->file = realpath(sys_get_temp_dir()).'/tmp.php'; | ||
| $this->time = time(); | ||
| touch($this->file, $this->time); | ||
|
|
||
| $this->class = __NAMESPACE__.'\Foo'; | ||
| $this->resource = new AutowireServiceResource( | ||
| $this->class, | ||
| $this->file, | ||
| array() | ||
| ); | ||
| } | ||
|
|
||
| public function testToString() | ||
| { | ||
| $this->assertSame('service.autowire.'.$this->class, (string) $this->resource); | ||
| } | ||
|
|
||
| public function testSerializeUnserialize() | ||
| { | ||
| $unserialized = unserialize(serialize($this->resource)); | ||
|
|
||
| $this->assertEquals($this->resource, $unserialized); | ||
| } | ||
|
|
||
| public function testIsFresh() | ||
| { | ||
| $this->assertTrue($this->resource->isFresh($this->time), '->isFresh() returns true if the resource has not changed in same second'); | ||
| $this->assertTrue($this->resource->isFresh($this->time + 10), '->isFresh() returns true if the resource has not changed'); | ||
| $this->assertFalse($this->resource->isFresh($this->time - 86400), '->isFresh() returns false if the resource has been updated'); | ||
|
There was a problem hiding this comment. Ok it this line, the object is returned instead of false. |
||
| } | ||
|
|
||
| public function testIsFreshForDeletedResources() | ||
| { | ||
| unlink($this->file); | ||
|
|
||
| $this->assertFalse($this->resource->isFresh($this->getStaleFileTime()), '->isFresh() returns false if the resource does not exist'); | ||
| } | ||
|
|
||
| public function testIsNotFreshChangedResource() | ||
| { | ||
| $oldResource = new AutowireServiceResource( | ||
| $this->class, | ||
| $this->file, | ||
| array('will_be_different') | ||
| ); | ||
|
|
||
| // test with a stale file *and* a resource that *will* be different than the actual | ||
| $this->assertFalse($oldResource->isFresh($this->getStaleFileTime()), '->isFresh() returns false if the constructor arguments have changed'); | ||
| } | ||
|
|
||
| public function testIsFreshSameConstructorArgs() | ||
| { | ||
| $oldResource = AutowirePass::createResourceForClass( | ||
|
There was a problem hiding this comment. |
||
| new \ReflectionClass(__NAMESPACE__.'\Foo') | ||
| ); | ||
|
|
||
| // test with a stale file *but* the resource will not be changed | ||
| $this->assertTrue($oldResource->isFresh($this->getStaleFileTime()), '->isFresh() returns false if the constructor arguments have changed'); | ||
| } | ||
|
|
||
| public function testNotFreshIfClassNotFound() | ||
| { | ||
| $resource = new AutowireServiceResource( | ||
| 'Some\Non\Existent\Class', | ||
| $this->file, | ||
| array() | ||
| ); | ||
|
|
||
| $this->assertFalse($resource->isFresh($this->getStaleFileTime()), '->isFresh() returns false if the class no longer exists'); | ||
| } | ||
|
|
||
| protected function tearDown() | ||
| { | ||
| if (!file_exists($this->file)) { | ||
| return; | ||
| } | ||
|
|
||
| unlink($this->file); | ||
| } | ||
|
|
||
| private function getStaleFileTime() | ||
| { | ||
| return $this->time - 10; | ||
| } | ||
| } | ||
|
|
||
| class Foo | ||
| { | ||
| public function __construct($foo) | ||
|
There was a problem hiding this comment.
|
||
| { | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't those static methods be extracted to a Factory class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just seen #18144 (comment)
Perfectly makes sense.