Skip to content

Commit fa7f939

Browse files
ENGCOM-5927: Allow use factories and OM for creating objects with variadic arguments in constructor #24556
2 parents c345add + f8dc00a commit fa7f939

File tree

6 files changed

+497
-55
lines changed

6 files changed

+497
-55
lines changed

lib/internal/Magento/Framework/Code/Reader/ClassReader.php

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@
55
*/
66
namespace Magento\Framework\Code\Reader;
77

8+
/**
9+
* Class ClassReader
10+
*/
811
class ClassReader implements ClassReaderInterface
912
{
1013
/**
1114
* Read class constructor signature
1215
*
13-
* @param string $className
16+
* @param string $className
1417
* @return array|null
1518
* @throws \ReflectionException
1619
*/
@@ -28,7 +31,8 @@ public function getConstructor($className)
2831
$parameter->getName(),
2932
$parameter->getClass() !== null ? $parameter->getClass()->getName() : null,
3033
!$parameter->isOptional() && !$parameter->isDefaultValueAvailable(),
31-
$parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null,
34+
$this->getReflectionParameterDefaultValue($parameter),
35+
$parameter->isVariadic(),
3236
];
3337
} catch (\ReflectionException $e) {
3438
$message = $e->getMessage();
@@ -40,6 +44,21 @@ public function getConstructor($className)
4044
return $result;
4145
}
4246

47+
/**
48+
* Get reflection parameter default value
49+
*
50+
* @param \ReflectionParameter $parameter
51+
* @return array|mixed|null
52+
*/
53+
private function getReflectionParameterDefaultValue(\ReflectionParameter $parameter)
54+
{
55+
if ($parameter->isVariadic()) {
56+
return [];
57+
}
58+
59+
return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null;
60+
}
61+
4362
/**
4463
* Retrieve parent relation information for type in a following format
4564
* array(
@@ -49,7 +68,7 @@ public function getConstructor($className)
4968
* ...
5069
* )
5170
*
52-
* @param string $className
71+
* @param string $className
5372
* @return string[]
5473
*/
5574
public function getParents($className)

lib/internal/Magento/Framework/ObjectManager/Factory/AbstractFactory.php

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
use Psr\Log\LoggerInterface;
1212
use Magento\Framework\App\ObjectManager;
1313

