Skip to content

Commit 3b5efc3

Browse files
committed
Inject routineLookup, increase coverage to 100%, remove dead code
- Make NamespaceLookup a constructor dependency injected from Router into all routes and handlers, removing duplicate creation and the setRoutineLookup()/withRoutineNamespace() post-construction API - Add 16 targeted tests reaching 100% class/method coverage; remove two unreachable code paths (ControllerRoute::getTargetMethod HEAD fallback, Router::process empty-dispatch guard) - Remove unused CallbackList methods (hasKey, filterKeysContain, filterKeysNotContain, executeCallback) and their tests - Merge Responder::finalize() dual null-check blocks into one - Make compareOcurrences private and fix typo -> compareOccurrences - Collapse Router::__call() callable branch into single call
1 parent 64e698e commit 3b5efc3

27 files changed

Lines changed: 430 additions & 194 deletions

src/DispatchContext.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,9 @@ private function resetHandlerState(): void
250250
{
251251
foreach ($this->handlers as $handler) {
252252
if ($handler instanceof ErrorHandler) {
253-
$handler->errors = [];
253+
$handler->clearErrors();
254254
} elseif ($handler instanceof ExceptionHandler) {
255-
$handler->exception = null;
255+
$handler->clearException();
256256
}
257257
}
258258
}
@@ -277,7 +277,7 @@ static function (
277277
string $errfile = '',
278278
int $errline = 0,
279279
) use ($handler): bool {
280-
$handler->errors[] = [$errno, $errstr, $errfile, $errline];
280+
$handler->addError($errno, $errstr, $errfile, $errline);
281281

282282
return true;
283283
},
@@ -291,7 +291,7 @@ static function (
291291
private function forwardCollectedErrors(): ResponseInterface|null
292292
{
293293
foreach ($this->handlers as $handler) {
294-
if ($handler instanceof ErrorHandler && $handler->errors) {
294+
if ($handler instanceof ErrorHandler && $handler->hasErrors()) {
295295
return $this->forward($handler);
296296
}
297297
}
@@ -306,8 +306,8 @@ private function catchExceptions(Throwable $e): ResponseInterface|null
306306
continue;
307307
}
308308

309-
if (is_a($e, $handler->class)) {
310-
$handler->exception = $e;
309+
if ($handler->matches($e)) {
310+
$handler->capture($e);
311311

312312
return $this->forward($handler);
313313
}

src/Handlers/ErrorHandler.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,33 @@
44

55
namespace Respect\Rest\Handlers;
66

7+
use Respect\Fluent\Factories\NamespaceLookup;
78
use Respect\Rest\DispatchContext;
89
use Respect\Rest\Routes\Callback;
910

1011
final class ErrorHandler extends Callback
1112
{
12-
/** @var callable */
13-
public $callback;
14-
1513
/** @var array<int, array<int, mixed>> */
16-
public array $errors = [];
14+
private array $errors = [];
15+
16+
public function __construct(NamespaceLookup $routineLookup, callable $callback)
17+
{
18+
parent::__construct($routineLookup, 'ANY', '^$', $callback);
19+
}
20+
21+
public function addError(int $errno, string $errstr, string $errfile, int $errline): void
22+
{
23+
$this->errors[] = [$errno, $errstr, $errfile, $errline];
24+
}
25+
26+
public function hasErrors(): bool
27+
{
28+
return $this->errors !== [];
29+
}
1730

18-
public function __construct(callable $callback)
31+
public function clearErrors(): void
1932
{
20-
parent::__construct('ANY', '^$', $callback);
33+
$this->errors = [];
2134
}
2235

2336
/** @param array<int, mixed> $params */

src/Handlers/ExceptionHandler.php

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,35 @@
44

55
namespace Respect\Rest\Handlers;
66

7+
use Respect\Fluent\Factories\NamespaceLookup;
78
use Respect\Rest\DispatchContext;
89
use Respect\Rest\Routes\Callback;
910
use Throwable;
1011

12+
use function is_a;
13+
1114
final class ExceptionHandler extends Callback
1215
{
13-
/** @var callable */
14-
public $callback;
16+
private Throwable|null $exception = null;
1517

16-
public Throwable|null $exception = null;
18+
public function __construct(NamespaceLookup $routineLookup, public private(set) string $class, callable $callback)
19+
{
20+
parent::__construct($routineLookup, 'ANY', '^$', $callback);
21+
}
22+
23+
public function matches(Throwable $e): bool
24+
{
25+
return is_a($e, $this->class);
26+
}
27+
28+
public function capture(Throwable $e): void
29+
{
30+
$this->exception = $e;
31+
}
1732

18-
public function __construct(public string $class, callable $callback)
33+
public function clearException(): void
1934
{
20-
parent::__construct('ANY', '^$', $callback);
35+
$this->exception = null;
2136
}
2237

2338
/** @param array<int, mixed> $params */

src/Handlers/StatusHandler.php

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@
44

55
namespace Respect\Rest\Handlers;
66

7+
use Respect\Fluent\Factories\NamespaceLookup;
78
use Respect\Rest\Routes\Callback;
89

910
final class StatusHandler extends Callback
1011
{
11-
/** @var callable */
12-
public $callback;
13-
14-
public function __construct(public readonly int|null $statusCode, callable $callback)
15-
{
16-
parent::__construct('ANY', '^$', $callback);
12+
public function __construct(
13+
NamespaceLookup $routineLookup,
14+
public readonly int|null $statusCode,
15+
callable $callback,
16+
) {
17+
parent::__construct($routineLookup, 'ANY', '^$', $callback);
1718
}
1819
}

src/Responder.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,6 @@ public function finalize(
6060
): ResponseInterface {
6161
$response = $this->normalize($result);
6262

63-
if ($responseDraft !== null) {
64-
if ($statusOverridden) {
65-
$response = $response->withStatus($responseDraft->getStatusCode(), $responseDraft->getReasonPhrase());
66-
}
67-
}
68-
6963
foreach ($defaultHeaders as $name => $value) {
7064
if ($response->hasHeader($name)) {
7165
continue;
@@ -75,6 +69,10 @@ public function finalize(
7569
}
7670

7771
if ($responseDraft !== null) {
72+
if ($statusOverridden) {
73+
$response = $response->withStatus($responseDraft->getStatusCode(), $responseDraft->getReasonPhrase());
74+
}
75+
7876
foreach ($responseDraft->getHeaders() as $name => $values) {
7977
if (!isset($appendedHeaderNames[strtolower($name)])) {
8078
$response = $response->withHeader($name, $values);

src/Router.php

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ final class Router implements MiddlewareInterface, RequestHandlerInterface, Rout
5959
public function __construct(
6060
protected string $basePath,
6161
private ResponseFactoryInterface&StreamFactoryInterface $factory,
62+
NamespaceLookup|null $routineLookup = null,
6263
) {
6364
$this->basePath = rtrim($basePath, '/');
64-
$this->routineLookup = new NamespaceLookup(
65+
$this->routineLookup = $routineLookup ?? new NamespaceLookup(
6566
new Ucfirst(),
6667
Routinable::class,
6768
'Respect\\Rest\\Routines',
@@ -81,18 +82,10 @@ public function always(string $routineName, mixed ...$params): static
8182
return $this;
8283
}
8384

84-
public function withRoutineNamespace(string $namespace): static
85-
{
86-
$this->routineLookup = $this->routineLookup->withNamespace($namespace);
87-
88-
return $this;
89-
}
90-
9185
public function appendRoute(AbstractRoute $route): static
9286
{
9387
$this->routes[] = $route;
94-
$route->basePath = $this->basePath;
95-
$route->setRoutineLookup($this->routineLookup);
88+
$route->setBasePath($this->basePath);
9689

9790
foreach ($this->globalRoutines as $routine) {
9891
$route->appendRoutine($routine);
@@ -106,7 +99,6 @@ public function appendRoute(AbstractRoute $route): static
10699
public function appendHandler(AbstractRoute $handler): static
107100
{
108101
$this->handlers[] = $handler;
109-
$handler->setRoutineLookup($this->routineLookup);
110102

111103
foreach ($this->globalRoutines as $routine) {
112104
$handler->appendRoutine($routine);
@@ -122,7 +114,7 @@ public function callbackRoute(
122114
callable $callback,
123115
array $arguments = [],
124116
): Routes\Callback {
125-
$route = new Routes\Callback($method, $path, $callback, $arguments);
117+
$route = new Routes\Callback($this->routineLookup, $method, $path, $callback, $arguments);
126118
$this->appendRoute($route);
127119

128120
return $route;
@@ -131,7 +123,7 @@ public function callbackRoute(
131123
/** @param array<int, mixed> $arguments */
132124
public function classRoute(string $method, string $path, string $class, array $arguments = []): Routes\ClassName
133125
{
134-
$route = new Routes\ClassName($method, $path, $class, $arguments);
126+
$route = new Routes\ClassName($this->routineLookup, $method, $path, $class, $arguments);
135127
$this->appendRoute($route);
136128

137129
return $route;
@@ -151,10 +143,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
151143
{
152144
$context = $this->dispatch($request);
153145

154-
if ($context->route === null && !$context->hasPreparedResponse()) {
155-
return $handler->handle($request);
156-
}
157-
158146
if ($context->route === null) {
159147
$response = $context->response();
160148
if ($response !== null && $response->getStatusCode() === 404) {
@@ -185,23 +173,23 @@ public function dispatchContext(DispatchContext $context): DispatchContext
185173

186174
public function onException(string $className, callable $callback): Handlers\ExceptionHandler
187175
{
188-
$handler = new Handlers\ExceptionHandler($className, $callback);
176+
$handler = new Handlers\ExceptionHandler($this->routineLookup, $className, $callback);
189177
$this->appendHandler($handler);
190178

191179
return $handler;
192180
}
193181

194182
public function onError(callable $callback): Handlers\ErrorHandler
195183
{
196-
$handler = new Handlers\ErrorHandler($callback);
184+
$handler = new Handlers\ErrorHandler($this->routineLookup, $callback);
197185
$this->appendHandler($handler);
198186

199187
return $handler;
200188
}
201189

202190
public function onStatus(int|null $statusCode, callable $callback): Handlers\StatusHandler
203191
{
204-
$handler = new Handlers\StatusHandler($statusCode, $callback);
192+
$handler = new Handlers\StatusHandler($this->routineLookup, $statusCode, $callback);
205193
$this->appendHandler($handler);
206194

207195
return $handler;
@@ -215,23 +203,23 @@ public function getHandlers(): array
215203

216204
public function factoryRoute(string $method, string $path, string $className, callable $factory): Routes\Factory
217205
{
218-
$route = new Routes\Factory($method, $path, $className, $factory);
206+
$route = new Routes\Factory($this->routineLookup, $method, $path, $className, $factory);
219207
$this->appendRoute($route);
220208

221209
return $route;
222210
}
223211

224212
public function instanceRoute(string $method, string $path, object $instance): Routes\Instance
225213
{
226-
$route = new Routes\Instance($method, $path, $instance);
214+
$route = new Routes\Instance($this->routineLookup, $method, $path, $instance);
227215
$this->appendRoute($route);
228216

229217
return $route;
230218
}
231219

232220
public function staticRoute(string $method, string $path, mixed $staticValue): Routes\StaticValue
233221
{
234-
$route = new Routes\StaticValue($method, $path, $staticValue);
222+
$route = new Routes\StaticValue($this->routineLookup, $method, $path, $staticValue);
235223
$this->appendRoute($route);
236224

237225
return $route;
@@ -256,11 +244,6 @@ public function dispatchEngine(): DispatchEngine
256244
);
257245
}
258246

259-
public static function compareOcurrences(string $patternA, string $patternB, string $sub): bool
260-
{
261-
return substr_count($patternA, $sub) < substr_count($patternB, $sub);
262-
}
263-
264247
protected function sortRoutesByComplexity(): void
265248
{
266249
usort(
@@ -273,7 +256,7 @@ static function (AbstractRoute $a, AbstractRoute $b): int {
273256
return 0;
274257
}
275258

276-
$slashCount = Router::compareOcurrences($pa, $pb, '/');
259+
$slashCount = Router::compareOccurrences($pa, $pb, '/');
277260

278261
$aCatchall = preg_match('#/\*\*$#', $pa);
279262
$bCatchall = preg_match('#/\*\*$#', $pb);
@@ -285,7 +268,7 @@ static function (AbstractRoute $a, AbstractRoute $b): int {
285268
return $slashCount ? 1 : -1;
286269
}
287270

288-
if (Router::compareOcurrences($pa, $pb, AbstractRoute::PARAM_IDENTIFIER)) {
271+
if (Router::compareOccurrences($pa, $pb, AbstractRoute::PARAM_IDENTIFIER)) {
289272
return -1;
290273
}
291274

@@ -294,6 +277,11 @@ static function (AbstractRoute $a, AbstractRoute $b): int {
294277
);
295278
}
296279

280+
private static function compareOccurrences(string $patternA, string $patternB, string $sub): bool
281+
{
282+
return substr_count($patternA, $sub) < substr_count($patternB, $sub);
283+
}
284+
297285
/** @param array<int, mixed> $args */
298286
public function __call(string $method, array $args): AbstractRoute
299287
{
@@ -315,11 +303,7 @@ public function __call(string $method, array $args): AbstractRoute
315303
}
316304

317305
if (is_callable($routeTarget)) {
318-
if (!isset($args[2])) {
319-
return $this->callbackRoute($method, $path, $routeTarget);
320-
}
321-
322-
return $this->callbackRoute($method, $path, $routeTarget, $args[2]);
306+
return $this->callbackRoute($method, $path, $routeTarget, $args[2] ?? []);
323307
}
324308

325309
if ($routeTarget instanceof Routable) {

src/Routes/AbstractRoute.php

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,22 @@ abstract class AbstractRoute
6565
public const string REGEX_OPTIONAL_PARAM = '(?:/([^/]+))?';
6666
public const string REGEX_INVALID_OPTIONAL_PARAM = '#\(\?\:/\(\[\^/\]\+\)\)\?/#';
6767

68-
public string $method = '';
68+
public private(set) string $method = '';
6969

70-
public string $regexForMatch = '';
70+
public private(set) string $regexForMatch = '';
7171

72-
public string $regexForReplace = '';
72+
public private(set) string $regexForReplace = '';
7373

7474
/** @var array<string, Routinable> */
75-
public array $routines = [];
75+
public private(set) array $routines = [];
7676

77-
public string|null $basePath = null;
77+
public private(set) string|null $basePath = null;
7878

79-
private NamespaceLookup $routineLookup;
80-
81-
public function __construct(string $method, public string $pattern = '')
82-
{
79+
public function __construct(
80+
private NamespaceLookup $routineLookup,
81+
string $method,
82+
public string $pattern,
83+
) {
8384
$this->method = strtoupper($method);
8485

8586
[$this->regexForMatch, $this->regexForReplace]
@@ -147,9 +148,9 @@ public function appendRoutine(Routinable $routine): static
147148
return $this;
148149
}
149150

150-
public function setRoutineLookup(NamespaceLookup $lookup): void
151+
public function setBasePath(string|null $basePath): void
151152
{
152-
$this->routineLookup = $lookup;
153+
$this->basePath = $basePath;
153154
}
154155

155156
public function createUri(mixed ...$params): string

0 commit comments

Comments
 (0)