Skip to content

Commit

Permalink
bug #25113 [Routing] Fix "config-file-relative" annotation loader res…
Browse files Browse the repository at this point in the history
…ources (nicolas-grekas, sroze)

This PR was merged into the 3.3 branch.

Discussion
----------

[Routing] Fix "config-file-relative" annotation loader resources

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | slight behavior change
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

By using the locator in its `supports()` method, the `AnnotationDirectoryLoader` breaks config-relative annotation resources. The workaround is to fallback on `kernel.root_dir`-relative paths, as done in https://github.com/symfony/recipes/blob/master/doctrine/annotations/1.0/config/routes/annotations.yaml
But as you can see, this is rather WTF: extra knowledge is required to know what to type there. All the other loader look relatively to the config file first.

This is a bug, but since this is a slight behavior change, I think it's best to merge it on 3.4.

Commits
-------

f4999d8 Add tests proving it can load annotated files
5998e9d [Routing] Fix "config-file-relative" annotation loader resources
  • Loading branch information
nicolas-grekas committed Nov 24, 2017
2 parents d141541 + f4999d8 commit db979e8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
Expand Up @@ -34,7 +34,9 @@ class AnnotationDirectoryLoader extends AnnotationFileLoader
*/
public function load($path, $type = null)
{
$dir = $this->locator->locate($path);
if (!is_dir($dir = $this->locator->locate($path))) {
return parent::supports($path, $type) ? parent::load($path, $type) : new RouteCollection();
}

$collection = new RouteCollection();
$collection->addResource(new DirectoryResource($dir, '/\.php$/'));
Expand Down Expand Up @@ -74,16 +76,18 @@ function (\SplFileInfo $current) {
*/
public function supports($resource, $type = null)
{
if (!is_string($resource)) {
if ('annotation' === $type) {
return true;
}

if ($type || !is_string($resource)) {
return false;
}

try {
$path = $this->locator->locate($resource);
return is_dir($this->locator->locate($resource));
} catch (\Exception $e) {
return false;
}

return is_dir($path) && (!$type || 'annotation' === $type);
}
}
Expand Up @@ -69,6 +69,24 @@ public function testSupports()
$this->assertFalse($this->loader->supports($fixturesDir, 'foo'), '->supports() checks the resource type if specified');
}

public function testItSupportsAnyAnnotation()
{
$this->assertTrue($this->loader->supports(__DIR__.'/../Fixtures/even-with-not-existing-folder', 'annotation'));
}

public function testLoadFileIfLocatedResourceIsFile()
{
$this->reader->expects($this->exactly(1))->method('getClassAnnotation');

$this->reader
->expects($this->any())
->method('getMethodAnnotations')
->will($this->returnValue(array()))
;

$this->loader->load(__DIR__.'/../Fixtures/AnnotatedClasses/FooClass.php');
}

private function expectAnnotationsToBeReadFrom(array $classes)
{
$this->reader->expects($this->exactly(count($classes)))
Expand Down

0 comments on commit db979e8

Please sign in to comment.