Issue #1915774 by tim.plunkett, amateescu, Crell, dawehner: Decide whether...

Issue #1915774 by tim.plunkett, amateescu, Crell, dawehner: Decide whether core route controllers should generally/always be DIC services or not.

core/lib/Drupal/Core/ControllerInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\ControllerInterface.
+ */
+
+namespace Drupal\Core;
+
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Defines a common interface for route controllers.
+ */
+interface ControllerInterface {
+
+  /**
+   * Instantiates a new instance of this controller.
+   *
+   * This is a factory method that returns a new instance of this object. The
+   * factory should pass any needed dependencies into the constructor of this
+   * object, but not the container itself. Every call to this method must return
+   * a new instance of this object; that is, it may not implement a singleton.
+   *
+   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+   *   The service container this object should use.
+   */
+  public static function create(ContainerInterface $container);
+}

core/lib/Drupal/Core/ControllerResolver.php

@@ -66,28 +66,35 @@ public function __construct(ContainerInterface $container, LoggerInterface $logg
    *   If the controller class does not exist
    */
   protected function createController($controller) {
-    // class::method
+    // Controller in the service:method notation.
+    $count = substr_count($controller, ':');
+    if ($count == 1) {
+      list($service, $method) = explode(':', $controller, 2);
+      return array($this->container->get($service), $method);
+    }
+
+    // Controller in the class::method notation.
     if (strpos($controller, '::') !== FALSE) {
       list($class, $method) = explode('::', $controller, 2);
-
       if (!class_exists($class)) {
         throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class));
       }
-
-      $controller = new $class();
-      if ($controller instanceof ContainerAwareInterface) {
-        $controller->setContainer($this->container);
+      if (in_array('Drupal\Core\ControllerInterface', class_implements($class))) {
+        $controller = $class::create($this->container);
       }
-      return array($controller, $method);
+      else {
+        $controller = new $class();
+      }
+    }
+    else {
+      throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
     }
 
-    // service:method
-    if (substr_count($controller, ':') == 1) {
-      // controller in the service:method notation
-      list($service, $method) = explode(':', $controller, 2);
-      return array($this->container->get($service), $method);
+    if ($controller instanceof ContainerAwareInterface) {
+      $controller->setContainer($this->container);
     }
 
-    throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
+    return array($controller, $method);
   }
+
 }

core/modules/user/lib/Drupal/user/UserAutocompleteController.php

@@ -8,11 +8,14 @@
 
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+use Drupal\Core\ControllerInterface;
 
 /**
  * Controller routines for taxonomy user routes.
  */
