From 946ccb6bd287c46bccf2f4f94a0ddded33e1c0c6 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 30 Aug 2011 20:57:43 +0200 Subject: [PATCH] [Routing] fixed annotation loaders for abstract classes, added more unit tests --- .../Routing/Loader/AnnotationClassLoader.php | 4 +++ .../Loader/AnnotationDirectoryLoader.php | 5 +++ .../AnnotatedClasses/AbstractClass.php | 7 ++++ .../Fixtures/AnnotatedClasses/FooClass.php | 7 ++++ .../Loader/AbstractAnnotationLoaderTest.php | 36 +++++++++++++++++++ .../Loader/AnnotationClassLoaderTest.php | 26 ++++++++++---- .../Loader/AnnotationDirectoryLoaderTest.php | 34 ++++++++++++------ .../Loader/AnnotationFileLoaderTest.php | 35 ++++++++++++------ 8 files changed, 125 insertions(+), 29 deletions(-) create mode 100644 tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/AbstractClass.php create mode 100644 tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/FooClass.php create mode 100644 tests/Symfony/Tests/Component/Routing/Loader/AbstractAnnotationLoaderTest.php diff --git a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php index eb460567898c..8a2c5fe0e993 100644 --- a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php +++ b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php @@ -105,6 +105,10 @@ public function load($class, $type = null) ); $class = new \ReflectionClass($class); + if ($class->isAbstract()) { + throw new \InvalidArgumentException(sprintf('Annotations from class "%s" cannot be read as it is abstract.', $class)); + } + if ($annot = $this->reader->getClassAnnotation($class, $this->routeAnnotationClass)) { if (null !== $annot->getPattern()) { $globals['pattern'] = $annot->getPattern(); diff --git a/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php index 57b6be04556f..8097cd67f96c 100644 --- a/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php +++ b/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php @@ -44,6 +44,11 @@ public function load($path, $type = null) } if ($class = $this->findClass($file)) { + $refl = new \ReflectionClass($class); + if ($refl->isAbstract()) { + continue; + } + $collection->addCollection($this->loader->load($class, $type)); } } diff --git a/tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/AbstractClass.php b/tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/AbstractClass.php new file mode 100644 index 000000000000..33e7e497773a --- /dev/null +++ b/tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/AbstractClass.php @@ -0,0 +1,7 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Tests\Component\Routing\Loader; + +use Symfony\Component\Config\Loader\LoaderResolver; +use Symfony\Component\Routing\Loader\AnnotationFileLoader; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; + +abstract class AbstractAnnotationLoaderTest extends \PHPUnit_Framework_TestCase +{ + public function getReader() + { + return $this->getMockBuilder('Doctrine\Common\Annotations\Reader') + ->disableOriginalConstructor() + ->getMock() + ; + } + + public function getClassLoader($reader) + { + return $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader') + ->setConstructorArgs(array($reader)) + ->getMockForAbstractClass() + ; + } +} diff --git a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationClassLoaderTest.php b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationClassLoaderTest.php index 96794462160b..cdf16b104dc9 100644 --- a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationClassLoaderTest.php +++ b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationClassLoaderTest.php @@ -16,20 +16,32 @@ use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; -class AnnotationClassLoaderTest extends \PHPUnit_Framework_TestCase +require_once __DIR__.'/../Fixtures/AnnotatedClasses/AbstractClass.php'; +require_once __DIR__.'/AbstractAnnotationLoaderTest.php'; + +class AnnotationClassLoaderTest extends AbstractAnnotationLoaderTest { protected $loader; - protected function setUp() + public function setUp() + { + $this->loader = $this->getClassLoader($this->getReader()); + } + + /** + * @expectedException \InvalidArgumentException + */ + public function testLoadMissingClass() { - $this->loader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader') - ->disableOriginalConstructor() - ->getMockForAbstractClass(); + $this->loader->load('MissingClass'); } - protected function tearDown() + /** + * @expectedException \InvalidArgumentException + */ + public function testLoadAbstractClass() { - $this->loader = null; + $this->loader->load('Symfony\Tests\Component\Routing\Fixtures\AnnotatedClasses\AbstractClass'); } /** diff --git a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationDirectoryLoaderTest.php b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationDirectoryLoaderTest.php index 7af085473fa1..c5fe2988e49f 100644 --- a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationDirectoryLoaderTest.php +++ b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationDirectoryLoaderTest.php @@ -17,25 +17,37 @@ use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Config\FileLocator; -class AnnotationDirectoryLoaderTest extends \PHPUnit_Framework_TestCase +require_once __DIR__.'/AbstractAnnotationLoaderTest.php'; + +class AnnotationDirectoryLoaderTest extends AbstractAnnotationLoaderTest { + protected $loader; + protected $reader; + + public function setUp() + { + $this->reader = $this->getReader(); + $this->loader = new AnnotationDirectoryLoader(new FileLocator(), $this->getClassLoader($this->reader)); + } + + public function testLoad() + { + $this->reader->expects($this->once())->method('getClassAnnotation'); + + $this->loader->load(__DIR__.'/../Fixtures/AnnotatedClasses'); + } + /** * @covers Symfony\Component\Routing\Loader\AnnotationDirectoryLoader::supports */ public function testSupports() { - $annotationClassLoader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader') - ->disableOriginalConstructor() - ->getMockForAbstractClass(); - - $loader = new AnnotationDirectoryLoader(new FileLocator(), $annotationClassLoader); - $fixturesDir = __DIR__.'/../Fixtures'; - $this->assertTrue($loader->supports($fixturesDir), '->supports() returns true if the resource is loadable'); - $this->assertFalse($loader->supports('foo.foo'), '->supports() returns true if the resource is loadable'); + $this->assertTrue($this->loader->supports($fixturesDir), '->supports() returns true if the resource is loadable'); + $this->assertFalse($this->loader->supports('foo.foo'), '->supports() returns true if the resource is loadable'); - $this->assertTrue($loader->supports($fixturesDir, 'annotation'), '->supports() checks the resource type if specified'); - $this->assertFalse($loader->supports($fixturesDir, 'foo'), '->supports() checks the resource type if specified'); + $this->assertTrue($this->loader->supports($fixturesDir, 'annotation'), '->supports() checks the resource type if specified'); + $this->assertFalse($this->loader->supports($fixturesDir, 'foo'), '->supports() checks the resource type if specified'); } } diff --git a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationFileLoaderTest.php b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationFileLoaderTest.php index 214087e73df2..809bdf0929e9 100644 --- a/tests/Symfony/Tests/Component/Routing/Loader/AnnotationFileLoaderTest.php +++ b/tests/Symfony/Tests/Component/Routing/Loader/AnnotationFileLoaderTest.php @@ -15,26 +15,39 @@ use Symfony\Component\Routing\Loader\AnnotationFileLoader; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Config\FileLocator; -class AnnotationFileLoaderTest extends \PHPUnit_Framework_TestCase +require_once __DIR__.'/AbstractAnnotationLoaderTest.php'; + +class AnnotationFileLoaderTest extends AbstractAnnotationLoaderTest { + protected $loader; + protected $reader; + + public function setUp() + { + $this->reader = $this->getReader(); + $this->loader = new AnnotationFileLoader(new FileLocator(), $this->getClassLoader($this->reader)); + } + + public function testLoad() + { + $this->reader->expects($this->once())->method('getClassAnnotation'); + + $this->loader->load(__DIR__.'/../Fixtures/AnnotatedClasses/FooClass.php'); + } + /** * @covers Symfony\Component\Routing\Loader\AnnotationFileLoader::supports */ public function testSupports() { - $annotationClassLoader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader') - ->disableOriginalConstructor() - ->getMockForAbstractClass(); - - $loader = new AnnotationFileLoader($this->getMock('Symfony\Component\Config\FileLocator'), $annotationClassLoader); - $fixture = __DIR__.'/../Fixtures/annotated.php'; - $this->assertTrue($loader->supports($fixture), '->supports() returns true if the resource is loadable'); - $this->assertFalse($loader->supports('foo.foo'), '->supports() returns true if the resource is loadable'); + $this->assertTrue($this->loader->supports($fixture), '->supports() returns true if the resource is loadable'); + $this->assertFalse($this->loader->supports('foo.foo'), '->supports() returns true if the resource is loadable'); - $this->assertTrue($loader->supports($fixture, 'annotation'), '->supports() checks the resource type if specified'); - $this->assertFalse($loader->supports($fixture, 'foo'), '->supports() checks the resource type if specified'); + $this->assertTrue($this->loader->supports($fixture, 'annotation'), '->supports() checks the resource type if specified'); + $this->assertFalse($this->loader->supports($fixture, 'foo'), '->supports() checks the resource type if specified'); } }