diff --git a/lib/internal/Magento/Framework/Code/Reader/ClassReader.php b/lib/internal/Magento/Framework/Code/Reader/ClassReader.php index fe96e9cc742aa..27e633c660689 100644 --- a/lib/internal/Magento/Framework/Code/Reader/ClassReader.php +++ b/lib/internal/Magento/Framework/Code/Reader/ClassReader.php @@ -5,12 +5,15 @@ */ namespace Magento\Framework\Code\Reader; +/** + * Class ClassReader + */ class ClassReader implements ClassReaderInterface { /** * Read class constructor signature * - * @param string $className + * @param string $className * @return array|null * @throws \ReflectionException */ @@ -28,7 +31,8 @@ public function getConstructor($className) $parameter->getName(), $parameter->getClass() !== null ? $parameter->getClass()->getName() : null, !$parameter->isOptional() && !$parameter->isDefaultValueAvailable(), - $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null, + $this->getReflectionParameterDefaultValue($parameter), + $parameter->isVariadic(), ]; } catch (\ReflectionException $e) { $message = $e->getMessage(); @@ -40,6 +44,21 @@ public function getConstructor($className) return $result; } + /** + * Get reflection parameter default value + * + * @param \ReflectionParameter $parameter + * @return array|mixed|null + */ + private function getReflectionParameterDefaultValue(\ReflectionParameter $parameter) + { + if ($parameter->isVariadic()) { + return []; + } + + return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; + } + /** * Retrieve parent relation information for type in a following format * array( @@ -49,7 +68,7 @@ public function getConstructor($className) * ... * ) * - * @param string $className + * @param string $className * @return string[] */ public function getParents($className) diff --git a/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php b/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php index 15c4cb098b84d..7094b116ead3f 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php +++ b/lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php @@ -11,6 +11,9 @@ use Psr\Log\LoggerInterface; use Magento\Framework\App\ObjectManager; +/** + * Class AbstractFactory + */ abstract class AbstractFactory implements \Magento\Framework\ObjectManager\FactoryInterface { /** @@ -49,10 +52,10 @@ abstract class AbstractFactory implements \Magento\Framework\ObjectManager\Facto protected $creationStack = []; /** - * @param \Magento\Framework\ObjectManager\ConfigInterface $config - * @param ObjectManagerInterface $objectManager + * @param \Magento\Framework\ObjectManager\ConfigInterface $config + * @param ObjectManagerInterface $objectManager * @param \Magento\Framework\ObjectManager\DefinitionInterface $definitions - * @param array $globalArguments + * @param array $globalArguments */ public function __construct( \Magento\Framework\ObjectManager\ConfigInterface $config, @@ -91,6 +94,8 @@ public function setArguments($arguments) } /** + * Get definitions + * * @return \Magento\Framework\ObjectManager\DefinitionInterface */ public function getDefinitions() @@ -105,7 +110,7 @@ public function getDefinitions() * Create object * * @param string $type - * @param array $args + * @param array $args * * @return object * @throws RuntimeException @@ -115,7 +120,9 @@ protected function createObject($type, $args) try { return new $type(...array_values($args)); } catch (\TypeError $exception) { - /** @var LoggerInterface $logger */ + /** + * @var LoggerInterface $logger + */ $logger = ObjectManager::getInstance()->get(LoggerInterface::class); $logger->critical( sprintf('Type Error occurred when creating object: %s, %s', $type, $exception->getMessage()) @@ -130,9 +137,9 @@ protected function createObject($type, $args) /** * Resolve an argument * - * @param array &$argument + * @param array $argument * @param string $paramType - * @param mixed $paramDefault + * @param mixed $paramDefault * @param string $paramName * @param string $requestedType * @@ -214,8 +221,8 @@ protected function parseArray(&$array) * Resolve constructor arguments * * @param string $requestedType - * @param array $parameters - * @param array $arguments + * @param array $parameters + * @param array $arguments * * @return array * @@ -226,27 +233,44 @@ protected function resolveArgumentsInRuntime($requestedType, array $parameters, { $resolvedArguments = []; foreach ($parameters as $parameter) { - list($paramName, $paramType, $paramRequired, $paramDefault) = $parameter; - $argument = null; - if (!empty($arguments) && (isset($arguments[$paramName]) || array_key_exists($paramName, $arguments))) { - $argument = $arguments[$paramName]; - } elseif ($paramRequired) { - if ($paramType) { - $argument = ['instance' => $paramType]; - } else { - $this->creationStack = []; - throw new \BadMethodCallException( - 'Missing required argument $' . $paramName . ' of ' . $requestedType . '.' - ); - } + $resolvedArguments[] = $this->getResolvedArgument((string)$requestedType, $parameter, $arguments); + } + + return empty($resolvedArguments) ? [] : array_merge(...$resolvedArguments); + } + + /** + * Get resolved argument from parameter + * + * @param string $requestedType + * @param array $parameter + * @param array $arguments + * @return array + */ + private function getResolvedArgument(string $requestedType, array $parameter, array $arguments): array + { + list($paramName, $paramType, $paramRequired, $paramDefault, $isVariadic) = $parameter; + $argument = null; + if (!empty($arguments) && (isset($arguments[$paramName]) || array_key_exists($paramName, $arguments))) { + $argument = $arguments[$paramName]; + } elseif ($paramRequired) { + if ($paramType) { + $argument = ['instance' => $paramType]; } else { - $argument = $paramDefault; + $this->creationStack = []; + throw new \BadMethodCallException( + 'Missing required argument $' . $paramName . ' of ' . $requestedType . '.' + ); } + } else { + $argument = $paramDefault; + } - $this->resolveArgument($argument, $paramType, $paramDefault, $paramName, $requestedType); - - $resolvedArguments[] = $argument; + if ($isVariadic) { + return is_array($argument) ? $argument : [$argument]; } - return $resolvedArguments; + + $this->resolveArgument($argument, $paramType, $paramDefault, $paramName, $requestedType); + return [$argument]; } } diff --git a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/CompiledTest.php b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/CompiledTest.php index 779a0d04ebc52..648a74b91495d 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/CompiledTest.php +++ b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/CompiledTest.php @@ -38,6 +38,9 @@ class CompiledTest extends \PHPUnit\Framework\TestCase /** @var ObjectManager */ private $objectManager; + /** + * Setup tests + */ protected function setUp() { $this->objectManager = new ObjectManager($this); @@ -57,6 +60,9 @@ protected function setUp() $this->objectManager->setBackwardCompatibleProperty($this->factory, 'definitions', $this->definitionsMock); } + /** + * Test create simple + */ public function testCreateSimple() { $expectedConfig = $this->getSimpleConfig(); @@ -106,6 +112,9 @@ public function testCreateSimple() $this->assertNull($result->getNullValue()); } + /** + * Test create simple configured arguments + */ public function testCreateSimpleConfiguredArguments() { $expectedConfig = $this->getSimpleNestedConfig(); @@ -170,6 +179,9 @@ public function testCreateSimpleConfiguredArguments() $this->assertNull($result->getNullValue()); } + /** + * Test create get arguments in runtime + */ public function testCreateGetArgumentsInRuntime() { // Stub OM to create test assets @@ -308,18 +320,21 @@ private function getRuntimeParameters() 1 => DependencyTesting::class, 2 => true, 3 => null, + 4 => false, ], 1 => [ 0 => 'sharedDependency', 1 => DependencySharedTesting::class, 2 => true, 3 => null, + 4 => false, ], 2 => [ 0 => 'value', 1 => null, 2 => false, 3 => 'value', + 4 => false, ], 3 => [ 0 => 'valueArray', @@ -329,18 +344,21 @@ private function getRuntimeParameters() 0 => 'default_value1', 1 => 'default_value2', ], + 4 => false, ], 4 => [ 0 => 'globalValue', 1 => null, 2 => false, 3 => '', + 4 => false, ], 5 => [ 0 => 'nullValue', 1 => null, 2 => false, 3 => null, + 4 => false, ], ]; } diff --git a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/FactoryTest.php b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/FactoryTest.php index 309bf48548ec7..cc86d794bd80a 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/FactoryTest.php +++ b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/FactoryTest.php @@ -10,6 +10,9 @@ use Magento\Framework\ObjectManager\Factory\Dynamic\Developer; use Magento\Framework\ObjectManager\ObjectManager; +/** + * Class FactoryTest + */ class FactoryTest extends \PHPUnit\Framework\TestCase { /** @@ -27,6 +30,9 @@ class FactoryTest extends \PHPUnit\Framework\TestCase */ private $objectManager; + /** + * Setup tests + */ protected function setUp() { $this->config = new Config(); @@ -35,28 +41,43 @@ protected function setUp() $this->factory->setObjectManager($this->objectManager); } + /** + * Test create without args + */ public function testCreateNoArgs() { $this->assertInstanceOf('StdClass', $this->factory->create(\StdClass::class)); } /** - * @expectedException \UnexpectedValueException + * @expectedException \UnexpectedValueException * @expectedExceptionMessage Invalid parameter configuration provided for $firstParam argument */ public function testResolveArgumentsException() { $configMock = $this->createMock(\Magento\Framework\ObjectManager\Config\Config::class); - $configMock->expects($this->once())->method('getArguments') - ->will($this->returnValue([ - 'firstParam' => 1, - ])); + $configMock->expects($this->once())->method('getArguments')->will( + $this->returnValue( + [ + 'firstParam' => 1, + ] + ) + ); $definitionsMock = $this->createMock(\Magento\Framework\ObjectManager\DefinitionInterface::class); - $definitionsMock->expects($this->once())->method('getParameters') - ->will($this->returnValue([[ - 'firstParam', 'string', true, 'default_val', - ]])); + $definitionsMock->expects($this->once())->method('getParameters')->will( + $this->returnValue( + [ + [ + 'firstParam', + 'string', + true, + 'default_val', + false + ] + ] + ) + ); $this->factory = new Developer( $configMock, @@ -71,9 +92,14 @@ public function testResolveArgumentsException() ); } + /** + * Test create with one arg + */ public function testCreateOneArg() { - /** @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar $result */ + /** + * @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar $result + */ $result = $this->factory->create( \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class, ['foo' => 'bar'] @@ -82,6 +108,9 @@ public function testCreateOneArg() $this->assertEquals('bar', $result->getFoo()); } + /** + * Test create with injectable + */ public function testCreateWithInjectable() { // let's imitate that One is injectable by providing DI configuration for it @@ -92,7 +121,9 @@ public function testCreateWithInjectable() ], ] ); - /** @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Two $result */ + /** + * @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Two $result + */ $result = $this->factory->create(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Two::class); $this->assertInstanceOf(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Two::class, $result); $this->assertInstanceOf( @@ -104,8 +135,8 @@ public function testCreateWithInjectable() } /** - * @param string $startingClass - * @param string $terminationClass + * @param string $startingClass + * @param string $terminationClass * @dataProvider circularDataProvider */ public function testCircular($startingClass, $terminationClass) @@ -130,23 +161,30 @@ public function circularDataProvider() ]; } + /** + * Test create using reflection + */ public function testCreateUsingReflection() { $type = \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Polymorphous::class; $definitions = $this->createMock(\Magento\Framework\ObjectManager\DefinitionInterface::class); // should be more than defined in "switch" of create() method - $definitions->expects($this->once())->method('getParameters')->with($type)->will($this->returnValue([ - ['one', null, false, null], - ['two', null, false, null], - ['three', null, false, null], - ['four', null, false, null], - ['five', null, false, null], - ['six', null, false, null], - ['seven', null, false, null], - ['eight', null, false, null], - ['nine', null, false, null], - ['ten', null, false, null], - ])); + $definitions->expects($this->once())->method('getParameters')->with($type)->will( + $this->returnValue( + [ + ['one', null, false, null, false], + ['two', null, false, null, false], + ['three', null, false, null, false], + ['four', null, false, null, false], + ['five', null, false, null, false], + ['six', null, false, null, false], + ['seven', null, false, null, false], + ['eight', null, false, null, false], + ['nine', null, false, null, false], + ['ten', null, false, null, false], + ] + ) + ); $factory = new Developer($this->config, null, $definitions); $result = $factory->create( $type, @@ -165,4 +203,257 @@ public function testCreateUsingReflection() ); $this->assertSame(10, $result->getArg(9)); } + + /** + * Test create objects with variadic argument in constructor + * + * @param $createArgs + * @param $expectedArg0 + * @param $expectedArg1 + * @dataProvider testCreateUsingVariadicDataProvider + */ + public function testCreateUsingVariadic( + $createArgs, + $expectedArg0, + $expectedArg1 + ) { + $type = \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Variadic::class; + $definitions = $this->createMock(\Magento\Framework\ObjectManager\DefinitionInterface::class); + + $definitions->expects($this->once())->method('getParameters')->with($type)->will( + $this->returnValue( + [ + [ + 'oneScalars', + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class, + false, + [], + true + ], + ] + ) + ); + $factory = new Developer($this->config, null, $definitions); + + /** + * @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Variadic $variadic + */ + $variadic = is_null($createArgs) + ? $factory->create($type) + : $factory->create($type, $createArgs); + + $this->assertSame($expectedArg0, $variadic->getOneScalarByKey(0)); + $this->assertSame($expectedArg1, $variadic->getOneScalarByKey(1)); + } + + /** + * @return array + */ + public function testCreateUsingVariadicDataProvider() + { + $oneScalar1 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + $oneScalar2 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + + return [ + 'without_args' => [ + null, + null, + null, + ], + 'with_empty_args' => [ + [], + null, + null, + ], + 'with_empty_args_value' => [ + [ + 'oneScalars' => [] + ], + null, + null, + ], + 'with_single_arg' => [ + [ + 'oneScalars' => $oneScalar1 + ], + $oneScalar1, + null, + ], + 'with_full_args' => [ + [ + 'oneScalars' => [ + $oneScalar1, + $oneScalar2, + ] + ], + $oneScalar1, + $oneScalar2, + ], + ]; + } + + /** + * Test data can be injected into variadic arguments from di config + */ + public function testCreateVariadicFromDiConfig() + { + $oneScalar1 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + $oneScalar2 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + + // let's imitate that Variadic is configured by providing DI configuration for it + $this->config->extend( + [ + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Variadic::class => [ + 'arguments' => [ + 'oneScalars' => [ + $oneScalar1, + $oneScalar2, + ] + ] + ], + ] + ); + /** + * @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Variadic $variadic + */ + $variadic = $this->factory->create(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\Variadic::class); + + $this->assertSame($oneScalar1, $variadic->getOneScalarByKey(0)); + $this->assertSame($oneScalar2, $variadic->getOneScalarByKey(1)); + } + + /** + * Test create objects with non variadic and variadic argument in constructor + * + * @param $createArgs + * @param $expectedFooValue + * @param $expectedArg0 + * @param $expectedArg1 + * @dataProvider testCreateUsingSemiVariadicDataProvider + */ + public function testCreateUsingSemiVariadic( + $createArgs, + $expectedFooValue, + $expectedArg0, + $expectedArg1 + ) { + $type = \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::class; + $definitions = $this->createMock(\Magento\Framework\ObjectManager\DefinitionInterface::class); + + $definitions->expects($this->once())->method('getParameters')->with($type)->will( + $this->returnValue( + [ + [ + 'foo', + null, + false, + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + false + ], + [ + 'oneScalars', + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class, + false, + [], + true + ], + ] + ) + ); + $factory = new Developer($this->config, null, $definitions); + + /** + * @var \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic $semiVariadic + */ + $semiVariadic = is_null($createArgs) + ? $factory->create($type) + : $factory->create($type, $createArgs); + + $this->assertSame($expectedFooValue, $semiVariadic->getFoo()); + $this->assertSame($expectedArg0, $semiVariadic->getOneScalarByKey(0)); + $this->assertSame($expectedArg1, $semiVariadic->getOneScalarByKey(1)); + } + + /** + * @return array + */ + public function testCreateUsingSemiVariadicDataProvider() + { + $oneScalar1 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + $oneScalar2 = $this->createMock(\Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\OneScalar::class); + + return [ + 'without_args' => [ + null, + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + null, + null, + ], + 'with_empty_args' => [ + [], + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + null, + null, + ], + 'only_with_foo_value' => [ + [ + 'foo' => 'baz' + ], + 'baz', + null, + null, + ], + 'only_with_oneScalars_empty_value' => [ + [ + 'oneScalars' => [] + ], + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + null, + null, + ], + 'only_with_oneScalars_single_value' => [ + [ + 'oneScalars' => $oneScalar1 + ], + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + $oneScalar1, + null, + ], + 'only_with_oneScalars_full_value' => [ + [ + 'oneScalars' => [ + $oneScalar1, + $oneScalar2, + ] + ], + \Magento\Framework\ObjectManager\Test\Unit\Factory\Fixture\SemiVariadic::DEFAULT_FOO_VALUE, + $oneScalar1, + $oneScalar2, + ], + 'with_all_values_defined_in_right_order' => [ + [ + 'foo' => 'baz', + 'oneScalars' => [ + $oneScalar1, + $oneScalar2, + ] + ], + 'baz', + $oneScalar1, + $oneScalar2, + ], + 'with_all_values_defined_in_reverse_order' => [ + [ + 'oneScalars' => [ + $oneScalar1, + $oneScalar2, + ], + 'foo' => 'baz', + ], + 'baz', + $oneScalar1, + $oneScalar2, + ], + ]; + } } diff --git a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/SemiVariadic.php b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/SemiVariadic.php new file mode 100644 index 0000000000000..8773dec1c48d6 --- /dev/null +++ b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/SemiVariadic.php @@ -0,0 +1,55 @@ +foo = $foo; + $this->oneScalars = $oneScalars; + } + + /** + * @param mixed $key + * @return mixed + */ + public function getOneScalarByKey($key) + { + return $this->oneScalars[$key] ?? null; + } + + /** + * @return string + */ + public function getFoo(): string + { + return $this->foo; + } +} diff --git a/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/Variadic.php b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/Variadic.php new file mode 100644 index 0000000000000..af26f7456fdde --- /dev/null +++ b/lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/Fixture/Variadic.php @@ -0,0 +1,35 @@ +oneScalars = $oneScalars; + } + + /** + * @param string $key + * @return mixed + */ + public function getOneScalarByKey($key) + { + return $this->oneScalars[$key] ?? null; + } +}