From 4ea484ca68c09af2fcacef6bfaa792aba7410c2f Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 12 Jun 2020 09:06:18 +0200 Subject: [PATCH] Fix #17722: Add action injection support --- docs/guide/concept-di-container.md | 18 +++ framework/CHANGELOG.md | 1 + framework/base/Controller.php | 30 ++++ framework/console/Controller.php | 39 ++++-- framework/web/Controller.php | 14 ++ tests/framework/console/ControllerTest.php | 100 ++++++++++++++ .../framework/console/FakePhp71Controller.php | 24 ++++ .../framework/console/stubs/DummyService.php | 16 +++ tests/framework/web/ControllerTest.php | 128 ++++++++++++++++++ tests/framework/web/FakePhp71Controller.php | 30 ++++ 10 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 tests/framework/console/FakePhp71Controller.php create mode 100644 tests/framework/console/stubs/DummyService.php create mode 100644 tests/framework/web/FakePhp71Controller.php diff --git a/docs/guide/concept-di-container.md b/docs/guide/concept-di-container.md index aa05bb83339..cbb050a4d2e 100644 --- a/docs/guide/concept-di-container.md +++ b/docs/guide/concept-di-container.md @@ -375,6 +375,24 @@ cannot be instantiated. This is because you need to tell the DI container how to Now if you access the controller again, an instance of `app\components\BookingService` will be created and injected as the 3rd parameter to the controller's constructor. +Since Yii 2.0.36 when using PHP 7 action injection is available for both web and console controllers: + +```php +namespace app\controllers; + +use yii\web\Controller; +use app\components\BookingInterface; + +class HotelController extends Controller +{ + public function actionBook($id, BookingInterface $bookingService) + { + $result = $bookingService->book($id); + // ... + } +} +``` + Advanced Practical Usage --------------- diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 21232e03e94..a6fc510d8f8 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -9,6 +9,7 @@ Yii Framework 2 Change Log - Bug #18026: Fix `ArrayHelper::getValue()` did not work with `ArrayAccess` objects (mikk150) - Enh #18048: Use `Instance::ensure()` to set `User::$accessChecker` (lav45) - Bug #18051: Fix missing support for custom validation method in EachValidator (bizley) +- Enh #17722: Add action injection support (SamMousa, samdark) - Bug #18041: Fix RBAC migration for MSSQL (darkdef) - Bug #18081: Fix for PDO_DBLIB/MSSQL. Set flag ANSI_NULL_DFLT_ON to ON for current connect to DB (darkdef) - Bug #13828: Fix retrieving inserted data for a primary key of type uniqueidentifier for SQL Server 2005 or later (darkdef) diff --git a/framework/base/Controller.php b/framework/base/Controller.php index 9921cfec6ee..d934810d2f9 100644 --- a/framework/base/Controller.php +++ b/framework/base/Controller.php @@ -523,4 +523,34 @@ public function findLayoutFile($view) return $path; } + + /** + * Fills parameters based on types and names in action method signature. + * @param \ReflectionType $type The reflected type of the action parameter. + * @param string $name The name of the parameter. + * @param array &$args The array of arguments for the action, this function may append items to it. + * @param array &$requestedParams The array with requested params, this function may write specific keys to it. + * @throws ErrorException when we cannot load a required service. + * @throws \yii\base\InvalidConfigException Thrown when there is an error in the DI configuration. + * @throws \yii\di\NotInstantiableException Thrown when a definition cannot be resolved to a concrete class + * (for example an interface type hint) without a proper definition in the container. + * @since 2.0.36 + */ + final protected function bindInjectedParams(\ReflectionType $type, $name, &$args, &$requestedParams) + { + // Since it is not a builtin type it must be DI injection. + $typeName = $type->getName(); + if (($component = $this->module->get($name, false)) instanceof $typeName) { + $args[] = $component; + $requestedParams[$name] = "Component: " . get_class($component) . " \$$name"; + } elseif (\Yii::$container->has($typeName) && ($service = \Yii::$container->get($typeName)) instanceof $typeName) { + $args[] = $service; + $requestedParams[$name] = "DI: $typeName \$$name"; + } elseif ($type->allowsNull()) { + $args[] = null; + $requestedParams[$name] = "Unavailable service: $name"; + } else { + throw new Exception('Could not load required service: ' . $name); + } + } } diff --git a/framework/console/Controller.php b/framework/console/Controller.php index 7bacd62bd9c..aa5021aeb76 100644 --- a/framework/console/Controller.php +++ b/framework/console/Controller.php @@ -182,19 +182,35 @@ public function bindActionParams($action, $params) $method = new \ReflectionMethod($action, 'run'); } - $args = array_values($params); - + $args = []; $missing = []; + $actionParams = []; + $requestedParams = []; foreach ($method->getParameters() as $i => $param) { - if ($param->isArray() && isset($args[$i])) { - $args[$i] = $args[$i] === '' ? [] : preg_split('/\s*,\s*/', $args[$i]); + $name = $param->getName(); + $key = null; + if (array_key_exists($i, $params)) { + $key = $i; + } elseif (array_key_exists($name, $params)) { + $key = $name; } - if (!isset($args[$i])) { - if ($param->isDefaultValueAvailable()) { - $args[$i] = $param->getDefaultValue(); - } else { - $missing[] = $param->getName(); + + if ($key !== null) { + if ($param->isArray()) { + $params[$key] = $params[$key] === '' ? [] : preg_split('/\s*,\s*/', $params[$key]); } + $args[] = $actionParams[$key] = $params[$key]; + unset($params[$key]); + } elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null && !$type->isBuiltin()) { + try { + $this->bindInjectedParams($type, $name, $args, $requestedParams); + } catch (\yii\base\Exception $e) { + throw new Exception($e->getMessage()); + } + } elseif ($param->isDefaultValueAvailable()) { + $args[] = $actionParams[$i] = $param->getDefaultValue(); + } else { + $missing[] = $name; } } @@ -202,6 +218,11 @@ public function bindActionParams($action, $params) throw new Exception(Yii::t('yii', 'Missing required arguments: {params}', ['params' => implode(', ', $missing)])); } + // We use a different array here, specifically one that doesn't contain service instances but descriptions instead. + if (\Yii::$app->requestedParams === null) { + \Yii::$app->requestedParams = array_merge($actionParams, $requestedParams); + } + return $args; } diff --git a/framework/web/Controller.php b/framework/web/Controller.php index 5492df62958..0175f0f5e5e 100644 --- a/framework/web/Controller.php +++ b/framework/web/Controller.php @@ -8,6 +8,8 @@ namespace yii\web; use Yii; +use yii\base\ErrorException; +use yii\base\Exception; use yii\base\InlineAction; use yii\helpers\Url; @@ -125,6 +127,7 @@ public function bindActionParams($action, $params) $args = []; $missing = []; $actionParams = []; + $requestedParams = []; foreach ($method->getParameters() as $param) { $name = $param->getName(); if (array_key_exists($name, $params)) { @@ -162,6 +165,12 @@ public function bindActionParams($action, $params) } $args[] = $actionParams[$name] = $params[$name]; unset($params[$name]); + } elseif (PHP_VERSION_ID >= 70100 && ($type = $param->getType()) !== null && !$type->isBuiltin()) { + try { + $this->bindInjectedParams($type, $name, $args, $requestedParams); + } catch (Exception $e) { + throw new ServerErrorHttpException($e->getMessage(), 0, $e); + } } elseif ($param->isDefaultValueAvailable()) { $args[] = $actionParams[$name] = $param->getDefaultValue(); } else { @@ -177,6 +186,11 @@ public function bindActionParams($action, $params) $this->actionParams = $actionParams; + // We use a different array here, specifically one that doesn't contain service instances but descriptions instead. + if (\Yii::$app->requestedParams === null) { + \Yii::$app->requestedParams = array_merge($actionParams, $requestedParams); + } + return $args; } diff --git a/tests/framework/console/ControllerTest.php b/tests/framework/console/ControllerTest.php index 6220a5357c4..01092790189 100644 --- a/tests/framework/console/ControllerTest.php +++ b/tests/framework/console/ControllerTest.php @@ -7,8 +7,13 @@ namespace yiiunit\framework\console; +use RuntimeException; +use yii\console\Exception; +use yiiunit\framework\console\stubs\DummyService; use Yii; +use yii\base\InlineAction; use yii\base\Module; +use yii\console\Application; use yii\console\Request; use yii\helpers\Console; use yiiunit\TestCase; @@ -18,6 +23,9 @@ */ class ControllerTest extends TestCase { + /** @var FakeController */ + private $controller; + protected function setUp() { parent::setUp(); @@ -76,6 +84,98 @@ public function testBindActionParams() $result = $controller->runAction('aksi3', $params); } + public function testNullableInjectedActionParams() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new Application([ + 'id' => 'app', + 'basePath' => __DIR__, + ])); + $this->mockApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionNullableInjection'); + $params = []; + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertEquals(\Yii::$app->request, $args[0]); + $this->assertNull($args[1]); + } + + public function testInjectionContainerException() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new Application([ + 'id' => 'app', + 'basePath' => __DIR__, + ])); + $this->mockApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->set(DummyService::className(), function() { throw new \RuntimeException('uh oh'); }); + + $this->expectException(get_class(new RuntimeException())); + $this->expectExceptionMessage('uh oh'); + $this->controller->bindActionParams($injectionAction, $params); + } + + public function testUnknownInjection() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new Application([ + 'id' => 'app', + 'basePath' => __DIR__, + ])); + $this->mockApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->clear(DummyService::className()); + $this->expectException(get_class(new Exception())); + $this->expectExceptionMessage('Could not load required service: dummyService'); + $this->controller->bindActionParams($injectionAction, $params); + } + + public function testInjectedActionParams() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new Application([ + 'id' => 'app', + 'basePath' => __DIR__, + ])); + $this->mockApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->set(DummyService::className(), DummyService::className()); + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertEquals($params['before'], $args[0]); + $this->assertEquals(\Yii::$app->request, $args[1]); + $this->assertEquals('Component: yii\console\Request $request', \Yii::$app->requestedParams['request']); + $this->assertEquals($params['between'], $args[2]); + $this->assertInstanceOf(DummyService::className(), $args[3]); + $this->assertEquals('DI: yiiunit\framework\console\stubs\DummyService $dummyService', \Yii::$app->requestedParams['dummyService']); + $this->assertNull($args[4]); + $this->assertEquals('Unavailable service: post', \Yii::$app->requestedParams['post']); + $this->assertEquals($params['after'], $args[5]); + } + public function assertResponseStatus($status, $response) { $this->assertInstanceOf('yii\console\Response', $response); diff --git a/tests/framework/console/FakePhp71Controller.php b/tests/framework/console/FakePhp71Controller.php new file mode 100644 index 00000000000..9b5ff601f76 --- /dev/null +++ b/tests/framework/console/FakePhp71Controller.php @@ -0,0 +1,24 @@ +controller, 'actionAksi1'); @@ -32,6 +37,129 @@ public function testBindActionParams() $this->assertEquals('avaliable', $other); } + public function testNullableInjectedActionParams() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([ + 'id' => 'app', + 'basePath' => __DIR__, + + 'components' => [ + 'request' => [ + 'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq', + 'scriptFile' => __DIR__ . '/index.php', + 'scriptUrl' => '/index.php', + ], + ], + ])); + $this->mockWebApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionNullableInjection'); + $params = []; + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertEquals(\Yii::$app->request, $args[0]); + $this->assertNull($args[1]); + } + + public function testInjectionContainerException() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([ + 'id' => 'app', + 'basePath' => __DIR__, + + 'components' => [ + 'request' => [ + 'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq', + 'scriptFile' => __DIR__ . '/index.php', + 'scriptUrl' => '/index.php', + ], + ], + ])); + $this->mockWebApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->set(VendorImage::className(), function() { throw new \RuntimeException('uh oh'); }); + + $this->expectException(get_class(new RuntimeException())); + $this->expectExceptionMessage('uh oh'); + $this->controller->bindActionParams($injectionAction, $params); + } + + public function testUnknownInjection() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([ + 'id' => 'app', + 'basePath' => __DIR__, + + 'components' => [ + 'request' => [ + 'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq', + 'scriptFile' => __DIR__ . '/index.php', + 'scriptUrl' => '/index.php', + ], + ], + ])); + $this->mockWebApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->clear(VendorImage::className()); + $this->expectException(get_class(new ServerErrorHttpException())); + $this->expectExceptionMessage('Could not load required service: vendorImage'); + $this->controller->bindActionParams($injectionAction, $params); + } + + public function testInjectedActionParams() + { + if (PHP_VERSION_ID < 70100) { + $this->markTestSkipped('Can not be tested on PHP < 7.1'); + return; + } + // Use the PHP71 controller for this test + $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([ + 'id' => 'app', + 'basePath' => __DIR__, + + 'components' => [ + 'request' => [ + 'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq', + 'scriptFile' => __DIR__ . '/index.php', + 'scriptUrl' => '/index.php', + ], + ], + ])); + $this->mockWebApplication(['controller' => $this->controller]); + + $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection'); + $params = ['between' => 'test', 'after' => 'another', 'before' => 'test']; + \Yii::$container->set(VendorImage::className(), VendorImage::className()); + $args = $this->controller->bindActionParams($injectionAction, $params); + $this->assertEquals($params['before'], $args[0]); + $this->assertEquals(\Yii::$app->request, $args[1]); + $this->assertEquals('Component: yii\web\Request $request', \Yii::$app->requestedParams['request']); + $this->assertEquals($params['between'], $args[2]); + $this->assertInstanceOf(VendorImage::className(), $args[3]); + $this->assertEquals('DI: yiiunit\framework\web\stubs\VendorImage $vendorImage', \Yii::$app->requestedParams['vendorImage']); + $this->assertNull($args[4]); + $this->assertEquals('Unavailable service: post', \Yii::$app->requestedParams['post']); + $this->assertEquals($params['after'], $args[5]); + } /** * @see https://github.com/yiisoft/yii2/issues/17701 */ diff --git a/tests/framework/web/FakePhp71Controller.php b/tests/framework/web/FakePhp71Controller.php new file mode 100644 index 00000000000..9c67256f520 --- /dev/null +++ b/tests/framework/web/FakePhp71Controller.php @@ -0,0 +1,30 @@ + + * @since 2.0.36 + */ +class FakePhp71Controller extends Controller +{ + public $enableCsrfValidation = false; + + public function actionInjection($before, Request $request, $between, VendorImage $vendorImage, Post $post = null, $after) + { + + } + + public function actionNullableInjection(?Request $request, ?Post $post) + { + } +}