From a3edb28e934f9061b82093a8d21014eb671d15e8 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Mon, 30 Oct 2023 11:01:39 +1000
Subject: [PATCH] Issue #3394870 by longwave, smustgrave, alexpott: Allow
controller service wiring via constructor parameter attributes
---
.../Drupal/Core/Controller/ControllerBase.php | 10 +---
.../DependencyInjection/AutowireTrait.php | 47 +++++++++++++++++
.../node/src/Controller/NodeController.php | 12 -----
.../Controller/BrokenSystemTestController.php | 23 +++++++++
.../src/Controller/SystemTestController.php | 25 ++++------
.../Core/Controller/ControllerBaseTest.php | 50 +++++++++++++++++++
6 files changed, 132 insertions(+), 35 deletions(-)
create mode 100644 core/lib/Drupal/Core/DependencyInjection/AutowireTrait.php
create mode 100644 core/modules/system/tests/modules/system_test/src/Controller/BrokenSystemTestController.php
create mode 100644 core/tests/Drupal/KernelTests/Core/Controller/ControllerBaseTest.php
diff --git a/core/lib/Drupal/Core/Controller/ControllerBase.php b/core/lib/Drupal/Core/Controller/ControllerBase.php
index a82770eaa555..a9569c622f55 100644
--- a/core/lib/Drupal/Core/Controller/ControllerBase.php
+++ b/core/lib/Drupal/Core/Controller/ControllerBase.php
@@ -2,12 +2,12 @@
namespace Drupal\Core\Controller;
+use Drupal\Core\DependencyInjection\AutowireTrait;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Logger\LoggerChannelTrait;
use Drupal\Core\Routing\RedirectDestinationTrait;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\Url;
-use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\Messenger\MessengerTrait;
use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -34,6 +34,7 @@
*/
abstract class ControllerBase implements ContainerInjectionInterface {
+ use AutowireTrait;
use LoggerChannelTrait;
use MessengerTrait;
use RedirectDestinationTrait;
@@ -102,13 +103,6 @@ abstract class ControllerBase implements ContainerInjectionInterface {
*/
protected $formBuilder;
- /**
- * {@inheritdoc}
- */
- public static function create(ContainerInterface $container) {
- return new static();
- }
-
/**
* Retrieves the entity type manager.
*
diff --git a/core/lib/Drupal/Core/DependencyInjection/AutowireTrait.php b/core/lib/Drupal/Core/DependencyInjection/AutowireTrait.php
new file mode 100644
index 000000000000..855bf20d82cd
--- /dev/null
+++ b/core/lib/Drupal/Core/DependencyInjection/AutowireTrait.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace Drupal\Core\DependencyInjection;
+
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
+
+/**
+ * Defines a trait for automatically wiring dependencies from the container.
+ *
+ * This trait uses reflection and may cause performance issues with classes
+ * that will be instantiated multiple times.
+ */
+trait AutowireTrait {
+
+ /**
+ * Instantiates a new instance of the implementing class using autowiring.
+ *
+ * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+ * The service container this instance should use.
+ *
+ * @return static
+ */
+ public static function create(ContainerInterface $container) {
+ $args = [];
+
+ if (method_exists(static::class, '__construct')) {
+ $constructor = new \ReflectionMethod(static::class, '__construct');
+ foreach ($constructor->getParameters() as $parameter) {
+ $service = (string) $parameter->getType();
+ foreach ($parameter->getAttributes(Autowire::class) as $attribute) {
+ $service = (string) $attribute->newInstance()->value;
+ }
+
+ 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));
+ }
+
+ $args[] = $container->get($service);
+ }
+ }
+
+ return new static(...$args);
+ }
+
+}
diff --git a/core/modules/node/src/Controller/NodeController.php b/core/modules/node/src/Controller/NodeController.php
index 25965374a72d..9cb1a50ad82c 100644
--- a/core/modules/node/src/Controller/NodeController.php
+++ b/core/modules/node/src/Controller/NodeController.php
@@ -13,7 +13,6 @@
use Drupal\node\NodeStorageInterface;
use Drupal\node\NodeTypeInterface;
use Drupal\node\NodeInterface;
-use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* Returns responses for Node routes.
@@ -57,17 +56,6 @@ public function __construct(DateFormatterInterface $date_formatter, RendererInte
$this->entityRepository = $entity_repository;
}
- /**
- * {@inheritdoc}
- */
- public static function create(ContainerInterface $container) {
- return new static(
- $container->get('date.formatter'),
- $container->get('renderer'),
- $container->get('entity.repository')
- );
- }
-
/**
* Displays add content links for available content types.
*
diff --git a/core/modules/system/tests/modules/system_test/src/Controller/BrokenSystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/BrokenSystemTestController.php
new file mode 100644
index 000000000000..8ebfa84c3813
--- /dev/null
+++ b/core/modules/system/tests/modules/system_test/src/Controller/BrokenSystemTestController.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace Drupal\system_test\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Lock\LockBackendInterface;
+
+/**
+ * A controller that does not specify its autowired dependencies correctly.
+ */
+class BrokenSystemTestController extends ControllerBase {
+
+ /**
+ * Constructs the BrokenSystemTestController.
+ *
+ * @param \Drupal\Core\Lock\LockBackendInterface $lock
+ * The lock service.
+ */
+ public function __construct(
+ protected LockBackendInterface $lock,
+ ) {}
+
+}
diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
index b806e582d6a4..f5cf04c5bdce 100644
--- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
+++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
@@ -11,11 +11,11 @@
use Drupal\Core\Render\Markup;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Url;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Drupal\Core\Lock\LockBackendInterface;
-use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* Controller routines for system_test routes.
@@ -71,7 +71,15 @@ class SystemTestController extends ControllerBase implements TrustedCallbackInte
* @param \Drupal\Core\Messenger\MessengerInterface $messenger
* The messenger service.
*/
- public function __construct(LockBackendInterface $lock, LockBackendInterface $persistent_lock, AccountInterface $current_user, RendererInterface $renderer, MessengerInterface $messenger) {
+ public function __construct(
+ #[Autowire(service: 'lock')]
+ LockBackendInterface $lock,
+ #[Autowire(service: 'lock.persistent')]
+ LockBackendInterface $persistent_lock,
+ AccountInterface $current_user,
+ RendererInterface $renderer,
+ MessengerInterface $messenger,
+ ) {
$this->lock = $lock;
$this->persistentLock = $persistent_lock;
$this->currentUser = $current_user;
@@ -79,19 +87,6 @@ public function __construct(LockBackendInterface $lock, LockBackendInterface $pe
$this->messenger = $messenger;
}
- /**
- * {@inheritdoc}
- */
- public static function create(ContainerInterface $container) {
- return new static(
- $container->get('lock'),
- $container->get('lock.persistent'),
- $container->get('current_user'),
- $container->get('renderer'),
- $container->get('messenger')
- );
- }
-
/**
* Tests main content fallback.
*
diff --git a/core/tests/Drupal/KernelTests/Core/Controller/ControllerBaseTest.php b/core/tests/Drupal/KernelTests/Core/Controller/ControllerBaseTest.php
new file mode 100644
index 000000000000..8a74fd0e357d
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Controller/ControllerBaseTest.php
@@ -0,0 +1,50 @@
+<?php
+
+namespace Drupal\KernelTests\Core\Controller;
+
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\system_test\Controller\BrokenSystemTestController;
+use Drupal\system_test\Controller\SystemTestController;
+use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
+
+/**
+ * Tests \Drupal\Core\Controller\ControllerBase.
+ *
+ * @coversDefaultClass \Drupal\Core\Controller\ControllerBase
+ * @group Controller
+ */
+class ControllerBaseTest extends KernelTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ protected static $modules = ['system_test', 'system'];
+
+ /**
+ * @covers ::create
+ */
+ public function testCreate() {
+ $controller = $this->container->get('class_resolver')->getInstanceFromDefinition(SystemTestController::class);
+
+ $property = new \ReflectionProperty(SystemTestController::class, 'lock');
+ $this->assertSame($this->container->get('lock'), $property->getValue($controller));
+
+ $property = new \ReflectionProperty(SystemTestController::class, 'persistentLock');
+ $this->assertSame($this->container->get('lock.persistent'), $property->getValue($controller));
+
+ $property = new \ReflectionProperty(SystemTestController::class, 'currentUser');
+ $this->assertSame($this->container->get('current_user'), $property->getValue($controller));
+ }
+
+ /**
+ * @covers ::create
+ */
+ public function testCreateException() {
+ $this->expectException(AutowiringFailedException::class);
+ $this->expectExceptionMessage('Cannot autowire service "Drupal\Core\Lock\LockBackendInterface": argument "$lock" of method "Drupal\system_test\Controller\BrokenSystemTestController::_construct()", you should configure its value explicitly.');
+ $this->container->get('class_resolver')->getInstanceFromDefinition(BrokenSystemTestController::class);
+ }
+
+}
--
GitLab