-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Class existence resource #20121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Class existence resource #20121
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
<?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\Config\Resource; | ||
|
||
/** | ||
* ClassExistenceResource represents a class existence. | ||
* Freshness is only evaluated against resource existence. | ||
* | ||
* The resource must be a fully-qualified class name. | ||
* | ||
* @author Fabien Potencier <fabien@symfony.com> | ||
*/ | ||
class ClassExistenceResource implements SelfCheckingResourceInterface, \Serializable | ||
{ | ||
private $resource; | ||
private $exists; | ||
|
||
/** | ||
* @param string $resource The fully-qualified class name | ||
*/ | ||
public function __construct($resource) | ||
{ | ||
$this->resource = $resource; | ||
$this->exists = class_exists($resource); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should use |
||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function __toString() | ||
{ | ||
return $this->resource; | ||
} | ||
|
||
/** | ||
* @return string The file path to the resource | ||
*/ | ||
public function getResource() | ||
{ | ||
return $this->resource; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function isFresh($timestamp) | ||
{ | ||
return class_exists($this->resource) === $this->exists; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function serialize() | ||
{ | ||
return serialize(array($this->resource, $this->exists)); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function unserialize($serialized) | ||
{ | ||
list($this->resource, $this->exists) = unserialize($serialized); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,54 @@ | ||||||||||||||||||||
<?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\Config\Tests\Resource; | ||||||||||||||||||||
|
||||||||||||||||||||
use Symfony\Component\Config\Resource\ClassExistenceResource; | ||||||||||||||||||||
|
||||||||||||||||||||
class ClassExistenceResourceTest extends \PHPUnit_Framework_TestCase | ||||||||||||||||||||
{ | ||||||||||||||||||||
public function testToString() | ||||||||||||||||||||
{ | ||||||||||||||||||||
$res = new ClassExistenceResource('BarClass'); | ||||||||||||||||||||
$this->assertSame('BarClass', (string) $res); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
public function testGetResource() | ||||||||||||||||||||
{ | ||||||||||||||||||||
$res = new ClassExistenceResource('BarClass'); | ||||||||||||||||||||
$this->assertSame('BarClass', $res->getResource()); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
public function testIsFreshWhenClassDoesNotExist() | ||||||||||||||||||||
{ | ||||||||||||||||||||
$res = new ClassExistenceResource('Symfony\Component\Config\Tests\Fixtures\BarClass'); | ||||||||||||||||||||
|
||||||||||||||||||||
$this->assertTrue($res->isFresh(time())); | ||||||||||||||||||||
|
||||||||||||||||||||
eval(<<<EOF | ||||||||||||||||||||
namespace Symfony\Component\Config\Tests\Fixtures; | ||||||||||||||||||||
|
||||||||||||||||||||
class BarClass | ||||||||||||||||||||
{ | ||||||||||||||||||||
} | ||||||||||||||||||||
EOF | ||||||||||||||||||||
); | ||||||||||||||||||||
|
||||||||||||||||||||
$this->assertFalse($res->isFresh(time())); | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Afaik, this should be
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The freshness is all about the state change. The resource is fresh if there is no change in class existence, so no need to refresh the cache. Not fresh means that the cache must be recalculated, so when the state changes. So, the table should be:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, sorry. Completely messed up the definition of "fresh"... |
||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
public function testIsFreshWhenClassExists() | ||||||||||||||||||||
{ | ||||||||||||||||||||
$res = new ClassExistenceResource('Symfony\Component\Config\Tests\Resource\ClassExistenceResourceTest'); | ||||||||||||||||||||
|
||||||||||||||||||||
$this->assertTrue($res->isFresh(time())); | ||||||||||||||||||||
} | ||||||||||||||||||||
} |
Uh oh!
There was an error while loading. Please reload this page.
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.
Related to this comment, just asking if we could replace this:
by this?
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.
The difference is that if one uses the
twig.extension.yaml
directly, the proposed version will break.I think the current approach is a more open 10000 thus a better practice, even if in this very case using the extension directly is so unlikely.