Skip to content

Commit

Permalink
bug #29274 [Routing] Remove duplicate schemes and methods for invokab…
Browse files Browse the repository at this point in the history
…le controllers (claudusd)

This PR was merged into the 3.4 branch.

Discussion
----------

[Routing] Remove duplicate schemes and methods for invokable controllers

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29225
| License       | MIT

This PR backport for 3.4 branch the same issue than the PR #29225.

I add a test to check the fix when annotation are on the class and rename another one when the route annotation is on the invoke method.

Commits
-------

640ccdf [Routing] Remove duplicate schemes and methods for invokable controllers
  • Loading branch information
fabpot committed Nov 24, 2018
2 parents d713671 + 640ccdf commit 812a878
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 15 deletions.
31 changes: 17 additions & 14 deletions src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php
Expand Up @@ -120,11 +120,9 @@ public function load($class, $type = null)
}

if (0 === $collection->count() && $class->hasMethod('__invoke')) {
$globals = $this->resetGlobals();
foreach ($this->reader->getClassAnnotations($class) as $annot) {
if ($annot instanceof $this->routeAnnotationClass) {
$globals['path'] = '';
$globals['name'] = '';

$this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
}
}
Expand Down Expand Up @@ -212,17 +210,7 @@ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMetho

protected function getGlobals(\ReflectionClass $class)
{
$globals = array(
'path' => '',
'requirements' => array(),
'options' => array(),
'defaults' => array(),
'schemes' => array(),
'methods' => array(),
'host' => '',
'condition' => '',
'name' => '',
);
$globals = $this->resetGlobals();

if ($annot = $this->reader->getClassAnnotation($class, $this->routeAnnotationClass)) {
if (null !== $annot->getName()) {
Expand Down Expand Up @@ -265,6 +253,21 @@ protected function getGlobals(\ReflectionClass $class)
return $globals;
}

private function resetGlobals()
{
return array(
'path' => '',
'requirements' => array(),
'options' => array(),
'defaults' => array(),
'schemes' => array(),
'methods' => array(),
'host' => '',
'condition' => '',
'name' => '',
);
}

protected function createRoute($path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition)
{
return new Route($path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition);
Expand Down
Expand Up @@ -181,7 +181,7 @@ public function testClassRouteLoad()
$this->assertEquals(array_merge($classRouteData['methods'], $methodRouteData['methods']), $route->getMethods(), '->load merges class and method route methods');
}

public function testInvokableClassRouteLoad()
public function testInvokableClassRouteLoadWithMethodAnnotation()
{
$classRouteData = array(
'name' => 'route1',
Expand Down Expand Up @@ -209,6 +209,41 @@ public function testInvokableClassRouteLoad()
$this->assertEquals($classRouteData['methods'], $route->getMethods(), '->load preserves class route methods');
}

public function testInvokableClassRouteLoadWithClassAnnotation()
{
$classRouteData = array(
'name' => 'route1',
'path' => '/',
'schemes' => array('https'),
'methods' => array('GET'),
);

$this->reader
->expects($this->exactly(1))
->method('getClassAnnotation')
->will($this->returnValue($this->getAnnotatedRoute($classRouteData)))
;

$this->reader
->expects($this->exactly(1))
->method('getClassAnnotations')
->will($this->returnValue(array($this->getAnnotatedRoute($classRouteData))))
;

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

$routeCollection = $this->loader->load('Symfony\Component\Routing\Tests\Fixtures\AnnotatedClasses\BazClass');
$route = $routeCollection->get($classRouteData['name']);

$this->assertSame($classRouteData['path'], $route->getPath(), '->load preserves class route path');
$this->assertEquals($classRouteData['schemes'], $route->getSchemes(), '->load preserves class route schemes');
$this->assertEquals($classRouteData['methods'], $route->getMethods(), '->load preserves class route methods');
}

public function testInvokableClassMultipleRouteLoad()
{
$classRouteData1 = array(
Expand Down

0 comments on commit 812a878

Please sign in to comment.