Merge pull request #10949 from robertpustulka/controller-properties

Deprecate public Controller properties

src/Controller/Controller.php

@@ -102,7 +102,7 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var string
      */
-    public $name;
+    protected $name;
 
     /**
      * An array containing the names of helpers this controller uses. The array elements should
@@ -115,6 +115,8 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var array
      * @link https://book.cakephp.org/3.0/en/controllers.html#configuring-helpers-to-load
+     *
+     * @deprecated 3.0.0 You should configure helpers in your AppView::initialize() method.
      */
     public $helpers = [];
 
@@ -125,6 +127,7 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var \Cake\Http\ServerRequest
      * @link https://book.cakephp.org/3.0/en/controllers/request-response.html#request
+     * @deprecated 3.6.0 The property will become protected in 4.0.0. Use getRequest()/setRequest instead.
      */
     public $request;
 
@@ -133,6 +136,7 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var \Cake\Http\Response
      * @link https://book.cakephp.org/3.0/en/controllers/request-response.html#response
+     * @deprecated 3.6.0 The property will become protected in 4.0.0. Use getResponse()/setResponse instead.
      */
     public $response;
 
@@ -160,7 +164,7 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var bool
      */
-    public $autoRender = true;
+    protected $autoRender = true;
 
     /**
      * Instance of ComponentRegistry used to create Components
@@ -180,6 +184,8 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var array
      * @link https://book.cakephp.org/3.0/en/controllers/components.html
+     *
+     * @deprecated 3.0.0 You should configure components in your Controller::initialize() method.
      */
     public $components = [];
 
@@ -207,7 +213,7 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
      *
      * @var string|null
      */
