Merge pull request #5650 from kenjis/remove-SPARKED

refactor: CodeIgniter has context

Author: kenjis

public/index.php

@@ -25,7 +25,10 @@
 
 // Location of the framework bootstrap file.
 $bootstrap = rtrim($paths->systemDirectory, '\\/ ') . DIRECTORY_SEPARATOR . 'bootstrap.php';
-$app       = require realpath($bootstrap) ?: $bootstrap;
+/** @var CodeIgniter\CodeIgniter $app */
+$app     = require realpath($bootstrap) ?: $bootstrap;
+$context = is_cli() ? 'php-cli' : 'web';
+$app->setContext($context);
 
 /*
  *---------------------------------------------------------------

rector.php

@@ -81,9 +81,11 @@
         // requires php 8
         RemoveUnusedPromotedPropertyRector::class,
 
-        // private method called via getPrivateMethodInvoker
         RemoveUnusedPrivateMethodRector::class => [
+            // private method called via getPrivateMethodInvoker
             __DIR__ . '/tests/system/Test/ReflectionHelperTest.php',
+            // Rector bug?
+            __DIR__ . '/system/CodeIgniter.php',
         ],
 
         // call on purpose for nothing happen check

spark

@@ -21,6 +21,11 @@
  * this class mainly acts as a passthru to the framework itself.
  */
 
+/**
+ * @var bool
+ *
+ * @deprecated No longer in use. `CodeIgniter` has `$context` property.
+ */
 define('SPARKED', true);
 
 /*
@@ -51,7 +56,9 @@ $paths = new Config\Paths();
 chdir(FCPATH);
 
 $bootstrap = rtrim($paths->systemDirectory, '\\/ ') . DIRECTORY_SEPARATOR . 'bootstrap.php';
-$app       = require realpath($bootstrap) ?: $bootstrap;
+/** @var CodeIgniter\CodeIgniter $app */
+$app = require realpath($bootstrap) ?: $bootstrap;
+$app->setContext('spark');
 
 // Grab our Console
 $console = new CodeIgniter\CLI\Console($app);

system/CodeIgniter.php

@@ -141,6 +141,16 @@ class CodeIgniter
      */
     protected $useSafeOutput = false;
 
+    /**
+     * Context
+     *  web:     Invoked by HTTP request
+     *  php-cli: Invoked by CLI via `php public/index.php`
+     *  spark:   Invoked by CLI via the `spark` command
+     *
+     * @phpstan-var 'php-cli'|'spark'|'web'
+     */
+    protected string $context;
+
     /**
      * Constructor.
      */
@@ -294,6 +304,11 @@ protected function initializeKint()
      */
     public function run(?RouteCollectionInterface $routes = null, bool $returnResponse = false)
     {
+        assert(
+            $this->context !== null,
+            'Context must be set before run() is called. If you are upgrading from 4.1.x, you need to merge `public/index.php` and `spark` file from `vendor/codeigniter4/framework`.'
+        );
+
         $this->startBenchmark();
 
         $this->getRequestObject();
@@ -321,7 +336,7 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon
         }
 
         // spark command has nothing to do with HTTP redirect and 404
-        if (defined('SPARKED')) {
+        if ($this->isSparked()) {
             return $this->handleRequest($routes, $cacheConfig, $returnResponse);
         }
 
@@ -358,6 +373,30 @@ public function useSafeOutput(bool $safe = true)
         return $this;
     }
 
