Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Commit 567313c

Browse files
committed
More cleanup and tests
1 parent 9481f26 commit 567313c

16 files changed

+1025
-424
lines changed

Diff for: src/PartialRouteResult.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static function fromMethodFailure(
3535
) : PartialRouteResult {
3636
$result = new self();
3737
$result->success = false;
38-
$result->allowedMethods = $allowedMethods;
38+
$result->setAllowedMethods($allowedMethods);
3939
$result->pathOffset = $pathOffset;
4040
$result->matchedPathLength = $matchedPathLength;
4141
return $result;

Diff for: src/Route/Chain.php

+6-3
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public static function factory($options = []) : RouteInterface
8585
}
8686

8787
if ($options['routes'] instanceof Traversable) {
88-
$options['routes'] = ArrayUtils::iteratorToArray($options['child_routes']);
88+
$options['routes'] = ArrayUtils::iteratorToArray($options['routes']);
8989
}
9090

9191
if (! isset($options['route_plugins'])) {
@@ -120,6 +120,9 @@ public function addRoute($name, $route, $priority = null) : void
120120

121121
public function partialMatch(Request $request, int $pathOffset = 0, array $options = []) : PartialRouteResult
122122
{
123+
if ($pathOffset < 0) {
124+
throw new Exception\InvalidArgumentException('Route path offset cannot be negative');
125+
}
123126
if ($this->chainRoutes !== null) {
124127
$this->addRoutes($this->chainRoutes);
125128
$this->chainRoutes = null;
@@ -192,7 +195,7 @@ public function assemble(array $params = [], array $options = []) : UriInterface
192195

193196
$uri = $options['uri'] ?? new Uri();
194197
if (! $uri instanceof UriInterface) {
195-
throw new Exception\DomainException(\sprintf(
198+
throw new Exception\InvalidArgumentException(\sprintf(
196199
'Route assemble option \'uri\' must be instance of %s, got %s',
197200
UriInterface::class,
198201
(\is_object($uri) ? \get_class($uri) : \gettype($uri))
@@ -214,7 +217,7 @@ public function assemble(array $params = [], array $options = []) : UriInterface
214217
$uri = $route->assemble($params, $chainOptions);
215218
$params = array_diff_key($params, array_flip($route->getLastAssembledParams()));
216219

217-
$this->assembledParams += $route->getLastAssembledParams();
220+
$this->assembledParams = \array_merge($this->assembledParams, $route->getLastAssembledParams());
218221
}
219222

220223
return $uri;

Diff for: src/Route/Part.php

+27-26
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ public function match(Request $request, int $pathOffset = 0, array $options = []
146146
{
147147
$partialResult = $this->route->partialMatch($request, $pathOffset, $options);
148148

149+
// no match, bail out
149150
if ($partialResult->isFailure() && ! $partialResult->isMethodFailure()) {
150151
return RouteResult::fromRouteFailure();
151152
}
@@ -161,54 +162,54 @@ public function match(Request $request, int $pathOffset = 0, array $options = []
161162
return RouteResult::fromMethodFailure($partialResult->getAllowedMethods());
162163
}
163164

165+
// @TODO get rid of lazy routes
164166
if ($this->childRoutes !== null) {
165167
$this->addRoutes($this->childRoutes);
166168
$this->childRoutes = null;
167169
}
168170

169-
$nextOffset = $pathOffset + $partialResult->getMatchedPathLength();
170-
171-
if (isset($options['translator'])
172-
&& ! isset($options['locale'])
173-
&& $partialResult->isSuccess()
174-
&& null !== ($locale = $partialResult->getMatchedParams()['locale'] ?? null)
175-
) {
176-
$options['locale'] = $locale;
171+
// pass matched params to child routes.
172+
// Could be used for obtaining locale from parent route match.
173+
if ($partialResult->isSuccess()) {
174+
$options['parent_match_params'] = $options['parent_match_params'] ?? [];
175+
$options['parent_match_params'] += $partialResult->getMatchedParams();
177176
}
178177

179-
$gatherMethods = $options['gather_allowed_methods'] ?? false;
180178
if ($partialResult->isMethodFailure()) {
179+
// we got partial method failure, keep matching to find all allowed methods
181180
$options['gather_allowed_methods'] = true;
182181
}
183182

183+
// match child routes
184+
$nextOffset = $pathOffset + $partialResult->getMatchedPathLength();
184185
$result = parent::match($request, $nextOffset, $options);
185186
if ($result->isFailure() && ! $result->isMethodFailure()) {
186187
return $result;
187188
}
188-
if ($partialResult->isMethodFailure()) {
189-
$allowed = $partialResult->getAllowedMethods();
190-
if (! empty($methods = $result->getAllowedMethods())) {
191-
$allowed = \array_intersect($allowed, $methods);
189+
190+
// gather allowed methods
191+
$partialAllowed = $partialResult->getAllowedMethods();
192+
$childAllowed = $result->getAllowedMethods();
193+
if (null !== $partialAllowed && null !== $childAllowed) {
194+
$allowed = \array_intersect($partialAllowed, $childAllowed);
195+
} else {
196+
$allowed = $partialAllowed ?? $childAllowed;
197+
}
198+
199+
// was it a method failure?
200+
if ($partialResult->isMethodFailure() || $result->isMethodFailure()) {
201+
if (empty($allowed)) {
202+
return RouteResult::fromRouteFailure();
192203
}
193204
return RouteResult::fromMethodFailure($allowed);
194205
}
195-
if ($result->isMethodFailure()) {
196-
return $result;
197-
}
206+
207+
// we got success
198208
$return = RouteResult::fromRouteMatch(
199209
\array_merge($partialResult->getMatchedParams(), $result->getMatchedParams()),
200210
$result->getMatchedRouteName()
201211
);
202-
$allowed = $partialResult->getAllowedMethods();
203-
$nested = $result->getAllowedMethods();
204-
if (! empty($allowed) && ! empty($nested)) {
205-
$allowed = \array_intersect($allowed, $nested);
206-
}
207-
$allowed = $allowed ?? $nested;
208-
if (! empty($allowed)) {
209-
$return->withAllowedMethods($allowed);
210-
}
211-
return $return;
212+
return $return->withAllowedMethods($allowed);
212213
}
213214

214215
/**

Diff for: src/Route/Segment.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,8 @@ public function partialMatch(Request $request, int $pathOffset = 0, array $optio
368368
}
369369

370370
$translator = $options['translator'];
371-
$textDomain = (isset($options['text_domain']) ? $options['text_domain'] : 'default');
372-
$locale = (isset($options['locale']) ? $options['locale'] : null);
371+
$textDomain = $options['text_domain'] ?? 'default';
372+
$locale = $options['locale'] ?? $options['parent_match_params']['locale'] ?? null;
373373

374374
foreach ($this->translationKeys as $key) {
375375
$regex = str_replace('#' . $key . '#', $translator->translate($key, $textDomain, $locale), $regex);

Diff for: src/RouteResult.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static function fromMethodFailure(array $allowedMethods) : RouteResult
2626
{
2727
$result = new self();
2828
$result->success = false;
29-
$result->allowedMethods = $allowedMethods;
29+
$result->setAllowedMethods($allowedMethods);
3030
return $result;
3131
}
3232

Diff for: src/RouteResultTrait.php

+14-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function withMatchedParams(array $params) : self
8585
public function withAllowedMethods(?array $allowedMethods) : self
8686
{
8787
$result = clone $this;
88-
$result->allowedMethods = $allowedMethods;
88+
$result->setAllowedMethods($allowedMethods);
8989
return $result;
9090
}
9191

@@ -142,4 +142,17 @@ public function getAllowedMethods() : ?array
142142
{
143143
return $this->allowedMethods;
144144
}
145+
146+
private function setAllowedMethods(?array $methods) : void
147+
{
148+
if (null !== $methods) {
149+
// @TODO verify this is faster than array_map + array_unique.
150+
// array is too small to see the difference I think
151+
$methods = \array_keys(\array_change_key_case(
152+
\array_flip($methods),
153+
\CASE_UPPER
154+
));
155+
}
156+
$this->allowedMethods = $methods;
157+
}
145158
}

Diff for: src/SimpleRouteStack.php

+6-7
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,6 @@ public function addRoutes($routes) : void
151151
* @param string $name
152152
* @param mixed $route
153153
* @param int $priority
154-
* @return SimpleRouteStack
155154
*/
156155
public function addRoute($name, $route, $priority = null) : void
157156
{
@@ -171,7 +170,6 @@ public function addRoute($name, $route, $priority = null) : void
171170
*
172171
* @see RouteStackInterface::removeRoute()
173172
* @param string $name
174-
* @return SimpleRouteStack
175173
*/
176174
public function removeRoute($name) : void
177175
{
@@ -281,8 +279,7 @@ protected function routeFromArray($specs) : RouteInterface
281279
/**
282280
* match(): defined by RouteInterface interface.
283281
*
284-
* @see \Zend\Router\RouteInterface::match()
285-
* @param Request $request
282+
* @param Request $request
286283
* @param int $pathOffset
287284
* @param array $options
288285
* @return RouteResult
@@ -294,17 +291,19 @@ public function match(Request $request, int $pathOffset = 0, array $options = []
294291
/** @var RouteInterface $route */
295292
$result = $route->match($request, $pathOffset, $options);
296293
if ($result->isMethodFailure()) {
297-
$allowedMethods += $result->getAllowedMethods();
294+
$allowedMethods = \array_merge($allowedMethods, $result->getAllowedMethods());
298295
}
299296
if ($result->isSuccess()) {
300297
return $result->withMatchedRouteName($name)
301298
->withMatchedParams(
302299
\array_merge($this->defaultParams, $result->getMatchedParams())
303-
);
300+
)
301+
// simple route stack does not gather allowed methods on success
302+
->withAllowedMethods(null);
304303
}
305304
}
306305
if (! empty($allowedMethods)) {
307-
return RouteResult::fromMethodFailure(\array_keys(\array_flip($allowedMethods)));
306+
return RouteResult::fromMethodFailure($allowedMethods);
308307
}
309308

310309
return RouteResult::fromRouteFailure();

Diff for: src/TreeRouteStack.php

+32-17
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,34 @@ public static function factory($options = []) : RouteInterface
5151
));
5252
}
5353

54-
/**
55-
* @var TreeRouteStack $instance
56-
*/
57-
$instance = parent::factory($options);
54+
if ($options instanceof Traversable) {
55+
$options = ArrayUtils::iteratorToArray($options);
56+
} elseif (! is_array($options)) {
57+
throw new Exception\InvalidArgumentException(sprintf(
58+
'%s expects an array or Traversable set of options',
59+
__METHOD__
60+
));
61+
}
62+
63+
$routePluginManager = null;
64+
if (isset($options['route_plugins'])) {
65+
$routePluginManager = $options['route_plugins'];
66+
}
67+
68+
$instance = new static($routePluginManager);
5869

5970
if (isset($options['prototypes'])) {
6071
$instance->addPrototypes($options['prototypes']);
6172
}
6273

74+
if (isset($options['routes'])) {
75+
$instance->addRoutes($options['routes']);
76+
}
77+
78+
if (isset($options['default_params'])) {
79+
$instance->setDefaultParams($options['default_params']);
80+
}
81+
6382
return $instance;
6483
}
6584

@@ -226,40 +245,36 @@ public function match(Request $request, int $pathOffset = 0, array $options = []
226245
// method failure have special handling
227246
continue;
228247
}
248+
249+
if (null !== $result->getAllowedMethods()) {
250+
$allowedMethods = \array_merge($allowedMethods, $result->getAllowedMethods());
251+
}
252+
229253
if ($result->isMethodFailure()) {
230-
$allowedMethods += $result->getAllowedMethods();
231254
continue;
232255
}
233256

234-
// now handling success
257+
// store first successful result, it will be returned later
235258
if (null === $successfulRouteResult) {
236259
$successfulRouteResult = $result->withParentRouteName($name)
237260
->withMatchedParams(\array_merge($this->defaultParams, $result->getMatchedParams()));
238261
}
239262

240-
// @TODO declare constant for the attribute
241263
if (($options['gather_allowed_methods'] ?? false) !== true
242264
|| empty($result->getAllowedMethods())
243265
) {
244266
// we do not need to gather allowed methods or all methods are allowed, we can return immediately
245-
return $successfulRouteResult;
267+
return $successfulRouteResult->withAllowedMethods(null);
246268
}
247-
248-
$allowedMethods += $result->getAllowedMethods();
249269
}
250270

251-
$allowedMethods = \array_keys($allowedMethods);
252271
if ($successfulRouteResult) {
253272
// return successful result with gathered methods
254-
return $successfulRouteResult->withAllowedMethods(
255-
\array_keys(\array_flip($allowedMethods))
256-
);
273+
return $successfulRouteResult->withAllowedMethods($allowedMethods);
257274
}
258275

259276
if (! empty($allowedMethods)) {
260-
return RouteResult::fromMethodFailure(
261-
\array_keys(\array_flip($allowedMethods))
262-
);
277+
return RouteResult::fromMethodFailure($allowedMethods);
263278
}
264279

265280
return RouteResult::fromRouteFailure();

Diff for: test/PartialRouteResultTest.php

+7
Original file line numberDiff line numberDiff line change
@@ -231,4 +231,11 @@ public function testWithMatchedParamsThrowsForUnsuccessfulResult()
231231
$result = PartialRouteResult::fromRouteFailure();
232232
$result->withMatchedParams(['foo' => 'bar']);
233233
}
234+
235+
public function testAllowedMethodsAreDisambiguatedAndCastToUppercase()
236+
{
237+
$methods = ['GeT', 'post', 'PoSt'];
238+
$result = PartialRouteResult::fromMethodFailure($methods, 0, 0);
239+
$this->assertEquals(['GET', 'POST'], $result->getAllowedMethods());
240+
}
234241
}

0 commit comments

Comments
 (0)