From 726478e12e1a44ec54d5118ae4b360bd85df0739 Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Fri, 12 Apr 2024 19:45:09 +0100 Subject: [PATCH] Issue #3440000 by amateescu, smustgrave: Fix Workspaces' hidden dependency on the path_alias module --- .../WorkspaceRequestSubscriber.php | 75 ++----------------- .../workspaces/src/WorkspaceManager.php | 4 +- .../workspaces/src/WorkspacesAliasManager.php | 59 +++++++++++++++ .../src/WorkspacesServiceProvider.php | 9 +++ .../Functional/WorkspaceEntityDeleteTest.php | 2 +- ...rtedNewEntitiesConstraintValidatorTest.php | 1 - ...rkspaceConflictConstraintValidatorTest.php | 1 - .../tests/src/Kernel/WorkspaceAccessTest.php | 1 - .../WorkspaceAssociationDeprecationTest.php | 2 +- .../src/Kernel/WorkspaceAssociationTest.php | 1 - .../tests/src/Kernel/WorkspaceCRUDTest.php | 2 - .../WorkspaceContentTranslationTest.php | 1 - .../src/Kernel/WorkspaceEntityDeleteTest.php | 10 ++- .../src/Kernel/WorkspaceInformationTest.php | 1 - .../tests/src/Kernel/WorkspaceMergerTest.php | 1 - .../Kernel/WorkspaceViewsIntegrationTest.php | 1 - .../src/Kernel/WorkspacesFileItemTest.php | 1 - .../Unit/WorkspaceRequestSubscriberTest.php | 20 +---- .../workspaces/workspaces.services.yml | 2 +- 19 files changed, 92 insertions(+), 102 deletions(-) create mode 100644 core/modules/workspaces/src/WorkspacesAliasManager.php diff --git a/core/modules/workspaces/src/EventSubscriber/WorkspaceRequestSubscriber.php b/core/modules/workspaces/src/EventSubscriber/WorkspaceRequestSubscriber.php index 7d9592fb08b6..c1950c53523b 100644 --- a/core/modules/workspaces/src/EventSubscriber/WorkspaceRequestSubscriber.php +++ b/core/modules/workspaces/src/EventSubscriber/WorkspaceRequestSubscriber.php @@ -2,83 +2,24 @@ namespace Drupal\workspaces\EventSubscriber; -use Drupal\path_alias\AliasManagerInterface; -use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Routing\CacheableRouteProviderInterface; use Drupal\Core\Routing\RouteProviderInterface; use Drupal\workspaces\WorkspaceManagerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Symfony\Component\HttpKernel\Event\ControllerEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\KernelEvents; /** * Provides a event subscriber for setting workspace-specific cache keys. + * + * @internal */ class WorkspaceRequestSubscriber implements EventSubscriberInterface { - /** - * The alias manager that caches alias lookups based on the request. - * - * @var \Drupal\path_alias\AliasManagerInterface - */ - protected $aliasManager; - - /** - * The current path. - * - * @var \Drupal\Core\Path\CurrentPathStack - */ - protected $currentPath; - - /** - * The route provider to load routes by name. - * - * @var \Drupal\Core\Routing\RouteProviderInterface - */ - protected $routeProvider; - - /** - * The workspace manager. - * - * @var \Drupal\workspaces\WorkspaceManagerInterface - */ - protected $workspaceManager; - - /** - * Constructs a new WorkspaceRequestSubscriber instance. - * - * @param \Drupal\path_alias\AliasManagerInterface $alias_manager - * The alias manager. - * @param \Drupal\Core\Path\CurrentPathStack $current_path - * The current path. - * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider - * The route provider. - * @param \Drupal\workspaces\WorkspaceManagerInterface $workspace_manager - * The workspace manager. - */ - public function __construct(AliasManagerInterface $alias_manager, CurrentPathStack $current_path, RouteProviderInterface $route_provider, WorkspaceManagerInterface $workspace_manager) { - $this->aliasManager = $alias_manager; - $this->currentPath = $current_path; - $this->routeProvider = $route_provider; - $this->workspaceManager = $workspace_manager; - } - - /** - * Sets the cache key on the alias manager cache decorator. - * - * KernelEvents::CONTROLLER is used in order to be executed after routing. - * - * @param \Symfony\Component\HttpKernel\Event\ControllerEvent $event - * The Event to process. - */ - public function onKernelController(ControllerEvent $event) { - // Set the cache key on the alias manager cache decorator. - if ($event->isMainRequest() && $this->workspaceManager->hasActiveWorkspace()) { - $cache_key = $this->workspaceManager->getActiveWorkspace()->id() . ':' . rtrim($this->currentPath->getPath($event->getRequest()), '/'); - $this->aliasManager->setCacheKey($cache_key); - } - } + public function __construct( + protected readonly RouteProviderInterface $routeProvider, + protected readonly WorkspaceManagerInterface $workspaceManager, + ) {} /** * Adds the active workspace as a cache key part to the route provider. @@ -96,10 +37,6 @@ public function onKernelRequest(RequestEvent $event) { * {@inheritdoc} */ public static function getSubscribedEvents(): array { - // Use a priority of 190 in order to run after the generic core subscriber. - // @see \Drupal\Core\EventSubscriber\PathSubscriber::getSubscribedEvents() - $events[KernelEvents::CONTROLLER][] = ['onKernelController', 190]; - // Use a priority of 33 in order to run before Symfony's router listener. // @see \Symfony\Component\HttpKernel\EventListener\RouterListener::getSubscribedEvents() $events[KernelEvents::REQUEST][] = ['onKernelRequest', 33]; diff --git a/core/modules/workspaces/src/WorkspaceManager.php b/core/modules/workspaces/src/WorkspaceManager.php index bba37e537094..b94eec48c05e 100644 --- a/core/modules/workspaces/src/WorkspaceManager.php +++ b/core/modules/workspaces/src/WorkspaceManager.php @@ -259,7 +259,9 @@ protected function doSwitchWorkspace($workspace) { // Clear the static cache for path aliases. We can't inject the path alias // manager service because it would create a circular dependency. - \Drupal::service('path_alias.manager')->cacheClear(); + if (\Drupal::hasService('path_alias.manager')) { + \Drupal::service('path_alias.manager')->cacheClear(); + } } /** diff --git a/core/modules/workspaces/src/WorkspacesAliasManager.php b/core/modules/workspaces/src/WorkspacesAliasManager.php new file mode 100644 index 000000000000..b7584db16162 --- /dev/null +++ b/core/modules/workspaces/src/WorkspacesAliasManager.php @@ -0,0 +1,59 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\workspaces; + +use Drupal\path_alias\AliasManagerInterface; + +/** + * Decorates the path_alias.manager service for workspace-specific caching. + * + * @internal + */ +class WorkspacesAliasManager implements AliasManagerInterface { + + public function __construct( + protected readonly AliasManagerInterface $inner, + protected readonly WorkspaceManagerInterface $workspaceManager, + ) {} + + /** + * {@inheritdoc} + */ + public function setCacheKey($key): void { + if ($this->workspaceManager->hasActiveWorkspace()) { + $key = $this->workspaceManager->getActiveWorkspace()->id() . ':' . $key; + } + $this->inner->setCacheKey($key); + } + + /** + * {@inheritdoc} + */ + public function writeCache(): void { + $this->inner->writeCache(); + } + + /** + * {@inheritdoc} + */ + public function getPathByAlias($alias, $langcode = NULL): string { + return $this->inner->getPathByAlias($alias, $langcode); + } + + /** + * {@inheritdoc} + */ + public function getAliasByPath($path, $langcode = NULL): string { + return $this->inner->getAliasByPath($path, $langcode); + } + + /** + * {@inheritdoc} + */ + public function cacheClear($source = NULL): void { + $this->inner->cacheClear($source); + } + +} diff --git a/core/modules/workspaces/src/WorkspacesServiceProvider.php b/core/modules/workspaces/src/WorkspacesServiceProvider.php index e9c869d977c7..424325bb1b80 100644 --- a/core/modules/workspaces/src/WorkspacesServiceProvider.php +++ b/core/modules/workspaces/src/WorkspacesServiceProvider.php @@ -21,6 +21,15 @@ public function alter(ContainerBuilder $container) { $renderer_config['required_cache_contexts'][] = 'workspace'; $container->setParameter('renderer.config', $renderer_config); + // Decorate the 'path_alias.manager' service. + if ($container->hasDefinition('path_alias.manager')) { + $container->register('workspaces.path_alias.manager', WorkspacesAliasManager::class) + ->setPublic(FALSE) + ->setDecoratedService('path_alias.manager', NULL, 50) + ->addArgument(new Reference('workspaces.path_alias.manager.inner')) + ->addArgument(new Reference('workspaces.manager')); + } + // Replace the class of the 'path_alias.repository' service. if ($container->hasDefinition('path_alias.repository')) { $definition = $container->getDefinition('path_alias.repository'); diff --git a/core/modules/workspaces/tests/src/Functional/WorkspaceEntityDeleteTest.php b/core/modules/workspaces/tests/src/Functional/WorkspaceEntityDeleteTest.php index 017a09e85c3b..7a3e96876428 100644 --- a/core/modules/workspaces/tests/src/Functional/WorkspaceEntityDeleteTest.php +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceEntityDeleteTest.php @@ -19,7 +19,7 @@ class WorkspaceEntityDeleteTest extends BrowserTestBase { /** * {@inheritdoc} */ - protected static $modules = ['block', 'node', 'path_alias', 'user', 'workspaces']; + protected static $modules = ['block', 'node', 'user', 'workspaces']; /** * {@inheritdoc} diff --git a/core/modules/workspaces/tests/src/Kernel/EntityReferenceSupportedNewEntitiesConstraintValidatorTest.php b/core/modules/workspaces/tests/src/Kernel/EntityReferenceSupportedNewEntitiesConstraintValidatorTest.php index 89d7d029b9bb..9c805d3ce9f2 100644 --- a/core/modules/workspaces/tests/src/Kernel/EntityReferenceSupportedNewEntitiesConstraintValidatorTest.php +++ b/core/modules/workspaces/tests/src/Kernel/EntityReferenceSupportedNewEntitiesConstraintValidatorTest.php @@ -28,7 +28,6 @@ class EntityReferenceSupportedNewEntitiesConstraintValidatorTest extends KernelT 'user', 'workspaces', 'entity_test', - 'path_alias', ]; /** diff --git a/core/modules/workspaces/tests/src/Kernel/EntityWorkspaceConflictConstraintValidatorTest.php b/core/modules/workspaces/tests/src/Kernel/EntityWorkspaceConflictConstraintValidatorTest.php index 7a7154c42b6b..cb64fba663b5 100644 --- a/core/modules/workspaces/tests/src/Kernel/EntityWorkspaceConflictConstraintValidatorTest.php +++ b/core/modules/workspaces/tests/src/Kernel/EntityWorkspaceConflictConstraintValidatorTest.php @@ -25,7 +25,6 @@ class EntityWorkspaceConflictConstraintValidatorTest extends KernelTestBase { */ protected static $modules = [ 'entity_test', - 'path_alias', 'system', 'user', 'workspaces', diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceAccessTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceAccessTest.php index cb9c3e2f8fec..ac3051b718bf 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceAccessTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceAccessTest.php @@ -27,7 +27,6 @@ class WorkspaceAccessTest extends KernelTestBase { 'system', 'workspaces', 'workspace_access_test', - 'path_alias', ]; /** diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationDeprecationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationDeprecationTest.php index a2f7209f9a1a..d72dbf114fce 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationDeprecationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationDeprecationTest.php @@ -16,7 +16,7 @@ class WorkspaceAssociationDeprecationTest extends KernelTestBase { /** * {@inheritdoc} */ - protected static $modules = ['path_alias', 'user', 'workspaces']; + protected static $modules = ['user', 'workspaces']; /** * {@inheritdoc} diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php index 7744a8c76ea2..a790d3b1984f 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php @@ -41,7 +41,6 @@ class WorkspaceAssociationTest extends KernelTestBase { 'text', 'user', 'system', - 'path_alias', 'workspaces', ]; diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceCRUDTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceCRUDTest.php index 619fc553b3bd..c8c9a5ba4962 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceCRUDTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceCRUDTest.php @@ -55,7 +55,6 @@ class WorkspaceCRUDTest extends KernelTestBase { 'filter', 'node', 'text', - 'path_alias', ]; /** @@ -71,7 +70,6 @@ protected function setUp(): void { $this->installEntitySchema('workspace'); $this->installSchema('workspaces', ['workspace_association']); $this->installEntitySchema('node'); - $this->installEntitySchema('path_alias'); $this->installConfig(['filter', 'node', 'system']); diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceContentTranslationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceContentTranslationTest.php index 414876be5311..eabe058b9160 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceContentTranslationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceContentTranslationTest.php @@ -33,7 +33,6 @@ class WorkspaceContentTranslationTest extends KernelTestBase { 'content_translation', 'entity_test', 'language', - 'path_alias', 'user', 'workspaces', ]; diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceEntityDeleteTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceEntityDeleteTest.php index da66a908654b..06bfec3db256 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceEntityDeleteTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceEntityDeleteTest.php @@ -25,7 +25,15 @@ class WorkspaceEntityDeleteTest extends KernelTestBase { /** * {@inheritdoc} */ - protected static $modules = ['field', 'filter', 'node', 'path_alias', 'user', 'text', 'system', 'workspaces']; + protected static $modules = [ + 'field', + 'filter', + 'node', + 'system', + 'text', + 'user', + 'workspaces', + ]; /** * The entity type manager. diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceInformationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceInformationTest.php index 5d79fd16fea6..602d3b339f63 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceInformationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceInformationTest.php @@ -49,7 +49,6 @@ class WorkspaceInformationTest extends KernelTestBase { */ protected static $modules = [ 'entity_test', - 'path_alias', 'user', 'workspaces', 'workspaces_test', diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php index e2aaf5fd606a..e46e1fab9c33 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceMergerTest.php @@ -47,7 +47,6 @@ class WorkspaceMergerTest extends KernelTestBase { 'text', 'user', 'system', - 'path_alias', ]; /** diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceViewsIntegrationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceViewsIntegrationTest.php index 4d3587b854b2..7ecc77559891 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceViewsIntegrationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceViewsIntegrationTest.php @@ -43,7 +43,6 @@ class WorkspaceViewsIntegrationTest extends ViewsKernelTestBase { 'node', 'language', 'text', - 'path_alias', 'views_ui', 'workspaces', ]; diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspacesFileItemTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspacesFileItemTest.php index eee017f9bd65..472660e2010f 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspacesFileItemTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspacesFileItemTest.php @@ -29,7 +29,6 @@ class WorkspacesFileItemTest extends FileItemTest { */ protected static $modules = [ 'file', - 'path_alias', 'workspaces', 'workspaces_test', ]; diff --git a/core/modules/workspaces/tests/src/Unit/WorkspaceRequestSubscriberTest.php b/core/modules/workspaces/tests/src/Unit/WorkspaceRequestSubscriberTest.php index 2e40fff58c48..df888ece36d8 100644 --- a/core/modules/workspaces/tests/src/Unit/WorkspaceRequestSubscriberTest.php +++ b/core/modules/workspaces/tests/src/Unit/WorkspaceRequestSubscriberTest.php @@ -4,8 +4,6 @@ namespace Drupal\Tests\workspaces\Unit; -use Drupal\path_alias\AliasManagerInterface; -use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Routing\CacheableRouteProviderInterface; use Drupal\Core\Routing\RouteProviderInterface; use Drupal\Tests\UnitTestCase; @@ -17,20 +15,10 @@ /** * @coversDefaultClass \Drupal\workspaces\EventSubscriber\WorkspaceRequestSubscriber * - * @group workspace + * @group workspaces */ class WorkspaceRequestSubscriberTest extends UnitTestCase { - /** - * @var \Drupal\path_alias\AliasManagerInterface - */ - protected $aliasManager; - - /** - * @var \Drupal\Core\Path\CurrentPathStack - */ - protected $currentPath; - /** * @var \Drupal\workspaces\WorkspaceManagerInterface */ @@ -42,8 +30,6 @@ class WorkspaceRequestSubscriberTest extends UnitTestCase { protected function setUp(): void { parent::setUp(); - $this->aliasManager = $this->prophesize(AliasManagerInterface::class)->reveal(); - $this->currentPath = $this->prophesize(CurrentPathStack::class)->reveal(); $this->workspaceManager = $this->prophesize(WorkspaceManagerInterface::class); $active_workspace = $this->prophesize(WorkspaceInterface::class); @@ -62,7 +48,7 @@ public function testOnKernelRequestWithCacheableRouteProvider() { // Check that WorkspaceRequestSubscriber::onKernelRequest() calls // addExtraCacheKeyPart() on a route provider that implements // CacheableRouteProviderInterface. - $workspace_request_subscriber = new WorkspaceRequestSubscriber($this->aliasManager, $this->currentPath, $route_provider->reveal(), $this->workspaceManager->reveal()); + $workspace_request_subscriber = new WorkspaceRequestSubscriber($route_provider->reveal(), $this->workspaceManager->reveal()); $event = $this->prophesize(RequestEvent::class)->reveal(); $this->assertNull($workspace_request_subscriber->onKernelRequest($event)); } @@ -76,7 +62,7 @@ public function testOnKernelRequestWithoutCacheableRouteProvider() { // Check that WorkspaceRequestSubscriber::onKernelRequest() doesn't call // addExtraCacheKeyPart() on a route provider that does not implement // CacheableRouteProviderInterface. - $workspace_request_subscriber = new WorkspaceRequestSubscriber($this->aliasManager, $this->currentPath, $route_provider->reveal(), $this->workspaceManager->reveal()); + $workspace_request_subscriber = new WorkspaceRequestSubscriber($route_provider->reveal(), $this->workspaceManager->reveal()); $event = $this->prophesize(RequestEvent::class)->reveal(); $this->assertNull($workspace_request_subscriber->onKernelRequest($event)); } diff --git a/core/modules/workspaces/workspaces.services.yml b/core/modules/workspaces/workspaces.services.yml index 2c9ca6e0445b..1c07e67988d7 100644 --- a/core/modules/workspaces/workspaces.services.yml +++ b/core/modules/workspaces/workspaces.services.yml @@ -48,7 +48,7 @@ services: arguments: ['@entity.definition_update_manager', '@entity.last_installed_schema.repository', '@workspaces.information'] workspaces.workspace_subscriber: class: Drupal\workspaces\EventSubscriber\WorkspaceRequestSubscriber - arguments: ['@path_alias.manager', '@path.current', '@router.route_provider', '@workspaces.manager'] + arguments: ['@router.route_provider', '@workspaces.manager'] cache_context.workspace: class: Drupal\workspaces\WorkspaceCacheContext -- GitLab