diff --git a/CHANGELOG.md b/CHANGELOG.md index edd4ea03..f72dbe10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ Changelog ========= +* **2014-09-05**: Dropped DynamicRouter::match and DynamicRouter no longer + implements RouterInterface but only RequestMatcherInterface and + UrlGeneratorInterface. The match method is redundant with matchRequest but + there was a potential information loss by re-creating the request. If you use + DynamicRouter directly, get access to the Request object or if you are + stand-alone create the request with Request::createFromGlobals(). + Deprecated ChainedRouterInterface as it adds no additional information over + VersatileGeneratorInterface. + 1.3.0-RC1 --------- diff --git a/ChainRouter.php b/ChainRouter.php index a544bb0e..f790c9ee 100644 --- a/ChainRouter.php +++ b/ChainRouter.php @@ -11,6 +11,7 @@ namespace Symfony\Cmf\Component\Routing; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RouterInterface; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext; @@ -78,8 +79,14 @@ public function getContext() /** * {@inheritdoc} */ - public function add(RouterInterface $router, $priority = 0) + public function add($router, $priority = 0) { + if (!($router instanceof RouterInterface + || $router instanceof RequestMatcherInterface + && $router instanceof UrlGeneratorInterface + )) { + throw new \InvalidArgumentException(sprintf('%s is not a valid router.', get_class($router))); + } if (empty($this->routers[$priority])) { $this->routers[$priority] = array(); } @@ -208,15 +215,13 @@ public function generate($name, $parameters = array(), $absolute = false) $debug = array(); foreach ($this->all() as $router) { - // if $router does not implement ChainedRouterInterface and $name is not a string, continue - if ($name && !$router instanceof ChainedRouterInterface) { - if (! is_string($name)) { - continue; - } + // if $router does not announce it is capable of handling non-string routes and $name is not a string, continue + if ($name && !is_string($name) && !$router instanceof VersatileGeneratorInterface) { + continue; } - // If $router implements ChainedRouterInterface but doesn't support this route name, continue - if ($router instanceof ChainedRouterInterface && !$router->supports($name)) { + // If $router is versatile and doesn't support this route name, continue + if ($router instanceof VersatileGeneratorInterface && !$router->supports($name)) { continue; } diff --git a/ChainRouterInterface.php b/ChainRouterInterface.php index a65cc9d6..b0d8cc9a 100644 --- a/ChainRouterInterface.php +++ b/ChainRouterInterface.php @@ -25,12 +25,14 @@ interface ChainRouterInterface extends RouterInterface, RequestMatcherInterface { /** - * Add a Router to the index + * Add a Router to the index. * - * @param RouterInterface $router The router instance - * @param integer $priority The priority + * The router must implement either RouterInterface or RequestMatcherInterface and UrlGeneratorInterface. + * + * @param RouterInterface|RequestMatcherInterface $router The router instance + * @param integer $priority The priority */ - public function add(RouterInterface $router, $priority = 0); + public function add($router, $priority = 0); /** * Sorts the routers and flattens them. diff --git a/ChainedRouterInterface.php b/ChainedRouterInterface.php index b0c40caa..7e33a3c9 100644 --- a/ChainedRouterInterface.php +++ b/ChainedRouterInterface.php @@ -11,11 +11,14 @@ namespace Symfony\Cmf\Component\Routing; -use Symfony\Component\Routing\RouterInterface; +use Symfony\Component\Routing\Matcher\RequestMatcherInterface; /** - * Interface to combine the VersatileGeneratorInterface with the RouterInterface + * Interface to combine the VersatileGeneratorInterface + * + * @deprecated This interface adds no value and should not be relied on. + * Better use VersatileGeneratorInterface directly. */ -interface ChainedRouterInterface extends RouterInterface, VersatileGeneratorInterface +interface ChainedRouterInterface extends VersatileGeneratorInterface { } diff --git a/DynamicRouter.php b/DynamicRouter.php index 03eaefaa..cb17cae0 100644 --- a/DynamicRouter.php +++ b/DynamicRouter.php @@ -14,7 +14,6 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\RouteCollection; -use Symfony\Component\Routing\RouterInterface; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\Matcher\UrlMatcherInterface; @@ -34,7 +33,7 @@ * @author Larry Garfield * @author David Buchmann */ -class DynamicRouter implements RouterInterface, RequestMatcherInterface, ChainedRouterInterface +class DynamicRouter implements UrlGeneratorInterface, RequestMatcherInterface, ChainedRouterInterface { /** * @var RequestMatcherInterface|UrlMatcherInterface @@ -183,45 +182,6 @@ public function supports($name) return is_string($name); } - /** - * Tries to match a URL path with a set of routes. - * - * If the matcher can not find information, it must throw one of the - * exceptions documented below. - * - * @param string $pathinfo The path info to be parsed (raw format, i.e. not - * urldecoded) - * - * @return array An array of parameters - * - * @throws ResourceNotFoundException If the resource could not be found - * @throws MethodNotAllowedException If the resource was found but the - * request method is not allowed - * - * @api - */ - public function match($pathinfo) - { - $request = Request::create($pathinfo); - if ($this->eventDispatcher) { - $event = new RouterMatchEvent(); - $this->eventDispatcher->dispatch(Events::PRE_DYNAMIC_MATCH, $event); - } - - if (! empty($this->uriFilterRegexp) && ! preg_match($this->uriFilterRegexp, $pathinfo)) { - throw new ResourceNotFoundException("$pathinfo does not match the '{$this->uriFilterRegexp}' pattern"); - } - - $matcher = $this->getMatcher(); - if (! $matcher instanceof UrlMatcherInterface) { - throw new \InvalidArgumentException('Wrong matcher type, you need to call matchRequest'); - } - - $defaults = $matcher->match($pathinfo); - - return $this->applyRouteEnhancers($defaults, $request); - } - /** * Tries to match a request with a set of routes and returns the array of * information for that route. diff --git a/Tests/Routing/ChainRouterTest.php b/Tests/Routing/ChainRouterTest.php index 89d1463e..0edbdf0b 100644 --- a/Tests/Routing/ChainRouterTest.php +++ b/Tests/Routing/ChainRouterTest.php @@ -11,6 +11,7 @@ namespace Symfony\Cmf\Component\Routing\Tests\Routing; +use Symfony\Cmf\Component\Routing\VersatileGeneratorInterface; use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Exception\RouteNotFoundException; @@ -569,7 +570,7 @@ public function testGenerateObjectNotFoundVersatile() $name = new \stdClass(); $parameters = array('test' => 'value'); - $chainedRouter = $this->getMock('Symfony\Cmf\Component\Routing\ChainedRouterInterface'); + $chainedRouter = $this->getMock('Symfony\Cmf\Component\Routing\Tests\Routing\VersatileRequestMatcher'); $chainedRouter ->expects($this->once()) ->method('supports') @@ -597,7 +598,7 @@ public function testGenerateObjectName() $parameters = array('test' => 'value'); $defaultRouter = $this->getMock('Symfony\Component\Routing\RouterInterface'); - $chainedRouter = $this->getMock('Symfony\Cmf\Component\Routing\ChainedRouterInterface'); + $chainedRouter = $this->getMock('Symfony\Cmf\Component\Routing\Tests\Routing\VersatileRequestMatcher'); $defaultRouter ->expects($this->never()) @@ -683,7 +684,7 @@ public function testRouteCollection() public function testSupport() { - $router = $this->getMock('Symfony\Cmf\Component\Routing\ChainedRouterInterface'); + $router = $this->getMock('Symfony\Cmf\Component\Routing\Tests\Routing\VersatileRequestMatcher'); $router ->expects($this->once()) ->method('supports') @@ -718,6 +719,10 @@ abstract class WarmableRouterMock implements \Symfony\Component\Routing\RouterIn { } -abstract class RequestMatcher implements \Symfony\Component\Routing\RouterInterface, \Symfony\Component\Routing\Matcher\RequestMatcherInterface +abstract class RequestMatcher implements \Symfony\Component\Routing\Generator\UrlGeneratorInterface, \Symfony\Component\Routing\Matcher\RequestMatcherInterface +{ +} + +abstract class VersatileRequestMatcher implements VersatileGeneratorInterface, \Symfony\Component\Routing\Matcher\RequestMatcherInterface { } diff --git a/Tests/Routing/DynamicRouterTest.php b/Tests/Routing/DynamicRouterTest.php index f3e24e92..b495b683 100644 --- a/Tests/Routing/DynamicRouterTest.php +++ b/Tests/Routing/DynamicRouterTest.php @@ -136,27 +136,6 @@ public function testGetMatcher() $this->assertSame($this->matcher, $matcher); } - public function testMatchUrl() - { - $routeDefaults = array('foo' => 'bar'); - $this->matcher->expects($this->once()) - ->method('match') - ->with($this->url) - ->will($this->returnValue($routeDefaults)) - ; - - $expected = array('this' => 'that'); - $this->enhancer->expects($this->once()) - ->method('enhance') - ->with($this->equalTo($routeDefaults), $this->equalTo($this->request)) - ->will($this->returnValue($expected)) - ; - - $results = $this->router->match($this->url); - - $this->assertEquals($expected, $results); - } - public function testMatchRequestWithUrlMatcher() { $routeDefaults = array('foo' => 'bar'); @@ -205,25 +184,6 @@ public function testMatchRequest() $this->assertEquals($expected, $router->matchRequest($this->request)); } - /** - * @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException - */ - public function testMatchFilter() - { - $router = new DynamicRouter($this->context, $this->matcher, $this->generator, '#/different/prefix.*#'); - $router->addRouteEnhancer($this->enhancer); - - $this->matcher->expects($this->never()) - ->method('match') - ; - - $this->enhancer->expects($this->never()) - ->method('enhance') - ; - - $router->match($this->url); - } - /** * @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException */ @@ -245,17 +205,6 @@ public function testMatchRequestFilter() $router->matchRequest($this->request); } - /** - * @expectedException \InvalidArgumentException - */ - public function testMatchUrlWithRequestMatcher() - { - $matcher = $this->buildMock('Symfony\Component\Routing\Matcher\RequestMatcherInterface', array('matchRequest', 'setContext', 'getContext')); - $router = new DynamicRouter($this->context, $matcher, $this->generator); - - $router->match($this->url); - } - /** * @expectedException \InvalidArgumentException */ @@ -282,26 +231,6 @@ public function testRouteDebugMessageNonversatile() $this->assertInternalType('string', $router->getRouteDebugMessage('test')); } - public function testEventHandler() - { - $eventDispatcher = $this->buildMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); - $router = new DynamicRouter($this->context, $this->matcher, $this->generator, '', $eventDispatcher); - - $eventDispatcher->expects($this->once()) - ->method('dispatch') - ->with(Events::PRE_DYNAMIC_MATCH, $this->equalTo(new RouterMatchEvent())) - ; - - $routeDefaults = array('foo' => 'bar'); - $this->matcher->expects($this->once()) - ->method('match') - ->with($this->url) - ->will($this->returnValue($routeDefaults)) - ; - - $this->assertEquals($routeDefaults, $router->match($this->url)); - } - public function testEventHandlerRequest() { $eventDispatcher = $this->buildMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');