From 6e69a357897b5a3b5b888baa7cd3dd4a58c27a41 Mon Sep 17 00:00:00 2001 From: Tim Plunkett Date: Mon, 25 Aug 2014 14:28:38 -0700 Subject: [PATCH 1/4] Allow ChainRouter::match() to use the correct exception message when a RequestMatcherInterface router is used. --- ChainRouter.php | 7 +++++-- Tests/Routing/ChainRouterTest.php | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ChainRouter.php b/ChainRouter.php index 82ab9536..0d49e5c0 100644 --- a/ChainRouter.php +++ b/ChainRouter.php @@ -170,10 +170,13 @@ private function doMatch($url, Request $request = null) // matching requests is more powerful than matching URLs only, so try that first if ($router instanceof RequestMatcherInterface) { if (null === $request) { - $request = Request::create($url); + $request_for_matching = Request::create($url); + } + else { + $request_for_matching = $request; } - return $router->matchRequest($request); + return $router->matchRequest($request_for_matching); } // every router implements the match method return $router->match($url); diff --git a/Tests/Routing/ChainRouterTest.php b/Tests/Routing/ChainRouterTest.php index 63a1fbf2..9a24523e 100644 --- a/Tests/Routing/ChainRouterTest.php +++ b/Tests/Routing/ChainRouterTest.php @@ -385,6 +385,32 @@ public function testMatchRequestNotFound() $this->router->matchRequest(Request::create('/test')); } + /** + * Call match on ChainRouter that has RequestMatcher in the chain. + * + * @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException + * @expectedExceptionMessage None of the routers in the chain matched url '/test' + */ + public function testMatchWithRequestMatchersNotFound() + { + $url = '/test'; + $request = Request::create('/test'); + + $high = $this->getMock('Symfony\Cmf\Component\Routing\Tests\Routing\RequestMatcher'); + + $high + ->expects($this->once()) + ->method('matchRequest') + ->with($request) + ->will($this->throwException(new \Symfony\Component\Routing\Exception\ResourceNotFoundException)) + ; + + $this->router->add($high, 20); + + $result = $this->router->match($url); + $this->assertEquals(array('test'), $result); + } + /** * If any of the routers throws a not allowed exception and no other matches, we need to see this * From 574266f98ea78403116e5b91f5e79b39cecbfc14 Mon Sep 17 00:00:00 2001 From: Tim Plunkett Date: Mon, 25 Aug 2014 14:53:47 -0700 Subject: [PATCH 2/4] Remove misleading assertion. --- Tests/Routing/ChainRouterTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/Routing/ChainRouterTest.php b/Tests/Routing/ChainRouterTest.php index 9a24523e..5671576d 100644 --- a/Tests/Routing/ChainRouterTest.php +++ b/Tests/Routing/ChainRouterTest.php @@ -407,8 +407,7 @@ public function testMatchWithRequestMatchersNotFound() $this->router->add($high, 20); - $result = $this->router->match($url); - $this->assertEquals(array('test'), $result); + $this->router->match($url); } /** From 3518d67e7fced9d6dfdf8c52c59a258308cad9a2 Mon Sep 17 00:00:00 2001 From: Tim Plunkett Date: Mon, 25 Aug 2014 23:18:21 -0700 Subject: [PATCH 3/4] Update for coding standards. --- ChainRouter.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ChainRouter.php b/ChainRouter.php index 0d49e5c0..f9c49f5f 100644 --- a/ChainRouter.php +++ b/ChainRouter.php @@ -170,13 +170,12 @@ private function doMatch($url, Request $request = null) // matching requests is more powerful than matching URLs only, so try that first if ($router instanceof RequestMatcherInterface) { if (null === $request) { - $request_for_matching = Request::create($url); - } - else { - $request_for_matching = $request; + $requestForMatching = Request::create($url); + } else { + $requestForMatching = $request; } - return $router->matchRequest($request_for_matching); + return $router->matchRequest($requestForMatching); } // every router implements the match method return $router->match($url); From c15199386bfdef0e1d93e18e6508e572c7f4d95a Mon Sep 17 00:00:00 2001 From: Tim Plunkett Date: Mon, 25 Aug 2014 23:33:54 -0700 Subject: [PATCH 4/4] Move local $requestForMatching variable initialization outside loop. --- ChainRouter.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ChainRouter.php b/ChainRouter.php index f9c49f5f..a544bb0e 100644 --- a/ChainRouter.php +++ b/ChainRouter.php @@ -164,15 +164,14 @@ private function doMatch($url, Request $request = null) { $methodNotAllowed = null; + $requestForMatching = $request; foreach ($this->all() as $router) { try { // the request/url match logic is the same as in Symfony/Component/HttpKernel/EventListener/RouterListener.php // matching requests is more powerful than matching URLs only, so try that first if ($router instanceof RequestMatcherInterface) { - if (null === $request) { + if (empty($requestForMatching)) { $requestForMatching = Request::create($url); - } else { - $requestForMatching = $request; } return $router->matchRequest($requestForMatching);