core/lib/Drupal/Core/Access/AccessResult.php | 76 ++- .../Drupal/Core/Access/AccessResultInterface.php | 10 +- .../Access/ContentTranslationOverviewAccess.php | 4 +- core/modules/system/entity.api.php | 2 +- .../Drupal/Tests/Core/Access/AccessResultTest.php | 560 ++++++++++++++++++--- 5 files changed, 542 insertions(+), 110 deletions(-) diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index 20d9a88..a3ef2d3 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -63,13 +63,7 @@ public function __construct() { } /** - * Creates an AccessResultInterface object wth isNeutral() === TRUE. - * - * This factory method exists to improve DX; it allows developers to fluently - * create access results. - * - * Defaults to a cacheable access result that neither explicitly allows nor - * explicitly forbids access. + * Creates an AccessResultInterface object with isNeutral() === TRUE. * * @return \Drupal\Core\Access\AccessResult * isNeutral() will be TRUE. @@ -79,7 +73,7 @@ public static function neutral() { } /** - * Creates an AccessResultInterface object wth isAllowed() === TRUE. + * Creates an AccessResultInterface object with isAllowed() === TRUE. * * @return \Drupal\Core\Access\AccessResult * isAllowed() will be TRUE. @@ -89,7 +83,7 @@ public static function allowed() { } /** - * Creates a forbidden access result. + * Creates an AccessResultInterface object with isForbidden() === TRUE. * * @return \Drupal\Core\Access\AccessResult * isForbidden() will be TRUE. @@ -105,7 +99,7 @@ public static function forbidden() { * The condition to evaluate. * * @return \Drupal\Core\Access\AccessResult - * If $condition is TRUE, isAlowed() will be TRUE, otherwise isNeutral() + * If $condition is TRUE, isAllowed() will be TRUE, otherwise isNeutral() * will be TRUE. */ public static function allowedIf($condition) { @@ -129,6 +123,8 @@ public static function forbiddenIf($condition) { /** * Creates an allowed access result if the permission is present, neutral otherwise. * + * Convenience method, checks the permission and calls ::cachePerRole(). + * * @param \Drupal\Core\Session\AccountInterface $account * The account for which to check a permission. * @param string $permission @@ -144,6 +140,8 @@ public static function allowedIfHasPermission(AccountInterface $account, $permis /** * {@inheritdoc} + * + * @see \Drupal\Core\Access\AccessResultAllowed */ public function isAllowed() { return FALSE; @@ -151,6 +149,8 @@ public function isAllowed() { /** * {@inheritdoc} + * + * \Drupal\Core\Access\AccessResultForbidden */ public function isForbidden() { return FALSE; @@ -158,6 +158,8 @@ public function isForbidden() { /** * {@inheritdoc} + * + * @see \Drupal\Core\Access\AccessResultNeutral */ public function isNeutral() { return FALSE; @@ -326,18 +328,33 @@ public function cacheUntilEntityChanges(EntityInterface $entity) { * {@inheritdoc} */ public function orIf(AccessResultInterface $other) { + // The other access result's cacheability metadata is merged if $merge_other + // gets set to TRUE. It gets set to TRUE in two cases: + // 1. The other access result is used. + // 2. This access result is not cacheable and the other access result is the + // same. i.e. attempt to return a cacheable access result. + $merge_other = FALSE; if ($this->isForbidden() || $other->isForbidden()) { $result = static::forbidden(); + if (!$this->isForbidden() || (!$this->isCacheable() && $other->isForbidden())) { + $merge_other = TRUE; + } } elseif ($this->isAllowed() || $other->isAllowed()) { $result = static::allowed(); + if (!$this->isAllowed() || (!$this->isCacheable() && $other->isAllowed())) { + $merge_other = TRUE; + } } else { $result = static::neutral(); + if (!$this->isNeutral() || (!$this->isCacheable() && $other->isNeutral())) { + $merge_other = TRUE; + } } - $result->mergeCacheabilityMetadata($this); - if (!$this->isForbidden()) { - $result->mergeCacheabilityMetadata($other); + $result->inheritCacheability($this); + if ($merge_other) { + $result->inheritCacheability($other); } return $result; } @@ -346,29 +363,50 @@ public function orIf(AccessResultInterface $other) { * {@inheritdoc} */ public function andIf(AccessResultInterface $other) { + // The other access result's cacheability metadata is merged if $merge_other + // gets set to TRUE. It gets set to TRUE in one case: if the other access + // result is used. + $merge_other = FALSE; if ($this->isForbidden() || $other->isForbidden()) { $result = static::forbidden(); + if (!$this->isForbidden()) { + $merge_other = TRUE; + } } elseif ($this->isAllowed() && $other->isAllowed()) { $result = static::allowed(); + $merge_other = TRUE; } else { $result = static::neutral(); + if (!$this->isNeutral()) { + $merge_other = TRUE; + } } - $result->mergeCacheabilityMetadata($this); - if ($this->isAllowed()) { - $result->mergeCacheabilityMetadata($other); + // Only when this access result already was isAllowed(), the second + $result->inheritCacheability($this); + if ($merge_other) { + $result->inheritCacheability($other); + // If this access result is not cacheable, then an AND with another access + // result must also not be cacheable, except if the other access result + // has isForbidden() === TRUE. isForbidden() access results are contagious + // in that they propagate regardless of the other value. + if (!$this->isCacheable() && !$result->isForbidden()) { + $result->setCacheable(FALSE); + } } return $result; } /** - * Merges the cacheability metadata of the other access result, if any. + * Inherits the cacheability of the other access result, if any. * * @param \Drupal\Core\Access\AccessResultInterface $other - * The other access result, whose cacheability data (if any) to merge. + * The other access result, whose cacheability (if any) to inherit. + * + * @return $this */ - protected function mergeCacheabilityMetadata(AccessResultInterface $other) { + public function inheritCacheability(AccessResultInterface $other) { if ($other instanceof CacheableInterface) { $this->setCacheable($other->isCacheable()); $this->addCacheContexts($other->getCacheKeys()); diff --git a/core/lib/Drupal/Core/Access/AccessResultInterface.php b/core/lib/Drupal/Core/Access/AccessResultInterface.php index 678a256..aa39c01 100644 --- a/core/lib/Drupal/Core/Access/AccessResultInterface.php +++ b/core/lib/Drupal/Core/Access/AccessResultInterface.php @@ -34,18 +34,18 @@ * Checks whether this access result indicates access is explicitly allowed. * * @return bool - * When TRUE then isForbidden() and isNeutral() is FALSE. + * When TRUE then isForbidden() and isNeutral() are FALSE. */ public function isAllowed(); /** * Checks whether this access result indicates access is explicitly forbidden. * - * This is a kill switch -- both orIf and andIf will result in isForbidden() - * if either results are isForbidden(). + * This is a kill switch — both orIf() and andIf() will result in + * isForbidden() if either results are isForbidden(). * * @return bool - * When TRUE then isAllowed() and isNeutral() is FALSE. + * When TRUE then isAllowed() and isNeutral() are FALSE. */ public function isForbidden(); @@ -53,7 +53,7 @@ public function isForbidden(); * Checks whether this access result indicates access is not yet determined. * * @return bool - * When TRUE then isAllowed() and isForbidden() is FALSE. + * When TRUE then isAllowed() and isForbidden() are FALSE. */ public function isNeutral(); diff --git a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php index bc55f70..d79b7f5 100644 --- a/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php +++ b/core/modules/content_translation/src/Access/ContentTranslationOverviewAccess.php @@ -66,7 +66,7 @@ public function access(RouteMatchInterface $route_match, AccountInterface $accou // Check "translate any entity" permission. if ($account->hasPermission('translate any entity')) { - return $access->orIf(AccessResult::allowed()->cachePerRole()); + return AccessResult::allowed()->cachePerRole()->inheritCacheability($access); } // Check per entity permission. @@ -74,7 +74,7 @@ public function access(RouteMatchInterface $route_match, AccountInterface $accou if ($definition->getPermissionGranularity() == 'bundle') { $permission = "translate {$bundle} {$entity_type_id}"; } - return $access->orIf(AccessResult::allowedIfHasPermission($account, $permission)); + return AccessResult::allowedIfHasPermission($account, $permission)->inheritCacheability($access); } // No opinion. diff --git a/core/modules/system/entity.api.php b/core/modules/system/entity.api.php index 7fc113f..18d9828 100644 --- a/core/modules/system/entity.api.php +++ b/core/modules/system/entity.api.php @@ -1868,7 +1868,7 @@ function hook_entity_field_access_alter(array &$grants, array $context) { // don't want to switch node module's grant to // AccessResultInterface::isAllowed() , because the grants of other modules // should still decide on their own if this field is accessible or not - $grants['node']->resetAccess(); + $grants['node'] = AccessResult::neutral()->inheritCacheability($grants['node']); } } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php index 6ffd3c0..5ef8685 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php @@ -8,8 +8,10 @@ namespace Drupal\Tests\Core\Access; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Access\AccessResultNeutral; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableInterface; use Drupal\Tests\UnitTestCase; /** @@ -37,6 +39,7 @@ public function testConstruction() { $verify = function (AccessResult $access) { $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertDefaultCacheability($access); }; @@ -56,11 +59,13 @@ public function testConstruction() { * @covers ::allowed * @covers ::isAllowed * @covers ::isForbidden + * @covers ::isNeutral */ public function testAccessAllowed() { $verify = function (AccessResult $access) { $this->assertTrue($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); }; @@ -74,11 +79,13 @@ public function testAccessAllowed() { * @covers ::forbidden * @covers ::isAllowed * @covers ::isForbidden + * @covers ::isNeutral */ public function testAccessForbidden() { $verify = function (AccessResult $access) { $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); }; @@ -92,11 +99,13 @@ public function testAccessForbidden() { * @covers ::allowedIf * @covers ::isAllowed * @covers ::isForbidden + * @covers ::isNeutral */ public function testAccessConditionallyAllowed() { - $verify = function (AccessResult $access, $allowed, $forbidden = FALSE) { + $verify = function (AccessResult $access, $allowed) { $this->assertSame($allowed, $access->isAllowed()); - $this->assertSame($forbidden, $access->isForbidden()); + $this->assertFalse($access->isForbidden()); + $this->assertSame(!$allowed, $access->isNeutral()); $this->assertDefaultCacheability($access); }; @@ -111,11 +120,13 @@ public function testAccessConditionallyAllowed() { * @covers ::forbiddenIf * @covers ::isAllowed * @covers ::isForbidden + * @covers ::isNeutral */ public function testAccessConditionallyForbidden() { - $verify = function (AccessResult $access, $forbidden, $allowed = FALSE) { - $this->assertSame($allowed, $access->isAllowed()); + $verify = function (AccessResult $access, $forbidden) { + $this->assertFalse($access->isAllowed()); $this->assertSame($forbidden, $access->isForbidden()); + $this->assertSame(!$forbidden, $access->isNeutral()); $this->assertDefaultCacheability($access); }; @@ -129,79 +140,81 @@ public function testAccessConditionallyForbidden() { * @covers ::andIf */ public function testAndIf() { - $no_opinion = AccessResult::neutral(); + $neutral = AccessResult::neutral(); $allowed = AccessResult::allowed(); $forbidden = AccessResult::forbidden(); $unused_access_result_due_to_lazy_evaluation = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); $unused_access_result_due_to_lazy_evaluation->expects($this->never()) ->method($this->anything()); - // ALLOW && ALLOW === ALLOW. + // ALLOWED && ALLOWED === ALLOWED. $access = $allowed->andIf($allowed); $this->assertTrue($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // ALLOW && DENY === DENY. - $access = $allowed->andIf($no_opinion); + // ALLOWED && NEUTRAL === NEUTRAL. + $access = $allowed->andIf($neutral); $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertDefaultCacheability($access); - // ALLOW && KILL === KILL. + // ALLOWED && FORBIDDEN === FORBIDDEN. $access = $allowed->andIf($forbidden); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY && ALLOW == DENY - $access = $no_opinion->andIf($allowed); + // NEUTRAL && ALLOW == NEUTRAL + $access = $neutral->andIf($allowed); $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY && DENY === DENY. - $access = $no_opinion->andIf($no_opinion); + // NEUTRAL && NEUTRAL === NEUTRAL. + $access = $neutral->andIf($neutral); $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY && KILL === KILL. - $access = $no_opinion->andIf($forbidden); + // NEUTRAL && FORBIDDEN === FORBIDDEN. + $access = $neutral->andIf($forbidden); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // KILL && ALLOW = KILL + // FORBIDDEN && ALLOWED = FORBIDDEN $access = $forbidden->andif($allowed); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // KILL && DENY = KILL - $access = $forbidden->andif($no_opinion); + // FORBIDDEN && NEUTRAL = FORBIDDEN + $access = $forbidden->andif($neutral); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // KILL && KILL = KILL + // FORBIDDEN && FORBIDDEN = FORBIDDEN $access = $forbidden->andif($forbidden); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY && * === DENY. - /* - $access = $no_opinion->andIf($unused_access_result_due_to_lazy_evaluation); - $this->assertFalse($access->isAllowed()); - $this->assertFalse($access->isForbidden()); - $this->assertDefaultCacheability($access); - */ - - // KILL && * === KILL. + // FORBIDDEN && * === FORBIDDEN: lazy evaluation verification. $access = $forbidden->andIf($unused_access_result_due_to_lazy_evaluation); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); } @@ -209,53 +222,81 @@ public function testAndIf() { * @covers ::orIf */ public function testOrIf() { - $no_opinion = AccessResult::neutral(); + $neutral = AccessResult::neutral(); $allowed = AccessResult::allowed(); $forbidden = AccessResult::forbidden(); $unused_access_result_due_to_lazy_evaluation = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); $unused_access_result_due_to_lazy_evaluation->expects($this->never()) ->method($this->anything()); - // ALLOW || ALLOW === ALLOW. + // ALLOWED || ALLOWED === ALLOWED. $access = $allowed->orIf($allowed); $this->assertTrue($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // ALLOW || DENY === ALLOW. - $access = $allowed->orIf($no_opinion); + // ALLOWED || NEUTRAL === ALLOWED. + $access = $allowed->orIf($neutral); $this->assertTrue($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // ALLOW || KILL === KILL. + // ALLOWED || FORBIDDEN === FORBIDDEN. $access = $allowed->orIf($forbidden); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY || DENY === DENY. - $access = $no_opinion->orIf($no_opinion); + // NEUTRAL || NEUTRAL === NEUTRAL. + $access = $neutral->orIf($neutral); $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY || ALLOW === ALLOW. - $access = $no_opinion->orIf($allowed); + // NEUTRAL || ALLOWED === ALLOWED. + $access = $neutral->orIf($allowed); $this->assertTrue($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertFalse($access->isNeutral()); + $this->assertDefaultCacheability($access); + + // NEUTRAL || FORBIDDEN === FORBIDDEN. + $access = $neutral->orIf($forbidden); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); + $this->assertDefaultCacheability($access); + + // FORBIDDEN || ALLOWED === FORBIDDEN. + $access = $forbidden->orIf($allowed); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); + $this->assertDefaultCacheability($access); + + // FORBIDDEN || NEUTRAL === FORBIDDEN. + $access = $forbidden->orIf($allowed); + $this->assertFalse($access->isAllowed()); + $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // DENY || KILL === KILL. - $access = $no_opinion->orIf($forbidden); + // FORBIDDEN || FORBIDDEN === FORBIDDEN. + $access = $forbidden->orIf($allowed); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); - // KILL || * === KILL. + // FORBIDDEN || * === FORBIDDEN. $access = $forbidden->orIf($unused_access_result_due_to_lazy_evaluation); $this->assertFalse($access->isAllowed()); $this->assertTrue($access->isForbidden()); + $this->assertFalse($access->isNeutral()); $this->assertDefaultCacheability($access); } @@ -291,6 +332,7 @@ public function testCacheContexts() { $verify = function (AccessResult $access, array $contexts) { $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertTrue($access->isCacheable()); $this->assertSame('default', $access->getCacheBin()); $this->assertSame(Cache::PERMANENT, $access->getCacheMaxAge()); @@ -368,6 +410,7 @@ public function testCacheTags() { $verify = function (AccessResult $access, array $tags) { $this->assertFalse($access->isAllowed()); $this->assertFalse($access->isForbidden()); + $this->assertTrue($access->isNeutral()); $this->assertTrue($access->isCacheable()); $this->assertSame('default', $access->getCacheBin()); $this->assertSame(Cache::PERMANENT, $access->getCacheMaxAge()); @@ -421,27 +464,13 @@ public function testCacheTags() { } /** - * @covers ::andIf - * @covers ::orIf - * @covers ::mergeCacheabilityMetadata + * @covers ::inheritCacheability */ - public function testCacheabilityMerging() { - $access_without_cacheability = $this->getMock('\Drupal\Core\Access\AccessResultInterface'); - $access_without_cacheability->expects($this->exactly(2)) - ->method('isAllowed') - ->willReturn(TRUE); - $access_without_cacheability->expects($this->exactly(2)) - ->method('isForbidden') - ->willReturn(FALSE); - $access_without_cacheability->expects($this->once()) - ->method('andIf') - ->will($this->returnSelf()); - + public function testInheritCacheability() { // andIf(); 1st has defaults, 2nd has custom tags, contexts and max-age. - $access = AccessResult::allowed() - ->andIf(AccessResult::allowed()->setCacheMaxAge(1500)->cachePerRole()->addCacheTags(['node' => [20011988]])); - $this->assertTrue($access->isAllowed()); - $this->assertFalse($access->isForbidden()); + $access = AccessResult::allowed(); + $other = AccessResult::allowed()->setCacheMaxAge(1500)->cachePerRole()->addCacheTags(['node' => [20011988]]); + $access->inheritCacheability($other); $this->assertTrue($access->isCacheable()); $this->assertSame(['cache_context.user.roles'], $access->getCacheKeys()); $this->assertSame(['node' => [20011988 => 20011988]], $access->getCacheTags()); @@ -449,39 +478,404 @@ public function testCacheabilityMerging() { $this->assertSame(1500, $access->getCacheMaxAge()); // andIf(); 1st has custom tags, max-age, 2nd has custom contexts and max-age. - $access = AccessResult::allowed()->cachePerUser()->setCacheMaxAge(43200) - ->andIf(AccessResult::forbidden()->addCacheTags(['node' => [14031991]])->setCacheMaxAge(86400)); - $this->assertFalse($access->isAllowed()); - $this->assertTrue($access->isForbidden()); + $access = AccessResult::allowed()->cachePerUser()->setCacheMaxAge(43200); + $other = AccessResult::forbidden()->addCacheTags(['node' => [14031991]])->setCacheMaxAge(86400); + $access->inheritCacheability($other); $this->assertTrue($access->isCacheable()); $this->assertSame(['cache_context.user'], $access->getCacheKeys()); $this->assertSame(['node' => [14031991 => 14031991]], $access->getCacheTags()); $this->assertSame('default', $access->getCacheBin()); $this->assertSame(43200, $access->getCacheMaxAge()); + } - // orIf(); 1st is cacheable, 2nd isn't. - $access = AccessResult::allowed()->orIf(AccessResult::allowed()->setCacheable(FALSE)); - $this->assertTrue($access->isAllowed()); - $this->assertFalse($access->isForbidden()); - $this->assertFalse($access->isCacheable()); + /** + * Provides a list of access result pairs and operations to test. + * + * This tests the propagation of cacheability metadata. Rather than testing + * every single bit of cacheability metadata, which would lead to a mind- + * boggling number of permutations, in this test, we only consider the + * permutations of all pairs of the following set: + * 1. Allowed, implements cacheable interface, is cacheable + * 2. Allowed, implements cacheable interface, is not cacheable + * 3. Allowed, does not implement cacheable interface (hence not cacheable) + * 4. Forbidden, implements cacheable interface, is cacheable + * 5. Forbidden, implements cacheable interface, is not cacheable + * 6. Forbidden, does not implement cacheable interface (hence not cacheable) + * 7. Neutral, implements cacheable interface, is cacheable + * 8. Neutral, implements cacheable interface, is not cacheable + * 9. Neutral, does not implement cacheable interface (hence not cacheable) + * + * This leads to 72 permutations (9!/(9-2)! = 9*8 = 72) per operation. There + * are two operations to test (AND and OR), so that leads to a grand total of + * 144 permutations, all of which are tested. + * + * There are two "contagious" patterns: + * 1. Any operation with a forbidden access result yields a forbidden result. + * This therefore also applies to the cacheability metadata associated with + * a forbidden result. + * This is the case for items 4, 5 and 6 in the set above. + * 2. Any operation yields an access result object that is of the same class + * (implementation) as the first operand. This is because operations are + * invoked on the first operand. Therefore, if the first implementation + * does not implement CacheableInterface, then the result won't either. + * This is the case for items 3, 6 and 9 in the set above. + */ + public function testAndOrCacheabilityPropagationProvider() { + // ct: cacheable=true, cf: cacheable=false, un: uncacheable. + $allowed_ct = AccessResult::allowed(); + $allowed_cf = AccessResult::allowed()->setCacheable(FALSE); + $allowed_un = new UncacheableTestAccessResult('ALLOWED'); + $forbidden_ct = AccessResult::forbidden(); + $forbidden_cf = AccessResult::forbidden()->setCacheable(FALSE); + $forbidden_un = new UncacheableTestAccessResult('FORBIDDEN'); + $neutral_ct = AccessResult::neutral(); + $neutral_cf = AccessResult::neutral()->setCacheable(FALSE); + $neutral_un = new UncacheableTestAccessResult('NEUTRAL'); + + // Structure: + // - First column: first access result. + // - Second column: operator ('OR' or 'AND'). + // - Third column: second access result. + // - Fourth column: whether the result implements CacheableInterface + // - Fifth column: whether the result is cacheable (if column 4 is TRUE) + return [ + // Allowed (ct) OR allowed (ct,cf,un). + [$allowed_ct, 'OR', $allowed_ct, TRUE, TRUE], + [$allowed_ct, 'OR', $allowed_cf, TRUE, TRUE], + [$allowed_ct, 'OR', $allowed_un, TRUE, TRUE], + // Allowed (cf) OR allowed (ct,cf,un). + [$allowed_cf, 'OR', $allowed_ct, TRUE, TRUE], + [$allowed_cf, 'OR', $allowed_cf, TRUE, FALSE], + [$allowed_cf, 'OR', $allowed_un, TRUE, FALSE], + // Allowed (un) OR allowed (ct,cf,un). + [$allowed_un, 'OR', $allowed_ct, FALSE, NULL], + [$allowed_un, 'OR', $allowed_cf, FALSE, NULL], + [$allowed_un, 'OR', $allowed_un, FALSE, NULL], + + // Allowed (ct) OR forbidden (ct,cf,un). + [$allowed_ct, 'OR', $forbidden_ct, TRUE, TRUE], + [$allowed_ct, 'OR', $forbidden_cf, TRUE, FALSE], + [$allowed_ct, 'OR', $forbidden_un, TRUE, FALSE], + // Allowed (cf) OR forbidden (ct,cf,un). + [$allowed_cf, 'OR', $forbidden_ct, TRUE, TRUE], + [$allowed_cf, 'OR', $forbidden_cf, TRUE, FALSE], + [$allowed_cf, 'OR', $forbidden_un, TRUE, FALSE], + // Allowed (un) OR forbidden (ct,cf,un). + [$allowed_un, 'OR', $forbidden_ct, FALSE, NULL], + [$allowed_un, 'OR', $forbidden_cf, FALSE, NULL], + [$allowed_un, 'OR', $forbidden_un, FALSE, NULL], + + // Allowed (ct) OR neutral (ct,cf,un). + [$allowed_ct, 'OR', $neutral_ct, TRUE, TRUE], + [$allowed_ct, 'OR', $neutral_cf, TRUE, TRUE], + [$allowed_ct, 'OR', $neutral_un, TRUE, TRUE], + // Allowed (cf) OR neutral (ct,cf,un). + [$allowed_cf, 'OR', $neutral_ct, TRUE, FALSE], + [$allowed_cf, 'OR', $neutral_cf, TRUE, FALSE], + [$allowed_cf, 'OR', $neutral_un, TRUE, FALSE], + // Allowed (un) OR neutral (ct,cf,un). + [$allowed_un, 'OR', $neutral_ct, FALSE, NULL], + [$allowed_un, 'OR', $neutral_cf, FALSE, NULL], + [$allowed_un, 'OR', $neutral_un, FALSE, NULL], + + + // Forbidden (ct) OR allowed (ct,cf,un). + [$forbidden_ct, 'OR', $allowed_ct, TRUE, TRUE], + [$forbidden_ct, 'OR', $allowed_cf, TRUE, TRUE], + [$forbidden_ct, 'OR', $allowed_un, TRUE, TRUE], + // Forbidden (cf) OR allowed (ct,cf,un). + [$forbidden_cf, 'OR', $allowed_ct, TRUE, FALSE], + [$forbidden_cf, 'OR', $allowed_cf, TRUE, FALSE], + [$forbidden_cf, 'OR', $allowed_un, TRUE, FALSE], + // Forbidden (un) OR allowed (ct,cf,un). + [$forbidden_un, 'OR', $allowed_ct, FALSE, NULL], + [$forbidden_un, 'OR', $allowed_cf, FALSE, NULL], + [$forbidden_un, 'OR', $allowed_un, FALSE, NULL], + + // Forbidden (ct) OR neutral (ct,cf,un). + [$forbidden_ct, 'OR', $neutral_ct, TRUE, TRUE], + [$forbidden_ct, 'OR', $neutral_cf, TRUE, TRUE], + [$forbidden_ct, 'OR', $neutral_un, TRUE, TRUE], + // Forbidden (cf) OR neutral (ct,cf,un). + [$forbidden_cf, 'OR', $neutral_ct, TRUE, FALSE], + [$forbidden_cf, 'OR', $neutral_cf, TRUE, FALSE], + [$forbidden_cf, 'OR', $neutral_un, TRUE, FALSE], + // Forbidden (un) OR neutral (ct,cf,un). + [$forbidden_un, 'OR', $neutral_ct, FALSE, NULL], + [$forbidden_un, 'OR', $neutral_cf, FALSE, NULL], + [$forbidden_un, 'OR', $neutral_un, FALSE, NULL], + + // Forbidden (ct) OR forbidden (ct,cf,un). + [$forbidden_ct, 'OR', $forbidden_ct, TRUE, TRUE], + [$forbidden_ct, 'OR', $forbidden_cf, TRUE, TRUE], + [$forbidden_ct, 'OR', $forbidden_un, TRUE, TRUE], + // Forbidden (cf) OR forbidden (ct,cf,un). + [$forbidden_cf, 'OR', $forbidden_ct, TRUE, TRUE], + [$forbidden_cf, 'OR', $forbidden_cf, TRUE, FALSE], + [$forbidden_cf, 'OR', $forbidden_un, TRUE, FALSE], + // Forbidden (un) OR forbidden (ct,cf,un). + [$forbidden_un, 'OR', $forbidden_ct, FALSE, NULL], + [$forbidden_un, 'OR', $forbidden_cf, FALSE, NULL], + [$forbidden_un, 'OR', $forbidden_un, FALSE, NULL], + + + // Neutral (ct) OR allowed (ct,cf,un). + [$neutral_ct, 'OR', $allowed_ct, TRUE, TRUE], + [$neutral_ct, 'OR', $allowed_cf, TRUE, FALSE], + [$neutral_ct, 'OR', $allowed_un, TRUE, FALSE], + // Neutral (cf) OR allowed (ct,cf,un). + [$neutral_cf, 'OR', $allowed_ct, TRUE, TRUE], + [$neutral_cf, 'OR', $allowed_cf, TRUE, FALSE], + [$neutral_cf, 'OR', $allowed_un, TRUE, FALSE], + // Neutral (un) OR allowed (ct,cf,un). + [$neutral_un, 'OR', $allowed_ct, FALSE, NULL], + [$neutral_un, 'OR', $allowed_cf, FALSE, NULL], + [$neutral_un, 'OR', $allowed_un, FALSE, NULL], + + // Neutral (ct) OR neutral (ct,cf,un). + [$neutral_ct, 'OR', $neutral_ct, TRUE, TRUE], + [$neutral_ct, 'OR', $neutral_cf, TRUE, TRUE], + [$neutral_ct, 'OR', $neutral_un, TRUE, TRUE], + // Neutral (cf) OR neutral (ct,cf,un). + [$neutral_cf, 'OR', $neutral_ct, TRUE, TRUE], + [$neutral_cf, 'OR', $neutral_cf, TRUE, FALSE], + [$neutral_cf, 'OR', $neutral_un, TRUE, FALSE], + // Neutral (un) OR neutral (ct,cf,un). + [$neutral_un, 'OR', $neutral_ct, FALSE, NULL], + [$neutral_un, 'OR', $neutral_cf, FALSE, NULL], + [$neutral_un, 'OR', $neutral_un, FALSE, NULL], + + // Neutral (ct) OR forbidden (ct,cf,un). + [$neutral_ct, 'OR', $forbidden_ct, TRUE, TRUE], + [$neutral_ct, 'OR', $forbidden_cf, TRUE, FALSE], + [$neutral_ct, 'OR', $forbidden_un, TRUE, FALSE], + // Neutral (cf) OR forbidden (ct,cf,un). + [$neutral_cf, 'OR', $forbidden_ct, TRUE, TRUE], + [$neutral_cf, 'OR', $forbidden_cf, TRUE, FALSE], + [$neutral_cf, 'OR', $forbidden_un, TRUE, FALSE], + // Neutral (un) OR forbidden (ct,cf,un). + [$neutral_un, 'OR', $forbidden_ct, FALSE, NULL], + [$neutral_un, 'OR', $forbidden_cf, FALSE, NULL], + [$neutral_un, 'OR', $forbidden_un, FALSE, NULL], + + + + + // Allowed (ct) AND allowed (ct,cf,un). + [$allowed_ct, 'AND', $allowed_ct, TRUE, TRUE], + [$allowed_ct, 'AND', $allowed_cf, TRUE, FALSE], + [$allowed_ct, 'AND', $allowed_un, TRUE, FALSE], + // Allowed (cf) AND allowed (ct,cf,un). + [$allowed_cf, 'AND', $allowed_ct, TRUE, FALSE], + [$allowed_cf, 'AND', $allowed_cf, TRUE, FALSE], + [$allowed_cf, 'AND', $allowed_un, TRUE, FALSE], + // Allowed (un) AND allowed (ct,cf,un). + [$allowed_un, 'AND', $allowed_ct, FALSE, NULL], + [$allowed_un, 'AND', $allowed_cf, FALSE, NULL], + [$allowed_un, 'AND', $allowed_un, FALSE, NULL], + + // Allowed (ct) AND forbidden (ct,cf,un). + [$allowed_ct, 'AND', $forbidden_ct, TRUE, TRUE], + [$allowed_ct, 'AND', $forbidden_cf, TRUE, FALSE], + [$allowed_ct, 'AND', $forbidden_un, TRUE, FALSE], + // Allowed (cf) AND forbidden (ct,cf,un). + [$allowed_cf, 'AND', $forbidden_ct, TRUE, TRUE], + [$allowed_cf, 'AND', $forbidden_cf, TRUE, FALSE], + [$allowed_cf, 'AND', $forbidden_un, TRUE, FALSE], + // Allowed (un) AND forbidden (ct,cf,un). + [$allowed_un, 'AND', $forbidden_ct, FALSE, NULL], + [$allowed_un, 'AND', $forbidden_cf, FALSE, NULL], + [$allowed_un, 'AND', $forbidden_un, FALSE, NULL], + + // Allowed (ct) AND neutral (ct,cf,un). + [$allowed_ct, 'AND', $neutral_ct, TRUE, TRUE], + [$allowed_ct, 'AND', $neutral_cf, TRUE, FALSE], + [$allowed_ct, 'AND', $neutral_un, TRUE, FALSE], + // Allowed (cf) AND neutral (ct,cf,un). + [$allowed_cf, 'AND', $neutral_ct, TRUE, FALSE], + [$allowed_cf, 'AND', $neutral_cf, TRUE, FALSE], + [$allowed_cf, 'AND', $neutral_un, TRUE, FALSE], + // Allowed (un) AND neutral (ct,cf,un). + [$allowed_un, 'AND', $neutral_ct, FALSE, NULL], + [$allowed_un, 'AND', $neutral_cf, FALSE, NULL], + [$allowed_un, 'AND', $neutral_un, FALSE, NULL], + + + // Forbidden (ct) AND allowed (ct,cf,un). + [$forbidden_ct, 'AND', $allowed_ct, TRUE, TRUE], + [$forbidden_ct, 'AND', $allowed_cf, TRUE, TRUE], + [$forbidden_ct, 'AND', $allowed_un, TRUE, TRUE], + // Forbidden (cf) AND allowed (ct,cf,un). + [$forbidden_cf, 'AND', $allowed_ct, TRUE, FALSE], + [$forbidden_cf, 'AND', $allowed_cf, TRUE, FALSE], + [$forbidden_cf, 'AND', $allowed_un, TRUE, FALSE], + // Forbidden (un) AND allowed (ct,cf,un). + [$forbidden_un, 'AND', $allowed_ct, FALSE, NULL], + [$forbidden_un, 'AND', $allowed_cf, FALSE, NULL], + [$forbidden_un, 'AND', $allowed_un, FALSE, NULL], + + // Forbidden (ct) AND neutral (ct,cf,un). + [$forbidden_ct, 'AND', $neutral_ct, TRUE, TRUE], + [$forbidden_ct, 'AND', $neutral_cf, TRUE, TRUE], + [$forbidden_ct, 'AND', $neutral_un, TRUE, TRUE], + // Forbidden (cf) AND neutral (ct,cf,un). + [$forbidden_cf, 'AND', $neutral_ct, TRUE, FALSE], + [$forbidden_cf, 'AND', $neutral_cf, TRUE, FALSE], + [$forbidden_cf, 'AND', $neutral_un, TRUE, FALSE], + // Forbidden (un) AND neutral (ct,cf,un). + [$forbidden_un, 'AND', $neutral_ct, FALSE, NULL], + [$forbidden_un, 'AND', $neutral_cf, FALSE, NULL], + [$forbidden_un, 'AND', $neutral_un, FALSE, NULL], + + // Forbidden (ct) AND forbidden (ct,cf,un). + [$forbidden_ct, 'AND', $forbidden_ct, TRUE, TRUE], + [$forbidden_ct, 'AND', $forbidden_cf, TRUE, TRUE], + [$forbidden_ct, 'AND', $forbidden_un, TRUE, TRUE], + // Forbidden (cf) AND forbidden (ct,cf,un). + [$forbidden_cf, 'AND', $forbidden_ct, TRUE, FALSE], + [$forbidden_cf, 'AND', $forbidden_cf, TRUE, FALSE], + [$forbidden_cf, 'AND', $forbidden_un, TRUE, FALSE], + // Forbidden (un) AND forbidden (ct,cf,un). + [$forbidden_un, 'AND', $forbidden_ct, FALSE, NULL], + [$forbidden_un, 'AND', $forbidden_cf, FALSE, NULL], + [$forbidden_un, 'AND', $forbidden_un, FALSE, NULL], + + + // Neutral (ct) AND allowed (ct,cf,un). + [$neutral_ct, 'AND', $allowed_ct, TRUE, TRUE], + [$neutral_ct, 'AND', $allowed_cf, TRUE, TRUE], + [$neutral_ct, 'AND', $allowed_un, TRUE, TRUE], + // Neutral (cf) AND allowed (ct,cf,un). + [$neutral_cf, 'AND', $allowed_ct, TRUE, FALSE], + [$neutral_cf, 'AND', $allowed_cf, TRUE, FALSE], + [$neutral_cf, 'AND', $allowed_un, TRUE, FALSE], + // Neutral (un) AND allowed (ct,cf,un). + [$neutral_un, 'AND', $allowed_ct, FALSE, NULL], + [$neutral_un, 'AND', $allowed_cf, FALSE, NULL], + [$neutral_un, 'AND', $allowed_un, FALSE, NULL], + + // Neutral (ct) AND neutral (ct,cf,un). + [$neutral_ct, 'AND', $neutral_ct, TRUE, TRUE], + [$neutral_ct, 'AND', $neutral_cf, TRUE, TRUE], + [$neutral_ct, 'AND', $neutral_un, TRUE, TRUE], + // Neutral (cf) AND neutral (ct,cf,un). + [$neutral_cf, 'AND', $neutral_ct, TRUE, FALSE], + [$neutral_cf, 'AND', $neutral_cf, TRUE, FALSE], + [$neutral_cf, 'AND', $neutral_un, TRUE, FALSE], + // Neutral (un) AND neutral (ct,cf,un). + [$neutral_un, 'AND', $neutral_ct, FALSE, NULL], + [$neutral_un, 'AND', $neutral_cf, FALSE, NULL], + [$neutral_un, 'AND', $neutral_un, FALSE, NULL], + + // Neutral (ct) AND forbidden (ct,cf,un). + [$neutral_ct, 'AND', $forbidden_ct, TRUE, TRUE], + [$neutral_ct, 'AND', $forbidden_cf, TRUE, FALSE], + [$neutral_ct, 'AND', $forbidden_un, TRUE, FALSE], + // Neutral (cf) AND forbidden (ct,cf,un). + [$neutral_cf, 'AND', $forbidden_ct, TRUE, TRUE], + [$neutral_cf, 'AND', $forbidden_cf, TRUE, FALSE], + [$neutral_cf, 'AND', $forbidden_un, TRUE, FALSE], + // Neutral (un) AND forbidden (ct,cf,un). + [$neutral_un, 'AND', $forbidden_ct, FALSE, NULL], + [$neutral_un, 'AND', $forbidden_cf, FALSE, NULL], + [$neutral_un, 'AND', $forbidden_un, FALSE, NULL], + ]; + } - // andIf(); 1st is cacheable, 2nd isn't. - $access = AccessResult::allowed()->andIf(AccessResult::allowed()->setCacheable(FALSE)); - $this->assertTrue($access->isAllowed()); - $this->assertFalse($access->isForbidden()); - $this->assertFalse($access->isCacheable()); + /** + * @covers ::andIf + * @covers ::orIf + * @covers ::inheritCacheability + * + * @dataProvider testAndOrCacheabilityPropagationProvider + */ + public function testAndOrCacheabilityPropagation(AccessResultInterface $first, $op, AccessResultInterface $second, $implements_cacheable_interface, $is_cacheable) { + if ($op === 'OR') { + $result = $first->orIf($second); + } + else if ($op === 'AND') { + $result = $first->andIf($second); + } + else { + throw new \LogicException('Invalid operator specified'); + } + if ($implements_cacheable_interface) { + $this->assertTrue($result instanceof CacheableInterface, 'Result is an instance of CacheableInterface.'); + if ($result instanceof CacheableInterface) { + $this->assertSame($is_cacheable, $result->isCacheable(), 'isCacheable() matches expectations.'); + } + } + else { + $this->assertFalse($result instanceof CacheableInterface, 'Result is not an instance of CacheableInterface.'); + } + } - // andIf(); 1st implements CacheableInterface, 2nd doesn't. - $access = AccessResult::allowed()->andIf($access_without_cacheability); - $this->assertTrue($access->isAllowed()); - $this->assertFalse($access->isForbidden()); - $this->assertFalse($access->isCacheable()); +} - // andIf(); 1st doesn't implement CacheableInterface, 2nd does. - $access = $access_without_cacheability->andIf(AccessResult::allowed()); - $this->assertTrue($access->isAllowed()); - $this->assertFalse($access->isForbidden()); - $this->assertFalse(method_exists($access, 'isCacheable')); +class UncacheableTestAccessResult implements AccessResultInterface { + + /** + * The access result value. 'ALLOWED', 'FORBIDDEN' or 'NEUTRAL'. + * + * @var string + */ + protected $value; + + /** + * Constructs a new UncacheableTestAccessResult object. + */ + public function __construct($value) { + $this->value = $value; + } + /** + * {@inheritdoc} + */ + public function isAllowed() { + return $this->value === 'ALLOWED'; + } + + /** + * {@inheritdoc} + */ + public function isForbidden() { + return $this->value === 'FORBIDDEN'; + } + + /** + * {@inheritdoc} + */ + public function isNeutral() { + return $this->value === 'NEUTRAL'; + } + + /** + * {@inheritdoc} + */ + public function orIf(AccessResultInterface $other) { + if ($this->isForbidden() || $other->isForbidden()) { + return new static('FORBIDDEN'); + } + elseif ($this->isAllowed() || $other->isAllowed()) { + return new static('ALLOWED'); + } + else { + return new static('NEUTRAL'); + } + } + + /** + * {@inheritdoc} + */ + public function andIf(AccessResultInterface $other) { + if ($this->isForbidden() || $other->isForbidden()) { + return new static('FORBIDDEN'); + } + elseif ($this->isAllowed() && $other->isAllowed()) { + return new static('ALLOWED'); + } + else { + return new static('NEUTRAL'); + } } }