14+
/**
15+
* Class AbstractFactory
16+
*/
1417
abstract class AbstractFactory implements \Magento\Framework\ObjectManager\FactoryInterface
1518
{
1619
/**
@@ -49,10 +52,10 @@ abstract class AbstractFactory implements \Magento\Framework\ObjectManager\Facto
4952
protected $creationStack = [];
5053

5154
/**
52-
* @param \Magento\Framework\ObjectManager\ConfigInterface $config
53-
* @param ObjectManagerInterface $objectManager
55+
* @param \Magento\Framework\ObjectManager\ConfigInterface $config
56+
* @param ObjectManagerInterface $objectManager
5457
* @param \Magento\Framework\ObjectManager\DefinitionInterface $definitions
55-
* @param array $globalArguments
58+
* @param array $globalArguments
5659
*/
5760
public function __construct(
5861
\Magento\Framework\ObjectManager\ConfigInterface $config,
@@ -91,6 +94,8 @@ public function setArguments($arguments)
9194
}
9295

9396
/**
97+
* Get definitions
98+
*
9499
* @return \Magento\Framework\ObjectManager\DefinitionInterface
95100
*/
96101
public function getDefinitions()
@@ -105,7 +110,7 @@ public function getDefinitions()
105110
* Create object
106111
*
107112
* @param string $type
108-
* @param array $args
113+
* @param array $args
109114
*
110115
* @return object
111116
* @throws RuntimeException
@@ -115,7 +120,9 @@ protected function createObject($type, $args)
115120
try {
116121
return new $type(...array_values($args));
117122
} catch (\TypeError $exception) {
118-
/** @var LoggerInterface $logger */
123+
/**
124+
* @var LoggerInterface $logger
125+
*/
119126
$logger = ObjectManager::getInstance()->get(LoggerInterface::class);
120127
$logger->critical(
121128
sprintf('Type Error occurred when creating object: %s, %s', $type, $exception->getMessage())
@@ -130,9 +137,9 @@ protected function createObject($type, $args)
130137
/**
131138
* Resolve an argument
132139
*
133-
* @param array &$argument
140+
* @param array $argument
134141
* @param string $paramType
135-
* @param mixed $paramDefault
142+
* @param mixed $paramDefault
136143
* @param string $paramName
137144
* @param string $requestedType
138145
*
@@ -214,8 +221,8 @@ protected function parseArray(&$array)
214221
* Resolve constructor arguments
215222
*
216223
* @param string $requestedType
217-
* @param array $parameters
218-
* @param array $arguments
224+
* @param array $parameters
225+
* @param array $arguments
219226
*
220227
* @return array
221228
*
@@ -226,27 +233,44 @@ protected function resolveArgumentsInRuntime($requestedType, array $parameters,
226233
{
227234
$resolvedArguments = [];
228235
foreach ($parameters as $parameter) {
229-
list($paramName, $paramType, $paramRequired, $paramDefault) = $parameter;
230-
$argument = null;
231-
if (!empty($arguments) && (isset($arguments[$paramName]) || array_key_exists($paramName, $arguments))) {
232-
$argument = $arguments[$paramName];
233-
} elseif ($paramRequired) {
234-
if ($paramType) {
235-
$argument = ['instance' => $paramType];
236-
} else {
237-
$this->creationStack = [];
238-
throw new \BadMethodCallException(
239-
'Missing required argument $' . $paramName . ' of ' . $requestedType . '.'
240-
);
241-
}
236+
$resolvedArguments[] = $this->getResolvedArgument((string)$requestedType, $parameter, $arguments);
237+
}
238+
239+
return empty($resolvedArguments) ? [] : array_merge(...$resolvedArguments);
240+
}
241+
242+
/**
243+
* Get resolved argument from parameter
244+
*
245+
* @param string $requestedType
246+
* @param array $parameter
247+
* @param array $arguments
248+
* @return array
249+
*/
250+
private function getResolvedArgument(string $requestedType, array $parameter, array $arguments): array
251+
{
252+
list($paramName, $paramType, $paramRequired, $paramDefault, $isVariadic) = $parameter;
253+
$argument = null;
254+
if (!empty($arguments) && (isset($arguments[$paramName]) || array_key_exists($paramName, $arguments))) {
255+
$argument = $arguments[$paramName];
256+
} elseif ($paramRequired) {
257+
if ($paramType) {
258+
$argument = ['instance' => $paramType];
242259
} else {
243-
$argument = $paramDefault;
260+
$this->creationStack = [];
261+
throw new \BadMethodCallException(
262+
'Missing required argument $' . $paramName . ' of ' . $requestedType . '.'
263+
);
244264
}
265+
} else {
266+
$argument = $paramDefault;
267+
}
245268

246-
$this->resolveArgument($argument, $paramType, $paramDefault, $paramName, $requestedType);
247-
248-
$resolvedArguments[] = $argument;
269+
if ($isVariadic) {
270+
return is_array($argument) ? $argument : [$argument];
249271
}
250-
return $resolvedArguments;
272+
273+
$this->resolveArgument($argument, $paramType, $paramDefault, $paramName, $requestedType);
274+
return [$argument];
251275
}
252276
}

lib/internal/Magento/Framework/ObjectManager/Test/Unit/Factory/CompiledTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class CompiledTest extends \PHPUnit\Framework\TestCase
3838
/** @var ObjectManager */
3939
private $objectManager;
4040

41+
/**
42+
* Setup tests
43+
*/
4144
protected function setUp()
4245
{
4346
$this->objectManager = new ObjectManager($this);
@@ -57,6 +60,9 @@ protected function setUp()
5760
$this->objectManager->setBackwardCompatibleProperty($this->factory, 'definitions', $this->definitionsMock);
5861
}
5962

63+
/**
64+
* Test create simple
65+
*/
6066
public function testCreateSimple()
6167
{
6268
$expectedConfig = $this->getSimpleConfig();
@@ -106,6 +112,9 @@ public function testCreateSimple()
106112
$this->assertNull($result->getNullValue());
107113
}
108114

115+
/**
116+
* Test create simple configured arguments
117+
*/
109118
public function testCreateSimpleConfiguredArguments()
110119
{
111120
$expectedConfig = $this->getSimpleNestedConfig();
@@ -170,6 +179,9 @@ public function testCreateSimpleConfiguredArguments()
170179
$this->assertNull($result->getNullValue());
171180
}
172181

182+
/**
183+
* Test create get arguments in runtime
184+
*/
173185
public function testCreateGetArgumentsInRuntime()
174186
{
175187
// Stub OM to create test assets
@@ -308,18 +320,21 @@ private function getRuntimeParameters()
308320
1 => DependencyTesting::class,
309321
2 => true,
310322
3 => null,
323+
4 => false,
311324
],
312325
1 => [
313326
0 => 'sharedDependency',
314327
1 => DependencySharedTesting::class,
315328
2 => true,
316329
3 => null,
330+
4 => false,
317331
],
318332
2 => [
319333
0 => 'value',
320334
1 => null,
321335
2 => false,
322336
3 => 'value',
337+
4 => false,
323338
],
324339
3 => [
325340
0 => 'valueArray',
@@ -329,18 +344,21 @@ private function getRuntimeParameters()
329344
0 => 'default_value1',
330345
1 => 'default_value2',
331346
],
347+
4 => false,
332348
],
333349
4 => [
334350
0 => 'globalValue',
335351
1 => null,
336352
2 => false,
337353
3 => '',
354+
4 => false,
338355
],
339356
5 => [
340357
0 => 'nullValue',
341358
1 => null,
342359
2 => false,
343360
3 => null,
361+
4 => false,
344362
],
345363
];
346364
}

0 commit comments

Comments
 (0)