API Refactor $flush into HTPPApplication

API Enforce health check in Controller::pushCurrent()
API Better global backup / restore
Updated Director::test() to use new API

cli-script.php

@@ -11,10 +11,9 @@ require __DIR__ . '/src/includes/autoload.php';
 
 // Build request and detect flush
 $request = HTTPRequest::createFromEnvironment();
-$flush = $request->getVar('flush') || strpos($request->getURL(), 'dev/build') === 0;
 
 // Default application
-$kernel = new AppKernel($flush);
+$kernel = new AppKernel();
 $app = new HTTPApplication($kernel);
 $app->addMiddleware(new OutputMiddleware());
 $app->handle($request);

main.php

@@ -10,10 +10,9 @@ require __DIR__ . '/src/includes/autoload.php';
 
 // Build request and detect flush
 $request = HTTPRequest::createFromEnvironment();
-$flush = $request->getVar('flush') || strpos($request->getURL(), 'dev/build') === 0;
 
 // Default application
-$kernel = new AppKernel($flush);
+$kernel = new AppKernel();
 $app = new HTTPApplication($kernel);
 $app->addMiddleware(new OutputMiddleware());
 $app->addMiddleware(new ErrorControlChainMiddleware($app, $request));

src/Control/Controller.php

@@ -154,10 +154,10 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
      */
     protected function beforeHandleRequest(HTTPRequest $request)
     {
-        //Push the current controller to protect against weird session issues
-        $this->pushCurrent();
         //Set up the internal dependencies (request, response)
         $this->setRequest($request);
+        //Push the current controller to protect against weird session issues
+        $this->pushCurrent();
         $this->setResponse(new HTTPResponse());
         //kick off the init functionality
         $this->doInit();
@@ -588,9 +588,14 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
      * Pushes this controller onto the stack of current controllers. This means that any redirection,
      * session setting, or other things that rely on Controller::curr() will now write to this
      * controller object.
+     *
+     * Note: Ensure this controller is assigned a request with a valid session before pushing
+     * it to the stack.
      */
     public function pushCurrent()
     {
+        // Ensure this controller has a valid session
+        $this->getRequest()->getSession();
         array_unshift(self::$controller_stack, $this);
     }
 

src/Control/Director.php

@@ -3,12 +3,10 @@
 namespace SilverStripe\Control;
 
 use SilverStripe\CMS\Model\SiteTree;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Kernel;
 use SilverStripe\Dev\Deprecation;
-use SilverStripe\ORM\ArrayLib;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\Requirements;
 use SilverStripe\View\Requirements_Backend;
@@ -188,76 +186,82 @@ class Director implements TemplateGlobalProvider
         $cookies = array(),
         &$request = null
     ) {
-        Config::nest();
+        // Build list of cleanup promises
-        Injector::nest();
+        $finally = [];
+
+        /** @var Kernel $kernel */
+        $kernel = Injector::inst()->get(Kernel::class);
+        $kernel->nest();
+        $finally[] = function () use ($kernel) {
+            $kernel->activate();
+        };
+
+        // backup existing vars, and create new vars
+        $existingVars = static::envToVars();
+        $finally[] = function () use ($existingVars) {
+            static::varsToEnv($existingVars);
+        };
+        $newVars = $existingVars;
 
         // These are needed so that calling Director::test() does not muck with whoever is calling it.
         // Really, it's some inappropriate coupling and should be resolved by making less use of statics.
-        $oldReadingMode = null;
         if (class_exists(Versioned::class)) {
             $oldReadingMode = Versioned::get_reading_mode();
-        }
+            $finally[] = function () use ($oldReadingMode) {
-        $getVars = array();
+                Versioned::set_reading_mode($oldReadingMode);
-
+            };
-        if (!$httpMethod) {
-            $httpMethod = ($postVars || is_array($postVars)) ? "POST" : "GET";
         }
 
-        if (!$session) {
+        // Default httpMethod
-            $session = new Session([]);
+        $newVars['_SERVER']['REQUEST_METHOD'] = $httpMethod
-        }
+            ?: (($postVars || is_array($postVars)) ? "POST" : "GET");
+
+        // Setup session
+        $newVars['_SESSION'] = $session instanceof Session
+            ? $session->getAll()
+            : ($session ?: []);
+
+        // Setup cookies
         $cookieJar = $cookies instanceof Cookie_Backend
             ? $cookies
             : Injector::inst()->createWithArgs(Cookie_Backend::class, array($cookies ?: []));
-
+        $newVars['_COOKIE'] = $cookieJar->getAll(false);
-        // Back up the current values of the superglobals
-        $existingRequestVars = isset($_REQUEST) ? $_REQUEST : array();
-        $existingGetVars = isset($_GET) ? $_GET : array();
-        $existingPostVars = isset($_POST) ? $_POST : array();
-        $existingSessionVars = isset($_SESSION) ? $_SESSION : array();
-        $existingCookies = isset($_COOKIE) ? $_COOKIE : array();
-        $existingServer = isset($_SERVER) ? $_SERVER : array();
-
-        $existingRequirementsBackend = Requirements::backend();
-
         Cookie::config()->update('report_errors', false);
-        Requirements::set_backend(Requirements_Backend::create());
+        Injector::inst()->registerService($cookieJar, Cookie_Backend::class);
 
-        if (strpos($url, '#') !== false) {
+        // Backup requirements
-            $url = substr($url, 0, strpos($url, '#'));
+        $existingRequirementsBackend = Requirements::backend();
-        }
+        Requirements::set_backend(Requirements_Backend::create());
+        $finally[] = function () use ($existingRequirementsBackend) {
+            Requirements::set_backend($existingRequirementsBackend);
+        };
+
+        // Strip any hash
+        $url = strtok($url, '#');
 
         // Handle absolute URLs
         if (parse_url($url, PHP_URL_HOST)) {
             $bits = parse_url($url);
+
             // If a port is mentioned in the absolute URL, be sure to add that into the HTTP host
-            if (isset($bits['port'])) {
+            $newVars['_SERVER']['HTTP_HOST'] = isset($bits['port'])
-                $_SERVER['HTTP_HOST'] = $bits['host'].':'.$bits['port'];
+                ? $bits['host'].':'.$bits['port']
-            } else {
+                : $bits['host'];
-                $_SERVER['HTTP_HOST'] = $bits['host'];
-            }
         }
 
         // Ensure URL is properly made relative.
         // Example: url passed is "/ss31/my-page" (prefixed with BASE_URL), this should be changed to "my-page"
         $url = self::makeRelative($url);
-
-        $urlWithQuerystring = $url;
         if (strpos($url, '?') !== false) {
             list($url, $getVarsEncoded) = explode('?', $url, 2);
-            parse_str($getVarsEncoded, $getVars);
+            parse_str($getVarsEncoded, $newVars['_GET']);
+        } else {
+            $newVars['_GET'] = [];
         }
+        $newVars['_SERVER']['REQUEST_URI'] = Director::baseURL() . $url;
 
-        // Replace the super globals with appropriate test values
+        // Create new request
-        $_REQUEST = ArrayLib::array_merge_recursive((array) $getVars, (array) $postVars);
+        $request = HTTPRequest::createFromVariables($newVars, $body);
-        $_GET = (array) $getVars;
-        $_POST = (array) $postVars;
-        $_SESSION = $session ? $session->getAll() : array();
-        $_COOKIE = $cookieJar->getAll(false);
-        Injector::inst()->registerService($cookieJar, Cookie_Backend::class);
-        $_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring;
-
-        $request = new HTTPRequest($httpMethod, $url, $getVars, $postVars, $body);
         if ($headers) {
             foreach ($headers as $k => $v) {
                 $request->addHeader($k, $v);
@@ -265,53 +269,13 @@ class Director implements TemplateGlobalProvider
         }
 
         try {
-            // Pre-request filtering
+            // Normal request handling
-            $requestProcessor = Injector::inst()->get(RequestProcessor::class);
+            return static::direct($request);
-            $output = $requestProcessor->preRequest($request);
-            if ($output === false) {
-                throw new HTTPResponse_Exception(_t('SilverStripe\\Control\\Director.INVALID_REQUEST', 'Invalid request'), 400);
-            }
-
-            // Process request
-            $result = Director::handleRequest($request);
-
-            // Ensure that the result is an HTTPResponse object
-            if (is_string($result)) {
-                if (substr($result, 0, 9) == 'redirect:') {
-                    $response = new HTTPResponse();
-                    $response->redirect(substr($result, 9));
-                    $result = $response;
-                } else {
-                    $result = new HTTPResponse($result);
-                }
-            }
-
-            $output = $requestProcessor->postRequest($request, $result);
-            if ($output === false) {
-                throw new HTTPResponse_Exception("Invalid response");
-            }
-
-            // Return valid response
-            return $result;
         } finally {
-            // Restore the super globals
+            // Restore state in reverse order to assignment
-            $_REQUEST = $existingRequestVars;
+            foreach (array_reverse($finally) as $callback) {
-            $_GET = $existingGetVars;
+                call_user_func($callback);
-            $_POST = $existingPostVars;
-            $_SESSION = $existingSessionVars;
-            $_COOKIE = $existingCookies;
-            $_SERVER = $existingServer;
-
-            Requirements::set_backend($existingRequirementsBackend);
-
-            // These are needed so that calling Director::test() does not muck with whoever is calling it.
-            // Really, it's some inappropriate coupling and should be resolved by making less use of statics
-            if (class_exists(Versioned::class)) {
-                Versioned::set_reading_mode($oldReadingMode);
             }
-
-            Injector::unnest(); // Restore old CookieJar, etc
-            Config::unnest();
         }
     }
 
@@ -372,6 +336,29 @@ class Director implements TemplateGlobalProvider
         return new HTTPResponse('No URL rule was matched', 404);
     }
 
+    /**
+     * Extract env vars prior to modification
+     *
+     * @return array List of all super globals
+     */
+    public static function envToVars()
+    {
+        // Suppress return by-ref
+        return array_merge($GLOBALS, []);
+    }
+
+    /**
+     * Restore a backed up or modified list of vars to $globals
+     *
+     * @param array $vars
+     */
+    public static function varsToEnv(array $vars)
+    {
+        foreach ($vars as $key => $value) {
+            $GLOBALS[$key] = $value;
+        }
+    }
+
     /**
      * Return the {@link SiteTree} object that is currently being viewed. If there is no SiteTree
      * object to return, then this will return the current controller.

src/Control/HTTPRequest.php

@@ -2,10 +2,10 @@
 
 namespace SilverStripe\Control;
 
+use ArrayAccess;
 use BadMethodCallException;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\ORM\ArrayLib;
-use ArrayAccess;
 
 /**
  * Represents a HTTP-request, including a URL that is tokenised for parsing, and a request method
@@ -159,8 +159,8 @@ class HTTPRequest implements ArrayAccess
     public static function createFromEnvironment()
     {
         // Health-check prior to creating environment
-        $variables = static::variablesFromEnvironment();
+        static::validateEnvironment();
-        return self::createFromVariables($variables, @file_get_contents('php://input'));
+        return self::createFromVariables(Director::envToVars(), @file_get_contents('php://input'));
     }
 
     /**
@@ -256,9 +256,8 @@ class HTTPRequest implements ArrayAccess
      * Error conditions will raise HTTPResponse_Exceptions
      *
      * @throws HTTPResponse_Exception
-     * @return array
      */
-    protected static function variablesFromEnvironment()
+    protected static function validateEnvironment()
     {
         // Validate $_FILES array before merging it with $_POST
         foreach ($_FILES as $key => $value) {
@@ -284,15 +283,6 @@ class HTTPRequest implements ArrayAccess
                 throw new HTTPResponse_Exception('Invalid Host', 400);
             }
         }
-
-        return [
-            '_SERVER' => $_SERVER,
-            '_GET' => $_GET,
-            '_POST' => $_POST,
-            '_FILES' => $_FILES,
-            '_SESSION' => isset($_SESSION) ? $_SESSION : null,
-            '_COOKIE' => $_COOKIE
-        ];
     }
 
     /**

src/Core/AppKernel.php

@@ -28,15 +28,8 @@ use SilverStripe\View\ThemeResourceLoader;
 
 class AppKernel extends CoreKernel
 {
-    /**
+    public function __construct()
-     * @var bool
-     */
-    protected $flush = false;
-
-    public function __construct($flush = false)
     {
-        $this->flush = $flush;
-
         // Initialise the dependency injector as soon as possible, as it is
         // subsequently used by some of the following code
         $injectorLoader = InjectorLoader::inst();
@@ -134,13 +127,10 @@ class AppKernel extends CoreKernel
         return null;
     }
 
