From 82fd1930e8f7e763d25475941a8ecbda2593ad8e Mon Sep 17 00:00:00 2001 From: adrian-martinez-interactiv4 Date: Thu, 12 Nov 2020 02:48:23 +0100 Subject: [PATCH 1/3] Allow use factories and OM for creating objects with variadic arguments in constructor Related with https://github.com/magento/magento2/pull/24556. That PR allowed to inject scalar values as variadic parameters, but do not work for objects injections. Object definitions don't get instantiated, and target class constructor receives an array of strings instead (with '_instance' key, that should have been transformed into real object) --- .../ObjectManager/Factory/AbstractFactory.php | 8 ++++++- .../Module/Di/Compiler/ArgumentsResolver.php | 24 +++++++++++++++++++ .../Di/Compiler/ConstructorArgument.php | 16 +++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php b/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php index b57f4665aff21..545a06f3d9480 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php +++ b/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php @@ -270,7 +270,13 @@ private function getResolvedArgument(string $requestedType, array $parameter, ar } if ($isVariadic) { - return is_array($argument) ? $argument : [$argument]; + $variadicArguments = is_array($argument) ? $argument : [$argument]; + + foreach ($variadicArguments as &$variadicArgument) { + $this->resolveArgument($variadicArgument, $paramType, $paramDefault, $paramName, $requestedType); + }; + + return $variadicArguments; } $this->resolveArgument($argument, $paramType, $paramDefault, $paramName, $requestedType); diff --git a/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php b/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php index 7a6d34fbcbe07..b431c593d30fd 100644 --- a/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php +++ b/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php @@ -89,11 +89,33 @@ public function getResolvedConstructorArguments($instanceType, $constructor) if (!$constructor) { return null; } + $configuredArguments = $this->getConfiguredArguments($instanceType); $arguments = []; /** @var ConstructorArgument $constructorArgument */ foreach ($constructor as $constructorArgument) { + if ($constructorArgument->isVariadic()) { + $variadicArguments = $configuredArguments[$constructorArgument->getName()] ?? []; + + if (!is_array($variadicArguments) || !count($variadicArguments)) { + // Variadic argument is always the last one + break; + } + + foreach ($variadicArguments as $variadicArgumentKey => $variadicArgument) { + $variadicArguments[$variadicArgumentKey] = $this->getConfiguredArgument( + $variadicArgument, + $constructorArgument + ); + } + + array_push($arguments, ...array_values($variadicArguments)); + + // Variadic argument is always the last one + break; + } + $argument = $this->getNonObjectArgument(null); if (!$constructorArgument->isRequired()) { $argument = $this->getNonObjectArgument($constructorArgument->getDefaultValue()); @@ -107,8 +129,10 @@ public function getResolvedConstructorArguments($instanceType, $constructor) $constructorArgument ); } + $arguments[$constructorArgument->getName()] = $argument; } + return $arguments; } diff --git a/setup/src/Magento/Setup/Module/Di/Compiler/ConstructorArgument.php b/setup/src/Magento/Setup/Module/Di/Compiler/ConstructorArgument.php index 051e1cd4bfaba..2f65683454fb8 100644 --- a/setup/src/Magento/Setup/Module/Di/Compiler/ConstructorArgument.php +++ b/setup/src/Magento/Setup/Module/Di/Compiler/ConstructorArgument.php @@ -29,6 +29,11 @@ class ConstructorArgument */ private $defaultValue; + /** + * @var bool + */ + private $isVariadic; + /** * @param array $configuration */ @@ -38,6 +43,7 @@ public function __construct(array $configuration) $this->type = $configuration[1]; $this->isRequired = $configuration[2]; $this->defaultValue = $configuration[3]; + $this->isVariadic = $configuration[4]; } /** @@ -79,4 +85,14 @@ public function getDefaultValue() { return $this->defaultValue; } + + /** + * Returns argument is variadic + * + * @return bool + */ + public function isVariadic(): bool + { + return $this->isVariadic; + } } From f3cfb9188dd82d637e11433f65f9b0ac406ab969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Mart=C3=ADnez=20Guantes?= Date: Mon, 15 Mar 2021 14:59:40 +0100 Subject: [PATCH 2/3] Allow objects to be properly injected through factories when variadic argument is the only one in constructor, and compiled mode is used --- .../ObjectManager/Factory/Compiled.php | 17 ++++++++++++- .../Module/Di/Compiler/ArgumentsResolver.php | 24 ++++++++++--------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Factory/Compiled.php b/lib/internal/Magento/Framework/ObjectManager/Factory/Compiled.php index b219d93b0f0fa..2c98cc3ee4c4d 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Factory/Compiled.php +++ b/lib/internal/Magento/Framework/ObjectManager/Factory/Compiled.php @@ -64,6 +64,7 @@ public function create($requestedType, array $arguments = []) * * Argument key meanings: * + * _vdic_: variadic argument * _i_: shared instance of a class or interface * _ins_: non-shared instance of a class or interface * _v_: non-array literal value @@ -73,7 +74,21 @@ public function create($requestedType, array $arguments = []) * _d_: default value in case environment variable specified by _a_ does not exist */ foreach ($args as $key => &$argument) { - if (isset($arguments[$key])) { + if (isset($argument['_vdic_'])) { + // Process variadic + if (isset($arguments[$key])) { + $argument = (array)$arguments[$key]; + } else { + $argument = (array)$argument['_vdic_']; + $this->parseArray($argument); + } + unset($args[$key]); + if (count($argument)) { + array_push($args, ...array_values($argument)); + } + // Variadic argument is always the last one + break; + } elseif (isset($arguments[$key])) { $argument = $arguments[$key]; } elseif (isset($argument['_i_'])) { $argument = $this->get($argument['_i_']); diff --git a/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php b/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php index b431c593d30fd..1b7b277164149 100644 --- a/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php +++ b/setup/src/Magento/Setup/Module/Di/Compiler/ArgumentsResolver.php @@ -59,6 +59,15 @@ class ArgumentsResolver '_vac_' => true, ]; + /** + * Variadic pattern used for configuration + * + * @var array + */ + private $variadicPattern = [ + '_vdic_' => [], + ]; + /** * Configured argument pattern used for configuration * @@ -96,21 +105,14 @@ public function getResolvedConstructorArguments($instanceType, $constructor) /** @var ConstructorArgument $constructorArgument */ foreach ($constructor as $constructorArgument) { if ($constructorArgument->isVariadic()) { + $argument = $this->variadicPattern; $variadicArguments = $configuredArguments[$constructorArgument->getName()] ?? []; - if (!is_array($variadicArguments) || !count($variadicArguments)) { - // Variadic argument is always the last one - break; - } - - foreach ($variadicArguments as $variadicArgumentKey => $variadicArgument) { - $variadicArguments[$variadicArgumentKey] = $this->getConfiguredArgument( - $variadicArgument, - $constructorArgument - ); + foreach ($variadicArguments as $variadicArgument) { + $argument['_vdic_'][] = $this->getConfiguredArgument($variadicArgument, $constructorArgument); } - array_push($arguments, ...array_values($variadicArguments)); + $arguments[$constructorArgument->getName()] = $argument; // Variadic argument is always the last one break; From c233932c689e88f28210e9e981eca7adcd5bc53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Mart=C3=ADnez=20Guantes?= Date: Mon, 15 Mar 2021 16:08:55 +0100 Subject: [PATCH 3/3] Fix unit tests --- .../Di/Compiler/ArgumentsResolverTest.php | 24 +++++++++---------- .../Di/Compiler/ConstructorArgumentTest.php | 3 ++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ArgumentsResolverTest.php b/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ArgumentsResolverTest.php index 8e22200b4bc05..b2f9fa0bb018d 100644 --- a/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ArgumentsResolverTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ArgumentsResolverTest.php @@ -36,11 +36,11 @@ public function testGetResolvedArgumentsConstructorFormat() $expectedResultDefault = $this->getResolvedSimpleConfigExpectation(); $constructor = [ - new ConstructorArgument(['type_dependency', 'Type\Dependency', true, null]), - new ConstructorArgument(['type_dependency_shared', 'Type\Dependency\Shared', true, null]), - new ConstructorArgument(['value', null, false, 'value']), - new ConstructorArgument(['value_array', null, false, ['default_value1', 'default_value2']]), - new ConstructorArgument(['value_null', null, false, null]), + new ConstructorArgument(['type_dependency', 'Type\Dependency', true, null, false]), + new ConstructorArgument(['type_dependency_shared', 'Type\Dependency\Shared', true, null, false]), + new ConstructorArgument(['value', null, false, 'value', false]), + new ConstructorArgument(['value_array', null, false, ['default_value1', 'default_value2', false]]), + new ConstructorArgument(['value_null', null, false, null, false]), ]; $this->diContainerConfig->expects($this->any()) ->method('isShared') @@ -68,13 +68,13 @@ public function testGetResolvedArgumentsConstructorConfiguredFormat() $expectedResultConfigured = $this->getResolvedConfigurableConfigExpectation(); $constructor = [ - new ConstructorArgument(['type_dependency_configured', 'Type\Dependency', true, null]), - new ConstructorArgument(['type_dependency_shared_configured', 'Type\Dependency\Shared', true, null]), - new ConstructorArgument(['global_argument', null, false, null]), - new ConstructorArgument(['global_argument_def', null, false, []]), - new ConstructorArgument(['value_configured', null, false, 'value']), - new ConstructorArgument(['value_array_configured', null, false, []]), - new ConstructorArgument(['value_null', null, false, null]), + new ConstructorArgument(['type_dependency_configured', 'Type\Dependency', true, null, false]), + new ConstructorArgument(['type_dependency_shared_configured', 'Type\Dependency\Shared', true, null, false]), + new ConstructorArgument(['global_argument', null, false, null, false]), + new ConstructorArgument(['global_argument_def', null, false, [], false]), + new ConstructorArgument(['value_configured', null, false, 'value', false]), + new ConstructorArgument(['value_array_configured', null, false, [], false]), + new ConstructorArgument(['value_null', null, false, null, false]), ]; $this->diContainerConfig->expects($this->any()) diff --git a/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ConstructorArgumentTest.php b/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ConstructorArgumentTest.php index c4ca51858c941..312e12cbb8733 100644 --- a/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ConstructorArgumentTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Module/Di/Compiler/ConstructorArgumentTest.php @@ -14,11 +14,12 @@ class ConstructorArgumentTest extends TestCase { public function testInterface() { - $argument = ['configuration', 'array', true, null]; + $argument = ['configuration', 'array', true, null, false]; $model = new ConstructorArgument($argument); $this->assertEquals($argument[0], $model->getName()); $this->assertEquals($argument[1], $model->getType()); $this->assertTrue($model->isRequired()); $this->assertNull($model->getDefaultValue()); + $this->assertFalse($model->isVariadic()); } }