-class UserAutocompleteController {
+class UserAutocompleteController implements ControllerInterface {
 
   /**
    * The user autocomplete helper class to find matching user names.
@@ -31,6 +34,15 @@ public function __construct(UserAutocomplete $user_autocomplete) {
     $this->userAutocomplete = $user_autocomplete;
   }
 
+  /**
+   * Implements \Drupal\Core\ControllerInterface::create().
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('user.autocomplete')
+    );
+  }
+
   /**
    * Returns response for the user autocompletion.
    *

core/modules/user/lib/Drupal/user/UserBundle.php

@@ -27,8 +27,6 @@ public function build(ContainerBuilder $container) {
     $container
       ->register('user.data', 'Drupal\user\UserData')
       ->addArgument(new Reference('database'));
-    $container->register('user.autocomplete_controller', 'Drupal\user\UserAutocompleteController')
-      ->addArgument(new Reference('user.autocomplete'));
     $container->register('user.autocomplete', 'Drupal\user\UserAutocomplete')
       ->addArgument(new Reference('database'))
       ->addArgument(new Reference('config.factory'));

core/modules/user/user.routing.yml

@@ -8,13 +8,13 @@ user_register:
 user_autocomplete:
   pattern: '/user/autocomplete'
   defaults:
-    _controller: 'user.autocomplete_controller:autocompleteUser'
+    _controller: '\Drupal\user\UserAutocompleteController::autocompleteUser'
   requirements:
     _permission: 'access user profiles'
 
 user_autocomplete_anonymous:
   pattern: '/user/autocomplete/anonymous'
   defaults:
-    _controller: 'user.autocomplete_controller:autocompleteUserAnonymous'
+    _controller: '\Drupal\user\UserAutocompleteController::autocompleteUserAnonymous'
   requirements:
     _permission: 'access user profiles'

core/modules/views/lib/Drupal/views/Tests/UI/CachedDataUITest.php

@@ -31,7 +31,6 @@ public static function getInfo() {
    * Tests the user tempstore views data in the UI.
    */
   public function testCacheData() {
-    $controller = $this->container->get('views_ui.controller');
     $view = entity_load('view', 'test_view');
 
     $temp_store = $this->container->get('user.tempstore')->get('views');

core/modules/views/lib/Drupal/views/Tests/UI/TagTest.php

@@ -8,6 +8,7 @@
 namespace Drupal\views\Tests\UI;
 
 use Drupal\views\Tests\ViewUnitTestBase;
+use Drupal\views_ui\Routing\ViewsUIController;
 
 /**
  * Tests the views ui tagging functionality.
@@ -45,7 +46,7 @@ public function testViewsUiAutocompleteTag() {
     }
 
     // Make sure just ten results are returns.
-    $controller = $this->container->get('views_ui.controller');
+    $controller = ViewsUIController::create($this->container);
     $request = $this->container->get('request');
     $request->query->set('q', 'autocomplete_tag_test');
     $result = $controller->autocompleteTag($request);

core/modules/views/views_ui/lib/Drupal/views_ui/Form/BreakLockForm.php

@@ -7,6 +7,9 @@
 
 namespace Drupal\views_ui\Form;
 
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+use Drupal\Core\ControllerInterface;
 use Drupal\Core\Form\FormInterface;
 use Drupal\views\ViewStorageInterface;
 use Drupal\Core\Entity\EntityManager;
@@ -15,7 +18,7 @@
 /**
  * Builds the form to break the lock of an edited view.
  */
-class BreakLockForm implements FormInterface {
+class BreakLockForm implements FormInterface, ControllerInterface {
 
   /**
    * Stores the Entity manager.
@@ -44,6 +47,16 @@ public function __construct(EntityManager $entity_manager, TempStoreFactory $tem
     $this->tempStore = $temp_store_factory->get('views');
   }
 
+  /**
+   * Implements \Drupal\Core\ControllerInterface::create().
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('plugin.manager.entity'),
+      $container->get('user.tempstore')
+    );
+  }
+
   /**
    * Creates a new instance of this form.
    *

core/modules/views/views_ui/lib/Drupal/views_ui/Form/SettingsFormBase.php

@@ -7,13 +7,16 @@
 
 namespace Drupal\views_ui\Form;
 
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
 use Drupal\Core\Config\ConfigFactory;
+use Drupal\Core\ControllerInterface;
 use Drupal\system\SystemConfigFormBase;
 
 /**
  * Form builder for the advanced admin settings page.
  */
-abstract class SettingsFormBase extends SystemConfigFormBase {
+abstract class SettingsFormBase extends SystemConfigFormBase implements ControllerInterface {
 
   /**
    * Stores the views configuration.
@@ -32,6 +35,15 @@ public function __construct(ConfigFactory $config_factory) {
     $this->config = $config_factory->get('views.settings');
   }
 
+  /**
+   * Implements \Drupal\Core\ControllerInterface::create().
+   */
+  public static function create(ContainerInterface $container) {
+   return new static(
+      $container->get('config.factory')
+    );
+  }
+
   /**
    * Creates a new instance of this form.
    *

core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php

@@ -13,7 +13,9 @@
 use Drupal\views\ViewsDataCache;
 use Drupal\user\TempStore;
 use Drupal\user\TempStoreFactory;
+use Drupal\Core\ControllerInterface;
 use Drupal\Core\Entity\EntityManager;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -23,7 +25,7 @@
 /**
  * Returns responses for Views UI routes.
  */
-class ViewsUIController {
+class ViewsUIController implements ControllerInterface {
 
   /**
    * Stores the Entity manager.
@@ -62,6 +64,17 @@ public function __construct(EntityManager $entity_manager, ViewsDataCache $views
     $this->tempStore = $temp_store_factory->get('views');
   }
 
+  /**
+   * Implements \Drupal\Core\ControllerInterface::create().
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('plugin.manager.entity'),
+      $container->get('views.views_data'),
+      $container->get('user.tempstore')
+    );
+  }
+
   /**
    * Lists all of the views.
    *

core/modules/views/views_ui/lib/Drupal/views_ui/ViewsUiBundle.php

@@ -20,17 +20,6 @@ class ViewsUiBundle extends Bundle {
    * Overrides \Symfony\Component\HttpKernel\Bundle\Bundle::build().
    */
   public function build(ContainerBuilder $container) {
-    $container->register('views_ui.controller', 'Drupal\views_ui\Routing\ViewsUIController')
-      ->addArgument(new Reference('plugin.manager.entity'))
-      ->addArgument(new Reference('views.views_data'))
-      ->addArgument(new Reference('user.tempstore'));
-    $container->register('views_ui.form.basic_settings', 'Drupal\views_ui\Form\BasicSettingsForm')
-      ->addArgument(new Reference('config.factory'));
-    $container->register('views_ui.form.advanced_settings', 'Drupal\views_ui\Form\AdvancedSettingsForm')
-      ->addArgument(new Reference('config.factory'));
-    $container->register('views_ui.form.breakLock', 'Drupal\views_ui\Form\BreakLockForm')
-      ->addArgument(new Reference('plugin.manager.entity'))
-      ->addArgument(new Reference('user.tempstore'));
     $container->register('paramconverter.views_ui', 'Drupal\views_ui\ParamConverter\ViewUIConverter')
       ->addArgument(new Reference('user.tempstore'))
       ->addTag('paramconverter');

core/modules/views/views_ui/views_ui.routing.yml

 views_ui.list:
   pattern: '/admin/structure/views'
   defaults:
-    _controller: 'views_ui.controller:listing'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::listing'
   requirements:
     _permission: 'administer views'
 
 views_ui.add:
   pattern: '/admin/structure/views/add'
   defaults:
-    _controller: 'views_ui.controller:add'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::add'
   requirements:
     _permission: 'administer views'
 
 views_ui.settings.basic:
   pattern: '/admin/structure/views/settings'
   defaults:
-    _controller: 'views_ui.form.basic_settings:getForm'
+    _controller: '\Drupal\views_ui\Form\BasicSettingsForm::getForm'
   requirements:
     _permission: 'administer views'
 
 views_ui.settings.advanced:
   pattern: '/admin/structure/views/settings/advanced'
   defaults:
-    _controller: 'views_ui.form.advanced_settings:getForm'
+    _controller: '\Drupal\views_ui\Form\AdvancedSettingsForm::getForm'
   requirements:
     _permission: 'administer views'
 
 views_ui.reports.fields:
   pattern: '/admin/reports/fields/views-fields'
   defaults:
-    _controller: 'views_ui.controller:reportFields'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::reportFields'
   requirements:
     _permission: 'administer views'
 
 views_ui.reports.plugins:
   pattern: '/admin/reports/views-plugins'
   defaults:
-    _controller: 'views_ui.controller:reportPlugins'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::reportPlugins'
   requirements:
     _permission: 'administer views'
 
 views_ui.operation:
   pattern: '/admin/structure/views/view/{view}/{op}'
   defaults:
-    _controller: 'views_ui.controller:ajaxOperation'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::ajaxOperation'
   requirements:
     _permission: 'administer views'
     op: 'enable|disable'
@@ -51,7 +51,7 @@ views_ui.operation:
 views_ui.clone:
   pattern: '/admin/structure/views/view/{view}/clone'
   defaults:
-    _controller: 'views_ui.controller:cloneForm'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::cloneForm'
   requirements:
     _permission: 'administer views'
 
@@ -65,7 +65,7 @@ views_ui.delete:
 views_ui.autocomplete:
   pattern: '/admin/views/ajax/autocomplete/tag'
   defaults:
-    _controller: 'views_ui.controller:autocompleteTag'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::autocompleteTag'
   requirements:
     _permission: 'administer views'
 
@@ -75,7 +75,7 @@ views_ui.edit:
     tempstore:
       view: 'views'
   defaults:
-    _controller: 'views_ui.controller:edit'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::edit'
   requirements:
     _permission: 'administer views'
 
@@ -85,7 +85,7 @@ views_ui.edit.display:
     tempstore:
       view: 'views'
   defaults:
-    _controller: 'views_ui.controller:edit'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::edit'
     display_id: NULL
   requirements:
     _permission: 'administer views'
@@ -96,7 +96,7 @@ views_ui.preview:
     tempstore:
       view: 'views'
   defaults:
-    _controller: 'views_ui.controller:preview'
+    _controller: '\Drupal\views_ui\Routing\ViewsUIController::preview'
     display_id: NULL
   requirements:
     _permission: 'administer views'
@@ -104,7 +104,7 @@ views_ui.preview:
 views_ui.breakLock:
   pattern: '/admin/structure/views/view/{view}/break-lock'
   defaults:
-    _controller: 'views_ui.form.breakLock:getForm'
+    _controller: '\Drupal\views_ui\Form\BreakLockForm::getForm'
     display_id: NULL
   requirements:
     _permission: 'administer views'