-    /**
+    public function boot($flush = false)
-     * @throws HTTPResponse_Exception
-     */
-    public function boot()
     {
         $this->bootPHP();
-        $this->bootManifests();
+        $this->bootManifests($flush);
         $this->bootErrorHandling();
         $this->bootDatabase();
     }
@@ -374,17 +364,19 @@ class AppKernel extends CoreKernel
 
     /**
      * Boot all manifests
+     *
+     * @param bool $flush
      */
-    protected function bootManifests()
+    protected function bootManifests($flush)
     {
         // Setup autoloader
-        $this->getClassLoader()->init($this->getIncludeTests(), $this->flush);
+        $this->getClassLoader()->init($this->getIncludeTests(), $flush);
 
         // Find modules
-        $this->getModuleLoader()->init($this->getIncludeTests(), $this->flush);
+        $this->getModuleLoader()->init($this->getIncludeTests(), $flush);
 
         // Flush config
-        if ($this->flush) {
+        if ($flush) {
             $config = $this->getConfigLoader()->getManifest();
             if ($config instanceof CachedConfigCollection) {
                 $config->setFlush(true);
@@ -397,7 +389,7 @@ class AppKernel extends CoreKernel
         // Find default templates
         $defaultSet = $this->getThemeResourceLoader()->getSet('$default');
         if ($defaultSet instanceof ThemeManifest) {
-            $defaultSet->init($this->getIncludeTests(), $this->flush);
+            $defaultSet->init($this->getIncludeTests(), $flush);
         }
     }
 

src/Core/Application.php

@@ -18,6 +18,8 @@ interface Application
      * Invoke the application control chain
      *
      * @param callable $callback
+     * @param bool $flush
+     * @return mixed
      */
-    public function execute(callable $callback);
+    public function execute(callable $callback, $flush = false);
 }

src/Core/CoreKernel.php

@@ -55,7 +55,7 @@ class CoreKernel implements Kernel
      */
     protected $themeResourceLoader = null;
 
-    public function boot()
+    public function boot($flush = false)
     {
     }
 

src/Core/HTTPApplication.php

@@ -90,26 +90,29 @@ class HTTPApplication implements Application
      */
     public function handle(HTTPRequest $request)
     {
+        $flush = $request->getVar('flush') || strpos($request->getURL(), 'dev/build') === 0;
+
         // Ensure boot is invoked
         return $this->execute(function () use ($request) {
             // Start session and execute
             $request->getSession()->init();
             return Director::direct($request);
-        });
+        }, $flush);
     }
 
     /**
      * Safely boot the application and execute the given main action
      *
      * @param callable $callback
+     * @param bool $flush
      * @return HTTPResponse
      */
