From 82c978f86a79693a01e0fbff79c584f80a6e334b Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Sun, 16 Jun 2013 17:04:28 +0200 Subject: [PATCH 1/3] draft idea of action based responses --- framework/yii/base/Action.php | 21 ++++++++++++++++++++- framework/yii/base/Application.php | 1 + framework/yii/base/Controller.php | 1 + framework/yii/base/InlineAction.php | 4 +++- framework/yii/console/Application.php | 14 +++++--------- framework/yii/web/Application.php | 16 ++++------------ 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/framework/yii/base/Action.php b/framework/yii/base/Action.php index 8778cab..9e0d073 100644 --- a/framework/yii/base/Action.php +++ b/framework/yii/base/Action.php @@ -7,6 +7,8 @@ namespace yii\base; +use Yii; + /** * Action is the base class for all controller action classes. * @@ -40,6 +42,21 @@ class Action extends Component */ public $controller; + private $_response; + + public function getResponse() + { + if ($this->_response === null) { + $this->_response = Yii::$app->createResponse(); + } + return $this->_response; + } + + public function setResponse($response) + { + $this->_response = $response; + } + /** * Constructor. * @param string $id the ID of this action @@ -75,6 +92,8 @@ class Action extends Component throw new InvalidConfigException(get_class($this) . ' must define a "run()" method.'); } $args = $this->controller->bindActionParams($this, $params); - return call_user_func_array(array($this, 'run'), $args); + $response = $this->getResponse(); + $response->result = call_user_func_array(array($this, 'run'), $args); + return $response; } } diff --git a/framework/yii/base/Application.php b/framework/yii/base/Application.php index 9eeb53b..01e8112 100644 --- a/framework/yii/base/Application.php +++ b/framework/yii/base/Application.php @@ -155,6 +155,7 @@ abstract class Application extends Module */ abstract public function handleRequest($request); + public abstract function createResponse(); private $_runtimePath; diff --git a/framework/yii/base/Controller.php b/framework/yii/base/Controller.php index 5b8debb..49af5fe 100644 --- a/framework/yii/base/Controller.php +++ b/framework/yii/base/Controller.php @@ -111,6 +111,7 @@ class Controller extends Component $oldAction = $this->action; $this->action = $action; $result = null; + // TODO beforeAction may also create a response somehow. if ($this->module->beforeAction($action)) { if ($this->beforeAction($action)) { $result = $action->runWithParams($params); diff --git a/framework/yii/base/InlineAction.php b/framework/yii/base/InlineAction.php index 75728e0..e683105 100644 --- a/framework/yii/base/InlineAction.php +++ b/framework/yii/base/InlineAction.php @@ -44,6 +44,8 @@ class InlineAction extends Action public function runWithParams($params) { $args = $this->controller->bindActionParams($this, $params); - return call_user_func_array(array($this->controller, $this->actionMethod), $args); + $response = $this->getResponse(); + $response->result = call_user_func_array(array($this->controller, $this->actionMethod), $args); + return $response; } } diff --git a/framework/yii/console/Application.php b/framework/yii/console/Application.php index c5e22ab..2baef94 100644 --- a/framework/yii/console/Application.php +++ b/framework/yii/console/Application.php @@ -93,22 +93,18 @@ class Application extends \yii\base\Application { list ($route, $params) = $request->resolve(); $result = $this->runAction($route, $params); - if ($result instanceof Response) { - return $result; - } else { - $response = $this->getResponse(); - $response->exitStatus = (int)$result; - return $response; - } + $response = $this->getResponse(); + $response->exitStatus = (int)$result; + return $response; } /** * Returns the response component. * @return Response the response component */ - public function getResponse() + public function createResponse() { - return $this->getComponent('response'); + return new Response(); } /** diff --git a/framework/yii/web/Application.php b/framework/yii/web/Application.php index 6536db7..3994ec2 100644 --- a/framework/yii/web/Application.php +++ b/framework/yii/web/Application.php @@ -65,16 +65,8 @@ class Application extends \yii\base\Application $params = array_splice($this->catchAll, 1); } try { - $result = $this->runAction($route, $params); - if ($result instanceof Response) { - return $result; - } else { - $response = $this->getResponse(); - if ($result !== null) { - $response->setContent($result); - } - return $response; - } + $response = $this->runAction($route, $params); + return $response; } catch (InvalidRouteException $e) { throw new HttpException(404, $e->getMessage(), $e->getCode(), $e); } @@ -119,9 +111,9 @@ class Application extends \yii\base\Application * Returns the response component. * @return Response the response component */ - public function getResponse() + public function createResponse() { - return $this->getComponent('response'); + return new Response(); } /** From b138199c59c0547429f62eb106e7b5d6c251f9cd Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Sun, 16 Jun 2013 18:12:27 +0200 Subject: [PATCH 2/3] implementation of Response tied to actions Result of a discussion with @qiangxue - there is not global Yii::$app->response anymore, every action has dedicated response class - implementation allows using HMVC pattern now, nested actions do not interfer --- framework/yii/base/Action.php | 18 +++++++++++++----- framework/yii/base/Application.php | 2 -- framework/yii/base/Controller.php | 22 +++++++++++++++++++--- framework/yii/base/InlineAction.php | 4 +--- framework/yii/base/Module.php | 6 +++--- framework/yii/console/Application.php | 33 ++------------------------------- framework/yii/console/Controller.php | 10 ++++++++++ framework/yii/web/Application.php | 15 +-------------- framework/yii/web/Controller.php | 11 +++++++++++ 9 files changed, 60 insertions(+), 61 deletions(-) diff --git a/framework/yii/base/Action.php b/framework/yii/base/Action.php index 9e0d073..574fd4c 100644 --- a/framework/yii/base/Action.php +++ b/framework/yii/base/Action.php @@ -41,17 +41,27 @@ class Action extends Component * @var Controller the controller that owns this action */ public $controller; - + /** + * @var Response + */ private $_response; + + /** + * @return Response|\yii\console\Response|\yii\web\Response + */ public function getResponse() { if ($this->_response === null) { - $this->_response = Yii::$app->createResponse(); + // TODO use applications response factory here + //$this->_response = new Response(); } return $this->_response; } + /** + * @param Response $response + */ public function setResponse($response) { $this->_response = $response; @@ -92,8 +102,6 @@ class Action extends Component throw new InvalidConfigException(get_class($this) . ' must define a "run()" method.'); } $args = $this->controller->bindActionParams($this, $params); - $response = $this->getResponse(); - $response->result = call_user_func_array(array($this, 'run'), $args); - return $response; + return call_user_func_array(array($this, 'run'), $args); } } diff --git a/framework/yii/base/Application.php b/framework/yii/base/Application.php index 01e8112..5b805ad 100644 --- a/framework/yii/base/Application.php +++ b/framework/yii/base/Application.php @@ -155,8 +155,6 @@ abstract class Application extends Module */ abstract public function handleRequest($request); - public abstract function createResponse(); - private $_runtimePath; /** diff --git a/framework/yii/base/Controller.php b/framework/yii/base/Controller.php index 49af5fe..af9b169 100644 --- a/framework/yii/base/Controller.php +++ b/framework/yii/base/Controller.php @@ -110,23 +110,39 @@ class Controller extends Component if ($action !== null) { $oldAction = $this->action; $this->action = $action; - $result = null; - // TODO beforeAction may also create a response somehow. if ($this->module->beforeAction($action)) { if ($this->beforeAction($action)) { $result = $action->runWithParams($params); + if ($result !== null) { + $this->handleActionResult($result, $action); + } $this->afterAction($action); } $this->module->afterAction($action); } $this->action = $oldAction; - return $result; + return $action->getResponse(); } else { throw new InvalidRouteException('Unable to resolve the request: ' . $this->getUniqueId() . '/' . $id); } } /** + * Handles the return value of an action + * @param mixed $result + * @param Action $action + */ + protected abstract function handleActionResult(&$result, $action); + + /** + * @return Response the response object of the current action. null if no action is running + */ + public function getResponse() + { + return $this->action !== null ? $this->action->getResponse() : null; + } + + /** * Runs a request specified in terms of a route. * The route can be either an ID of an action within this controller or a complete route consisting * of module IDs, controller ID and action ID. If the route starts with a slash '/', the parsing of diff --git a/framework/yii/base/InlineAction.php b/framework/yii/base/InlineAction.php index e683105..75728e0 100644 --- a/framework/yii/base/InlineAction.php +++ b/framework/yii/base/InlineAction.php @@ -44,8 +44,6 @@ class InlineAction extends Action public function runWithParams($params) { $args = $this->controller->bindActionParams($this, $params); - $response = $this->getResponse(); - $response->result = call_user_func_array(array($this->controller, $this->actionMethod), $args); - return $response; + return call_user_func_array(array($this->controller, $this->actionMethod), $args); } } diff --git a/framework/yii/base/Module.php b/framework/yii/base/Module.php index 6603b28..fbe6064 100644 --- a/framework/yii/base/Module.php +++ b/framework/yii/base/Module.php @@ -571,7 +571,7 @@ abstract class Module extends Component * If the route is empty, the method will use [[defaultRoute]]. * @param string $route the route that specifies the action. * @param array $params the parameters to be passed to the action - * @return mixed the result of the action. + * @return Response the resulting response of the action. * @throws InvalidRouteException if the requested route cannot be resolved into an action successfully */ public function runAction($route, $params = array()) @@ -582,9 +582,9 @@ abstract class Module extends Component list($controller, $actionID) = $parts; $oldController = Yii::$app->controller; Yii::$app->controller = $controller; - $result = $controller->runAction($actionID, $params); + $response = $controller->runAction($actionID, $params); Yii::$app->controller = $oldController; - return $result; + return $response; } else { throw new InvalidRouteException('Unable to resolve the request "' . trim($this->getUniqueId() . '/' . $route, '/') . '".'); } diff --git a/framework/yii/console/Application.php b/framework/yii/console/Application.php index 2baef94..1a58ed6 100644 --- a/framework/yii/console/Application.php +++ b/framework/yii/console/Application.php @@ -88,39 +88,13 @@ class Application extends \yii\base\Application * Handles the specified request. * @param Request $request the request to be handled * @return Response the resulting response + * @throws Exception if the route is invalid */ public function handleRequest($request) { list ($route, $params) = $request->resolve(); - $result = $this->runAction($route, $params); - $response = $this->getResponse(); - $response->exitStatus = (int)$result; - return $response; - } - - /** - * Returns the response component. - * @return Response the response component - */ - public function createResponse() - { - return new Response(); - } - - /** - * Runs a controller action specified by a route. - * This method parses the specified route and creates the corresponding child module(s), controller and action - * instances. It then calls [[Controller::runAction()]] to run the action with the given parameters. - * If the route is empty, the method will use [[defaultRoute]]. - * @param string $route the route that specifies the action. - * @param array $params the parameters to be passed to the action - * @return integer the status code returned by the action execution. 0 means normal, and other values mean abnormal. - * @throws Exception if the route is invalid - */ - public function runAction($route, $params = array()) - { try { - return parent::runAction($route, $params); + return $this->runAction($route, $params); } catch (InvalidRouteException $e) { throw new Exception(\Yii::t('yii', 'Unknown command "{command}".', array('{command}' => $route)), 0, $e); } @@ -152,9 +126,6 @@ class Application extends \yii\base\Application 'request' => array( 'class' => 'yii\console\Request', ), - 'response' => array( - 'class' => 'yii\console\Response', - ), )); } } diff --git a/framework/yii/console/Controller.php b/framework/yii/console/Controller.php index e4d4211..01a70ff 100644 --- a/framework/yii/console/Controller.php +++ b/framework/yii/console/Controller.php @@ -135,6 +135,16 @@ class Controller extends \yii\base\Controller } /** + * Handles the return value of an action + * @param mixed $result + * @param Action $action + */ + protected function handleActionResult(&$result, $action) + { + $action->getResponse()->exitStatus = (int)$result; + } + + /** * Formats a string with ANSI codes * * You may pass additional parameters using the constants defined in [[yii\helpers\base\Console]]. diff --git a/framework/yii/web/Application.php b/framework/yii/web/Application.php index 3994ec2..4babc22 100644 --- a/framework/yii/web/Application.php +++ b/framework/yii/web/Application.php @@ -65,8 +65,7 @@ class Application extends \yii\base\Application $params = array_splice($this->catchAll, 1); } try { - $response = $this->runAction($route, $params); - return $response; + return $this->runAction($route, $params); } catch (InvalidRouteException $e) { throw new HttpException(404, $e->getMessage(), $e->getCode(), $e); } @@ -108,15 +107,6 @@ class Application extends \yii\base\Application } /** - * Returns the response component. - * @return Response the response component - */ - public function createResponse() - { - return new Response(); - } - - /** * Returns the session component. * @return Session the session component */ @@ -154,9 +144,6 @@ class Application extends \yii\base\Application 'request' => array( 'class' => 'yii\web\Request', ), - 'response' => array( - 'class' => 'yii\web\Response', - ), 'session' => array( 'class' => 'yii\web\Session', ), diff --git a/framework/yii/web/Controller.php b/framework/yii/web/Controller.php index 6214c54..ee25afc 100644 --- a/framework/yii/web/Controller.php +++ b/framework/yii/web/Controller.php @@ -8,6 +8,7 @@ namespace yii\web; use Yii; +use yii\base\Action; use yii\base\InlineAction; /** @@ -62,6 +63,16 @@ class Controller extends \yii\base\Controller } /** + * Handles the return value of an action + * @param mixed $result + * @param Action $action + */ + protected function handleActionResult(&$result, $action) + { + $action->getResponse()->setContent($result); + } + + /** * Creates a URL using the given route and parameters. * * This method enhances [[UrlManager::createUrl()]] by supporting relative routes. From 71782b344e7d38cc046fcef7eda532f85b6052ea Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Sun, 16 Jun 2013 18:52:56 +0200 Subject: [PATCH 3/3] Changed existing code to match the changes regarding response --- apps/advanced/backend/controllers/SiteController.php | 4 ++-- apps/advanced/frontend/controllers/SiteController.php | 6 +++--- apps/basic/controllers/SiteController.php | 6 +++--- framework/yii/base/Controller.php | 2 +- framework/yii/base/ErrorHandler.php | 10 ++++++++-- framework/yii/web/CaptchaAction.php | 2 +- framework/yii/web/HttpCache.php | 9 +++++---- framework/yii/web/Request.php | 2 +- framework/yii/web/User.php | 9 ++++----- framework/yii/web/VerbFilter.php | 2 +- tests/unit/framework/helpers/HtmlTest.php | 3 --- 11 files changed, 29 insertions(+), 26 deletions(-) diff --git a/apps/advanced/backend/controllers/SiteController.php b/apps/advanced/backend/controllers/SiteController.php index 851fcec..7f6eb58 100644 --- a/apps/advanced/backend/controllers/SiteController.php +++ b/apps/advanced/backend/controllers/SiteController.php @@ -17,7 +17,7 @@ class SiteController extends Controller { $model = new LoginForm(); if ($this->populate($_POST, $model) && $model->login()) { - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } else { return $this->render('login', array( 'model' => $model, @@ -28,6 +28,6 @@ class SiteController extends Controller public function actionLogout() { Yii::$app->user->logout(); - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } } diff --git a/apps/advanced/frontend/controllers/SiteController.php b/apps/advanced/frontend/controllers/SiteController.php index b0f8ec2..7405ba1 100644 --- a/apps/advanced/frontend/controllers/SiteController.php +++ b/apps/advanced/frontend/controllers/SiteController.php @@ -27,7 +27,7 @@ class SiteController extends Controller { $model = new LoginForm(); if ($this->populate($_POST, $model) && $model->login()) { - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } else { return $this->render('login', array( 'model' => $model, @@ -38,7 +38,7 @@ class SiteController extends Controller public function actionLogout() { Yii::$app->user->logout(); - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } public function actionContact() @@ -46,7 +46,7 @@ class SiteController extends Controller $model = new ContactForm; if ($this->populate($_POST, $model) && $model->contact(Yii::$app->params['adminEmail'])) { Yii::$app->session->setFlash('contactFormSubmitted'); - return Yii::$app->response->refresh(); + return $this->response->refresh(); } else { return $this->render('contact', array( 'model' => $model, diff --git a/apps/basic/controllers/SiteController.php b/apps/basic/controllers/SiteController.php index d79b728..670e05e 100644 --- a/apps/basic/controllers/SiteController.php +++ b/apps/basic/controllers/SiteController.php @@ -27,7 +27,7 @@ class SiteController extends Controller { $model = new LoginForm(); if ($this->populate($_POST, $model) && $model->login()) { - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } else { return $this->render('login', array( 'model' => $model, @@ -38,7 +38,7 @@ class SiteController extends Controller public function actionLogout() { Yii::$app->user->logout(); - return Yii::$app->response->redirect(array('site/index')); + return $this->response->redirect(array('site/index')); } public function actionContact() @@ -46,7 +46,7 @@ class SiteController extends Controller $model = new ContactForm; if ($this->populate($_POST, $model) && $model->contact(Yii::$app->params['adminEmail'])) { Yii::$app->session->setFlash('contactFormSubmitted'); - return Yii::$app->response->refresh(); + return $this->response->refresh(); } else { return $this->render('contact', array( 'model' => $model, diff --git a/framework/yii/base/Controller.php b/framework/yii/base/Controller.php index af9b169..f36241e 100644 --- a/framework/yii/base/Controller.php +++ b/framework/yii/base/Controller.php @@ -15,7 +15,7 @@ use Yii; * @author Qiang Xue * @since 2.0 */ -class Controller extends Component +abstract class Controller extends Component { /** * @event ActionEvent an event raised right before executing a controller action. diff --git a/framework/yii/base/ErrorHandler.php b/framework/yii/base/ErrorHandler.php index 338a392..dc1e00b 100644 --- a/framework/yii/base/ErrorHandler.php +++ b/framework/yii/base/ErrorHandler.php @@ -9,6 +9,7 @@ namespace yii\base; use Yii; use yii\web\HttpException; +use yii\web\Response; /** * ErrorHandler handles uncaught PHP errors and exceptions. @@ -89,8 +90,13 @@ class ErrorHandler extends Component $useErrorView = !YII_DEBUG || $exception instanceof UserException; - $response = Yii::$app->getResponse(); - $response->getHeaders()->removeAll(); + if (Yii::$app->controller !== null) { + $response = Yii::$app->controller->getResponse(); + $response->getHeaders()->removeAll(); + } + if (empty($response)) { + $response = new Response(); + } if ($useErrorView && $this->errorAction !== null) { $result = Yii::$app->runAction($this->errorAction); diff --git a/framework/yii/web/CaptchaAction.php b/framework/yii/web/CaptchaAction.php index 1e22627..47eded8 100644 --- a/framework/yii/web/CaptchaAction.php +++ b/framework/yii/web/CaptchaAction.php @@ -325,7 +325,7 @@ class CaptchaAction extends Action */ protected function setHttpHeaders() { - Yii::$app->getResponse()->getHeaders() + $this->getResponse()->getHeaders() ->set('Pragma', 'public') ->set('Expires', '0') ->set('Cache-Control', 'must-revalidate, post-check=0, pre-check=0') diff --git a/framework/yii/web/HttpCache.php b/framework/yii/web/HttpCache.php index cc9e6ed..785c7dd 100644 --- a/framework/yii/web/HttpCache.php +++ b/framework/yii/web/HttpCache.php @@ -74,8 +74,8 @@ class HttpCache extends ActionFilter $etag = $this->generateEtag($seed); } - $this->sendCacheControlHeader(); - $response = Yii::$app->getResponse(); + $response = $action->getResponse(); + $this->sendCacheControlHeader($response); if ($etag !== null) { $response->getHeaders()->set('Etag', $etag); } @@ -109,12 +109,13 @@ class HttpCache extends ActionFilter /** * Sends the cache control header to the client + * @param Response $response * @see cacheControl */ - protected function sendCacheControlHeader() + protected function sendCacheControlHeader($response) { session_cache_limiter('public'); - $headers = Yii::$app->getResponse()->getHeaders(); + $headers = $response->getHeaders(); $headers->set('Pragma'); if ($this->cacheControlHeader !== null) { $headers->set('Cache-Control', $this->cacheControlHeader); diff --git a/framework/yii/web/Request.php b/framework/yii/web/Request.php index 1027011..0af80d5 100644 --- a/framework/yii/web/Request.php +++ b/framework/yii/web/Request.php @@ -788,7 +788,7 @@ class Request extends \yii\base\Request $this->_csrfCookie = $this->getCookies()->get($this->csrfTokenName); if ($this->_csrfCookie === null) { $this->_csrfCookie = $this->createCsrfCookie(); - Yii::$app->getResponse()->getCookies()->add($this->_csrfCookie); + Yii::$app->controller->getResponse()->getCookies()->add($this->_csrfCookie); } } diff --git a/framework/yii/web/User.php b/framework/yii/web/User.php index d4646a6..7091dd5 100644 --- a/framework/yii/web/User.php +++ b/framework/yii/web/User.php @@ -283,8 +283,7 @@ class User extends Component $this->setReturnUrl($request->getUrl()); } if ($this->loginUrl !== null) { - $response = Yii::$app->getResponse(); - $response->redirect($this->loginUrl)->send(); + Yii::$app->controller->getResponse()->redirect($this->loginUrl)->send(); exit(); } else { throw new HttpException(403, Yii::t('yii', 'Login Required')); @@ -372,7 +371,7 @@ class User extends Component $cookie = new Cookie($this->identityCookie); $cookie->value = $value; $cookie->expire = time() + (int)$data[2]; - Yii::$app->getResponse()->getCookies()->add($cookie); + Yii::$app->controller->getResponse()->getCookies()->add($cookie); } } } @@ -395,7 +394,7 @@ class User extends Component $duration, )); $cookie->expire = time() + $duration; - Yii::$app->getResponse()->getCookies()->add($cookie); + Yii::$app->controller->getResponse()->getCookies()->add($cookie); } /** @@ -429,7 +428,7 @@ class User extends Component $this->sendIdentityCookie($identity, $duration); } } elseif ($this->enableAutoLogin) { - Yii::$app->getResponse()->getCookies()->remove(new Cookie($this->identityCookie)); + Yii::$app->controller->getResponse()->getCookies()->remove(new Cookie($this->identityCookie)); } } diff --git a/framework/yii/web/VerbFilter.php b/framework/yii/web/VerbFilter.php index 4f45190..ffd82c2 100644 --- a/framework/yii/web/VerbFilter.php +++ b/framework/yii/web/VerbFilter.php @@ -80,7 +80,7 @@ class VerbFilter extends Behavior if (!in_array($verb, $allowed)) { $event->isValid = false; // http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.7 - Yii::$app->getResponse()->getHeaders()->set('Allow', implode(', ', $allowed)); + $event->action->getResponse()->getHeaders()->set('Allow', implode(', ', $allowed)); throw new HttpException(405, 'Method Not Allowed. This url can only handle the following request methods: ' . implode(', ', $allowed)); } } diff --git a/tests/unit/framework/helpers/HtmlTest.php b/tests/unit/framework/helpers/HtmlTest.php index 018a820..0399a4e 100644 --- a/tests/unit/framework/helpers/HtmlTest.php +++ b/tests/unit/framework/helpers/HtmlTest.php @@ -17,9 +17,6 @@ class HtmlTest extends TestCase 'class' => 'yii\web\Request', 'url' => '/test', ), - 'response' => array( - 'class' => 'yii\web\Response', - ), ), )); }