From 2dcccbd3464b49a39317eb4712a51e7881d2b98d Mon Sep 17 00:00:00 2001 From: Claudiu Cristea Date: Sun, 3 Nov 2024 15:56:09 +0200 Subject: [PATCH 1/2] Autowire container params --- src/Commands/AutowireTrait.php | 65 +++++++++++++++++++--- src/Commands/generate/GenerateCommands.php | 3 +- src/Runtime/ServiceManager.php | 64 ++++++++++++++++----- 3 files changed, 109 insertions(+), 23 deletions(-) diff --git a/src/Commands/AutowireTrait.php b/src/Commands/AutowireTrait.php index d4d77ef095..e98b52dd04 100644 --- a/src/Commands/AutowireTrait.php +++ b/src/Commands/AutowireTrait.php @@ -2,6 +2,9 @@ namespace Drush\Commands; +use Drupal\Component\DependencyInjection\ContainerInterface as DrupalContainer; +use Drush\Drush; +use League\Container\Container as DrushContainer; use Psr\Container\ContainerInterface; use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException; @@ -16,7 +19,18 @@ */ trait AutowireTrait { - /** + /** + * Limit to service and param or plain value. + * + * @see \Symfony\Component\DependencyInjection\Attribute\Autowire::__construct + */ + private const ACCEPTED_AUTOWIRE_ARGUMENTS = [ + 0 => 'value', + 1 => 'service', + 4 => 'param', + ]; + + /** * Instantiates a new instance of the implementing class using autowiring. * * @param ContainerInterface $container @@ -24,23 +38,56 @@ trait AutowireTrait * * @return static */ - public static function create(ContainerInterface $container) + public static function create(ContainerInterface $container, ?ContainerInterface $drushContainer = null): self { $args = []; if (method_exists(static::class, '__construct')) { + $drushContainer = $container instanceof DrushContainer ? $container : ($drushContainer instanceof DrushContainer ? $drushContainer : Drush::getContainer()); + $drupalContainer = $container instanceof DrupalContainer ? $container : null; + $constructor = new \ReflectionMethod(static::class, '__construct'); foreach ($constructor->getParameters() as $parameter) { - $service = ltrim((string) $parameter->getType(), '?'); - foreach ($parameter->getAttributes(Autowire::class) as $attribute) { - $service = (string) $attribute->newInstance()->value; + if (!$attributes = $parameter->getAttributes(Autowire::class)) { + // No #[Autowire()] attribute. + $service = ltrim((string) $parameter->getType(), '?'); + if (!$drushContainer->has($service)) { + throw new AutowiringFailedException($service, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s::_construct()", you should configure its value explicitly.', $service, $parameter->getName(), static::class)); + } + $args[] = $drushContainer->get($service); + continue; } - if (!$container->has($service)) { - throw new AutowiringFailedException($service, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s::_construct()", you should configure its value explicitly.', $service, $parameter->getName(), static::class)); - } + // This parameter has an #[Autowire()] attribute. + [$attribute] = $attributes; + $value = null; + foreach ($attribute->getArguments() as $key => $argument) { + // Resolve argument name when arguments are passed as list. + if (is_int($key)) { + if ($argument === null || !isset(self::ACCEPTED_AUTOWIRE_ARGUMENTS[$key])) { + continue; + } + $key = self::ACCEPTED_AUTOWIRE_ARGUMENTS[$key]; + } + + if (!in_array($key, self::ACCEPTED_AUTOWIRE_ARGUMENTS, true)) { + continue; + } - $args[] = $container->get($service); + $value = $attribute->newInstance()->value; + $valueAsString = (string) $value; + $value = match ($key) { + 'service' => $drushContainer->has($valueAsString) ? $drushContainer->get($valueAsString) : throw new AutowiringFailedException($valueAsString, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s::_construct()", you should configure its value explicitly.', $valueAsString, $parameter->getName(), static::class)), + // Container param comes as %foo.bar.param%. + 'param' => $drupalContainer ? $drupalContainer->getParameter(trim($valueAsString, '%')) : $valueAsString, + default => $value, + }; + // Done as Autowire::__construct() only needs one argument. + break; + } + if ($value !== null) { + $args[] = $value; + } } } diff --git a/src/Commands/generate/GenerateCommands.php b/src/Commands/generate/GenerateCommands.php index 2e7d621d40..45515c25bd 100644 --- a/src/Commands/generate/GenerateCommands.php +++ b/src/Commands/generate/GenerateCommands.php @@ -8,7 +8,7 @@ use Drush\Commands\core\DocsCommands; use Drush\Commands\DrushCommands; use Drush\Commands\help\ListCommands; -use Psr\Container\ContainerInterface as DrushContainer; +use League\Container\DefinitionContainerInterface as DrushContainer; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Completion\CompletionInput; use Symfony\Component\Console\Completion\CompletionSuggestions; @@ -21,6 +21,7 @@ final class GenerateCommands extends DrushCommands protected function __construct( private readonly DrushContainer $drush_container, ) { + parent::__construct(); } public static function create(DrushContainer $container): self diff --git a/src/Runtime/ServiceManager.php b/src/Runtime/ServiceManager.php index 0588d63549..b60f82b56d 100644 --- a/src/Runtime/ServiceManager.php +++ b/src/Runtime/ServiceManager.php @@ -23,7 +23,9 @@ use Grasmash\YamlCli\Command\UnsetKeyCommand; use Grasmash\YamlCli\Command\UpdateKeyCommand; use Grasmash\YamlCli\Command\UpdateValueCommand; -use League\Container\Container as DrushContainer; +use League\Container\Container; +use League\Container\DefinitionContainerInterface as DrushContainer; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Robo\ClassDiscovery\RelativeNamespaceDiscovery; @@ -342,6 +344,7 @@ public function instantiateServices(array $bootstrapCommandClasses, DrushContain // Prevent duplicate calls to delegate() by checking for state. if ($container && !$drushContainer->has('state')) { + assert($drushContainer instanceof Container); // Combine the two containers. $drushContainer->delegate($container); } @@ -349,11 +352,8 @@ public function instantiateServices(array $bootstrapCommandClasses, DrushContain $commandHandler = null; try { - if ($this->hasStaticCreateFactory($class) && $this->supportsCompoundContainer($class, $drushContainer)) { - // Hurray, this class is compatible with the container with delegate. - $commandHandler = $class::create($drushContainer); - } elseif ($container && $this->hasStaticCreateFactory($class)) { - $commandHandler = $class::create($container, $drushContainer); + if ($staticCreateFactoryArguments = $this->getStaticCreateFactoryArguments($class, $drushContainer, $container)) { + $commandHandler = $class::create(...$staticCreateFactoryArguments); } elseif (!$container && $this->hasStaticCreateEarlyFactory($class)) { $commandHandler = $class::createEarly($drushContainer); } else { @@ -372,14 +372,43 @@ public function instantiateServices(array $bootstrapCommandClasses, DrushContain return $commandHandlers; } - /** - * Determine if the first parameter of the create method supports our container with delegate. - */ - protected function supportsCompoundContainer($class, $drush_container): bool + protected function getStaticCreateFactoryArguments(string $class, DrushContainer $drushContainer, ?DrupalContainer $drupalContainer = null): ?array { + if (!method_exists($class, 'create')) { + return null; + } + $reflection = new \ReflectionMethod($class, 'create'); - $hint = (string)$reflection->getParameters()[0]->getType(); - return is_a($drush_container, $hint); + if (!$reflection->isStatic()) { + return null; + } + + $params = $reflection->getParameters(); + + $args = []; + $type = ltrim((string) $params[0]->getType(), '?'); + if ($drupalContainer && is_a($type, DrupalContainer::class, true)) { + // The factory create() method explicitly expects a Drupal container as 1st argument. + $args[] = $drupalContainer; + } elseif (is_a($type, DrushContainer::class, true)) { + // The factory create() method explicitly expects a Drush container as 1st argument. Don't add a 2nd + // argument. If the Drupal container has been initialized, it's already a delegate. + return [$drushContainer]; + } elseif ($drupalContainer && is_a($type, ContainerInterface::class, true)) { + // The factory create() method expects a container of any type as st argument and the Drupal container has + // been initialized. Pass it as 1st argument. + $args[] = $drupalContainer; + } + + // Add Drush container as 2nd argument if the method expects one. + if (isset($params[1])) { + $type = ltrim((string) $params[1]->getType(), '?'); + if (is_a($type, ContainerInterface::class, true)) { + $args[] = $drushContainer; + } + } + + return $args; } /** @@ -387,7 +416,16 @@ protected function supportsCompoundContainer($class, $drush_container): bool */ protected function hasStaticCreateFactory(string $class): bool { - return static::hasStaticMethod($class, 'create'); + if (!$hasStaticCreateFactory = static::hasStaticMethod($class, 'create')) { + return false; + } + $reflection = new \ReflectionMethod($class, 'create'); + // Check first two param typings. + foreach (array_slice($reflection->getParameters(), 0, 2) as $param) { + $typing = ltrim((string) $param->getType(), '?'); + $hasStaticCreateFactory = $hasStaticCreateFactory && is_a($typing, ContainerInterface::class, true); + } + return $hasStaticCreateFactory; } /** From 02471eee443e4da0ae02df4d2d4b6a7c6a1599e9 Mon Sep 17 00:00:00 2001 From: Claudiu Cristea Date: Sun, 3 Nov 2024 15:56:24 +0200 Subject: [PATCH 2/2] Test coverage --- .../unish/woot/src/AutowireTestService.php | 13 ++++ .../woot/src/AutowireTestServiceInterface.php | 7 ++ sut/modules/unish/woot/woot.services.yml | 6 ++ .../lib/CreateExpectsDrupalContainer.php | 23 ++++++ .../lib/CreateExpectsDrushContainer.php | 22 ++++++ .../Drush/Commands/AutowireTestCommands.php | 64 ++++++++++++++++ tests/integration/AutowireArgumentsTest.php | 76 +++++++++++++++++++ 7 files changed, 211 insertions(+) create mode 100644 sut/modules/unish/woot/src/AutowireTestService.php create mode 100644 sut/modules/unish/woot/src/AutowireTestServiceInterface.php create mode 100644 tests/fixtures/lib/CreateExpectsDrupalContainer.php create mode 100644 tests/fixtures/lib/CreateExpectsDrushContainer.php create mode 100644 tests/fixtures/lib/Drush/Commands/AutowireTestCommands.php create mode 100644 tests/integration/AutowireArgumentsTest.php diff --git a/sut/modules/unish/woot/src/AutowireTestService.php b/sut/modules/unish/woot/src/AutowireTestService.php new file mode 100644 index 0000000000..1547001cca --- /dev/null +++ b/sut/modules/unish/woot/src/AutowireTestService.php @@ -0,0 +1,13 @@ +get('redirect.destination')); + } +} diff --git a/tests/fixtures/lib/CreateExpectsDrushContainer.php b/tests/fixtures/lib/CreateExpectsDrushContainer.php new file mode 100644 index 0000000000..435ffe3e1b --- /dev/null +++ b/tests/fixtures/lib/CreateExpectsDrushContainer.php @@ -0,0 +1,22 @@ +get('logger')); + } +} diff --git a/tests/fixtures/lib/Drush/Commands/AutowireTestCommands.php b/tests/fixtures/lib/Drush/Commands/AutowireTestCommands.php new file mode 100644 index 0000000000..a05899be76 --- /dev/null +++ b/tests/fixtures/lib/Drush/Commands/AutowireTestCommands.php @@ -0,0 +1,64 @@ +getParameters() as $param) { + $values[] = (string) $this->{$param->getName()}; + } + return implode("\n", $values); + } + + #[CLI\Command(name: 'test_autowire:drush-container')] + #[CLI\Bootstrap(level: DrupalBootLevels::NONE)] + #[CLI\Help(hidden: true)] + public function drush(): string + { + $values = []; + $constructor = new \ReflectionMethod($this, '__construct'); + foreach ($constructor->getParameters() as $param) { + $values[] = (string) $this->{$param->getName()}; + } + return implode("\n", $values); + } +} diff --git a/tests/integration/AutowireArgumentsTest.php b/tests/integration/AutowireArgumentsTest.php new file mode 100644 index 0000000000..9bb6799574 --- /dev/null +++ b/tests/integration/AutowireArgumentsTest.php @@ -0,0 +1,76 @@ +drush(PmCommands::INSTALL, ['woot']); + + $expected = [ + // Autowire('a string as it is') + 'a string as it is', + // Autowire(null, 'woot.autowire_test') + 'Hello World!', + // Autowire(null, null, null, null, 'foo') + 'bar', + // Autowire(value: 'a string as it is') + 'a string as it is', + // Autowire(service: 'woot.autowire_test') + 'Hello World!', + // Autowire(param: 'foo') + 'bar', + // Autowire by service full qualified interface name + // @see \Drupal\woot\AutowireTestServiceInterface + 'Hello World!', + ]; + + $this->drush('test_autowire:drupal-container'); + $this->assertSame(implode("\n", $expected), $this->getOutput()); + + $this->drush(PmCommands::UNINSTALL, ['woot']); + } + + // @todo This test will fail because I coudn't find a way to add a new + // service to Drush container. + public function testWithDrushContainer(): void + { + $drushContainer = Drush::getContainer(); + $drushContainer->add('woot.autowire_test', AutowireTestService::class); + $drushContainer->add(AutowireTestServiceInterface::class, AutowireTestService::class); + Drush::setContainer($drushContainer); + + $expected = [ + // Autowire('a string as it is') + 'a string as it is', + // Autowire(null, 'woot.autowire_test') + 'Hello World!', + // Autowire(null, null, null, null, 'foo') + 'bar', + // Autowire(value: 'a string as it is') + 'a string as it is', + // Autowire(service: 'woot.autowire_test') + 'Hello World!', + // Autowire(param: 'foo') + 'bar', + // Autowire by service full qualified interface name + // @see \Drupal\woot\AutowireTestServiceInterface + 'Hello World!', + ]; + + $this->drush('test_autowire:drush-container'); + $this->assertSame(implode("\n", $expected), $output); + } +}