-    public function execute(callable $callback)
+    public function execute(callable $callback, $flush = false)
     {
         try {
-            return $this->callMiddleware(function () use ($callback) {
+            return $this->callMiddleware(function () use ($callback, $flush) {
                 // Pre-request boot
-                $this->getKernel()->boot();
+                $this->getKernel()->boot($flush);
                 return call_user_func($callback);
             });
         } finally {

src/Core/Kernel.php

@@ -32,8 +32,10 @@ interface Kernel
 
     /*
      * Boots the current kernel
+     *
+     * @param bool $flush
      */
-    public function boot();
+    public function boot($flush = false);
 
     /**
      * Shutdowns the kernel.

src/Core/Startup/ErrorControlChainMiddleware.php

@@ -90,7 +90,7 @@ class ErrorControlChainMiddleware
     protected function safeReloadWithToken($reloadToken)
     {
         // Safe reload requires manual boot
-        $this->getApplication()->getKernel()->boot();
+        $this->getApplication()->getKernel()->boot(false);
 
         // Ensure session is started
         $this->getRequest()->getSession()->init();

src/Core/TestKernel.php

@@ -7,10 +7,10 @@ namespace SilverStripe\Core;
  */
 class TestKernel extends AppKernel
 {
-    public function __construct($flush = true)
+    public function __construct()
     {
         $this->setEnvironment(self::DEV);
-        parent::__construct($flush);
+        parent::__construct();
     }
 
     /**

src/Dev/Install/install.php5

@@ -9,6 +9,8 @@
  ************************************************************************************
  ************************************************************************************/
 use SilverStripe\Control\Controller;
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\Session;
 use SilverStripe\Core\Startup\ParameterConfirmationToken;
 use SilverStripe\Dev\Install\DatabaseAdapterRegistry;
 use SilverStripe\Dev\Install\DatabaseConfigurationHelper;
@@ -1503,7 +1505,10 @@ PHP
 		require_once 'Core/Core.php';
 
 		// Build database
+        $request = new HTTPRequest('GET', '/');
+        $request->setSession(new Session([]));
 		$con = new Controller();
+		$con->setRequest($request);
 		$con->pushCurrent();
 
 		global $databaseConfig;

src/Dev/SapphireTest.php

@@ -897,6 +897,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase
 
         // Custom application
         $app->execute(function () use ($request) {
+            // Start session and execute
+            $request->getSession()->init();
+
             // Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class
             // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly)
             DataObject::reset();
@@ -906,7 +909,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase
             $controller->setRequest($request);
             $controller->pushCurrent();
             $controller->doInit();
-        });
+        }, true);
 
         // Register state
         static::$state = SapphireTestState::singleton();
@@ -1137,7 +1140,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase
      */
     public function logOut()
     {
-        Injector::inst()->get(IdentityStore::class)->logOut();
+        /** @var IdentityStore $store */
+        $store = Injector::inst()->get(IdentityStore::class);
+        $store->logOut();
     }
 
     /**

src/Dev/TestSession.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Dev;
 
 use SilverStripe\Control\Cookie_Backend;
+use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Director;
@@ -55,8 +56,10 @@ class TestSession
     {
         $this->session = Injector::inst()->create(Session::class, array());
         $this->cookies = Injector::inst()->create(Cookie_Backend::class);
+        $request = new HTTPRequest('GET', '/');
+        $request->setSession($this->session());
         $this->controller = new Controller();
-        // @todo - Ensure $this->session is set on all requests
+        $this->controller->setRequest($request);
         $this->controller->pushCurrent();
     }