8000 Merge pull request #48 from symfony-cmf/remove_content_parameter_support · dawehner/symfony@5a1c0d1 · GitHub
[go: up one dir, main page]

Skip to content

Commit 5a1c0d1

Browse files
committed
Merge pull request symfony#48 from symfony-cmf/remove_content_parameter_support
removed support to pass in a content instance via a 'content' parameter
2 parents 3c1b600 + a4bf90b commit 5a1c0d1

9 files changed

+265
-60
lines changed

ChainRouter.php

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function add(RouterInterface $router, $priority = 0)
8484
/**
8585
* Sorts the routers and flattens them.
8686
*
87-
* @return array
87+
* @return RouterInterface[]
8888
*/
8989
public function all()
9090
{
@@ -133,26 +133,7 @@ protected function sortRouters()
133133
*/
134134
public function match($url)
135135
{
136-
$methodNotAllowed = null;
137-
138-
/** @var $router RouterInterface */
139-
foreach ($this->all() as $router) {
140-
try {
141-
return $router->match($url);
142-
} catch (ResourceNotFoundException $e) {
143-
if ($this->logger) {
144-
$this->logger->info('Router '.get_class($router).' was not able to match, message "'.$e->getMessage().'"');
145-
}
146-
// Needs special care
147-
} catch (MethodNotAllowedException $e) {
148-
if ($this->logger) {
149-
$this->logger->info('Router '.get_class($router).' throws MethodNotAllowedException with message "'.$e->getMessage().'"');
150-
}
151-
$methodNotAllowed = $e;
152-
}
153-
}
154-
155-
throw $methodNotAllowed ?: new ResourceNotFoundException("None of the routers in the chain matched '$url'");
136+
return $this->doMatch($url);
156137
}
157138

158139
/**
@@ -161,6 +142,20 @@ public function match($url)
161142
* Loops through all routes and tries to match the passed request.
162143
*/
163144
public function matchRequest(Request $request)
145+
{
146+
return $this->doMatch($request->getPathInfo(), $request);
147+
}
148+
149+
/**
150+
* Loops through all routers and tries to match the passed request or url.
151+
*
152+
* At least the url must be provided, if a request is additionally provided
153+
* the request takes precedence.
154+
*
155+
* @param string $url
156+
* @param Request $request
157+
*/
158+
private function doMatch($url, Request $request = null)
164159
{
165160
$methodNotAllowed = null;
166161

@@ -171,7 +166,8 @@ public function matchRequest(Request $request)
171166
if ($router instanceof RequestMatcherInterface) {
172167
return $router->matchRequest($request);
173168
}
174-
return $router->match($request->getPathInfo());
169+
// every router implements the match method
170+
return $router->match($url);
175171
} catch (ResourceNotFoundException $e) {
176172
if ($this->logger) {
177173
$this->logger->info('Router '.get_class($router).' was not able to match, message "'.$e->getMessage().'"');
@@ -185,7 +181,10 @@ public function matchRequest(Request $request)
185181
}
186182
}
187183

188-
throw $methodNotAllowed ?: new ResourceNotFoundException("None of the routers in the chain matched this request");
184+
$info = $request
185+
? "this request\n$request"
186+
: "url '$url'";
187+
throw $methodNotAllowed ?: new ResourceNotFoundException("None of the routers in the chain matched $info");
189188
}
190189

