Merge pull request #3138 from ADmad/3.0-controller-action

3.0 - WIP - Made Response instance the only valid return type for controller action.

src/Routing/Dispatcher.php

@@ -163,7 +163,7 @@ class Dispatcher implements EventListener {
 			));
 		}
 
-		$response = $this->_invoke($controller, $response);
+		$response = $this->_invoke($controller);
 		if (isset($request->params['return'])) {
 			return $response->body();
 		}
@@ -175,31 +175,31 @@ class Dispatcher implements EventListener {
 
 /**
  * Initializes the components and models a controller will be using.
- * Triggers the controller action, and invokes the rendering if Controller::$autoRender is true and echo's the output.
- * Otherwise the return value of the controller action are returned.
+ * Triggers the controller action and invokes the rendering if Controller::$autoRender
+ * is true. If a response object is returned by controller action that is returned
+ * else controller's $response property is returned.
  *
  * @param Controller $controller Controller to invoke
- * @param \Cake\Network\Response $response The response object to receive the output
- * @return \Cake\Network\Response the resulting response object
+ * @return \Cake\Network\Response The resulting response object
+ * @throws \Cake\Error\Exception If data returned by controller action is not an
+ *   instance of Response
  */
-	protected function _invoke(Controller $controller, Response $response) {
+	protected function _invoke(Controller $controller) {
 		$controller->constructClasses();
 		$controller->startupProcess();
 
-		$render = true;
-		$result = $controller->invokeAction();
-		if ($result instanceof Response) {
-			$render = false;
-			$response = $result;
+		$response = $controller->invokeAction();
+		if ($response !== null && !($response instanceof Response)) {
+			throw new Error\Exception('Controller action can only return an instance of Response');
 		}
 
-		if ($render && $controller->autoRender) {
+		if (!$response && $controller->autoRender) {
 			$response = $controller->render();
-		} elseif (!($result instanceof CakeResponse) && $response->body() === null) {
-			$response->body($result);
+		} elseif (!$response) {
+			$response = $controller->response;
 		}
-		$controller->shutdownProcess();
 
+		$controller->shutdownProcess();
 		return $response;
 	}
 

tests/TestCase/Routing/DispatcherTest.php

@@ -58,12 +58,11 @@ class TestDispatcher extends Dispatcher {
  * invoke method
  *
  * @param \Cake\Controller\Controller $controller
- * @param \Cake\Network\Response $response
- * @return void
+ * @return \Cake\Network\Response $response
  */
-	protected function _invoke(Controller $controller, Response $response) {
+	protected function _invoke(Controller $controller) {
 		$this->controller = $controller;
-		return parent::_invoke($controller, $response);
+		return parent::_invoke($controller);
 	}
 
 /**

tests/TestCase/Routing/RequestActionTraitTest.php

@@ -75,7 +75,7 @@ class RequestActionTraitTest extends TestCase {
 		$this->assertEquals($expected, $result);
 
 		$result = $this->object->requestAction('/request_action/paginate_request_action');
-		$this->assertTrue($result);
+		$this->assertNull($result);
 
 		$result = $this->object->requestAction('/request_action/normal_request_action');
 		$expected = 'Hello World';
@@ -158,13 +158,13 @@ class RequestActionTraitTest extends TestCase {
 		$result = $this->object->requestAction(
 			array('controller' => 'request_action', 'action' => 'paginate_request_action')
 		);
-		$this->assertTrue($result);
+		$this->assertNull($result);
 
 		$result = $this->object->requestAction(
 			array('controller' => 'request_action', 'action' => 'paginate_request_action'),
 			array('pass' => array(5))
 		);
-		$this->assertTrue($result);
+		$this->assertNull($result);
 	}
 
 /**
@@ -186,10 +186,11 @@ class RequestActionTraitTest extends TestCase {
  */
 	public function testRequestActionParamParseAndPass() {
 		$result = $this->object->requestAction('/request_action/params_pass');
-		$this->assertEquals('request_action/params_pass', $result->url);
-		$this->assertEquals('request_action', $result['controller']);
-		$this->assertEquals('params_pass', $result['action']);
-		$this->assertEquals(null, $result['plugin']);
+		$result = json_decode($result, true);
+		$this->assertEquals('request_action/params_pass', $result['url']);
+		$this->assertEquals('request_action', $result['params']['controller']);
+		$this->assertEquals('params_pass', $result['params']['action']);
+		$this->assertNull($result['params']['plugin']);
 	}
 
 /**
@@ -203,13 +204,14 @@ class RequestActionTraitTest extends TestCase {
 			'item' => 'value'
 		);
 		$result = $this->object->requestAction(array('controller' => 'request_action', 'action' => 'post_pass'));
-		$expected = null;
+		$result = json_decode($result, true);
 		$this->assertEmpty($result);
 
 		$result = $this->object->requestAction(
 			array('controller' => 'request_action', 'action' => 'post_pass'),
 			array('post' => $_POST)
 		);
+		$result = json_decode($result, true);
 		$expected = $_POST;
 		$this->assertEquals($expected, $result);
 	}
@@ -228,6 +230,7 @@ class RequestActionTraitTest extends TestCase {
 			['controller' => 'request_action', 'action' => 'query_pass'],
 			['query' => $query]
 		);
+		$result = json_decode($result, true);
 		$this->assertEquals($query, $result);
 
 		$result = $this->object->requestAction([
@@ -235,11 +238,13 @@ class RequestActionTraitTest extends TestCase {
 			'action' => 'query_pass',
 			'?' => $query
 		]);
+		$result = json_decode($result, true);
 		$this->assertEquals($query, $result);
 
 		$result = $this->object->requestAction(
 			'/request_action/query_pass?page=3&sort=body'
 		);
+		$result = json_decode($result, true);
 		$expected = ['page' => 3, 'sort' => 'body'];
 		$this->assertEquals($expected, $result);
 	}
@@ -257,12 +262,14 @@ class RequestActionTraitTest extends TestCase {
 			array('controller' => 'request_action', 'action' => 'post_pass'),
 			array('post' => $data)
 		);
+		$result = json_decode($result, true);
 		$this->assertEquals($data, $result);
 
 		$result = $this->object->requestAction(
 			'/request_action/post_pass',
 			array('post' => $data)
 		);
+		$result = json_decode($result, true);
 		$this->assertEquals($data, $result);
 	}
 
@@ -275,13 +282,15 @@ class RequestActionTraitTest extends TestCase {
 		$result = $this->object->requestAction(
 			'/request_action/params_pass?get=value&limit=5'
 		);
-		$this->assertEquals('value', $result->query['get']);
+		$result = json_decode($result, true);
+		$this->assertEquals('value', $result['query']['get']);
 
 		$result = $this->object->requestAction(
 			array('controller' => 'request_action', 'action' => 'params_pass'),
 			array('query' => array('get' => 'value', 'limit' => 5))
 		);
-		$this->assertEquals('value', $result->query['get']);
+		$result = json_decode($result, true);
+		$this->assertEquals('value', $result['query']['get']);
 	}
 
 }

tests/test_app/Plugin/TestPlugin/Controller/TestsController.php

@@ -34,7 +34,7 @@ class TestsController extends TestPluginAppController {
 	}
 
 	public function some_method() {
-		return 25;
+		$this->response->body(25);
 	}
 
 }

tests/test_app/TestApp/Controller/OrangeSessionTestController.php

@@ -35,9 +35,9 @@ class OrangeSessionTestController extends Controller {
 /**
  * session_id method
  *
- * @return string
+ * @return void
  */
 	public function session_id() {
-		return $this->Session->id();
+		$this->response->body($this->Session->id());
 	}
 }

tests/test_app/TestApp/Controller/RequestActionController.php

@@ -33,10 +33,11 @@ class RequestActionController extends AppController {
 /**
  * test_request_action method
  *
- * @return string
+ * @return \Cake\Network\Response
  */
 	public function test_request_action() {
-		return 'This is a test';
+		$this->response->body('This is a test');
+		return $this->response;
 	}
 
 /**
@@ -44,39 +45,40 @@ class RequestActionController extends AppController {
  *
  * @param mixed $id
  * @param mixed $other
- * @access public
- * @return string
+ * @return \Cake\Network\Response
  */
 	public function another_ra_test($id, $other) {
-		return $id + $other;
+		$this->response->body($id + $other);
+		return $this->response;
 	}
 
 /**
  * normal_request_action method
  *
- * @return string
+ * @return \Cake\Network\Response
  */
 	public function normal_request_action() {
-		return 'Hello World';
+		$this->response->body('Hello World');
+		return $this->response;
 	}
 
 /**
- * returns $this->here
+ * returns $this->here as body
  *
- * @return string
+ * @return \Cake\Network\Response
  */
 	public function return_here() {
-		return $this->here;
+		$this->response->body($this->here);
+		return $this->response;
 	}
 
 /**
  * paginate_request_action method
  *
- * @return boolean
+ * @return void
  */
 	public function paginate_request_action() {
 		$data = $this->paginate();
-		return true;
 	}
 
 /**
@@ -85,7 +87,7 @@ class RequestActionController extends AppController {
  * @return array
  */
 	public function post_pass() {
-		return $this->request->data;
+		$this->response->body(json_encode($this->request->data));
 	}
 
 /**
@@ -94,16 +96,20 @@ class RequestActionController extends AppController {
  * @return array
  */
 	public function query_pass() {
-		return $this->request->query;
+		$this->response->body(json_encode($this->request->query));
 	}
 
 /**
  * test param passing and parsing.
  *
- * @return array
+ * @return void
  */
 	public function params_pass() {
-		return $this->request;
+		$this->response->body(json_encode([
+			'params' => $this->request->params,
+			'query' => $this->request->query,
+			'url' => $this->request->url
+		]));
 	}
 
 /**

tests/test_app/TestApp/Controller/SessionTestController.php

@@ -31,9 +31,9 @@ class SessionTestController extends Controller {
 /**
  * session_id method
  *
- * @return string
+ * @return void
  */
 	public function session_id() {
-		return $this->Session->id();
+		$this->response->body($this->Session->id());
 	}
 }

tests/test_app/TestApp/Controller/SomePostsController.php

@@ -57,7 +57,6 @@ class SomePostsController extends Controller {
  * @return void
  */
 	public function index() {
-		return true;
 	}
 
 /**
@@ -66,7 +65,6 @@ class SomePostsController extends Controller {
  * @return void
  */
 	public function change() {
-		return true;
 	}
 
 }

tests/test_app/TestApp/Controller/TestDispatchPagesController.php

@@ -34,7 +34,6 @@ class TestDispatchPagesController extends Controller {
  * @return void
  */
 	public function admin_index() {
-		return true;
 	}
 
 /**
@@ -43,7 +42,6 @@ class TestDispatchPagesController extends Controller {
  * @return void
  */
 	public function camelCased() {
-		return true;
 	}
 
 }

tests/test_app/TestApp/Controller/TestsAppsController.php

@@ -36,7 +36,7 @@ class TestsAppsController extends AppController {
 	}
 
 	public function some_method() {
-		return 5;
+		$this->response->body(5);
 	}
 
 	public function set_action() {