bug #25113 [Routing] Fix "config-file-relative" annotation loader resources (nicolas-grekas, sroze)

nicolas-grekas · nicolas-grekas · commit db979e88e8c2 · 2017-11-24T12:32:34.000+01:00
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
diff --git a/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php
@@ -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$/'));
@@ -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);
     }
 }
diff --git a/src/Symfony/Component/Routing/Tests/Loader/AnnotationDirectoryLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/AnnotationDirectoryLoaderTest.php
@@ -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)))

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,9 @@ class AnnotationDirectoryLoader extends AnnotationFileLoader`
`34`	`34`	`*/`
`35`	`35`	`public function load($path, $type = null)`
`36`	`36`	`{`
`37`		`- $dir = $this->locator->locate($path);`
	`37`	`+ if (!is_dir($dir = $this->locator->locate($path))) {`
	`38`	`+ return parent::supports($path, $type) ? parent::load($path, $type) : new RouteCollection();`
	`39`	`+ }`
`38`	`40`
`39`	`41`	`$collection = new RouteCollection();`
`40`	`42`	`$collection->addResource(new DirectoryResource($dir, '/\.php$/'));`
`@@ -74,16 +76,18 @@ function (\SplFileInfo $current) {`
`74`	`76`	`*/`
`75`	`77`	`public function supports($resource, $type = null)`
`76`	`78`	`{`
`77`		`- if (!is_string($resource)) {`
	`79`	`+ if ('annotation' === $type) {`
	`80`	`+ return true;`
	`81`	`+ }`
	`82`	`+`
	`83`	`+ if ($type \|\| !is_string($resource)) {`
`78`	`84`	`return false;`
`79`	`85`	`}`
`80`	`86`
`81`	`87`	`try {`
`82`		`- $path = $this->locator->locate($resource);`
	`88`	`+ return is_dir($this->locator->locate($resource));`
`83`	`89`	`} catch (\Exception $e) {`
`84`	`90`	`return false;`
`85`	`91`	`}`
`86`		`-`
`87`		`- return is_dir($path) && (!$type \|\| 'annotation' === $type);`
`88`	`92`	`}`
`89`	`93`	`}`