diff --git a/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php b/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php index 1172428..0035cf3 100644 --- a/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php +++ b/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Routing; use Drupal\Core\Routing\RoutingEvents; +use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Routing\RouteCollection; @@ -17,8 +18,21 @@ abstract class RouteSubscriberBase implements EventSubscriberInterface { /** + * The event dispatcher. + * + * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface + */ + protected $eventDispatcher; + + /** * Alters existing routes for a specific collection. * + * In case a route subscriber adds additional routes in addition to (or + * instead of) altering existing ones, it should allow those routes to be + * altered by other modules as well by invoking an additional + * RoutingEvents::ALTER event. RouteSubscriberBase::invokeAdditionalAlter() is + * provided for that purpose. + * * @param \Symfony\Component\Routing\RouteCollection $collection * The route collection for adding routes. * @param string $provider @@ -29,10 +43,43 @@ protected function alterRoutes(RouteCollection $collection, $provider) { } /** + * Invokes an additional alter event for routes added during alter events. + * + * It is suggested to create a new route collection with the newly added + * routes, then invoke the RoutingEvents::ALTER event using this method and + * then add them to the existing route collection passed into + * RouteSubscriberBase::alterRoutes(). For example: + * @code + * public function alterRoutes(RouteCollection $collection, $provider) { + * // Create an additional route collection and add routes to it. + * $additional_collection = new RouteCollection(); + * $additional_collection->add(..., ...); + * ... + * // Invoke the RoutingEvents::ALTER event. + * $this->invokeAdditionalAlter($additional_collection); + * + * // Add the new routes to the existing route collection. + * $collection->addCollection($additional_collection); + * } + * @endcode + * The provider of the newly added routes is the class name of the route + * subscriber. To avoid infinite recursion, RouteSubscriberBase::alterRoutes() + * will not be called for the additional route collection. + * + * @param \Symfony\Component\Routing\RouteCollection $additional_collection + * The collection of additional routes to be altered. + */ + protected function invokeAdditionalAlter(RouteCollection $additional_collection) { + $new_event = new RouteBuildEvent($additional_collection, get_class($this)); + $this->eventDispatcher->dispatch(RoutingEvents::ALTER, $new_event); + } + + /** * {@inheritdoc} */ public static function getSubscribedEvents() { $events[RoutingEvents::ALTER] = 'onAlterRoutes'; + $events[RoutingEvents::FINISHED] = 'onFinishedRebuilding'; return $events; } @@ -43,8 +90,25 @@ public static function getSubscribedEvents() { * The route build event. */ public function onAlterRoutes(RouteBuildEvent $event) { + $this->eventDispatcher = $event->getDispatcher(); + $collection = $event->getRouteCollection(); - $this->alterRoutes($collection, $event->getProvider()); + $provider = $event->getProvider(); + // Avoid infinite recursion for additional routes that were provided by + // a route subscriber. + // @see \Drupal\Core\Routing\RouteSubscriberBase::invokeAdditionalAlter() + if ($provider != get_class($this)) { + $this->alterRoutes($collection, $provider); + } + } + + /** + * Provides an opportunity to run code when route rebuilding has finished. + * + * @param \Symfony\Component\EventDispatcher\Event $event + * The RoutingEvents::FINISHED event. + */ + public function onFinishedRebuilding(Event $event) { } } diff --git a/core/modules/config_translation/lib/Drupal/config_translation/ConfigMapperInterface.php b/core/modules/config_translation/lib/Drupal/config_translation/ConfigMapperInterface.php index b572708..4def427 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/ConfigMapperInterface.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/ConfigMapperInterface.php @@ -9,6 +9,7 @@ use Drupal\Core\Language\Language; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Route; /** * Defines an interface for configuration mapper. @@ -47,6 +48,16 @@ public function getBaseRouteParameters(); public function getBaseRoute(); /** + * Sets the base route object. + * + * @param \Symfony\Component\Routing\Route $route + * The route object used as base route. + * + * @see \Drupal\config_translation\Routing\RouteSubscriber::alterRoutes() + */ + public function setBaseRoute(Route $route); + + /** * Returns a processed path for the base route the mapper is attached to. * * @return string diff --git a/core/modules/config_translation/lib/Drupal/config_translation/ConfigNamesMapper.php b/core/modules/config_translation/lib/Drupal/config_translation/ConfigNamesMapper.php index 2df7387..1351e72 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/ConfigNamesMapper.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/ConfigNamesMapper.php @@ -91,14 +91,13 @@ class ConfigNamesMapper extends PluginBase implements ConfigMapperInterface, Con public function __construct($plugin_id, array $plugin_definition, ConfigFactory $config_factory, LocaleConfigManager $locale_config_manager, ConfigMapperManagerInterface $config_mapper_manager, RouteProviderInterface $route_provider, TranslationInterface $translation_manager) { $this->pluginId = $plugin_id; $this->pluginDefinition = $plugin_definition; + $this->routeProvider = $route_provider; $this->configFactory = $config_factory; $this->localeConfigManager = $locale_config_manager; $this->configMapperManager = $config_mapper_manager; $this->setTranslationManager($translation_manager); - - $this->baseRoute = $route_provider->getRouteByName($this->getBaseRouteName()); } /** @@ -145,12 +144,25 @@ public function getBaseRouteParameters() { * {@inheritdoc} */ public function getBaseRoute() { + // The base route cannot be set directly in the constructor because during + // route rebuilding we need to instantiate the mappers before the respective + // routes are available. + if (!isset($this->baseRoute)) { + $this->baseRoute = $this->routeProvider->getRouteByName($this->getBaseRouteName()); + } return $this->baseRoute; } /** * {@inheritdoc} */ + public function setBaseRoute(Route $route) { + $this->baseRoute = $route; + } + + /** + * {@inheritdoc} + */ public function getBasePath() { return $this->getPathFromRoute($this->getBaseRoute(), $this->getBaseRouteParameters()); } diff --git a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php index 335faa5..1782536 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php @@ -9,6 +9,8 @@ use Drupal\Core\Routing\RouteSubscriberBase; use Drupal\config_translation\ConfigMapperManagerInterface; +use Drupal\Core\Routing\RoutingEvents; +use Symfony\Component\EventDispatcher\Event; use Symfony\Component\Routing\RouteCollection; /** @@ -17,11 +19,29 @@ class RouteSubscriber extends RouteSubscriberBase { /** - * The mapper plugin discovery service. + * An array of all configuration mappers, keyed by ID. * - * @var \Drupal\config_translation\ConfigMapperManagerInterface + * @var \Drupal\config_translation\ConfigMapperInterface[] */ - protected $mapperManager; + protected $mappers; + + /** + * A mapping of configuration mappers to their base route name. + * + * @var array + */ + protected $baseRouteNames = array(); + + /** + * An array of base route names that need to processed. + * + * At the beginning of each route rebuilding process this is identical to + * RouteSubscriber::$baseRouteNames and after route rebuilding this should + * be empty. + * + * @var array + */ + protected $baseRouteNamesToProcess; /** * Constructs a new RouteSubscriber. @@ -30,26 +50,67 @@ class RouteSubscriber extends RouteSubscriberBase { * The mapper plugin discovery service. */ public function __construct(ConfigMapperManagerInterface $mapper_manager) { - $this->mapperManager = $mapper_manager; + $this->mappers = $mapper_manager->getMappers(); + + // The entire list of base route names is needed so we can check each route + // collection for matching routes in RouteSubscriber::alterRoutes(). In + // order to pass the route information to the configuration mapper we then + // need the respective mapper ID for the base route name. + foreach ($this->mappers as $mapper_id => $mapper) { + $this->baseRouteNames[$mapper_id] = $mapper->getBaseRouteName(); + } } /** * {@inheritdoc} */ protected function alterRoutes(RouteCollection $collection, $provider) { - // @todo \Drupal\config_translation\ConfigNamesMapper uses the route - // provider directly, which is unsafe during rebuild. This currently only - // works by coincidence; fix in https://drupal.org/node/2158571. - if ($provider != 'dynamic_routes') { - return; + // Reset RouteSubscriber::$baseRouteNamesToProcess at the beginning of the + // route rebuilding process. + if (!isset($this->baseRouteNamesToProcess)) { + $this->baseRouteNamesToProcess = $this->baseRouteNames; } - $mappers = $this->mapperManager->getMappers(); - foreach ($mappers as $mapper) { - $collection->add($mapper->getOverviewRouteName(), $mapper->getOverviewRoute()); - $collection->add($mapper->getAddRouteName(), $mapper->getAddRoute()); - $collection->add($mapper->getEditRouteName(), $mapper->getEditRoute()); - $collection->add($mapper->getDeleteRouteName(), $mapper->getDeleteRoute()); + // Create an additional route collection for the translation routes. + $additional_collection = new RouteCollection(); + + foreach ($this->baseRouteNamesToProcess as $mapper_id => $route_name) { + if ($route = $collection->get($route_name)) { + $mapper = $this->mappers[$mapper_id]; + $mapper->setBaseRoute($route); + + $additional_collection->add($mapper->getOverviewRouteName(), $mapper->getOverviewRoute()); + $additional_collection->add($mapper->getAddRouteName(), $mapper->getAddRoute()); + $additional_collection->add($mapper->getEditRouteName(), $mapper->getEditRoute()); + $additional_collection->add($mapper->getDeleteRouteName(), $mapper->getDeleteRoute()); + + // We do not need to check for the same base route name on the next + // invocation. + unset($this->baseRouteNamesToProcess[$mapper_id]); + } + } + + if ($additional_collection->count()) { + // Allow the new routes to be altered before adding them to the given + // collection. + $this->invokeAdditionalAlter($additional_collection); + $collection->addCollection($additional_collection); + } + } + + /** + * {@inheritdoc} + */ + public function onFinishedRebuilding(Event $event) { + $base_route_names_to_process = $this->baseRouteNamesToProcess; + // Unset RouteSubscriber::$baseRouteNamesToProcess so that if route + // rebuilding happens again in the same request, + // RouteSubscriber::alterRoutes() will re-initialize it. + unset($this->baseRouteNamesToProcess); + // If there are any routes left after route rebuilding has finished, + // something has gone wrong. + if (!empty($base_route_names_to_process)) { + throw new \RuntimeException(sprintf('The following routes are needed by configuration mappers but were not found: %s', implode(', ', $base_route_names_to_process))); } } diff --git a/core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiTest.php b/core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiTest.php index dfda109..255042e 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiTest.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/Tests/ConfigTranslationUiTest.php @@ -658,9 +658,8 @@ public function testAlterInfo() { public function testThemeDiscovery() { // Enable the test theme and rebuild routes. $theme = 'config_translation_test_theme'; - theme_enable(array($theme)); + \Drupal::service('theme_handler')->enable(array($theme)); \Drupal::service('router.builder')->rebuild(); - menu_router_rebuild(); $this->drupalLogin($this->admin_user); diff --git a/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigEntityMapperTest.php b/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigEntityMapperTest.php index b7ed1f5..46911d2 100644 --- a/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigEntityMapperTest.php +++ b/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigEntityMapperTest.php @@ -65,12 +65,6 @@ public function setUp() { $this->routeProvider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface'); - $this->routeProvider - ->expects($this->once()) - ->method('getRouteByName') - ->with('language.edit') - ->will($this->returnValue(new Route('/admin/config/regional/language/edit/{language_entity}'))); - $definition = array( 'class' => '\Drupal\config_translation\ConfigEntityMapper', 'base_route_name' => 'language.edit', diff --git a/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigNamesMapperTest.php b/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigNamesMapperTest.php index 3df6ac0..522d0fc 100644 --- a/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigNamesMapperTest.php +++ b/core/modules/config_translation/tests/Drupal/config_translation/Tests/ConfigNamesMapperTest.php @@ -96,12 +96,6 @@ public function setUp() { $this->baseRoute = new Route('/admin/config/system/site-information'); - $this->routeProvider - ->expects($this->once()) - ->method('getRouteByName') - ->with('system.site_information_settings') - ->will($this->returnValue($this->baseRoute)); - $this->configNamesMapper = new TestConfigNamesMapper( 'system.site_information_settings', $this->pluginDefinition, @@ -139,8 +133,26 @@ public function testGetBaseRouteParameters() { /** * Tests ConfigNamesMapper::getBaseRoute(). + * + * Also tests ConfigNamesMapper::setBaseRoute(). */ - public function testGetBaseRoute() { + public function testBaseRoute() { + // Test that the route can be set manually. + $this->configNamesMapper->setBaseRoute($this->baseRoute); + $result = $this->configNamesMapper->getBaseRoute(); + $this->assertSame($this->baseRoute, $result); + + // Test that the route is fetched from the route provider if it is not set. + $this->configNamesMapper->unsetBaseRoute(); + $this->routeProvider + ->expects($this->once()) + ->method('getRouteByName') + ->with('system.site_information_settings') + ->will($this->returnValue($this->baseRoute)); + $result = $this->configNamesMapper->getBaseRoute(); + $this->assertSame($this->baseRoute, $result); + + // Test that the base route is stored on the mapper. $result = $this->configNamesMapper->getBaseRoute(); $this->assertSame($this->baseRoute, $result); } @@ -149,6 +161,7 @@ public function testGetBaseRoute() { * Tests ConfigNamesMapper::getBasePath(). */ public function testGetBasePath() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $result = $this->configNamesMapper->getBasePath(); $this->assertSame('/admin/config/system/site-information', $result); } @@ -174,6 +187,7 @@ public function testGetOverviewRouteParameters() { * Tests ConfigNamesMapper::getOverviewRoute(). */ public function testGetOverviewRoute() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $expected = new Route('/admin/config/system/site-information/translate', array( '_controller' => '\Drupal\config_translation\Controller\ConfigTranslationController::itemPage', @@ -191,6 +205,7 @@ public function testGetOverviewRoute() { * Tests ConfigNamesMapper::getOverviewPath(). */ public function testGetOverviewPath() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $result = $this->configNamesMapper->getOverviewPath(); $this->assertSame('/admin/config/system/site-information/translate', $result); } @@ -221,6 +236,7 @@ public function testGetAddRouteParameters() { * Tests ConfigNamesMapper::getAddRoute(). */ public function testGetAddRoute() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $expected = new Route('/admin/config/system/site-information/translate/{langcode}/add', array( '_form' => '\Drupal\config_translation\Form\ConfigTranslationAddForm', @@ -260,6 +276,7 @@ public function testGetEditRouteParameters() { * Tests ConfigNamesMapper::getEditRoute(). */ public function testGetEditRoute() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $expected = new Route('/admin/config/system/site-information/translate/{langcode}/edit', array( '_form' => '\Drupal\config_translation\Form\ConfigTranslationEditForm', @@ -298,6 +315,7 @@ public function testGetDeleteRouteParameters() { * Tests ConfigNamesMapper::getRoute(). */ public function testGetDeleteRoute() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $expected = new Route('/admin/config/system/site-information/translate/{langcode}/delete', array( '_form' => '\Drupal\config_translation\Form\ConfigTranslationDeleteForm', @@ -604,6 +622,7 @@ public function testGetTypeName() { * Tests ConfigNamesMapper::hasTranslation(). */ public function testGetOperations() { + $this->configNamesMapper->setBaseRoute($this->baseRoute); $expected = array( 'translate' => array( 'title' => 'Translate', @@ -653,4 +672,11 @@ public function setConfigFactory(ConfigFactory $config_factory) { $this->configFactory = $config_factory; } + /** + * Unsets the base route that is stored in the mapper. + */ + public function unsetBaseRoute() { + unset($this->baseRoute); + } + } diff --git a/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php b/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php index b78a88f..a63f6ab 100644 --- a/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php +++ b/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php @@ -48,6 +48,9 @@ protected function alterRoutes(RouteCollection $collection, $provider) { } $path = $entity_route->getPath(); + // Create an additional route collection for the field UI routes. + $additional_collection = new RouteCollection(); + $route = new Route( "$path/fields/{field_instance}", array( @@ -56,21 +59,21 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ), array('_permission' => 'administer ' . $entity_type . ' fields') ); - $collection->add("field_ui.instance_edit_$entity_type", $route); + $additional_collection->add("field_ui.instance_edit_$entity_type", $route); $route = new Route( "$path/fields/{field_instance}/field", array('_form' => '\Drupal\field_ui\Form\FieldEditForm'), array('_permission' => 'administer ' . $entity_type . ' fields') ); - $collection->add("field_ui.field_edit_$entity_type", $route); + $additional_collection->add("field_ui.field_edit_$entity_type", $route); $route = new Route( "$path/fields/{field_instance}/delete", array('_entity_form' => 'field_instance.delete'), array('_permission' => 'administer ' . $entity_type . ' fields') ); - $collection->add("field_ui.delete_$entity_type", $route); + $additional_collection->add("field_ui.delete_$entity_type", $route); // If the entity type has no bundles, use the entity type. $defaults['entity_type'] = $entity_type; @@ -85,7 +88,7 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ) + $defaults, array('_permission' => 'administer ' . $entity_type . ' fields') ); - $collection->add("field_ui.overview_$entity_type", $route); + $additional_collection->add("field_ui.overview_$entity_type", $route); $route = new Route( "$path/form-display", @@ -95,7 +98,7 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ) + $defaults, array('_field_ui_form_mode_access' => 'administer ' . $entity_type . ' form display') ); - $collection->add("field_ui.form_display_overview_$entity_type", $route); + $additional_collection->add("field_ui.form_display_overview_$entity_type", $route); $route = new Route( "$path/form-display/{form_mode_name}", @@ -105,7 +108,7 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ) + $defaults, array('_field_ui_form_mode_access' => 'administer ' . $entity_type . ' form display') ); - $collection->add("field_ui.form_display_overview_form_mode_$entity_type", $route); + $additional_collection->add("field_ui.form_display_overview_form_mode_$entity_type", $route); $route = new Route( "$path/display", @@ -115,7 +118,7 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ) + $defaults, array('_field_ui_view_mode_access' => 'administer ' . $entity_type . ' display') ); - $collection->add("field_ui.display_overview_$entity_type", $route); + $additional_collection->add("field_ui.display_overview_$entity_type", $route); $route = new Route( "$path/display/{view_mode_name}", @@ -125,7 +128,12 @@ protected function alterRoutes(RouteCollection $collection, $provider) { ) + $defaults, array('_field_ui_view_mode_access' => 'administer ' . $entity_type . ' display') ); - $collection->add("field_ui.display_overview_view_mode_$entity_type", $route); + $additional_collection->add("field_ui.display_overview_view_mode_$entity_type", $route); + + // Allow the new routes to be altered before adding them to the given + // collection. + $this->invokeAdditionalAlter($additional_collection); + $collection->addCollection($additional_collection); } } } diff --git a/core/modules/system/system.local_tasks.yml b/core/modules/system/system.local_tasks.yml index 4d597ee..dedbf26 100644 --- a/core/modules/system/system.local_tasks.yml +++ b/core/modules/system/system.local_tasks.yml @@ -1,3 +1,8 @@ +system.performance_settings_tab: + route_name: system.performance_settings + title: Settings + base_route: system.performance_settings_tab + system.rss_feeds_settings_tab: route_name: system.rss_feeds_settings title: Settings