Skip to content

Commit e934674

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 e934674

1 file changed

Lines changed: 13 additions & 21 deletions

File tree

system/CodeIgniter.php

Lines changed: 13 additions & 21 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
@@ -513,8 +508,6 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
513508
$this->gatherOutput($cacheConfig, $returned);
514509
$gathered = true;
515510
}
516-
// Closure controller has already run inside startController().
517-
// Use instanceof instead of is_callable() — 88x faster for this check.
518511
elseif (! $this->controller instanceof Closure) {
519512
$controller = $this->createController();
520513

@@ -540,7 +533,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
540533

541534
if ($this->enableFilters) {
542535
/** @var Filters $filters */
543-
$filters = Services::filters();
536+
$filters = service('filters');
544537
$filters->setResponse($this->response);
545538

546539
// Run "after" filters
@@ -554,7 +547,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
554547
}
555548

556549
// Execute controller attributes' after() methods AFTER framework filters.
557-
if (config(Routing::class)->useControllerAttributes === true) {
550+
if ((config('Routing')->useControllerAttributes ?? true) === true) {
558551
$this->benchmark->start('route_attributes_after');
559552
$this->response = $this->router->executeAfterAttributes($this->request, $this->response);
560553
$this->benchmark->stop('route_attributes_after');
@@ -678,7 +671,7 @@ protected function getRequestObject()
678671
Services::createRequest($this->config);
679672
}
680673

681-
$this->request = Services::request();
674+
$this->request = service('request');
682675

683676
$this->spoofRequestMethod();
684677
}
@@ -841,19 +834,19 @@ public function displayPerformanceMetrics(string $output): string
841834
*
842835
* @throws RedirectException
843836
*/
844-
protected function tryToRouteIt(?RouteCollectionInterface $routes = null, ?string $uri = null)
837+
protected function tryToRouteIt(?RouteCollectionInterface $routes = null)
845838
{
846839
$this->benchmark->start('routing');
847840

848841
if (! $routes instanceof RouteCollectionInterface) {
849-
$routes = Services::routes()->loadRoutes();
842+
$routes = service('routes')->loadRoutes();
850843
}
851844

852845
// $routes is defined in Config/Routes.php
853846
$this->router = Services::router($routes, $this->request);
854847

855848
// $uri is URL-encoded.
856-
$uri ??= $this->request->getPath();
849+
$uri = $this->request->getPath();
857850

858851
$this->outputBufferingStart();
859852

@@ -896,7 +889,7 @@ protected function startController()
896889
$this->benchmark->start('controller');
897890
$this->benchmark->start('controller_constructor');
898891

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

@@ -1088,8 +1081,7 @@ public function storePreviousURL($uri)
10881081
if (! $this->isWeb()) {
10891082
return;
10901083
}
1091-
// Ignore AJAX requests. Use instanceof instead of method_exists() — faster
1092-
// since CLIRequest never has isAJAX(), only IncomingRequest does.
1084+
// Ignore AJAX requests
10931085
if ($this->request instanceof IncomingRequest && $this->request->isAJAX()) {
10941086
return;
10951087
}
@@ -1140,7 +1132,7 @@ public function spoofRequestMethod()
11401132
}
11411133

11421134
// Only allows PUT, PATCH, DELETE
1143-
if (in_array($method, self::SPOOFABLE_METHODS, true)) {
1135+
if (in_array($method, [Method::PUT, Method::PATCH, Method::DELETE], true)) {
11441136
$this->request = $this->request->setMethod($method);
11451137
}
11461138
}

0 commit comments

Comments
 (0)