Merge pull request #120 from symfony-cmf/deprecate-dynamicrouter-match

deprecate DynamicRouter::match and cleanup interface usage

CHANGELOG.md

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+* **2014-09-29**: ChainRouter does not require a RouterInterface, as a
+  RequestMatcher and UrlGenerator is fine too. Fixed chain router interface to
+  not force a RouterInterface.
+* **2014-09-29**: Deprecated DynamicRouter::match in favor of matchRequest.
+
 1.3.0-RC1
 ---------
 

ChainRouter.php

@@ -12,6 +12,7 @@
 namespace Symfony\Cmf\Component\Routing;
 
 use Symfony\Component\Routing\RouterInterface;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
 use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\RequestContextAwareInterface;
@@ -24,17 +25,15 @@
 use Psr\Log\LoggerInterface;
 
 /**
- * ChainRouter
- *
- * Allows access to a lot of different routers.
+ * The ChainRouter allows to combine several routers to try in a defined order.
  *
  * @author Henrik Bjornskov <[email protected]>
  * @author Magnus Nordlander <[email protected]>
  */
 class ChainRouter implements ChainRouterInterface, WarmableInterface
 {
     /**
-     * @var \Symfony\Component\Routing\RequestContext
+     * @var RequestContext
      */
     private $context;
 
@@ -45,17 +44,17 @@ class ChainRouter implements ChainRouterInterface, WarmableInterface
     private $routers = array();
 
     /**
-     * @var \Symfony\Component\Routing\RouterInterface[] Array of routers, sorted by priority
+     * @var RouterInterface[] Array of routers, sorted by priority
      */
     private $sortedRouters;
 
     /**
-     * @var \Symfony\Component\Routing\RouteCollection
+     * @var RouteCollection
      */
     private $routeCollection;
 
     /**
-     * @var null|\Psr\Log\LoggerInterface
+     * @var null|LoggerInterface
      */
     protected $logger;
 
@@ -78,8 +77,13 @@ 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();
         }
@@ -159,6 +163,10 @@ public function matchRequest(Request $request)
      *
      * @param string  $url
      * @param Request $request
+     *
+     * @return array An array of parameters
+     *
+     * @throws ResourceNotFoundException If no router matched.
      */
     private function doMatch($url, Request $request = null)
     {
@@ -208,15 +216,14 @@ 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;
             }
 

ChainRouterInterface.php

@@ -15,27 +15,25 @@
 use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
 
 /**
- * ChainRouterInterface
+ * Interface for a router that proxies routing to other routers.
  *
- * Allows access to a lot of different routers.
- *
- * @author Henrik Bjornskov <[email protected]>
- * @author Magnus Nordlander <[email protected]>
+ * @author Daniel Wehner <[email protected]>
  */
 interface ChainRouterInterface extends RouterInterface, RequestMatcherInterface
 {
     /**
-     * Add a Router to the index
+     * Add a Router to the index.
      *
-     * @param RouterInterface $router   The router instance
+     * @param RouterInterface $router   The router instance. Instead of RouterInterface, may also
+     *                                  be RequestMatcherInterface and UrlGeneratorInterface.
      * @param integer         $priority The priority
      */
-    public function add(RouterInterface $router, $priority = 0);
+    public function add($router, $priority = 0);
 
     /**
      * Sorts the routers and flattens them.
      *
-     * @return RouterInterface[]
+     * @return RouterInterface[] or RequestMatcherInterface and UrlGeneratorInterface.
      */
     public function all();
 }

ChainedRouterInterface.php

@@ -14,7 +14,7 @@
 use Symfony\Component\Routing\RouterInterface;
 
 /**
- * Interface to combine the VersatileGeneratorInterface with the RouterInterface
+ * Interface to combine the VersatileGeneratorInterface with the RouterInterface.
  */
 interface ChainedRouterInterface extends RouterInterface, VersatileGeneratorInterface
 {

DynamicRouter.php

@@ -198,6 +198,7 @@ public function supports($name)
      * @throws MethodNotAllowedException If the resource was found but the
      *                                   request method is not allowed
      *
+     * @deprecated Use matchRequest exclusively to avoid problems. This method will be removed in version 2.0
      * @api
      */
     public function match($pathinfo)

Tests/Routing/ChainRouterTest.php

@@ -11,9 +11,12 @@
 
 namespace Symfony\Cmf\Component\Routing\Tests\Routing;
 
+use Symfony\Cmf\Component\Routing\VersatileGeneratorInterface;
+use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface;
 use Symfony\Component\Routing\Exception\MethodNotAllowedException;
 use Symfony\Component\Routing\Exception\ResourceNotFoundException;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
+use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
 use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\HttpFoundation\Request;
@@ -569,7 +572,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\VersatileRouter');
         $chainedRouter
             ->expects($this->once())
             ->method('supports')
@@ -597,7 +600,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\VersatileRouter');
 
         $defaultRouter
             ->expects($this->never())
@@ -683,7 +686,7 @@ public function testRouteCollection()
     public function testSupport()
     {
 
-        $router = $this->getMock('Symfony\Cmf\Component\Routing\ChainedRouterInterface');
+        $router = $this->getMock('Symfony\Cmf\Component\Routing\Tests\Routing\VersatileRouter');
         $router
             ->expects($this->once())
             ->method('supports')
@@ -714,10 +717,14 @@ protected function createRouterMocks()
     }
 }
 
-abstract class WarmableRouterMock implements \Symfony\Component\Routing\RouterInterface, \Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface
+abstract class WarmableRouterMock implements RouterInterface, WarmableInterface
 {
 }
 
-abstract class RequestMatcher implements \Symfony\Component\Routing\RouterInterface, \Symfony\Component\Routing\Matcher\RequestMatcherInterface
+abstract class RequestMatcher implements RouterInterface, RequestMatcherInterface
+{
+}
+
+abstract class VersatileRouter implements VersatileGeneratorInterface, RequestMatcherInterface
 {
 }

composer.json

@@ -19,7 +19,7 @@
         "psr/log": "~1.0"
     },
     "require-dev": {
-        "symfony/dependency-injection": "~2.0",
+        "symfony/dependency-injection": "~2.0@stable",
         "symfony/config": "~2.2",
         "symfony/event-dispatcher": "~2.1"
     },