Skip to content

Commit 1787992

Browse files
fabpotfabpot
committed
merged branch niklasf/request-matcher-interface (PR #4363)
Commits ------- 2277500 [Routing][HttpKernel] Add RequestMatcherInterface. Discussion ---------- [Routing][HttpKernel] Add RequestMatcherInterface. First try at implementing #4351. Bug fix: no Feature addition: yes Backwards compatibility break: no Fixes the following tickets: - Todo: - License of the code: MIT --------------------------------------------------------------------------- by travisbot at 2012-05-21T19:37:07Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1392716) (merged 457496db into ea33d4d). --------------------------------------------------------------------------- by travisbot at 2012-05-21T19:47:51Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1392939) (merged 78effa98 into ea33d4d). --------------------------------------------------------------------------- by everzet at 2012-05-21T20:17:03Z No tests? --------------------------------------------------------------------------- by travisbot at 2012-05-21T20:18:18Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1393392) (merged 6564fb6a into ea33d4d). --------------------------------------------------------------------------- by schmittjoh at 2012-05-21T20:35:14Z You need to remove the type-hint from the constructor, and probably add an exception instead where the matching methods are called to ensure that either ``UrlMatcherInterface``, or ``RequestMatcherInterface`` were passed. Alternatively, you could also add such a check to the constructor. --------------------------------------------------------------------------- by fabpot at 2012-05-22T06:52:01Z related to #4020 --------------------------------------------------------------------------- by niklasf at 2012-05-25T11:11:45Z Reverted the changes to UrlMatcher.php. --------------------------------------------------------------------------- by fabpot at 2012-05-25T12:46:06Z @niklasf: it looks good now except for the listener constructor (see @schmittjoh suggestion above). Can you fix that and add some unit tests to ensure that everything works as expected? Thanks. --------------------------------------------------------------------------- by stof at 2012-05-25T12:52:59Z Another solution could be to make the ``RequestMatcherInterface`` extend the ``MatcherInterface`` to keep the typehint in the constructor --------------------------------------------------------------------------- by fabpot at 2012-05-25T13:52:26Z I thought about that as well, but that does not make sense. --------------------------------------------------------------------------- by stof at 2012-05-25T14:12:19Z Well, the RouterInterface extends UrlMatcherInterface anyway (and it should stay this way as it would be a huge BC break) so I guess most people will implement both interfaces when implementing the RquestMatcherInterface --------------------------------------------------------------------------- by travisbot at 2012-05-25T15:26:22Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1433963) (merged 8f36204c into ff4d446). --------------------------------------------------------------------------- by travisbot at 2012-05-25T15:33:13Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1433996) (merged 6d2f2cd9 into ff4d446). --------------------------------------------------------------------------- by travisbot at 2012-05-25T15:39:01Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1434060) (merged 3c1d89e2 into ff4d446). --------------------------------------------------------------------------- by travisbot at 2012-05-25T22:06:34Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1437398) (merged 3ab997c1 into ff4d446). --------------------------------------------------------------------------- by travisbot at 2012-05-25T22:06:47Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1437385) (merged d8c0e387 into ff4d446). --------------------------------------------------------------------------- by fabpot at 2012-05-26T06:41:31Z Can you add a note in the CHANGELOG of the component? --------------------------------------------------------------------------- by travisbot at 2012-05-26T08:12:40Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1440435) (merged c7458733 into 9e95199). --------------------------------------------------------------------------- by niklasf at 2012-05-26T08:14:41Z @fabpot: Sorry, not sure how: Under 2.1.0 or in a new section? As the first or last entry of the list? --------------------------------------------------------------------------- by stof at 2012-05-26T10:20:23Z @niklasf the new interface should be mentioned in the changelog of the Routing component --------------------------------------------------------------------------- by travisbot at 2012-05-26T10:30:06Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1440837) (merged 34ea86a9 into 9e95199). --------------------------------------------------------------------------- by niklasf at 2012-06-02T15:27:01Z Ah ... so there were two pitfalls: - PHPUnit clones the arguments of mocked functions. So they wouldn't equal. - createGetResponseEventForUri() disables routing on purpose. So not using that helper, now. Tests should be passing. --------------------------------------------------------------------------- by travisbot at 2012-06-02T15:30:28Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1509054) (merged 7fab0118 into 1541fe2).
2 parents fc40e53 + 2277500 commit 1787992

File tree

4 files changed

+85
-5
lines changed

4 files changed

+85
-5
lines changed

‎src/Symfony/Component/HttpKernel/EventListener/RouterListener.php

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
2121
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
2222
use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
23+
use Symfony\Component\Routing\Matcher\RequestMatcherInterface;
2324
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
2425

2526
/**
@@ -29,12 +30,16 @@
2930
*/
3031
class RouterListener implements EventSubscriberInterface
3132
{
32-
private $urlMatcher;
33+
private $matcher;
3334
private $logger;
3435

35-
public function __construct(UrlMatcherInterface $urlMatcher, LoggerInterface $logger = null)
36+
public function __construct($matcher, LoggerInterface $logger = null)
3637
{
37-
$this->urlMatcher = $urlMatcher;
38+
if (!$matcher instanceof UrlMatcherInterface && !$matcher instanceof RequestMatcherInterface) {
39+
throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
40+
}
41+
42+
$this->matcher = $matcher;
3843
$this->logger = $logger;
3944
}
4045

@@ -43,7 +48,7 @@ public function onKernelRequest(GetResponseEvent $event)
4348
$request = $event->getRequest();
4449

4550
if (HttpKernelInterface::MASTER_REQUEST === $event->getRequestType()) {
46-
$this->urlMatcher->getContext()->fromRequest($request);
51+
$this->matcher->getContext()->fromRequest($request);
4752
}
4853

4954
if ($request->attributes->has('_controller')) {
@@ -53,7 +58,12 @@ public function onKernelRequest(GetResponseEvent $event)
5358

5459
// add attributes based on the path info (routing)
5560
try {
56-
$parameters = $this->urlMatcher->match($request->getPathInfo());
61+
// matching requests is more powerful than matching URLs only, so try that first
62+
if ($this->matcher instanceof RequestMatcherInterface) {
63+
$parameters = $this->matcher->matchRequest($request);
64+
} else {
65+
$parameters = $this->matcher->match($request->getPathInfo());
66+
}
5767

5868
if (null !== $this->logger) {
5969
$this->logger->info(sprintf('Matched route "%s" (parameters: %s)', $parameters['_route'], $this->parametersToString($parameters)));

‎src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,33 @@ private function createGetResponseEventForUri($uri)
7777

7878
return new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
7979
}
80+
81+
/**
82+
* @expectedException \InvalidArgumentException
83+
*/
84+
public function testInvalidMatcher()
85+
{
86+
new RouterListener(new \stdClass());
87+
}
88+
89+
public function testRequestMatcher()
90+
{
91+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
92+
$request = Request::create('http://localhost/');
93+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
94+
95+
$context = new RequestContext();
96+
97+
$requestMatcher = $this->getMock('Symfony\Component\Routing\Matcher\RequestMatcherInterface');
98+
$requestMatcher->expects($this->any())
99+
->method('getContext')
100+
->will($this->returnValue($context));
101+
$requestMatcher->expects($this->once())
102+
->method('matchRequest')
103+
->with($this->isInstanceOf('Symfony\Component\HttpFoundation\Request'))
104+
->will($this->returnValue(array()));
105+
106+
$listener = new RouterListener($requestMatcher);
107+
$listener->onKernelRequest($event);
108+
}
80109
}

‎src/Symfony/Component/Routing/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
2.1.0
55
-----
66

7+
* added RequestMatcherInterface
78
* added RequestContext::fromRequest()
89
* the UrlMatcher does not throw a \LogicException anymore when the required
910
scheme is not the current one

‎src/Symfony/Component/Routing/Matcher/RequestMatcherInterface.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Routing\Matcher;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\Routing\RequestContextAwareInterface;
16+
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
17+
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
18+
19+
/**
20+
* UrlMatcherInterface is the interface that all URL matcher classes must implement.
21+
*
22+
* @author Fabien Potencier <fabien@symfony.com>
23+
*/
24+
interface RequestMatcherInterface extends RequestContextAwareInterface
25+
{
26+
/**
27+
* Tries to match a request with a set of routes.
28+
*
29+
* If the matcher can not find information, it must throw one of the exceptions documented
30+
* below.
31+
*
32+
* @param Request $request The request to match
33+
*
34+
* @return array An array of parameters
35+
*
36+
* @throws ResourceNotFoundException If no matching resource could be found
37+
* @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed
38+
*/
39+
function matchRequest(Request $request);
40+
}

0 commit comments

Comments
 (0)