Problem/Motivation

In #3585079: Convert remaining system module routes to use attributes I mentioned we could move some routes and controllers out of system module now that they can be discovered by attributes. For example, EntityAutocompleteController could move to the Drupal\Core\Entity\Controller namespace.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3615109

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mstrelan created an issue. See original summary.

mstrelan’s picture

From @godotislate in the other issue:

This is a great idea. Discovery loops through the container namespaces, which include these in Drupal\Core:

   'Drupal\\Core\\Action' => 'core/lib/Drupal/Core/Action',
   'Drupal\\Core\\Block' => 'core/lib/Drupal/Core/Block',
   'Drupal\\Core\\Config' => 'core/lib/Drupal/Core/Config',
   'Drupal\\Core\\Datetime' => 'core/lib/Drupal/Core/Datetime',
   'Drupal\\Core\\DefaultContent' => 'core/lib/Drupal/Core/DefaultContent',
   'Drupal\\Core\\Entity' => 'core/lib/Drupal/Core/Entity',
   'Drupal\\Core\\Extension' => 'core/lib/Drupal/Core/Extension',
   'Drupal\\Core\\Field' => 'core/lib/Drupal/Core/Field',
   'Drupal\\Core\\Mail' => 'core/lib/Drupal/Core/Mail',
   'Drupal\\Core\\Menu' => 'core/lib/Drupal/Core/Menu',
   'Drupal\\Core\\Path' => 'core/lib/Drupal/Core/Path',
   'Drupal\\Core\\Plugin' => 'core/lib/Drupal/Core/Plugin',
   'Drupal\\Core\\Recipe' => 'core/lib/Drupal/Core/Recipe',
   'Drupal\\Core\\Render' => 'core/lib/Drupal/Core/Render',
   'Drupal\\Core\\TempStore' => 'core/lib/Drupal/Core/TempStore',
   'Drupal\\Core\\Theme' => 'core/lib/Drupal/Core/Theme',
   'Drupal\\Core\\TypedData' => 'core/lib/Drupal/Core/TypedData',
   'Drupal\\Core\\Validation' => 'core/lib/Drupal/Core/Validation',
   'Drupal\\Core\\Config\\Action' => 'core/lib/Drupal/Core/Config/Action',

I tested moving EntityAutocompleteController to Drupal\Core\Entity\Controller locally, and it seems to work correctly. I'm not sure if we should do that in this issue though, because maybe we'd want to move and update some corresponding tests as well. We can probably do a separate issue to evaluate what routes in system should be moved to Core and where.

mstrelan’s picture

I think this only works for EntityAutocompleteController and TimezoneController. For the others we need to add 'Controller' and 'Form' in \Drupal\Core\DrupalKernel::compileContainer, as below:

if (!$component->isDot() && $component->isDir() && (
  is_dir($pathname . '/Command') ||
  is_dir($pathname . '/Controller') ||
  is_dir($pathname . '/Plugin') ||
  is_dir($pathname . '/Entity') ||
  is_dir($pathname . '/Element') ||
  is_dir($pathname . '/Form')
)) {
  $namespaces[$parent_namespace . '\\' . $component->getFilename()] = $path . '/' . $component->getFilename();
}

That would allow the following, and possibly others:

  • BatchController
  • CronController
  • CsrfTokenController
  • Http4xxController
mstrelan’s picture

We probably want to deprecate the old route names too, system.entity_autocomplete won't make sense once it's out of system module. Maybe core.entity_autocomplete would be a good replacement. Apparently we can use \Symfony\Component\Routing\Attribute\DeprecatedAlias in Route attributes to do this.

mstrelan’s picture

godotislate made their first commit to this issue’s fork.

godotislate’s picture

Yes, changing the route names and adding aliases to the old names makes sense to me.

I addressed the PHPStan issue, basically a return statement was missing outside the if conditions, but since nothing should presumably get there, threw an access denied exception.

One FJ test has failed multiple times, and since this MR still needs work anyway, not going to try running it again for now.