Issue #3416353 by longwave, Spokje: Convert CheckProvider to use a service locator

merge-requests/5425/merge
catch 2024-02-12 11:13:30 +00:00
parent 8bc5040e28
commit e85a9e85f3
5 changed files with 45 additions and 40 deletions

View File

@ -1308,8 +1308,7 @@ services:
Drupal\Core\Access\AccessManagerInterface: '@access_manager' Drupal\Core\Access\AccessManagerInterface: '@access_manager'
access_manager.check_provider: access_manager.check_provider:
class: Drupal\Core\Access\CheckProvider class: Drupal\Core\Access\CheckProvider
calls: arguments: ['%dynamic_access_check_services%']
- [setContainer, ['@service_container']]
public: false public: false
Drupal\Core\Access\CheckProviderInterface: '@access_manager.check_provider' Drupal\Core\Access\CheckProviderInterface: '@access_manager.check_provider'
access_check.default: access_check.default:

View File

@ -3,17 +3,14 @@
namespace Drupal\Core\Access; namespace Drupal\Core\Access;
use Drupal\Core\Routing\Access\AccessInterface; use Drupal\Core\Routing\Access\AccessInterface;
use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\Routing\Route; use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection; use Symfony\Component\Routing\RouteCollection;
/** /**
* Loads access checkers from the container. * Loads access checkers from the container.
*/ */
class CheckProvider implements CheckProviderInterface, ContainerAwareInterface { class CheckProvider implements CheckProviderInterface {
use ContainerAwareTrait;
/** /**
* Array of registered access check service ids. * Array of registered access check service ids.
@ -55,6 +52,29 @@ class CheckProvider implements CheckProviderInterface, ContainerAwareInterface {
*/ */
protected $dynamicRequirementMap; protected $dynamicRequirementMap;
/**
* Constructs a CheckProvider object.
*
* @param array|null $dynamic_requirements_map
* An array to map dynamic requirement keys to service IDs.
* @param \Psr\Container\ContainerInterface|null $container
* The check provider service locator.
*/
public function __construct(
array $dynamic_requirements_map = NULL,
protected ?ContainerInterface $container = NULL,
) {
$this->dynamicRequirementMap = $dynamic_requirements_map;
if (is_null($this->dynamicRequirementMap)) {
@trigger_error('Calling ' . __METHOD__ . ' without the $dynamic_requirements_map argument is deprecated in drupal:10.3.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3416353', E_USER_DEPRECATED);
$this->dynamicRequirementMap = \Drupal::getContainer()->getParameter('dynamic_access_check_services');
}
if (!$this->container) {
@trigger_error('Calling ' . __METHOD__ . ' without the $container argument is deprecated in drupal:10.3.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3416353', E_USER_DEPRECATED);
$this->container = \Drupal::getContainer();
}
}
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
@ -80,7 +100,6 @@ class CheckProvider implements CheckProviderInterface, ContainerAwareInterface {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function setChecks(RouteCollection $routes) { public function setChecks(RouteCollection $routes) {
$this->loadDynamicRequirementMap();
foreach ($routes as $route) { foreach ($routes as $route) {
if ($checks = $this->applies($route)) { if ($checks = $this->applies($route)) {
$route->setOption('_access_checks', $checks); $route->setOption('_access_checks', $checks);

View File

@ -3,8 +3,10 @@
namespace Drupal\Core\DependencyInjection\Compiler; namespace Drupal\Core\DependencyInjection\Compiler;
use Drupal\Core\Access\AccessCheckInterface; use Drupal\Core\Access\AccessCheckInterface;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;
/** /**
* Adds services tagged 'access_check' to the access_manager service. * Adds services tagged 'access_check' to the access_manager service.
@ -18,6 +20,7 @@ class RegisterAccessChecksPass implements CompilerPassInterface {
if (!$container->hasDefinition('access_manager')) { if (!$container->hasDefinition('access_manager')) {
return; return;
} }
$services = [];
$dynamic_access_check_services = []; $dynamic_access_check_services = [];
// Add services tagged 'access_check' to the access_manager service. // Add services tagged 'access_check' to the access_manager service.
$access_manager = $container->getDefinition('access_manager.check_provider'); $access_manager = $container->getDefinition('access_manager.check_provider');
@ -43,7 +46,10 @@ class RegisterAccessChecksPass implements CompilerPassInterface {
if (in_array(AccessCheckInterface::class, class_implements($class), TRUE)) { if (in_array(AccessCheckInterface::class, class_implements($class), TRUE)) {
$dynamic_access_check_services[] = $id; $dynamic_access_check_services[] = $id;
} }
$services[$id] = new Reference($id);
} }
$access_manager->addArgument(ServiceLocatorTagPass::register($container, $services));
$container->setParameter('dynamic_access_check_services', $dynamic_access_check_services); $container->setParameter('dynamic_access_check_services', $dynamic_access_check_services);
} }

View File

@ -192,22 +192,6 @@ parameters:
count: 1 count: 1
path: lib/Drupal/Core/Access/AccessResult.php path: lib/Drupal/Core/Access/AccessResult.php
-
message: """
#^Class Drupal\\\\Core\\\\Access\\\\CheckProvider implements deprecated interface Symfony\\\\Component\\\\DependencyInjection\\\\ContainerAwareInterface\\:
since Symfony 6\\.4, use dependency injection instead$#
"""
count: 1
path: lib/Drupal/Core/Access/CheckProvider.php
-
message: """
#^Usage of deprecated trait Symfony\\\\Component\\\\DependencyInjection\\\\ContainerAwareTrait in class Drupal\\\\Core\\\\Access\\\\CheckProvider\\:
since Symfony 6\\.4, use dependency injection instead$#
"""
count: 1
path: lib/Drupal/Core/Access/CheckProvider.php
- -
message: "#^Method Drupal\\\\Core\\\\Access\\\\CsrfRequestHeaderAccessCheck\\:\\:applies\\(\\) should return bool but return statement is missing\\.$#" message: "#^Method Drupal\\\\Core\\\\Access\\\\CsrfRequestHeaderAccessCheck\\:\\:applies\\(\\) should return bool but return statement is missing\\.$#"
count: 1 count: 1

View File

@ -124,8 +124,7 @@ class AccessManagerTest extends UnitTestCase {
$this->account = $this->createMock('Drupal\Core\Session\AccountInterface'); $this->account = $this->createMock('Drupal\Core\Session\AccountInterface');
$this->currentUser = $this->createMock('Drupal\Core\Session\AccountInterface'); $this->currentUser = $this->createMock('Drupal\Core\Session\AccountInterface');
$this->argumentsResolverFactory = $this->createMock('Drupal\Core\Access\AccessArgumentsResolverFactoryInterface'); $this->argumentsResolverFactory = $this->createMock('Drupal\Core\Access\AccessArgumentsResolverFactoryInterface');
$this->checkProvider = new CheckProvider(); $this->checkProvider = new CheckProvider([], $this->container);
$this->checkProvider->setContainer($this->container);
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider); $this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
} }
@ -154,15 +153,15 @@ class AccessManagerTest extends UnitTestCase {
* Tests setChecks with a dynamic access checker. * Tests setChecks with a dynamic access checker.
*/ */
public function testSetChecksWithDynamicAccessChecker() { public function testSetChecksWithDynamicAccessChecker() {
// Setup the access manager.
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
// Setup the dynamic access checker. // Setup the dynamic access checker.
$access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface');
$this->container->set('test_access', $access_check); $this->container->set('test_access', $access_check);
$this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider = new CheckProvider(['test_access'], $this->container);
$this->checkProvider->addCheckService('test_access', 'access'); $this->checkProvider->addCheckService('test_access', 'access');
// Setup the access manager.
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$route = new Route('/test-path', [], ['_foo' => '1', '_bar' => '1']); $route = new Route('/test-path', [], ['_foo' => '1', '_bar' => '1']);
$route2 = new Route('/test-path', [], ['_foo' => '1', '_bar' => '2']); $route2 = new Route('/test-path', [], ['_foo' => '1', '_bar' => '2']);
$collection = new RouteCollection(); $collection = new RouteCollection();
@ -375,8 +374,6 @@ class AccessManagerTest extends UnitTestCase {
return $route_match->getParameters()->get('value') == 'upcasted_value'; return $route_match->getParameters()->get('value') == 'upcasted_value';
})); }));
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface');
$access_check->expects($this->atLeastOnce()) $access_check->expects($this->atLeastOnce())
->method('applies') ->method('applies')
@ -386,11 +383,12 @@ class AccessManagerTest extends UnitTestCase {
->willReturn(AccessResult::forbidden()); ->willReturn(AccessResult::forbidden());
$this->container->set('test_access', $access_check); $this->container->set('test_access', $access_check);
$this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider = new CheckProvider(['test_access'], $this->container);
$this->checkProvider->addCheckService('test_access', 'access'); $this->checkProvider->addCheckService('test_access', 'access');
$this->checkProvider->setChecks($this->routeCollection); $this->checkProvider->setChecks($this->routeCollection);
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_1', ['value' => 'example'], $this->account)); $this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_1', ['value' => 'example'], $this->account));
$this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', ['value' => 'example'], $this->account, TRUE)); $this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', ['value' => 'example'], $this->account, TRUE));
} }
@ -424,8 +422,6 @@ class AccessManagerTest extends UnitTestCase {
return $route_match->getParameters()->get('value') == 'upcasted_value'; return $route_match->getParameters()->get('value') == 'upcasted_value';
})); }));
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface');
$access_check->expects($this->atLeastOnce()) $access_check->expects($this->atLeastOnce())
->method('applies') ->method('applies')
@ -435,11 +431,12 @@ class AccessManagerTest extends UnitTestCase {
->willReturn(AccessResult::forbidden()); ->willReturn(AccessResult::forbidden());
$this->container->set('test_access', $access_check); $this->container->set('test_access', $access_check);
$this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider = new CheckProvider(['test_access'], $this->container);
$this->checkProvider->addCheckService('test_access', 'access'); $this->checkProvider->addCheckService('test_access', 'access');
$this->checkProvider->setChecks($this->routeCollection); $this->checkProvider->setChecks($this->routeCollection);
$this->accessManager = new AccessManager($this->routeProvider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_1', [], $this->account)); $this->assertEquals(FALSE, $this->accessManager->checkNamedRoute('test_route_1', [], $this->account));
$this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', [], $this->account, TRUE)); $this->assertEquals(AccessResult::forbidden(), $this->accessManager->checkNamedRoute('test_route_1', [], $this->account, TRUE));
} }
@ -497,9 +494,9 @@ class AccessManagerTest extends UnitTestCase {
->willReturn($return_value); ->willReturn($return_value);
$container->set('test_incorrect_value', $access_check); $container->set('test_incorrect_value', $access_check);
$access_manager = new AccessManager($route_provider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider); $this->checkProvider = new CheckProvider([], $container);
$this->checkProvider->setContainer($container);
$this->checkProvider->addCheckService('test_incorrect_value', 'access'); $this->checkProvider->addCheckService('test_incorrect_value', 'access');
$access_manager = new AccessManager($route_provider, $this->paramConverter, $this->argumentsResolverFactory, $this->currentUser, $this->checkProvider);
$this->expectException(AccessException::class); $this->expectException(AccessException::class);
$access_manager->checkNamedRoute('test_incorrect_value', [], $this->account); $access_manager->checkNamedRoute('test_incorrect_value', [], $this->account);