-    public $plugin;
+    protected $plugin;
 
     /**
      * Holds all passed params.
@@ -245,15 +251,16 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
             $this->name = substr($name, 0, -10);
         }
 
-        $this->setRequest($request !== null ? $request : new ServerRequest());
-        $this->response = $response !== null ? $response : new Response();
+        $this->setRequest($request ?: new ServerRequest());
+        $this->setResponse($response ?: new Response());
 
         if ($eventManager !== null) {
             $this->setEventManager($eventManager);
         }
 
         $this->modelFactory('Table', [$this->getTableLocator(), 'get']);
-        $modelClass = ($this->plugin ? $this->plugin . '.' : '') . $this->name;
+        $plugin = $this->request->getParam('plugin');
+        $modelClass = ($plugin ? $plugin . '.' : '') . $this->name;
         $this->_setModelClass($modelClass);
 
         if ($components !== null) {
@@ -333,6 +340,18 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
     public function __get($name)
     {
         $deprecated = [
+            'name' => 'getName',
+            'plugin' => 'getPlugin',
+            'autoRender' => 'isAutoRenderEnabled',
+        ];
+        if (isset($deprecated[$name])) {
+            $method = $deprecated[$name];
+            deprecationWarning(sprintf('Controller::$%s is deprecated. Use $this->%s instead.', $name, $method));
+
+            return $this->{$method}();
+        }
+
+        $deprecated = [
             'layout' => 'getLayout',
             'view' => 'getTemplate',
             'theme' => 'getTheme',
@@ -365,6 +384,23 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
     public function __set($name, $value)
     {
         $deprecated = [
+            'name' => 'setName',
+            'plugin' => 'setPlugin'
+        ];
+        if (isset($deprecated[$name])) {
+            $method = $deprecated[$name];
+            deprecationWarning(sprintf('Controller::$%s is deprecated. Use $this->%s() instead.', $name, $method));
+            $this->{$method}($value);
+
+            return;
+        }
+        if ($name === 'autoRender') {
+            $value ? $this->enableAutoRender() : $this->disableAutoRender();
+            deprecationWarning(sprintf('Controller::$%s is deprecated. Use $this->enableAutoRender/disableAutoRender() instead.', $name));
+
+            return;
+        }
+        $deprecated = [
             'layout' => 'setLayout',
             'view' => 'setTemplate',
             'theme' => 'setTheme',
@@ -385,17 +421,113 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
     }
 
     /**
+     * Returns the controller name.
+     *
+     * @return string
+     * @since 3.6.0
+     */
+    public function getName()
+    {
+        return $this->name;
+    }
+
+    /**
+     * Sets the controller name.
+     *
+     * @param string $name Controller name.
+     * @return $this
+     * @since 3.6.0
+     */
+    public function setName($name)
+    {
+        $this->name = $name;
+
+        return $this;
+    }
+
+    /**
+     * Returns the plugin name.
+     *
+     * @return string|null
+     * @since 3.6.0
+     */
+    public function getPlugin()
+    {
+        return $this->plugin;
+    }
+
+    /**
+     * Sets the plugin name.
+     *
+     * @param string $name Plugin name.
+     * @return $this
+     * @since 3.6.0
+     */
+    public function setPlugin($name)
+    {
+        $this->plugin = $name;
+
+        return $this;
+    }
+
+    /**
+     * Returns true if an action should be rendered automatically.
+     *
+     * @return bool
+     * @since 3.6.0
+     */
+    public function isAutoRenderEnabled()
+    {
+        return $this->autoRender;
+    }
+
+    /**
+     * Enable automatic action rendering.
+     *
+     * @return $this
+     * @since 3.6.0
+     */
+    public function enableAutoRender()
+    {
+        $this->autoRender = true;
+
+        return $this;
+    }
+
+    /**
+     * Disbale automatic action rendering.
+     *
+     * @return $this
+     * @since 3.6.0
+     */
+    public function disableAutoRender()
+    {
+        $this->autoRender = false;
+
+        return $this;
+    }
+
+    /**
+     * Gets the request instance.
+     *
+     * @return \Cake\Http\ServerRequest
+     * @since 3.6.0
+     */
+    public function getRequest()
+    {
+        return $this->request;
+    }
+
+    /**
      * Sets the request objects and configures a number of controller properties
      * based on the contents of the request. Controller acts as a proxy for certain View variables
      * which must also be updated here. The properties that get set are:
      *
      * - $this->request - To the $request parameter
-     * - $this->plugin - To the $request->params['plugin']
      * - $this->passedArgs - Same as $request->params['pass]
-     * - View::$plugin - $this->plugin
      *
      * @param \Cake\Http\ServerRequest $request Request instance.
-     * @return void
+     * @return $this
      */
     public function setRequest(ServerRequest $request)
     {
@@ -405,6 +537,33 @@ class Controller implements EventListenerInterface, EventDispatcherInterface
         if ($request->getParam('pass')) {
             $this->passedArgs = $request->getParam('pass');
         }
+
+        return $this;
+    }
+
+    /**
+     * Gets the response instance.
+     *
+     * @return \Cake\Http\Response
+     * @since 3.6.0
+     */
+    public function getResponse()
+    {
+        return $this->response;
+    }
+
+    /**
+     * Sets the response instance.
+     *
+     * @param \Cake\Http\Response $response Response instance.
+     * @return $this
+     * @since 3.6.0
+     */
+    public function setResponse(Response $response)
+    {
+        $this->response = $response;
+
+        return $this;
     }
 
     /**

src/Error/ExceptionRenderer.php

@@ -338,8 +338,8 @@ class ExceptionRenderer implements ExceptionRendererInterface
             return $this->_outputMessage('error500');
         } catch (MissingPluginException $e) {
             $attributes = $e->getAttributes();
-            if (isset($attributes['plugin']) && $attributes['plugin'] === $this->controller->plugin) {
-                $this->controller->plugin = null;
+            if (isset($attributes['plugin']) && $attributes['plugin'] === $this->controller->getPlugin()) {
+                $this->controller->setPlugin(null);
             }
 
             return $this->_outputMessageSafe('error500');

src/Http/ActionDispatcher.php

@@ -121,7 +121,7 @@ class ActionDispatcher
             throw new LogicException('Controller actions can only return Cake\Http\Response or null.');
         }
 
-        if (!$response && $controller->autoRender) {
+        if (!$response && $controller->isAutoRenderEnabled()) {
             $controller->render();
         }
 

tests/TestCase/Controller/ControllerTest.php

@@ -322,7 +322,7 @@ class ControllerTest extends TestCase
         Plugin::load('TestPlugin');
 
         $Controller = new TestPluginController();
-        $Controller->plugin = 'TestPlugin';
+        $Controller->setPlugin('TestPlugin');
 
         $this->assertFalse(isset($Controller->TestPluginComments));
 
@@ -497,7 +497,7 @@ class ControllerTest extends TestCase
         $this->assertSame($response, $Controller->response);
         $this->assertEquals($code, $response->getStatusCode());
         $this->assertEquals('http://cakephp.org', $response->getHeaderLine('Location'));
-        $this->assertFalse($Controller->autoRender);
+        $this->assertFalse($Controller->isAutoRenderEnabled());
     }
 
     /**
@@ -1086,6 +1086,169 @@ class ControllerTest extends TestCase
     }
 
     /**
+     * Test name getter and setter.
+     *
+     * @return void
+     */
+    public function testName()
+    {
+        $controller = new PostsController();
+        $this->assertEquals('Posts', $controller->getName());
+
+        $this->assertSame($controller, $controller->setName('Articles'));
+        $this->assertEquals('Articles', $controller->getName());
+    }
+
+    /**
+     * Test plugin getter and setter.
+     *
+     * @return void
+     */
+    public function testPlugin()
+    {
+        $controller = new PostsController();
+        $this->assertEquals('', $controller->getPlugin());
+
+        $this->assertSame($controller, $controller->setPlugin('Articles'));
+        $this->assertEquals('Articles', $controller->getPlugin());
+    }
+
+    /**
+     * Test request getter and setter.
+     *
+     * @return void
+     */
+    public function testRequest()
+    {
+        $controller = new PostsController();
+        $this->assertInstanceOf(ServerRequest::class, $controller->getRequest());
+
+        $request = new ServerRequest([
+            'params' => [
+                'plugin' => 'Posts',
+                'pass' => [
+                    'foo',
+                    'bar'
+                ]
+            ]
+        ]);
+        $this->assertSame($controller, $controller->setRequest($request));
+        $this->assertSame($request, $controller->getRequest());
+
+        $this->assertEquals('Posts', $controller->getRequest()->getParam('plugin'));
+        $this->assertEquals(['foo', 'bar'], $controller->passedArgs);
+    }
+
+    /**
+     * Test response getter and setter.
+     *
+     * @return void
+     */
+    public function testResponse()
+    {
+        $controller = new PostsController();
+        $this->assertInstanceOf(Response::class, $controller->getResponse());
+
+        $response = new Response;
+        $this->assertSame($controller, $controller->setResponse($response));
+        $this->assertSame($response, $controller->getResponse());
+    }
+
+    /**
+     * Test autoRender getter and setter.
+     *
+     * @return void
+     */
+    public function testAutoRender()
+    {
+        $controller = new PostsController();
+        $this->assertTrue($controller->isAutoRenderEnabled());
+
+        $this->assertSame($controller, $controller->disableAutoRender());
+        $this->assertFalse($controller->isAutoRenderEnabled());
+
+        $this->assertSame($controller, $controller->enableAutoRender());
+        $this->assertTrue($controller->isAutoRenderEnabled());
+    }
+
+    /**
+     * Tests deprecated controller properties work
+     *
+     * @group deprecated
+     * @param $property Deprecated property name
+     * @param $getter Getter name
+     * @param $setter Setter name
+     * @param mixed $value Value to be set
+     * @return void
+     * @dataProvider deprecatedControllerPropertyProvider
+     */
+    public function testDeprecatedControllerProperty($property, $getter, $setter, $value)
+    {
+        $controller = new AnotherTestController();
+        $this->deprecated(function () use ($controller, $property, $value) {
+            $controller->$property = $value;
+            $this->assertSame($value, $controller->$property);
+        });
+        $this->assertSame($value, $controller->{$getter}());
+    }
+
+    /**
+     * Tests deprecated controller properties message
+     *
+     * @group deprecated
+     * @param $property Deprecated property name
+     * @param $getter Getter name
+     * @param $setter Setter name
+     * @param mixed $value Value to be set
+     * @return void
+     * @expectedException PHPUnit\Framework\Error\Deprecated
+     * @expectedExceptionMessageRegExp /^Controller::\$\w+ is deprecated(.*)/
+     * @dataProvider deprecatedControllerPropertyProvider
+     */
+    public function testDeprecatedControllerPropertySetterMessage($property, $getter, $setter, $value)
+    {
+        $controller = new AnotherTestController();
+        $this->withErrorReporting(E_ALL, function () use ($controller, $property, $value) {
+            $controller->$property = $value;
+        });
+    }
+
+    /**
+     * Tests deprecated controller properties message
+     *
+     * @param $property Deprecated property name
+     * @param $getter Getter name
+     * @param $setter Setter name
+     * @param mixed $value Value to be set
+     * @return void
+     * @expectedException PHPUnit\Framework\Error\Deprecated
+     * @expectedExceptionMessageRegExp /^Controller::\$\w+ is deprecated(.*)/
+     * @dataProvider deprecatedControllerPropertyProvider
+     */
+    public function testDeprecatedControllerPropertyGetterMessage($property, $getter, $setter, $value)
+    {
+        $controller = new AnotherTestController();
+        $controller->{$setter}($value);
+        $this->withErrorReporting(E_ALL, function () use ($controller, $property) {
+            $controller->$property;
+        });
+    }
+
+    /**
+     * Data provider for testing deprecated view properties
+     *
+     * @return array
+     */
+    public function deprecatedControllerPropertyProvider()
+    {
+        return [
+            ['name', 'getName', 'setName', 'Foo'],
+            ['plugin', 'getPlugin', 'setPlugin', 'Foo'],
+            ['autoRender', 'isAutoRenderEnabled', 'disableAutoRender', false],
+        ];
+    }
+
+    /**
      * Tests deprecated view properties work
      *
      * @group deprecated

tests/TestCase/Error/ExceptionRendererTest.php

@@ -781,7 +781,7 @@ class ExceptionRendererTest extends TestCase
         $ExceptionRenderer->controller = $this->getMockBuilder('Cake\Controller\Controller')
             ->setMethods(['render'])
             ->getMock();
-        $ExceptionRenderer->controller->plugin = 'TestPlugin';
+        $ExceptionRenderer->controller->setPlugin('TestPlugin');
         $ExceptionRenderer->controller->request = $this->getMockBuilder('Cake\Http\ServerRequest')->getMock();
 
         $exception = new MissingPluginException(['plugin' => 'TestPlugin']);
@@ -810,7 +810,7 @@ class ExceptionRendererTest extends TestCase
         $ExceptionRenderer->controller = $this->getMockBuilder('Cake\Controller\Controller')
             ->setMethods(['render'])
             ->getMock();
-        $ExceptionRenderer->controller->plugin = 'TestPlugin';
+        $ExceptionRenderer->controller->setPlugin('TestPlugin');
         $ExceptionRenderer->controller->request = $this->getMockBuilder('Cake\Http\ServerRequest')->getMock();
 
         $exception = new MissingPluginException(['plugin' => 'TestPluginTwo']);

tests/TestCase/View/JsonViewTest.php

@@ -318,7 +318,7 @@ class JsonViewTest extends TestCase
         $Request = new ServerRequest();
         $Response = new Response();
         $Controller = new Controller($Request, $Response);
-        $Controller->name = 'Posts';
+        $Controller->setName('Posts');
 
         $data = [
             'User' => [
@@ -332,7 +332,7 @@ class JsonViewTest extends TestCase
         $Controller->set('user', $data);
         $Controller->viewBuilder()->setClassName('Json');
         $View = $Controller->createView();
-        $View->setTemplatePath($Controller->name);
+        $View->setTemplatePath($Controller->getName());
         $output = $View->render('index');
 
         $expected = json_encode(['user' => 'fake', 'list' => ['item1', 'item2'], 'paging' => null]);

tests/TestCase/View/ViewTest.php

@@ -1071,7 +1071,7 @@ class ViewTest extends TestCase
     public function testViewEvent()
     {
         $View = $this->PostsController->createView();
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
         $View->enableAutoLayout(false);
         $listener = new TestViewEventListenerInterface();
 
@@ -1180,7 +1180,7 @@ class ViewTest extends TestCase
     public function testHelperCallbackTriggering()
     {
         $View = $this->PostsController->createView();
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
 
         $manager = $this->getMockBuilder('Cake\Event\EventManager')->getMock();
         $View->setEventManager($manager);
@@ -1272,7 +1272,7 @@ class ViewTest extends TestCase
             'Html'
         ];
         $View = $this->PostsController->createView();
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
         $View->render('index');
         $this->assertEquals('Valuation', $View->helpers()->TestBeforeAfter->property);
     }
@@ -1291,7 +1291,7 @@ class ViewTest extends TestCase
         $this->PostsController->set('variable', 'values');
 
         $View = $this->PostsController->createView();
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
 
         $content = 'This is my view output';
         $result = $View->renderLayout($content, 'default');
@@ -1308,7 +1308,7 @@ class ViewTest extends TestCase
     {
         $this->PostsController->helpers = ['Form', 'Number'];
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
 
         $result = $View->render('index', false);
         $this->assertEquals('posts index', $result);
@@ -1319,7 +1319,7 @@ class ViewTest extends TestCase
 
         $this->PostsController->helpers = ['Html', 'Form', 'Number', 'TestPlugin.PluggedHelper'];
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
 
         $result = $View->render('index', false);
         $this->assertEquals('posts index', $result);
@@ -1337,7 +1337,7 @@ class ViewTest extends TestCase
     public function testRender()
     {
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
         $result = $View->render('index');
 
         $this->assertRegExp("/<meta charset=\"utf-8\"\/>\s*<title>/", $result);
@@ -1356,7 +1356,7 @@ class ViewTest extends TestCase
         Configure::write('Cache.check', true);
 
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
         $result = $View->render('index');
 
         $this->assertRegExp("/<meta charset=\"utf-8\"\/>\s*<title>/", $result);
@@ -1371,7 +1371,7 @@ class ViewTest extends TestCase
     public function testRenderUsingViewProperty()
     {
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
-        $View->setTemplatePath($this->PostsController->name);
+        $View->setTemplatePath($this->PostsController->getName());
         $View->setTemplate('cache_form');
 
         $this->assertEquals('cache_form', $View->getTemplate());
@@ -1408,8 +1408,8 @@ class ViewTest extends TestCase
      */
     public function testGetViewFileNameSubdirWithPluginAndViewPath()
     {
-        $this->PostsController->plugin = 'TestPlugin';
-        $this->PostsController->name = 'Posts';
+        $this->PostsController->setPlugin('TestPlugin');
+        $this->PostsController->setName('Posts');
         $View = $this->PostsController->createView('Cake\Test\TestCase\View\TestView');
         $View->setTemplatePath('Element');
 
@@ -1431,7 +1431,7 @@ class ViewTest extends TestCase
         $Controller->helpers = ['Html'];
         $Controller->set('html', 'I am some test html');
         $View = $Controller->createView();
-        $View->setTemplatePath($Controller->name);
+        $View->setTemplatePath($Controller->getName());
         $result = $View->render('helper_overwrite', false);
 
         $this->assertRegExp('/I am some test html/', $result);
@@ -1974,8 +1974,8 @@ TEXT;
     public function testMemoryLeakInPaths()
     {
         $this->skipIf(env('CODECOVERAGE') == 1, 'Running coverage this causes this tests to fail sometimes.');
-        $this->ThemeController->plugin = null;
-        $this->ThemeController->name = 'Posts';
+        $this->ThemeController->setPlugin(null);
+        $this->ThemeController->setName('Posts');
 
         $View = $this->ThemeController->createView();
         $View->setTemplatePath('Posts');

tests/TestCase/View/XmlViewTest.php

@@ -281,7 +281,7 @@ class XmlViewTest extends TestCase
         $Request = new ServerRequest();
         $Response = new Response();
         $Controller = new Controller($Request, $Response);
-        $Controller->name = 'Posts';
+        $Controller->setName('Posts');
 
         $data = [
             [