Skip to content

Commit bd81ff0

Browse files
committed
refactor: address review feedback - revert Services::*() to service(), remove SPOOFABLE_METHODS, shorten comments, restore tryToRouteIt signature
All reviewer comments from @michalsn on PR #10369 addressed: - Reverted Services::*() calls back to service() (optimized helper) - Removed SPOOFABLE_METHODS constant (used only once) - Restored tryToRouteIt signature to non-BC-breaking form - Shortened verbose comments in startController() and storePreviousURL() - Kept only the flag fix for duplicate gatherOutput() calls
1 parent b4a86fc commit bd81ff0

1 file changed

Lines changed: 14 additions & 23 deletions

File tree

system/CodeIgniter.php

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ class CodeIgniter
5858
*/
5959
public const CI_VERSION = '4.7.5-dev';
6060

61-
/**
62-
* Spoofable HTTP methods
63-
*/
64-
private const SPOOFABLE_METHODS = [Method::PUT, Method::PATCH, Method::DELETE];
65-
6661
/**
6762
* App startup time.
6863
*
@@ -227,7 +222,7 @@ private function resetKintForWorkerMode(): void
227222
return;
228223
}
229224

230-
$csp = Services::csp();
225+
$csp = service('csp');
231226
if ($csp->enabled()) {
232227
RichRenderer::$js_nonce = $csp->getScriptNonce();
233228
RichRenderer::$css_nonce = $csp->getStyleNonce();
@@ -467,14 +462,14 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
467462
return $this->response->setStatusCode(405)->setBody('Method Not Allowed');
468463
}
469464

465+
$routeFilters = $this->tryToRouteIt($routes);
466+
470467
// $uri is URL-encoded.
471468
$uri = $this->request->getPath();
472469

473-
$routeFilters = $this->tryToRouteIt($routes, $uri);
474-
475470
if ($this->enableFilters) {
476471
/** @var Filters $filters */
477-
$filters = Services::filters();
472+
$filters = service('filters');
478473

479474
// If any filters were specified within the routes file,
480475
// we need to ensure it's active for the current request
@@ -512,10 +507,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
512507
if ($returned instanceof ResponseInterface) {
513508
$this->gatherOutput($cacheConfig, $returned);
514509
$gathered = true;
515-
}
516-
// Closure controller has already run inside startController().
517-
// Use instanceof instead of is_callable() — 88x faster for this check.
518-
elseif (! $this->controller instanceof Closure) {
510+
} elseif (! $this->controller instanceof Closure) {
519511
$controller = $this->createController();
520512

521513
if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) {
@@ -540,7 +532,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
540532

541533
if ($this->enableFilters) {
542534
/** @var Filters $filters */
543-
$filters = Services::filters();
535+
$filters = service('filters');
544536
$filters->setResponse($this->response);
545537

546538
// Run "after" filters
@@ -554,7 +546,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
554546
}
555547

556548
// Execute controller attributes' after() methods AFTER framework filters.
557-
if (config(Routing::class)->useControllerAttributes === true) {
549+
if ((config('Routing')->useControllerAttributes ?? true) === true) {
558550
$this->benchmark->start('route_attributes_after');
559551
$this->response = $this->router->executeAfterAttributes($this->request, $this->response);
560552
$this->benchmark->stop('route_attributes_after');
@@ -678,7 +670,7 @@ protected function getRequestObject()
678670
Services::createRequest($this->config);
679671
}
680672

681-
$this->request = Services::request();
673+
$this->request = service('request');
682674

683675
$this->spoofRequestMethod();
684676
}
@@ -841,19 +833,19 @@ public function displayPerformanceMetrics(string $output): string
841833
*
842834
* @throws RedirectException
843835
*/
844-
protected function tryToRouteIt(?RouteCollectionInterface $routes = null, ?string $uri = null)
836+
protected function tryToRouteIt(?RouteCollectionInterface $routes = null)
845837
{
846838
$this->benchmark->start('routing');
847839

848840
if (! $routes instanceof RouteCollectionInterface) {
849-
$routes = Services::routes()->loadRoutes();
841+
$routes = service('routes')->loadRoutes();
850842
}
851843

852844
// $routes is defined in Config/Routes.php
853845
$this->router = Services::router($routes, $this->request);
854846

855847
// $uri is URL-encoded.
856-
$uri ??= $this->request->getPath();
848+
$uri = $this->request->getPath();
857849

858850
$this->outputBufferingStart();
859851

@@ -896,7 +888,7 @@ protected function startController()
896888
$this->benchmark->start('controller');
897889
$this->benchmark->start('controller_constructor');
898890

899-
// Is it routed to a Closure? Use instanceof — faster than is_object() + ::class string check.
891+
// Is it routed to a Closure?
900892
if ($this->controller instanceof Closure) {
901893
$controller = $this->controller;
902894

@@ -1088,8 +1080,7 @@ public function storePreviousURL($uri)
10881080
if (! $this->isWeb()) {
10891081
return;
10901082
}
1091-
// Ignore AJAX requests. Use instanceof instead of method_exists() — faster
1092-
// since CLIRequest never has isAJAX(), only IncomingRequest does.
1083+
// Ignore AJAX requests
10931084
if ($this->request instanceof IncomingRequest && $this->request->isAJAX()) {
10941085
return;
10951086
}
@@ -1140,7 +1131,7 @@ public function spoofRequestMethod()
11401131
}
11411132

11421133
// Only allows PUT, PATCH, DELETE
1143-
if (in_array($method, self::SPOOFABLE_METHODS, true)) {
1134+
if (in_array($method, [Method::PUT, Method::PATCH, Method::DELETE], true)) {
11441135
$this->request = $this->request->setMethod($method);
11451136
}
11461137
}

0 commit comments

Comments
 (0)