Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow use factories and OM for creating objects with variadic arguments in constructor #24556

25 changes: 22 additions & 3 deletions lib/internal/Magento/Framework/Code/Reader/ClassReader.php
Expand Up @@ -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
*/
Expand All @@ -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();
Expand All @@ -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(
Expand All @@ -49,7 +68,7 @@ public function getConstructor($className)
* ...
* )
*
* @param string $className
* @param string $className
* @return string[]
*/
public function getParents($className)
Expand Down
Expand Up @@ -11,6 +11,9 @@
use Psr\Log\LoggerInterface;
use Magento\Framework\App\ObjectManager;

/**
* Class AbstractFactory
*/
abstract class AbstractFactory implements \Magento\Framework\ObjectManager\FactoryInterface
{
/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -91,6 +94,8 @@ public function setArguments($arguments)
}

/**
* Get definitions
*
* @return \Magento\Framework\ObjectManager\DefinitionInterface
*/
public function getDefinitions()
Expand All @@ -105,7 +110,7 @@ public function getDefinitions()
* Create object
*
* @param string $type
* @param array $args
* @param array $args
*
* @return object
* @throws RuntimeException
Expand All @@ -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())
Expand All @@ -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
*
Expand Down Expand Up @@ -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
*
Expand All @@ -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];
}
}
Expand Up @@ -38,6 +38,9 @@ class CompiledTest extends \PHPUnit\Framework\TestCase
/** @var ObjectManager */
private $objectManager;

/**
* Setup tests
*/
protected function setUp()
{
$this->objectManager = new ObjectManager($this);
Expand All @@ -57,6 +60,9 @@ protected function setUp()
$this->objectManager->setBackwardCompatibleProperty($this->factory, 'definitions', $this->definitionsMock);
}

/**
* Test create simple
*/
public function testCreateSimple()
{
$expectedConfig = $this->getSimpleConfig();
Expand Down Expand Up @@ -106,6 +112,9 @@ public function testCreateSimple()
$this->assertNull($result->getNullValue());
}

/**
* Test create simple configured arguments
*/
public function testCreateSimpleConfiguredArguments()
{
$expectedConfig = $this->getSimpleNestedConfig();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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,
],
];
}
Expand Down