Controller::$request and $response

[brandonkelly]

This PR sets $request and $response objects on yii\base\Controller, adding support for those objects to be dependency-injected (e.g. with a mock object when testing).

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌
Tests pass?	✔️

[samdark]

@brandonkelly there's #18032 that is more general fix. It's still missing console support (that's why it is not merged yet) but web is fine.

[brandonkelly]

@samdark That PR seems more focused on DI for controller actions; I don’t see anything about request & response in that one. Am I missing something?

[samdark]

Oh, sorry. You're right. It's different.

[brandonkelly]

@xepozz Those changes don’t seem consistent with other DI components in Yii, e.g.

yii2/framework/caching/DbCache.php

Lines 42 to 48 in 60a3965

	/**
	* @var Connection|array|string the DB connection object or the application component ID of the DB connection.
	* After the DbCache object is created, if you want to change this property, you should only assign it
	* with a DB connection object.
	* Starting from version 2.0.2, this can also be a configuration array for creating the object.
	*/
	public $db = 'db';

yii2/framework/filters/AccessControl.php

Lines 59 to 64 in 60a3965

	/**
	* @var User|array|string|false the user object representing the authentication status or the ID of the user application component.
	* Starting from version 2.0.2, this can also be a configuration array for creating the object.
	* Starting from version 2.0.12, you can set it to `false` to explicitly switch this component support off for the filter.
	*/
	public $user = 'user';

yii2/framework/filters/PageCache.php

Lines 69 to 75 in 60a3965

	/**
	* @var CacheInterface|array|string the cache object or the application component ID of the cache object.
	* After the PageCache object is created, if you want to change this property,
	* you should only assign it with a cache object.
	* Starting from version 2.0.2, this can also be a configuration array for creating the object.
	*/
	public $cache = 'cache';

If their initial values are null, then Instance::ensure() won’t know what to assign them to by default.

[brandonkelly]

w00t!

[samdark]

Thanks!

[justSinner]

@samdark , @brandonkelly , we have a problem right after composer update.
Call to a member function validateCsrfToken() on string in yiisoft/yii2/web/Controller.php : 203.

Exactly here $this->request is a string "request"!

[brandonkelly]

@justSinner What’s the stack trace? Yii’s built-in CSRF validation is in beforeAction() which is called after init(), so not a problem there.

[justSinner]

@brandonkelly The request is not an object there at all
Stack trace:

2020-06-18 12:42:05 [...][33][ojh39gvi4b5brbqr68bjainrqp][error][yii\base\UnknownPropertyException] yii\base\UnknownPropertyException: Getting unknown property: frontend\controllers\ApiController::request in /.../vendor/yiisoft/yii2/base/Component.php:154
Stack trace:
#0 /.../vendor/yiisoft/yii2/web/Controller.php(203): yii\base\Component->__get('request')
#1 /.../frontend/controllers/ApiController.php(109): yii\web\Controller->beforeAction(Object(yii\base\InlineAction))
#2 /.../vendor/yiisoft/yii2/base/Controller.php(155): frontend\controllers\ApiController->beforeAction(Object(yii\base\InlineAction))
#3 /.../vendor/yiisoft/yii2/base/Module.php(528): yii\base\Controller->runAction('notifications-g...', Array)
#4 /.../vendor/yiisoft/yii2/web/Application.php(103): yii\base\Module->runAction('api/notificatio...', Array)
#5 /.../vendor/yiisoft/yii2/base/Application.php(386): yii\web\Application->handleRequest(Object(common\components\core\Request))
#6 /.../frontend/web/index.php(21): yii\base\Application->run()

[brandonkelly]

Hm, I’m guessing your frontend\controllers\ApiController has an init() method, which is not calling parent::init() like it should.

@samdark do you think I should move the Instance::ensure() calls over to the constructor to work around that? Or just put something in the upgrade notes letting people know that their controllers need to be calling parent::init()?

[justSinner]

@brandonkelly , http://prntscr.com/t23jnr - ApiController extends yii\web\Controller and has no init() method at all.

[brandonkelly]

Actually, looking at your actual error message, it looks like PHP isn’t seeing the new $request property on yii\base\Controller at all. Otherwise yii\web\Controller::beforeAction() wouldn’t be invoking the __get() method (step #0 in your stack trace).

Make sure that those new properties are actually defined on yii\base\Controller for you. If not, maybe something went wrong with the Composer update. Or if they are, maybe OPcache or some other type of caching (sshfs, etc.) could be preventing PHP from seeing the new file.

[samdark]

@justSinner it would likely be helpful to start with a clean basic project template and reproduce it.

[justSinner]

I'm sorry, wrong stack trace, a lot of logs...

There is stacktrace for this error:

2020-06-18 14:01:28 [127.0.0.1][43][fpbvg0s72ul95c4epfsge8nel5][error][Error] Error: Call to a member function validateCsrfToken() on string in /.../vendor/yiisoft/yii2/web/Controller.php:203
Stack trace:
#0 /.../frontend/controllers/BaseController.php(69): yii\web\Controller->beforeAction(Object(yii\base\InlineAction))
#1 /.../vendor/yiisoft/yii2/base/Controller.php(177): frontend\controllers\BaseController->beforeAction(Object(yii\base\InlineAction))
#2 /.../vendor/yiisoft/yii2/base/Module.php(528): yii\base\Controller->runAction('index', Array)
#3 /.../vendor/yiisoft/yii2/web/Application.php(103): yii\base\Module->runAction('site/index', Array)
#4 /.../vendor/yiisoft/yii2/base/Application.php(386): yii\web\Application->handleRequest(Object(common\components\core\Request))
#5 /.../frontend/web/index.php(21): yii\base\Application->run()
#6 {main}

[justSinner]

There wasn't init() method in yii\web\Controller previously. We've found, that our BaseController has init() method without parent::init(). Thx for quick reply.

[brandonkelly]

Thanks, figured. So @samdark thoughts on this question? #18083 (comment)

We added some logic to Craft’s base web controller’s init() method at one point and there were a couple plugins/modules that were affected by it and needed to add their own parent::init() calls, so I suspect this isn’t the last time we’ll hear about this. So I could either move the code to the constructor (inconsistent with the rest of Yii, but will save some people from errors like this), or just mention in the upgrade notes that they need to make sure they are calling parent::init() (which I think is already the expectation, per https://www.yiiframework.com/doc/guide/2.0/en/concept-components).

[samdark]

I think UPGRADE is better.

[brandonkelly]

PR submitted: #18119

[jdgutierrezm]

So I ran composer update on July 08, and my application crashed.

1. First fatal error: Access level to app\modules\v1\controllers\AuthController::$request must be public (as in class yii\rest\Controller)
2. I changed the access level from protected to public and got the following (attached):

yii-error.log

I don't have an init() method on my controller,
I rely a lot of my application on using the variabls $request and $response (on child controller)
And my Controller extends yii\rest\Controller

Solution on my end has been changing the variable $request and $response to something else, but since when $request and $response became reserved words? Was that the plan? or is this a different issue and I am looking at it the wrong way?

[brandonkelly]

Error line:

yii2/framework/di/Instance.php

Line 126 in 700ac02

throw new InvalidConfigException('The required component is not specified.');

@jdprime yii\base\Controller and subclasses now expect $request to be an instance of yii\base\Request; and $response to be an instance of yii\base\Response. Sounds like maybe you are setting $request to something else.

There’s a risk of this happening any time a new property is added to a base class. Sorry you got bit by it.

[jdgutierrezm]

Good to know. Thank you for replying.

However I was setting my $request variable like this $request = Yii::$app->request; - and reading on the documentation this should have inherited from yii\web\Request - right? or is Yii::$app->request something else?

[brandonkelly]

The issue probably is that it is set to null from the property definition, but it needs to be set to either a yii\base\Request instance or the ID of the request component on the app (e.g. 'request'); otherwise Instance::ensure() won’t know how to resolve it.

The good news is, it sounds like all you really need to do is remove your own public $request; property as well as the line that sets it – all of the rest of your controller code referencing $this->request will continue to work.

[DmLapin]

* @since 2.0.36...
$this->request = Instance::ensure($this->request, Request::className());

 * @deprecated since 2.0.14. On PHP >=5.5, use `::class` instead.
public static function className()

🤦‍♂️🙂

[samdark]

@DmLapin yes, that's fine since Yii 2 still works fine on 5.4.

[azzi907]

Yii::$app->getResponse() changed to $this->response
and because of it $this->response->redirect is causing error of providing string as url
where Yii::$app->getResponse()->redirect was working just fine
I have tried providing array to $this->response->redirect but still same error

[bizley]

@azzi907 if you believe there is an error here please report a new issue about it with steps to reproduce it.