core/lib/Drupal/Core/Access/AccessResult.php | 119 +++-------------- core/lib/Drupal/Core/Cache/CacheableMetadata.php | 142 +++++---------------- .../Cache/RefinableCacheableDependencyTrait.php | 10 +- .../ContentTranslationManageAccessCheckTest.php | 8 ++ .../Unit/Access/EditEntityFieldAccessCheckTest.php | 62 +++++---- .../src/Unit/UserAccessControlHandlerTest.php | 8 ++ .../Drupal/Tests/Core/Access/AccessManagerTest.php | 4 + .../Drupal/Tests/Core/Access/AccessResultTest.php | 30 +++-- .../Tests/Core/Entity/EntityAccessCheckTest.php | 7 + .../Core/Entity/EntityCreateAccessCheckTest.php | 30 +++-- .../Menu/DefaultMenuLinkTreeManipulatorsTest.php | 7 + .../Tests/Core/Route/RoleAccessCheckTest.php | 7 + 12 files changed, 169 insertions(+), 265 deletions(-) diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index b364baf..bf88316 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -8,6 +8,8 @@ use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyTrait; use Drupal\Core\Config\ConfigBase; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; @@ -25,47 +27,10 @@ * * When using ::orIf() and ::andIf(), cacheability metadata will be merged * accordingly as well. - * - * @todo Use RefinableCacheableDependencyInterface and the corresponding trait in - * https://www.drupal.org/node/2526326. */ -abstract class AccessResult implements AccessResultInterface, CacheableDependencyInterface { - - /** - * The cache context IDs (to vary a cache item ID based on active contexts). - * - * @see \Drupal\Core\Cache\Context\CacheContextInterface - * @see \Drupal\Core\Cache\Context\CacheContextsManager::convertTokensToKeys() - * - * @var string[] - */ - protected $contexts; - - /** - * The cache tags. - * - * @var array - */ - protected $tags; - - /** - * The maximum caching time in seconds. - * - * @var int - */ - protected $maxAge; +abstract class AccessResult implements AccessResultInterface, RefinableCacheableDependencyInterface { - /** - * Constructs a new AccessResult object. - */ - public function __construct() { - $this->resetCacheContexts() - ->resetCacheTags() - // Max-age must be non-zero for an access result to be cacheable. - // Typically, cache items are invalidated via associated cache tags, not - // via a maximum age. - ->setCacheMaxAge(Cache::PERMANENT); - } + use RefinableCacheableDependencyTrait; /** * Creates an AccessResultInterface object with isNeutral() === TRUE. @@ -215,35 +180,21 @@ public function isNeutral() { * {@inheritdoc} */ public function getCacheContexts() { - sort($this->contexts); - return $this->contexts; + return $this->cacheContexts; } /** * {@inheritdoc} */ public function getCacheTags() { - return $this->tags; + return $this->cacheTags; } /** * {@inheritdoc} */ public function getCacheMaxAge() { - return $this->maxAge; - } - - /** - * Adds cache contexts associated with the access result. - * - * @param string[] $contexts - * An array of cache context IDs, used to generate a cache ID. - * - * @return $this - */ - public function addCacheContexts(array $contexts) { - $this->contexts = array_unique(array_merge($this->contexts, $contexts)); - return $this; + return $this->cacheMaxAge; } /** @@ -252,20 +203,7 @@ public function addCacheContexts(array $contexts) { * @return $this */ public function resetCacheContexts() { - $this->contexts = array(); - return $this; - } - - /** - * Adds cache tags associated with the access result. - * - * @param array $tags - * An array of cache tags. - * - * @return $this - */ - public function addCacheTags(array $tags) { - $this->tags = Cache::mergeTags($this->tags, $tags); + $this->cacheContexts = array(); return $this; } @@ -275,7 +213,7 @@ public function addCacheTags(array $tags) { * @return $this */ public function resetCacheTags() { - $this->tags = array(); + $this->cacheTags = array(); return $this; } @@ -288,7 +226,7 @@ public function resetCacheTags() { * @return $this */ public function setCacheMaxAge($max_age) { - $this->maxAge = $max_age; + $this->cacheMaxAge = $max_age; return $this; } @@ -343,28 +281,6 @@ public function cacheUntilConfigurationChanges(ConfigBase $configuration) { } /** - * Adds a dependency on an object: merges its cacheability metadata. - * - * @param \Drupal\Core\Cache\CacheableDependencyInterface|object $other_object - * The dependency. If the object implements CacheableDependencyInterface, - * then its cacheability metadata will be used. Otherwise, the passed in - * object must be assumed to be uncacheable, so max-age 0 is set. - * - * @return $this - */ - public function addCacheableDependency($other_object) { - if ($other_object instanceof CacheableDependencyInterface) { - $this->contexts = Cache::mergeContexts($this->contexts, $other_object->getCacheContexts()); - $this->tags = Cache::mergeTags($this->tags, $other_object->getCacheTags()); - $this->maxAge = Cache::mergeMaxAges($this->maxAge, $other_object->getCacheMaxAge()); - } - else { - $this->maxAge = 0; - } - return $this; - } - - /** * {@inheritdoc} */ public function orIf(AccessResultInterface $other) { @@ -452,12 +368,19 @@ public function andIf(AccessResultInterface $other) { /** * Inherits the cacheability of the other access result, if any. * + * inheritCacheability() differs from addCacheableDependency() in how it + * handles max-age, because it is designed to inherit the cacheability of the + * second operand in the andIf() and orIf() operations. There, the situation + * "allowed, max-age=0 OR allowed, max-age=1000" needs to yield max-age 1000 + * as the end result. + * * @param \Drupal\Core\Access\AccessResultInterface $other * The other access result, whose cacheability (if any) to inherit. * * @return $this */ public function inheritCacheability(AccessResultInterface $other) { + $this->addCacheableDependency($other); if ($other instanceof CacheableDependencyInterface) { if ($this->getCacheMaxAge() !== 0 && $other->getCacheMaxAge() !== 0) { $this->setCacheMaxAge(Cache::mergeMaxAges($this->getCacheMaxAge(), $other->getCacheMaxAge())); @@ -465,14 +388,6 @@ public function inheritCacheability(AccessResultInterface $other) { else { $this->setCacheMaxAge($other->getCacheMaxAge()); } - $this->addCacheContexts($other->getCacheContexts()); - $this->addCacheTags($other->getCacheTags()); - } - // If any of the access results don't provide cacheability metadata, then - // we cannot cache the combined access result, for we may not make - // assumptions. - else { - $this->setCacheMaxAge(0); } return $this; } diff --git a/core/lib/Drupal/Core/Cache/CacheableMetadata.php b/core/lib/Drupal/Core/Cache/CacheableMetadata.php index 33d5afb..33429bf 100644 --- a/core/lib/Drupal/Core/Cache/CacheableMetadata.php +++ b/core/lib/Drupal/Core/Cache/CacheableMetadata.php @@ -11,50 +11,16 @@ * * @ingroup cache * - * @todo Use RefinableCacheableDependencyInterface and the corresponding trait in - * https://www.drupal.org/node/2526326. */ -class CacheableMetadata implements CacheableDependencyInterface { +class CacheableMetadata implements RefinableCacheableDependencyInterface { - /** - * Cache contexts. - * - * @var string[] - */ - protected $contexts = []; - - /** - * Cache tags. - * - * @var string[] - */ - protected $tags = []; - - /** - * Cache max-age. - * - * @var int - */ - protected $maxAge = Cache::PERMANENT; + use RefinableCacheableDependencyTrait; /** * {@inheritdoc} */ public function getCacheTags() { - return $this->tags; - } - - /** - * Adds cache tags. - * - * @param string[] $cache_tags - * The cache tags to be added. - * - * @return $this - */ - public function addCacheTags(array $cache_tags) { - $this->tags = Cache::mergeTags($this->tags, $cache_tags); - return $this; + return $this->cacheTags; } /** @@ -66,7 +32,7 @@ public function addCacheTags(array $cache_tags) { * @return $this */ public function setCacheTags(array $cache_tags) { - $this->tags = $cache_tags; + $this->cacheTags = $cache_tags; return $this; } @@ -74,20 +40,7 @@ public function setCacheTags(array $cache_tags) { * {@inheritdoc} */ public function getCacheContexts() { - return $this->contexts; - } - - /** - * Adds cache contexts. - * - * @param string[] $cache_contexts - * The cache contexts to be added. - * - * @return $this - */ - public function addCacheContexts(array $cache_contexts) { - $this->contexts = Cache::mergeContexts($this->contexts, $cache_contexts); - return $this; + return $this->cacheContexts; } /** @@ -99,7 +52,7 @@ public function addCacheContexts(array $cache_contexts) { * @return $this */ public function setCacheContexts(array $cache_contexts) { - $this->contexts = $cache_contexts; + $this->cacheContexts = $cache_contexts; return $this; } @@ -107,7 +60,7 @@ public function setCacheContexts(array $cache_contexts) { * {@inheritdoc} */ public function getCacheMaxAge() { - return $this->maxAge; + return $this->cacheMaxAge; } /** @@ -128,36 +81,7 @@ public function setCacheMaxAge($max_age) { throw new \InvalidArgumentException('$max_age must be an integer'); } - $this->maxAge = $max_age; - return $this; - } - - /** - * Adds a dependency on an object: merges its cacheability metadata. - * - * @param \Drupal\Core\Cache\CacheableDependencyInterface|mixed $other_object - * The dependency. If the object implements CacheableDependencyInterface, - * then its cacheability metadata will be used. Otherwise, the passed in - * object must be assumed to be uncacheable, so max-age 0 is set. - * - * @return $this - */ - public function addCacheableDependency($other_object) { - if ($other_object instanceof CacheableDependencyInterface) { - $this->addCacheTags($other_object->getCacheTags()); - $this->addCacheContexts($other_object->getCacheContexts()); - if ($this->maxAge === Cache::PERMANENT) { - $this->maxAge = $other_object->getCacheMaxAge(); - } - elseif (($max_age = $other_object->getCacheMaxAge()) && $max_age !== Cache::PERMANENT) { - $this->maxAge = Cache::mergeMaxAges($this->maxAge, $max_age); - } - } - else { - // Not a cacheable dependency, this can not be cached. - $this->maxAge = 0; - } - + $this->cacheMaxAge = $max_age; return $this; } @@ -175,34 +99,34 @@ public function merge(CacheableMetadata $other) { // This is called many times per request, so avoid merging unless absolutely // necessary. - if (empty($this->contexts)) { - $result->contexts = $other->contexts; + if (empty($this->cacheContexts)) { + $result->cacheContexts = $other->cacheContexts; } - elseif (empty($other->contexts)) { - $result->contexts = $this->contexts; + elseif (empty($other->cacheContexts)) { + $result->cacheContexts = $this->cacheContexts; } else { - $result->contexts = Cache::mergeContexts($this->contexts, $other->contexts); + $result->cacheContexts = Cache::mergeContexts($this->cacheContexts, $other->cacheContexts); } - if (empty($this->tags)) { - $result->tags = $other->tags; + if (empty($this->cacheTags)) { + $result->cacheTags = $other->cacheTags; } - elseif (empty($other->tags)) { - $result->tags = $this->tags; + elseif (empty($other->cacheTags)) { + $result->cacheTags = $this->cacheTags; } else { - $result->tags = Cache::mergeTags($this->tags, $other->tags); + $result->cacheTags = Cache::mergeTags($this->cacheTags, $other->cacheTags); } - if ($this->maxAge === Cache::PERMANENT) { - $result->maxAge = $other->maxAge; + if ($this->cacheMaxAge === Cache::PERMANENT) { + $result->cacheMaxAge = $other->cacheMaxAge; } - elseif ($other->maxAge === Cache::PERMANENT) { - $result->maxAge = $this->maxAge; + elseif ($other->cacheMaxAge === Cache::PERMANENT) { + $result->cacheMaxAge = $this->cacheMaxAge; } else { - $result->maxAge = Cache::mergeMaxAges($this->maxAge, $other->maxAge); + $result->cacheMaxAge = Cache::mergeMaxAges($this->cacheMaxAge, $other->cacheMaxAge); } return $result; } @@ -214,9 +138,9 @@ public function merge(CacheableMetadata $other) { * A render array. */ public function applyTo(array &$build) { - $build['#cache']['contexts'] = $this->contexts; - $build['#cache']['tags'] = $this->tags; - $build['#cache']['max-age'] = $this->maxAge; + $build['#cache']['contexts'] = $this->cacheContexts; + $build['#cache']['tags'] = $this->cacheTags; + $build['#cache']['max-age'] = $this->cacheMaxAge; } /** @@ -229,9 +153,9 @@ public function applyTo(array &$build) { */ public static function createFromRenderArray(array $build) { $meta = new static(); - $meta->contexts = (isset($build['#cache']['contexts'])) ? $build['#cache']['contexts'] : []; - $meta->tags = (isset($build['#cache']['tags'])) ? $build['#cache']['tags'] : []; - $meta->maxAge = (isset($build['#cache']['max-age'])) ? $build['#cache']['max-age'] : Cache::PERMANENT; + $meta->cacheContexts = (isset($build['#cache']['contexts'])) ? $build['#cache']['contexts'] : []; + $meta->cacheTags = (isset($build['#cache']['tags'])) ? $build['#cache']['tags'] : []; + $meta->cacheMaxAge = (isset($build['#cache']['max-age'])) ? $build['#cache']['max-age'] : Cache::PERMANENT; return $meta; } @@ -249,16 +173,16 @@ public static function createFromRenderArray(array $build) { public static function createFromObject($object) { if ($object instanceof CacheableDependencyInterface) { $meta = new static(); - $meta->contexts = $object->getCacheContexts(); - $meta->tags = $object->getCacheTags(); - $meta->maxAge = $object->getCacheMaxAge(); + $meta->cacheContexts = $object->getCacheContexts(); + $meta->cacheTags = $object->getCacheTags(); + $meta->cacheMaxAge = $object->getCacheMaxAge(); return $meta; } // Objects that don't implement CacheableDependencyInterface must be assumed // to be uncacheable, so set max-age 0. $meta = new static(); - $meta->maxAge = 0; + $meta->cacheMaxAge = 0; return $meta; } diff --git a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php index 63b1d3d..b94d7f3 100644 --- a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php +++ b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php @@ -44,7 +44,7 @@ public function addCacheableDependency($other_object) { } else { // Not a cacheable dependency, this can not be cached. - $this->maxAge = 0; + $this->cacheMaxAge = 0; } return $this; } @@ -53,7 +53,9 @@ public function addCacheableDependency($other_object) { * {@inheritdoc} */ public function addCacheContexts(array $cache_contexts) { - $this->cacheContexts = Cache::mergeContexts($this->cacheContexts, $cache_contexts); + if ($cache_contexts) { + $this->cacheContexts = Cache::mergeContexts($this->cacheContexts, $cache_contexts); + } return $this; } @@ -61,7 +63,9 @@ public function addCacheContexts(array $cache_contexts) { * {@inheritdoc} */ public function addCacheTags(array $cache_tags) { - $this->cacheTags = Cache::mergeTags($this->cacheTags, $cache_tags); + if ($cache_tags) { + $this->cacheTags = Cache::mergeTags($this->cacheTags, $cache_tags); + } return $this; } diff --git a/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php b/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php index 6c95f7e..266a89b 100644 --- a/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php +++ b/core/modules/content_translation/tests/src/Unit/Access/ContentTranslationManageAccessCheckTest.php @@ -10,6 +10,8 @@ use Drupal\content_translation\Access\ContentTranslationManageAccessCheck; use Drupal\Core\Access\AccessResult; use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\Cache\Cache; +use Drupal\Core\DependencyInjection\Container; use Drupal\Core\Language\Language; use Drupal\Tests\UnitTestCase; use Symfony\Component\Routing\Route; @@ -97,6 +99,12 @@ public function testCreateAccess() { ->with() ->will($this->returnValue(array())); $entity->expects($this->once()) + ->method('getCacheContexts') + ->willReturn([]); + $entity->expects($this->once()) + ->method('getCacheMaxAge') + ->willReturn(Cache::PERMANENT); + $entity->expects($this->once()) ->method('getCacheTags') ->will($this->returnValue(array('node:1337'))); $entity->expects($this->once()) diff --git a/core/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php b/core/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php index 6f352ae..8b91791 100644 --- a/core/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php +++ b/core/modules/quickedit/tests/src/Unit/Access/EditEntityFieldAccessCheckTest.php @@ -8,6 +8,8 @@ namespace Drupal\Tests\quickedit\Unit\Access; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Drupal\quickedit\Access\EditEntityFieldAccessCheck; use Drupal\Tests\UnitTestCase; use Drupal\field\FieldStorageConfigInterface; @@ -33,6 +35,11 @@ class EditEntityFieldAccessCheckTest extends UnitTestCase { */ protected function setUp() { $this->editAccessCheck = new EditEntityFieldAccessCheck(); + + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); } /** @@ -41,34 +48,11 @@ protected function setUp() { * @see \Drupal\Tests\edit\Unit\quickedit\Access\EditEntityFieldAccessCheckTest::testAccess() */ public function providerTestAccess() { - $editable_entity = $this->createMockEntity(); - $editable_entity->expects($this->any()) - ->method('access') - ->will($this->returnValue(AccessResult::allowed()->cachePerPermissions())); - - $non_editable_entity = $this->createMockEntity(); - $non_editable_entity->expects($this->any()) - ->method('access') - ->will($this->returnValue(AccessResult::neutral()->cachePerPermissions())); - - $field_storage_with_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') - ->disableOriginalConstructor() - ->getMock(); - $field_storage_with_access->expects($this->any()) - ->method('access') - ->will($this->returnValue(AccessResult::allowed())); - $field_storage_without_access = $this->getMockBuilder('Drupal\field\Entity\FieldStorageConfig') - ->disableOriginalConstructor() - ->getMock(); - $field_storage_without_access->expects($this->any()) - ->method('access') - ->will($this->returnValue(AccessResult::neutral())); - $data = array(); - $data[] = array($editable_entity, $field_storage_with_access, AccessResult::allowed()->cachePerPermissions()); - $data[] = array($non_editable_entity, $field_storage_with_access, AccessResult::neutral()->cachePerPermissions()); - $data[] = array($editable_entity, $field_storage_without_access, AccessResult::neutral()->cachePerPermissions()); - $data[] = array($non_editable_entity, $field_storage_without_access, AccessResult::neutral()->cachePerPermissions()); + $data[] = array(TRUE, TRUE, AccessResult::allowed()); + $data[] = array(FALSE, TRUE, AccessResult::neutral()); + $data[] = array(TRUE, FALSE, AccessResult::neutral()); + $data[] = array(FALSE, FALSE, AccessResult::neutral()); return $data; } @@ -76,16 +60,28 @@ public function providerTestAccess() { /** * Tests the method for checking access to routes. * - * @param \Drupal\Core\Entity\EntityInterface $entity - * A mocked entity. - * @param \Drupal\field\FieldStorageConfigInterface $field_storage - * A mocked field storage. - * @param bool|null $expected_result + * @param bool $entity_is_editable + * Whether the subject entity is editable. + * @param bool $field_storage_is_accessible + * Whether the user has access to the field storage entity. + * @param \Drupal\Core\Access\AccessResult $expected_result * The expected result of the access call. * * @dataProvider providerTestAccess */ - public function testAccess(EntityInterface $entity, FieldStorageConfigInterface $field_storage = NULL, $expected_result) { + public function testAccess($entity_is_editable, $field_storage_is_accessible, AccessResult $expected_result) { + $entity = $this->createMockEntity(); + $entity->expects($this->any()) + ->method('access') + ->willReturn(AccessResult::allowedIf($entity_is_editable)->cachePerPermissions()); + + $field_storage = $this->getMock('Drupal\field\FieldStorageConfigInterface'); + $field_storage->expects($this->any()) + ->method('access') + ->willReturn(AccessResult::allowedIf($field_storage_is_accessible)); + + $expected_result->cachePerPermissions(); + $field_name = 'valid'; $entity_with_field = clone $entity; $entity_with_field->expects($this->any()) diff --git a/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php b/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php index 34f7590..05d7223 100644 --- a/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php +++ b/core/modules/user/tests/src/Unit/UserAccessControlHandlerTest.php @@ -9,6 +9,8 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Drupal\Tests\UnitTestCase; use Drupal\user\UserAccessControlHandler; @@ -62,6 +64,12 @@ class UserAccessControlHandlerTest extends UnitTestCase { */ public function setUp() { parent::setUp(); + + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); + $this->viewer = $this->getMock('\Drupal\Core\Session\AccountInterface'); $this->viewer ->expects($this->any()) diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 128f2d4..3a03535 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -11,6 +11,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Access\CheckProvider; +use Drupal\Core\Cache\Context\CacheContextsManager; use Drupal\Core\Routing\RouteMatch; use Drupal\Core\Access\AccessManager; use Drupal\Core\Access\DefaultAccessCheck; @@ -94,6 +95,9 @@ protected function setUp() { parent::setUp(); $this->container = new ContainerBuilder(); + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $this->container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($this->container); $this->routeCollection = new RouteCollection(); $this->routeCollection->add('test_route_1', new Route('/test-route-1')); diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php index 881b096..ae5dadc 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php @@ -52,7 +52,6 @@ protected function assertDefaultCacheability(AccessResult $access) { /** * Tests the construction of an AccessResult object. * - * @covers ::__construct * @covers ::neutral */ public function testConstruction() { @@ -852,8 +851,15 @@ public function testOrIfCacheabilityMerging() { * @covers ::allowedIfHasPermissions * * @dataProvider providerTestAllowedIfHasPermissions + * + * @param string[] $permissions + * The permissions to check for. + * @param string $conjunction + * The conjunction to use when checking for permission. 'AND' or 'OR'. + * @param \Drupal\Core\Access\AccessResult $expected_access + * The expected access check result. */ - public function testAllowedIfHasPermissions($permissions, $conjunction, $expected_access) { + public function testAllowedIfHasPermissions($permissions, $conjunction, AccessResult $expected_access) { $account = $this->getMock('\Drupal\Core\Session\AccountInterface'); $account->expects($this->any()) ->method('hasPermission') @@ -862,6 +868,10 @@ public function testAllowedIfHasPermissions($permissions, $conjunction, $expecte ['denied', FALSE], ]); + if ($permissions) { + $expected_access->cachePerPermissions(); + } + $access_result = AccessResult::allowedIfHasPermissions($account, $permissions, $conjunction); $this->assertEquals($expected_access, $access_result); } @@ -875,14 +885,14 @@ public function providerTestAllowedIfHasPermissions() { return [ [[], 'AND', AccessResult::allowedIf(FALSE)], [[], 'OR', AccessResult::allowedIf(FALSE)], - [['allowed'], 'OR', AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions'])], - [['allowed'], 'AND', AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions'])], - [['denied'], 'OR', AccessResult::allowedIf(FALSE)->addCacheContexts(['user.permissions'])], - [['denied'], 'AND', AccessResult::allowedIf(FALSE)->addCacheContexts(['user.permissions'])], - [['allowed', 'denied'], 'OR', AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions'])], - [['denied', 'allowed'], 'OR', AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions'])], - [['allowed', 'denied', 'other'], 'OR', AccessResult::allowedIf(TRUE)->addCacheContexts(['user.permissions'])], - [['allowed', 'denied'], 'AND', AccessResult::allowedIf(FALSE)->addCacheContexts(['user.permissions'])], + [['allowed'], 'OR', AccessResult::allowedIf(TRUE)], + [['allowed'], 'AND', AccessResult::allowedIf(TRUE)], + [['denied'], 'OR', AccessResult::allowedIf(FALSE)], + [['denied'], 'AND', AccessResult::allowedIf(FALSE)], + [['allowed', 'denied'], 'OR', AccessResult::allowedIf(TRUE)], + [['denied', 'allowed'], 'OR', AccessResult::allowedIf(TRUE)], + [['allowed', 'denied', 'other'], 'OR', AccessResult::allowedIf(TRUE)], + [['allowed', 'denied'], 'AND', AccessResult::allowedIf(FALSE)], ]; } diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php index bc2e5cc..0865933 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Entity; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\Routing\Route; use Drupal\Core\Access\AccessResult; @@ -25,6 +27,11 @@ class EntityAccessCheckTest extends UnitTestCase { * Tests the method for checking access to routes. */ public function testAccess() { + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); + $route = new Route('/foo', array(), array('_entity_access' => 'node.update')); $upcasted_arguments = new ParameterBag(); $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php index 5d0a966..b0b4c8c 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php @@ -8,6 +8,8 @@ namespace Drupal\Tests\Core\Entity; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Drupal\Core\Entity\EntityCreateAccessCheck; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; @@ -32,6 +34,11 @@ class EntityCreateAccessCheckTest extends UnitTestCase { */ protected function setUp() { parent::setUp(); + + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); } /** @@ -40,9 +47,8 @@ protected function setUp() { * @return array */ public function providerTestAccess() { - $no_access = AccessResult::neutral()->cachePerPermissions(); - $access = AccessResult::allowed()->cachePerPermissions(); - $no_access_due_to_errors = AccessResult::neutral(); + $no_access = FALSE; + $access = TRUE; return array( array('', 'entity_test', $no_access, $no_access), @@ -51,10 +57,10 @@ public function providerTestAccess() { array('test_entity', 'entity_test:test_entity', $no_access, $no_access), array('test_entity', 'entity_test:{bundle_argument}', $access, $access), array('test_entity', 'entity_test:{bundle_argument}', $no_access, $no_access), - array('', 'entity_test:{bundle_argument}', $no_access, $no_access_due_to_errors), + array('', 'entity_test:{bundle_argument}', $no_access, $no_access, FALSE), // When the bundle is not provided, access should be denied even if the // access control handler would allow access. - array('', 'entity_test:{bundle_argument}', $access, $no_access_due_to_errors), + array('', 'entity_test:{bundle_argument}', $access, $no_access, FALSE), ); } @@ -63,7 +69,15 @@ public function providerTestAccess() { * * @dataProvider providerTestAccess */ - public function testAccess($entity_bundle, $requirement, $access, $expected) { + public function testAccess($entity_bundle, $requirement, $access, $expected, $expect_permission_context = TRUE) { + + // Set up the access result objects for allowing or denying access. + $access_result = $access ? AccessResult::allowed()->cachePerPermissions() : AccessResult::neutral()->cachePerPermissions(); + $expected_access_result = $expected ? AccessResult::allowed() : AccessResult::neutral(); + if ($expect_permission_context) { + $expected_access_result->cachePerPermissions(); + } + $entity_manager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface'); // Don't expect a call to the access control handler when we have a bundle @@ -73,7 +87,7 @@ public function testAccess($entity_bundle, $requirement, $access, $expected) { $access_control_handler->expects($this->once()) ->method('createAccess') ->with($entity_bundle) - ->will($this->returnValue($access)); + ->will($this->returnValue($access_result)); $entity_manager->expects($this->any()) ->method('getAccessControlHandler') @@ -101,7 +115,7 @@ public function testAccess($entity_bundle, $requirement, $access, $expected) { ->will($this->returnValue($raw_variables)); $account = $this->getMock('Drupal\Core\Session\AccountInterface'); - $this->assertEquals($expected, $applies_check->access($route, $route_match, $account)); + $this->assertEquals($expected_access_result, $applies_check->access($route, $route_match, $account)); } } diff --git a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php index c20a99b..17702e7 100644 --- a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php @@ -8,6 +8,8 @@ namespace Drupal\Tests\Core\Menu; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Drupal\Core\Menu\DefaultMenuLinkTreeManipulators; use Drupal\Core\Menu\MenuLinkTreeElement; use Drupal\Tests\UnitTestCase; @@ -76,6 +78,11 @@ protected function setUp() { ->getMock(); $this->defaultMenuTreeManipulators = new DefaultMenuLinkTreeManipulators($this->accessManager, $this->currentUser, $this->queryFactory); + + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); } /** diff --git a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php index d818e47..9d0d148 100644 --- a/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php +++ b/core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php @@ -8,6 +8,8 @@ namespace Drupal\Tests\Core\Route; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Context\CacheContextsManager; +use Drupal\Core\DependencyInjection\Container; use Drupal\Core\Session\UserSession; use Drupal\Tests\UnitTestCase; use Drupal\user\Access\RoleAccessCheck; @@ -143,6 +145,11 @@ public function roleAccessProvider() { * @dataProvider roleAccessProvider */ public function testRoleAccess($path, $grant_accounts, $deny_accounts) { + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); + $container = new Container(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); + $role_access_check = new RoleAccessCheck(); $collection = $this->getTestRouteCollection();