191190
/**
@@ -196,9 +195,9 @@ public function matchRequest(Request $request)
196195
*/
197196
public function generate($name, $parameters = array(), $absolute = false)
198197
{
199-
/** @var $router RouterInterface */
200-
foreach ($this->all() as $router) {
198+
$debug = array();
201199

200+
foreach ($this->all() as $router) {
202201
// if $router does not implement ChainedRouterInterface and $name is not a string, continue
203202
if ($name && !$router instanceof ChainedRouterInterface) {
204203
if (! is_string($name)) {
@@ -214,13 +213,24 @@ public function generate($name, $parameters = array(), $absolute = false)
214213
try {
215214
return $router->generate($name, $parameters, $absolute);
216215
} catch (RouteNotFoundException $e) {
216+
$hint = ($router instanceof VersatileGeneratorInterface)
217+
? $router->getRouteDebugMessage($name, $parameters)
218+
: "Route '$name' not found";
219+
$debug[] = $hint;
217220
if ($this->logger) {
218-
$this->logger->info($e->getMessage());
221+
$this->logger->info('Router '.get_class($router)." was unable to generate route. Reason: '$hint': ".$e->getMessage());
219222
}
220223
}
221224
}
222225

223-
throw new RouteNotFoundException(sprintf('None of the chained routers were able to generate route "%s".', $name));
226+
if ($debug) {
227+
$debug = array_unique($debug);
228+
$info = implode(', ', $debug);
229+
} else {
230+
$info = "No route '$name' found";
231+
}
232+
233+
throw new RouteNotFoundException(sprintf('None of the chained routers were able to generate route: %s', $info));
224234
}
225235

226236
/**

ContentAwareGenerator.php

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1010
use Symfony\Component\Routing\Generator\UrlGenerator;
1111

12-
use Symfony\Cmf\Component\Routing\RouteProviderInterface;
13-
1412
/**
1513
* A generator that tries to generate routes from object, route names or
1614
* content objects or names.
@@ -46,8 +44,8 @@ public function setContentRepository(ContentRepositoryInterface $contentReposito
4644
*
4745
* @param string $name ignored
4846
* @param array $parameters must either contain the field 'route' with a
49-
* RouteObjectInterface or the field 'content' with the document
50-
* instance to get the route for (implementing RouteAwareInterface)
47+
* RouteObjectInterface or the field 'content_id' with a document
48+
* id to get the route for (implementing RouteAwareInterface)
5149
*
5250
* @throws RouteNotFoundException If there is no such route in the database
5351
*/
@@ -120,51 +118,55 @@ protected function getBestLocaleRoute(SymfonyRoute $route, $parameters)
120118
}
121119

122120
/**
123-
* Get the route based on the content field in parameters
121+
* Get the route based on the $name that is a RouteAwareInterface or a
122+
* RouteAwareInterface content found in the content repository with the
123+
* content_id specified in parameters.
124124
*
125125
* Called in generate when there is no route given in the parameters.
126126
*
127127
* If there is more than one route for the content, tries to find the
128128
* first one that matches the _locale (provided in $parameters or otherwise
129129
* defaulting to the request locale).
130130
*
131-
* If none is found, falls back to just return the first route.
131+
* If no route with matching locale is found, falls back to just return the
132+
* first route.
132133
*
133134
* @param mixed $name
134135
* @param array $parameters which should contain a content field containing a RouteAwareInterface object
135136
*
136137
* @return SymfonyRoute the route instance
137138
*
138-
* @throws RouteNotFoundException if there is no content field in the
139-
* parameters or its not possible to build a route from that object
139+
* @throws RouteNotFoundException if no route can be determined
140140
*/
141141
protected function getRouteByContent($name, &$parameters)
142142
{
143143
if ($name instanceof RouteAwareInterface) {
144144
$content = $name;
145-
} elseif (isset($parameters['content_id']) && null !== $this->contentRepository) {
145+
} elseif (isset($parameters['content_id'])
146+
&& null !== $this->contentRepository
147+
) {
146148
$content = $this->contentRepository->findById($parameters['content_id']);
147-
} elseif (isset($parameters['content'])) {
148-
$content = $parameters['content'];
149-
}
150-
151-
unset($parameters['content'], $parameters['content_id']);
152-
153-
if (empty($content)) {
154-
throw new RouteNotFoundException('Neither the route name, nor a parameter "content" or "content_id" could be resolved to an content instance');
155-
}
156-
157-
if (!$content instanceof RouteAwareInterface) {
158-
$hint = is_object($content) ? get_class($content) : gettype($content);
159-
throw new RouteNotFoundException('The content does not implement RouteAwareInterface: ' . $hint);
149+
if (empty($content)) {
150+
throw new RouteNotFoundException('The content repository found nothing at id ' . $parameters['content_id']);
151+
}
152+
if (!$content instanceof RouteAwareInterface) {
153+
throw new RouteNotFoundException('Content repository did not return a RouteAwareInterface for id ' . $parameters['content_id']);
154+
}
155+
} else {
156+
$hint = is_object($name) ? get_class($name) : gettype($name);
157+
throw new RouteNotFoundException("The route name argument '$hint' is not RouteAwareInterface and there is no 'content_id' parameter");
160158
}
161159

162160
$routes = $content->getRoutes();
163161
if (empty($routes)) {
164-
$hint = method_exists($content, 'getPath') ? $content->getPath() : get_class($content);
165-
throw new RouteNotFoundException('Document has no route: ' . $hint);
162+
$hint = ($this->contentRepository && $this->contentRepository->getContentId($content))
163+
? $this->contentRepository->getContentId($content)
164+
: get_class($content);
165+
throw new RouteNotFoundException('Content document has no route: ' . $hint);
166166
}
167167

168+
unset($parameters['content_id']);
169+
168170
$route = $this->getRouteByLocale($routes, $this->getLocale($parameters));
169171
if ($route) {
170172
return $route;
@@ -233,4 +235,20 @@ public function supports($name)
233235
{
234236
return ! $name || parent::supports($name) || $name instanceof RouteAwareInterface;
235237
}
238+
239+
/**
240+
* {@inheritDoc}
241+
*/
242+
public function getRouteDebugMessage($name, array $parameters = array())
243+
{
244+
if (empty($name) && isset($parameters['content_id'])) {
245+
return 'Content id ' . $parameters['content_id'];
246+
}
247+
248+
if ($name instanceof RouteAwareInterface) {
249+
return 'Route aware content ' . $name;
250+
}
251+
252+
return parent::getRouteDebugMessage($name, $parameters);
253+
}
236254
}

ContentRepositoryInterface.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,13 @@ interface ContentRepositoryInterface
2424
* @return object A content that matches this id.
2525
*/
2626
public function findById($id);
27+
28+
/**
29+
* Return the content id for the provided content object
30+
*
31+
* @param object $content A content instance
32+
*
33+
* @return string|null $id id of the content object or null if unable to determine an id
34+
*/
35+
public function getContentId($content);
2736
}

DynamicRouter.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,4 +298,18 @@ public function getContext()
298298
{
299299
return $this->context;
300300
}
301+
302+
/**
303+
* {@inheritDoc}
304+
*
305+
* Forwards to the generator.
306+
*/
307+
public function getRouteDebugMessage($name, array $parameters = array())
308+
{
309+
if ($this->generator instanceof VersatileGeneratorInterface) {
310+
return $this->generator->getRouteDebugMessage($name, $parameters);
311+
}
312+
313+
return "Route '$name' not found";
314+
}
301315
}

ProviderBasedGenerator.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,21 @@ public function supports($name)
6464
{
6565
return is_string($name) || $name instanceof SymfonyRoute;
6666
}
67+
68+
/**
69+
* {@inheritDoc}
70+
*/
71+
public function getRouteDebugMessage($name, array $parameters = array())
72+
{
73+
if ($name instanceof RouteObjectInterface) {
74+
return 'Route with key ' . $name->getRouteKey();
75+
}
76+
77+
if ($name instanceof SymfonyRoute) {
78+
return 'Route with pattern ' . $name->getPattern();
79+
}
80+
81+
return $name;
82+
}
83+
6784
}

0 commit comments

Comments
 (0)
0