+    /**
+     * Invoked via spark command?
+     */
+    private function isSparked(): bool
+    {
+        return $this->context === 'spark';
+    }
+
+    /**
+     * Invoked via php-cli command?
+     */
+    private function isPhpCli(): bool
+    {
+        return $this->context === 'php-cli';
+    }
+
+    /**
+     * Web access?
+     */
+    private function isWeb(): bool
+    {
+        return $this->context === 'web';
+    }
+
     /**
      * Handles the main request logic and fires the controller.
      *
@@ -389,7 +428,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
         }
 
         // Never run filters when running through Spark cli
-        if (! defined('SPARKED')) {
+        if (! $this->isSparked()) {
             // Run "before" filters
             $this->benchmark->start('before_filters');
             $possibleResponse = $filters->run($uri, 'before');
@@ -430,7 +469,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
         $this->gatherOutput($cacheConfig, $returned);
 
         // Never run filters when running through Spark cli
-        if (! defined('SPARKED')) {
+        if (! $this->isSparked()) {
             $filters->setResponse($this->response);
 
             // Run "after" filters
@@ -548,10 +587,8 @@ protected function getRequestObject()
             return;
         }
 
-        if (is_cli() && ENVIRONMENT !== 'testing') {
-            // @codeCoverageIgnoreStart
+        if ($this->isSparked() || $this->isPhpCli()) {
             $this->request = Services::clirequest($this->config);
-        // @codeCoverageIgnoreEnd
         } else {
             $this->request = Services::request($this->config);
             // guess at protocol if needed
@@ -567,7 +604,7 @@ protected function getResponseObject()
     {
         $this->response = Services::response($this->config);
 
-        if (! is_cli() || ENVIRONMENT === 'testing') {
+        if ($this->isWeb()) {
             $this->response->setProtocolVersion($this->request->getProtocolVersion());
         }
 
@@ -828,7 +865,7 @@ protected function createController()
     protected function runController($class)
     {
         // If this is a console request then use the input segments as parameters
-        $params = defined('SPARKED') ? $this->request->getSegments() : $this->router->params();
+        $params = $this->isSparked() ? $this->request->getSegments() : $this->router->params();
 
         if (method_exists($class, '_remap')) {
             $output = $class->_remap($this->method, ...$params);
@@ -888,7 +925,9 @@ protected function display404errors(PageNotFoundException $e)
             ob_end_flush(); // @codeCoverageIgnore
         }
 
-        throw PageNotFoundException::forPageNotFound(ENVIRONMENT !== 'production' || is_cli() ? $e->getMessage() : '');
+        throw PageNotFoundException::forPageNotFound(
+            (ENVIRONMENT !== 'production' || ! $this->isWeb()) ? $e->getMessage() : ''
+        );
     }
 
     /**
@@ -954,8 +993,8 @@ protected function gatherOutput(?Cache $cacheConfig = null, $returned = null)
     public function storePreviousURL($uri)
     {
         // Ignore CLI requests
-        if (is_cli() && ENVIRONMENT !== 'testing') {
-            return; // @codeCoverageIgnore
+        if (! $this->isWeb()) {
+            return;
         }
         // Ignore AJAX requests
         if (method_exists($this->request, 'isAJAX') && $this->request->isAJAX()) {
@@ -1019,4 +1058,18 @@ protected function callExit($code)
     {
         exit($code); // @codeCoverageIgnore
     }
+
+    /**
+     * Sets the app context.
+     *
+     * @phpstan-param 'php-cli'|'spark'|'web' $context
+     *
+     * @return $this
+     */
+    public function setContext(string $context)
+    {
+        $this->context = $context;
+
+        return $this;
+    }
 }

system/Test/FeatureTestCase.php

@@ -194,6 +194,7 @@ public function call(string $method, string $path, ?array $params = null)
         Services::injectMock('filters', Services::filters(null, false));
 
         $response = $this->app
+            ->setContext('web')
             ->setRequest($request)
             ->run($routes, true);
 

system/Test/FeatureTestTrait.php

@@ -184,6 +184,7 @@ public function call(string $method, string $path, ?array $params = null)
         Services::injectMock('filters', Services::filters(null, false));
 
         $response = $this->app
+            ->setContext('web')
             ->setRequest($request)
             ->run($routes, true);
 

system/Test/Mock/MockCodeIgniter.php

@@ -15,6 +15,8 @@
 
 class MockCodeIgniter extends CodeIgniter
 {
+    protected string $context = 'web';
+
     protected function callExit($code)
     {
         // Do not call exit() in testing.

tests/system/CLI/ConsoleTest.php

@@ -49,6 +49,7 @@ protected function setUp(): void
         CLI::init();
 
         $this->app = new MockCodeIgniter(new MockCLIConfig());
+        $this->app->setContext('spark');
     }
 
     protected function tearDown(): void

tests/system/CodeIgniterTest.php

@@ -276,6 +276,7 @@ public function testRunForceSecure()
         $config->forceGlobalSecureRequests = true;
 
         $codeigniter = new MockCodeIgniter($config);
+        $codeigniter->setContext('web');
 
         $this->getPrivateMethodInvoker($codeigniter, 'getRequestObject')();
         $this->getPrivateMethodInvoker($codeigniter, 'getResponseObject')();

user_guide_src/source/installation/upgrade_420.rst

@@ -0,0 +1,60 @@
+#############################
+Upgrading from 4.1.8 to 4.2.0
+#############################
+
+Please refer to the upgrade instructions corresponding to your installation method.
+
+- :ref:`Composer Installation App Starter Upgrading <app-starter-upgrading>`
+- :ref:`Composer Installation Adding CodeIgniter4 to an Existing Project Upgrading <adding-codeigniter4-upgrading>`
+- :ref:`Manual Installation Upgrading <installing-manual-upgrading>`
+
+.. contents::
+    :local:
+    :depth: 2
+
+Mandatory File Changes
+**********************
+
+The following files received significant changes and
+**you must merge the updated versions** with your application:
+
+* ``public/index.php``
+* ``spark``
+
+Breaking Changes
+****************
+
+
+
+Breaking Enhancements
+*********************
+
+none.
+
+Project Files
+*************
+
+Numerous files in the **project space** (root, app, public, writable) received updates. Due to
+these files being outside of the **system** scope they will not be changed without your intervention.
+There are some third-party CodeIgniter modules available to assist with merging changes to
+the project space: `Explore on Packagist <https://packagist.org/explore/?query=codeigniter4%20updates>`_.
+
+.. note:: Except in very rare cases for bug fixes, no changes made to files for the project space
+    will break your application. All changes noted here are optional until the next major version,
+    and any mandatory changes will be covered in the sections above.
+
+Content Changes
+===============
+
+The following files received significant changes (including deprecations or visual adjustments)
+and it is recommended that you merge the updated versions with your application:
+
+*
+
+All Changes
+===========
+
+This is a list of all files in the **project space** that received changes;
+many will be simple comments or formatting that have no effect on the runtime:
+
+*