Fix forms namespace

open-sausages · Jun 20, 2017 · 2a278e2 · 2a278e2
1 parent b65c212
commit 2a278e2
Show file tree

Hide file tree

Showing 41 changed files with 283 additions and 201 deletions.
diff --git a/src/Control/Director.php b/src/Control/Director.php
@@ -131,18 +131,9 @@ public static function direct(HTTPRequest $request)
         // Generate output
         $result = static::handleRequest($request);
 
-        // Save session data. Note that inst_save() will start/resume the session if required.
+        // Save session data. Note that save() will start/resume the session if required.
         $request->getSession()->save();
 
-        // Return code for a redirection request
-        // @todo: Refactor into CLIApplication
-        if ($result->isRedirect() && static::is_cli()) {
-            $url = Director::makeRelative($result->getHeader('Location'));
-            $request = clone $request;
-            $request->setUrl($url);
-            return static::direct($request);
-        }
-
         // Post-request handling
         $postRequest = RequestProcessor::singleton()->postRequest($request, $result);
         if ($postRequest === false) {
@@ -178,7 +169,7 @@ public static function direct(HTTPRequest $request)
      */
     public static function test(
         $url,
-        $postVars = null,
+        $postVars = [],
         $session = array(),
         $httpMethod = null,
         $body = null,
@@ -213,13 +204,24 @@ public static function test(
         }
 
         // Default httpMethod
-        $newVars['_SERVER']['REQUEST_METHOD'] = $httpMethod
-            ?: (($postVars || is_array($postVars)) ? "POST" : "GET");
+        $newVars['_SERVER']['REQUEST_METHOD'] = $httpMethod ?: ($postVars ? "POST" : "GET");
+        $newVars['_POST'] = (array)$postVars;
 
         // Setup session
-        $newVars['_SESSION'] = $session instanceof Session
-            ? $session->getAll()
-            : ($session ?: []);
+        if ($session instanceof Session) {
+            // Note: If passing $session as object, ensure that changes are written back
+            // This is important for classes such as FunctionalTest which emulate cross-request persistence
+            $newVars['_SESSION'] = $session->getAll();
+            $finally[] = function () use ($session) {
+                if (isset($_SESSION)) {
+                    foreach ($_SESSION as $key => $value) {
+                        $session->set($key, $value);
+                    }
+                }
+            };
+        } else {
+            $newVars['_SESSION'] = $session ?: [];
+        }
 
         // Setup cookies
         $cookieJar = $cookies instanceof Cookie_Backend
@@ -268,6 +270,9 @@ public static function test(
             }
         }
 
+        // Apply new vars to environment
+        static::varsToEnv($newVars);
+
         try {
             // Normal request handling
             return static::direct($request);

diff --git a/src/Dev/ExtensionTestState.php b/src/Dev/ExtensionTestState.php
@@ -55,7 +55,7 @@ public function setUpOnce($class)
                     continue;
                 }
                 if (!isset($this->extensionsToReapply[$dataClass])) {
-                    $this->extensionsToReapply[$dataClass] = array();
+                    $this->extensionsToReapply[$dataClass] = [];
                 }
                 $this->extensionsToReapply[$dataClass][] = $extension;
                 $dataClass::remove_extension($extension);
@@ -70,14 +70,13 @@ public function setUpOnce($class)
             }
             $this->extensionsToRemove[$dataClass] = array();
             foreach ($extensions as $extension) {
-                $dataClass = Extension::get_classname_without_arguments($extension);
-                if (!class_exists($dataClass)) {
-                    $self = static::class;
-                    throw new LogicException("Test {$self} requires extension {$extension} which doesn't exist");
+                $extension = Extension::get_classname_without_arguments($extension);
+                if (!class_exists($extension)) {
+                    throw new LogicException("Test {$class} requires extension {$extension} which doesn't exist");
                 }
                 if (!$dataClass::has_extension($extension)) {
                     if (!isset($this->extensionsToRemove[$dataClass])) {
-                        $this->extensionsToReapply[$dataClass] = array();
+                        $this->extensionsToReapply[$dataClass] = [];
                     }
                     $this->extensionsToRemove[$dataClass][] = $extension;
                     $dataClass::add_extension($extension);

diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Dev;
 
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Config\Config;
@@ -408,11 +409,11 @@ public function assertExactHTMLMatchBySelector($selector, $expectedMatches, $mes
     public function useDraftSite($enabled = true)
     {
         if ($enabled) {
-            $this->session()->inst_set('readingMode', 'Stage.Stage');
-            $this->session()->inst_set('unsecuredDraftSite', true);
+            $this->session()->set('readingMode', 'Stage.Stage');
+            $this->session()->set('unsecuredDraftSite', true);
         } else {
-            $this->session()->inst_set('readingMode', 'Stage.Live');
-            $this->session()->inst_set('unsecuredDraftSite', false);
+            $this->session()->set('readingMode', 'Stage.Live');
+            $this->session()->set('unsecuredDraftSite', false);
         }
     }
 

diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php
@@ -904,7 +904,7 @@ public static function start()
             // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly)
             DataObject::reset();
 
-            // Set dummy controller
+            // Set dummy controller;
             $controller = Controller::create();
             $controller->setRequest($request);
             $controller->pushCurrent();

diff --git a/src/Dev/TestSession.php b/src/Dev/TestSession.php
@@ -61,6 +61,7 @@ public function __construct()
         $this->controller = new Controller();
         $this->controller->setRequest($request);
         $this->controller->pushCurrent();
+        $this->controller->doInit();
     }
 
     public function __destruct()

diff --git a/src/Forms/DefaultFormFactory.php b/src/Forms/DefaultFormFactory.php
@@ -26,6 +26,12 @@ public function __construct()
         $this->constructExtensions();
     }
 
+    /**
+     * @param RequestHandler $controller
+     * @param string $name
+     * @param array $context
+     * @return Form
+     */
     public function getForm(RequestHandler $controller = null, $name = FormFactory::DEFAULT_NAME, $context = [])
     {
         // Validate context

diff --git a/src/Forms/Form.php b/src/Forms/Form.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Forms;
 
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\HasRequestHandler;
 use SilverStripe\Control\HTTP;
 use SilverStripe\Control\RequestHandler;
@@ -347,7 +348,9 @@ public function clearFormState()
      */
     protected function getSession()
     {
-        return $this->getRequestHandler()->getRequest()->getSession();
+        // Note: Session may not be available if this form doesn't have a request handler
+        $controller = $this->getController() ?: Controller::curr();
+        return $controller->getRequest()->getSession();
     }
 
     /**

diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php
@@ -4,9 +4,9 @@
 
 use SilverStripe\Admin\LeftAndMain;
 use SilverStripe\Control\Controller;
-use SilverStripe\Control\RequestHandler;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\RequestHandler;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
 use SilverStripe\Forms\FormAction;

diff --git a/src/Forms/GridField/GridFieldExportButton.php b/src/Forms/GridField/GridFieldExportButton.php
@@ -134,9 +134,7 @@ protected function getExportColumnsForGridField(GridField $gridField)
         }
 
         /** @var GridFieldDataColumns $dataCols */
-        $dataCols = $gridField->getConfig()->getComponentByType(
-            'SilverStripe\\Forms\\GridField\\GridFieldDataColumns'
-        );
+        $dataCols = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class);
         if ($dataCols) {
             return $dataCols->getDisplayFields($gridField);
         }

diff --git a/src/Forms/HTMLEditor/TinyMCEConfig.php b/src/Forms/HTMLEditor/TinyMCEConfig.php
@@ -365,7 +365,7 @@ public function enablePlugins($plugin)
 
     /**
      * Enable one or several plugins. Will properly handle being passed a plugin that is already disabled
-     * @param string $plugin,... a string, or several strings, or a single array of strings - The plugins to enable
+     * @param string|array $plugin,... a string, or several strings, or a single array of strings - The plugins to enable
      * @return $this
      */
     public function disablePlugins($plugin)

diff --git a/tests/php/Control/ControllerTest/TestController.php b/tests/php/Control/ControllerTest/TestController.php
@@ -10,6 +10,14 @@
  */
 class TestController extends Controller implements TestOnly
 {
+    public function __construct()
+    {
+        parent::__construct();
+        if (Controller::has_curr()) {
+            $this->setRequest(Controller::curr()->getRequest());
+        }
+    }
+
     private static $url_segment = 'TestController';
 
     public $Content = "default content";

diff --git a/tests/php/Control/DirectorTest/TestController.php b/tests/php/Control/DirectorTest/TestController.php
@@ -7,6 +7,14 @@
 
 class TestController extends Controller implements TestOnly
 {
+    public function __construct()
+    {
+        parent::__construct();
+        if (Controller::has_curr()) {
+            $this->setRequest(Controller::curr()->getRequest());
+        }
+    }
+
     private static $url_segment = 'TestController';
 
     private static $allowed_actions = array(

diff --git a/tests/php/Control/RequestHandlingTest/TestController.php b/tests/php/Control/RequestHandlingTest/TestController.php
@@ -3,6 +3,7 @@
 namespace SilverStripe\Control\Tests\RequestHandlingTest;
 
 use SilverStripe\Control\Controller;
+use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Dev\TestOnly;
@@ -41,25 +42,28 @@ public function __construct()
     {
         $this->failover = new ControllerFailover();
         parent::__construct();
+        if (Controller::has_curr()) {
+            $this->setRequest(Controller::curr()->getRequest());
+        }
     }
 
-    public function index($request)
+    public function index(HTTPRequest $request)
     {
         return "This is the controller";
     }
 
-    public function method($request)
+    public function method(HTTPRequest $request)
     {
         return "This is a method on the controller: " . $request->param('ID') . ', ' . $request->param('OtherID');
     }
 
-    public function legacymethod($request)
+    public function legacymethod(HTTPRequest $request)
     {
         return "\$this->urlParams can be used, for backward compatibility: " . $this->urlParams['ID'] . ', '
         . $this->urlParams['OtherID'];
     }
 
-    public function virtualfile($request)
+    public function virtualfile(HTTPRequest $request)
     {
         return "This is the virtualfile method";
     }

diff --git a/tests/php/Forms/CheckboxSetFieldTest.php b/tests/php/Forms/CheckboxSetFieldTest.php
@@ -169,17 +169,14 @@ public function testSaveWithArrayValueSet()
 
     public function testLoadDataFromObject()
     {
-        $article = $this->objFromFixture(Article::class, 'articlewithouttags');
         $articleWithTags = $this->objFromFixture(Article::class, 'articlewithtags');
         $tag1 = $this->objFromFixture(Tag::class, 'tag1');
         $tag2 = $this->objFromFixture(Tag::class, 'tag2');
 
         $field = new CheckboxSetField("Tags", "Test field", DataObject::get(Tag::class)->map());
-        /**
- * @skipUpgrade
-*/
+        /** @skipUpgrade */
         $form = new Form(
-            new Controller(),
+            Controller::curr(),
             'Form',
             new FieldList($field),
             new FieldList()

diff --git a/tests/php/Forms/ConfirmedPasswordFieldTest.php b/tests/php/Forms/ConfirmedPasswordFieldTest.php
@@ -2,14 +2,13 @@
 
 namespace SilverStripe\Forms\Tests;
 
-use SilverStripe\Control\Tests\ControllerTest\TestController;
-use SilverStripe\Security\Member;
-use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Control\Controller;
+use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\ConfirmedPasswordField;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
 use SilverStripe\Forms\RequiredFields;
+use SilverStripe\Security\Member;
 
 class ConfirmedPasswordFieldTest extends SapphireTest
 {
@@ -39,10 +38,8 @@ public function testHashHidden()
         $member->Password = "valueB";
         $member->write();
 
-        /**
- * @skipUpgrade
-*/
-        $form = new Form(new TestController(), 'Form', new FieldList($field), new FieldList());
+        /** @skipUpgrade */
+        $form = new Form(Controller::curr(), 'Form', new FieldList($field), new FieldList());
         $form->loadDataFrom($member);
 
         $this->assertEquals('', $field->Value());
@@ -92,10 +89,8 @@ public function testValidation()
             )
         );
         $validator = new RequiredFields();
-        /**
- * @skipUpgrade
-*/
-        $form = new Form(new TestController(), 'Form', new FieldList($field), new FieldList(), $validator);
+        /** @skipUpgrade */
+        new Form(Controller::curr(), 'Form', new FieldList($field), new FieldList(), $validator);
         $this->assertTrue(
             $field->validate($validator),
             "Validates when both passwords are the same"
@@ -120,11 +115,9 @@ public function testValidation()
 
     public function testFormValidation()
     {
-        /**
- * @skipUpgrade
-*/
+        /** @skipUpgrade */
         $form = new Form(
-            new Controller(),
+            Controller::curr(),
             'Form',
             new FieldList($field = new ConfirmedPasswordField('Password')),
             new FieldList()

diff --git a/tests/php/Forms/DatetimeFieldTest.php b/tests/php/Forms/DatetimeFieldTest.php
@@ -2,16 +2,14 @@
 
 namespace SilverStripe\Forms\Tests;
 
-use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Control\Controller;
+use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\DatetimeField;
-use SilverStripe\Forms\RequiredFields;
-use SilverStripe\Forms\DateField;
-use SilverStripe\Forms\Tests\DatetimeFieldTest\Model;
-use SilverStripe\Forms\TimeField;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\FormAction;
 use SilverStripe\Forms\Form;
+use SilverStripe\Forms\FormAction;
+use SilverStripe\Forms\RequiredFields;
+use SilverStripe\Forms\Tests\DatetimeFieldTest\Model;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\FieldType\DBDatetime;
 
@@ -452,7 +450,7 @@ protected function getMockForm()
     {
         /** @skipUpgrade */
         return new Form(
-            new Controller(),
+            Controller::curr(),
             'Form',
             new FieldList(),
             new FieldList(

diff --git a/tests/php/Forms/DropdownFieldTest.php b/tests/php/Forms/DropdownFieldTest.php
@@ -479,7 +479,7 @@ public function testValidation()
             )
         );
         $validator = new RequiredFields();
-        $form = new Form(null, 'Form', new FieldList($field), new FieldList(), $validator);
+        new Form(null, 'Form', new FieldList($field), new FieldList(), $validator);
         $field->setValue("One");
         $this->assertTrue($field->validate($validator));
         $field->setName("TestNew"); //try changing name of field