diff --git a/core/modules/block_content/src/Access/AccessGroupAnd.php b/core/modules/block_content/src/Access/AccessGroupAnd.php index 85a90d648a..f8b7c1fa67 100644 --- a/core/modules/block_content/src/Access/AccessGroupAnd.php +++ b/core/modules/block_content/src/Access/AccessGroupAnd.php @@ -2,20 +2,49 @@ namespace Drupal\block_content\Access; -use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Access\AccessibleInterface; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Session\AccountInterface; /** * An access group where all the dependencies must be allowed. * * @internal */ -class AccessGroupAnd extends AccessibleGroupBase { +class AccessGroupAnd implements AccessibleInterface{ + + + /** + * The access dependencies. + * + * @var \Drupal\Core\Access\AccessibleInterface[] + */ + protected $dependencies = []; + + /** + * {@inheritdoc} + */ + public function addDependency(AccessibleInterface $dependency) { + $this->dependencies[] = $dependency; + return $this; + } + + /** + * {@inheritdoc} + */ + public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { + $access_result = AccessResult::neutral(); + foreach (array_slice($this->dependencies, 1) as $dependency) { + $access_result = $access_result->andIf($dependency->access($operation, $account, TRUE)); + } + return $return_as_object ? $access_result : $access_result->isAllowed(); + } /** * {@inheritdoc} */ - protected function combineAccess(AccessResultInterface $accumulated_access, AccessResultInterface $dependency_access) { - return $accumulated_access->andIf($dependency_access); + public function getDependencies() { + return $this->dependencies; } } diff --git a/core/modules/block_content/src/Access/AccessGroupOr.php b/core/modules/block_content/src/Access/AccessGroupOr.php deleted file mode 100644 index 2414a048f4..0000000000 --- a/core/modules/block_content/src/Access/AccessGroupOr.php +++ /dev/null @@ -1,21 +0,0 @@ -orIf($dependency_access); - } - -} diff --git a/core/modules/block_content/src/Access/AccessibleGroupBase.php b/core/modules/block_content/src/Access/AccessibleGroupBase.php deleted file mode 100644 index 174724c782..0000000000 --- a/core/modules/block_content/src/Access/AccessibleGroupBase.php +++ /dev/null @@ -1,68 +0,0 @@ -dependencies[] = $dependency; - return $this; - } - - /** - * {@inheritdoc} - */ - public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { - if (empty($this->dependencies)) { - return AccessResult::neutral(); - } - $access_result = $this->dependencies[0]->access($operation, $account, TRUE); - if (count($this->dependencies) > 1) { - foreach (array_slice($this->dependencies, 1) as $dependency) { - $access_result = $this->combineAccess($access_result, $dependency->access($operation, $account, TRUE)); - } - } - return $return_as_object ? $access_result : $access_result->isAllowed(); - } - - /** - * {@inheritdoc} - */ - public function getDependencies() { - return $this->dependencies; - } - - /** - * Combines the access result of one dependency to previous dependencies. - * - * @param \Drupal\Core\Access\AccessResultInterface $accumulated_access - * The combined access result of previous dependencies. - * @param \Drupal\Core\Access\AccessResultInterface $dependency_access - * The access result of the current dependency. - * - * @return \Drupal\Core\Access\AccessResultInterface - * The combined access result. - */ - abstract protected function combineAccess(AccessResultInterface $accumulated_access, AccessResultInterface $dependency_access); - -} diff --git a/core/modules/block_content/src/Access/AccessibleGroupInterface.php b/core/modules/block_content/src/Access/AccessibleGroupInterface.php deleted file mode 100644 index 991eacd40b..0000000000 --- a/core/modules/block_content/src/Access/AccessibleGroupInterface.php +++ /dev/null @@ -1,32 +0,0 @@ -accessDependency = $access_dependency; return $this; } - if (!$this->accessDependency instanceof AccessibleGroupInterface) { + if (!$this->accessDependency instanceof AccessGroupAnd) { $accessGroup = new AccessGroupAnd(); $this->accessDependency = $accessGroup->addDependency($this->accessDependency); } diff --git a/core/modules/block_content/src/BlockContentEvents.php b/core/modules/block_content/src/BlockContentEvents.php index 661f98de51..85931b7a72 100644 --- a/core/modules/block_content/src/BlockContentEvents.php +++ b/core/modules/block_content/src/BlockContentEvents.php @@ -6,6 +6,8 @@ * Defines events for the block_content module. * * @see \Drupal\block_content\Event\BlockContentGetDependencyEvent + * + * @internal */ final class BlockContentEvents { diff --git a/core/modules/block_content/src/Event/BlockContentGetDependencyEvent.php b/core/modules/block_content/src/Event/BlockContentGetDependencyEvent.php index 977c8593fc..e705172aa5 100644 --- a/core/modules/block_content/src/Event/BlockContentGetDependencyEvent.php +++ b/core/modules/block_content/src/Event/BlockContentGetDependencyEvent.php @@ -8,6 +8,8 @@ /** * Block content event to allow setting an access dependency. + * + * @internal */ class BlockContentGetDependencyEvent extends Event { diff --git a/core/modules/block_content/tests/src/Unit/Access/AccessGroupTest.php b/core/modules/block_content/tests/src/Unit/Access/AccessGroupAndTest.php similarity index 59% rename from core/modules/block_content/tests/src/Unit/Access/AccessGroupTest.php rename to core/modules/block_content/tests/src/Unit/Access/AccessGroupAndTest.php index 639af5af86..8915674862 100644 --- a/core/modules/block_content/tests/src/Unit/Access/AccessGroupTest.php +++ b/core/modules/block_content/tests/src/Unit/Access/AccessGroupAndTest.php @@ -3,7 +3,6 @@ namespace Drupal\Tests\block_content\Unit\Access; use Drupal\block_content\Access\AccessGroupAnd; -use Drupal\block_content\Access\AccessGroupOr; use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\Tests\UnitTestCase; @@ -11,9 +10,9 @@ /** * Tests accessible groups. * - * @group Access + * @group block_content */ -class AccessGroupTest extends UnitTestCase { +class AccessGroupAndTest extends UnitTestCase { use AccessibleTestingTrait; @@ -27,42 +26,28 @@ protected function setUp() { /** * @covers \Drupal\block_content\Access\AccessGroupAnd - * @covers \Drupal\block_content\Access\AccessGroupOr */ public function testGroups() { $allowedAccessible = $this->createAccessibleDouble(AccessResult::allowed()); $forbiddenAccessible = $this->createAccessibleDouble(AccessResult::forbidden()); $neutralAccessible = $this->createAccessibleDouble(AccessResult::neutral()); - // Ensure that groups with no dependencies return a forbidden access result. - $this->assertTrue((new AccessGroupOr())->access('view', $this->account, TRUE)->isNeutral()); + // Ensure that groups with no dependencies return a neutral access result. $this->assertTrue((new AccessGroupAnd())->access('view', $this->account, TRUE)->isNeutral()); - $orForbidden = new AccessGroupOr(); - $orForbidden->addDependency($allowedAccessible)->addDependency($forbiddenAccessible); - $this->assertTrue($orForbidden->access('view', $this->account, TRUE)->isForbidden()); - - $orAllowed = new AccessGroupOr(); - $orAllowed->addDependency($allowedAccessible)->addDependency($neutralAccessible); - $this->assertTrue($orAllowed->access('view', $this->account, TRUE)->isAllowed()); - $andNeutral = new AccessGroupAnd(); $andNeutral->addDependency($allowedAccessible)->addDependency($neutralAccessible); $this->assertTrue($andNeutral->access('view', $this->account, TRUE)->isNeutral()); - // We can also add groups and dependencies!!!!! Nested!!!!! - $andNeutral->addDependency($orAllowed); - $this->assertTrue($andNeutral->access('view', $this->account, TRUE)->isNeutral()); - $andForbidden = $andNeutral; $andForbidden->addDependency($forbiddenAccessible); $this->assertTrue($andForbidden->access('view', $this->account, TRUE)->isForbidden()); - // We can make groups from other groups! + // Ensure that groups added to other groups works. $andGroupsForbidden = new AccessGroupAnd(); - $andGroupsForbidden->addDependency($andNeutral)->addDependency($andForbidden)->addDependency($orForbidden); + $andGroupsForbidden->addDependency($andNeutral)->addDependency($andForbidden); $this->assertTrue($andGroupsForbidden->access('view', $this->account, TRUE)->isForbidden()); - // But then would could also add a non-group accessible. + // Ensure you can add a non-group accessible object. $andGroupsForbidden->addDependency($allowedAccessible); $this->assertTrue($andGroupsForbidden->access('view', $this->account, TRUE)->isForbidden()); } diff --git a/core/modules/block_content/tests/src/Unit/Access/DependentAccessTest.php b/core/modules/block_content/tests/src/Unit/Access/DependentAccessTest.php index 7637790123..2be368601d 100644 --- a/core/modules/block_content/tests/src/Unit/Access/DependentAccessTest.php +++ b/core/modules/block_content/tests/src/Unit/Access/DependentAccessTest.php @@ -3,8 +3,6 @@ namespace Drupal\Tests\block_content\Unit\Access; use Drupal\block_content\Access\AccessGroupAnd; -use Drupal\block_content\Access\AccessGroupOr; -use Drupal\block_content\Access\AccessibleGroupInterface; use Drupal\Core\Access\AccessResult; use Drupal\block_content\Access\RefinableDependentAccessInterface; use Drupal\block_content\Access\RefinableDependentAccessTrait; @@ -14,7 +12,7 @@ /** * @coversDefaultClass \Drupal\block_content\Access\RefinableDependentAccessTrait * - * @group Access + * @group block_content */ class DependentAccessTest extends UnitTestCase { use AccessibleTestingTrait; @@ -66,7 +64,7 @@ public function testSetAccessDependency($use_set_first) { // Calling setAccessDependency() replaces the existing dependency. $testRefinable->setAccessDependency($this->neutral); $dependency = $testRefinable->getAccessDependency(); - $this->assertFalse($dependency instanceof AccessibleGroupInterface); + $this->assertFalse($dependency instanceof AccessGroupAnd); $accessResult = $dependency->access('view', $this->account, TRUE); $this->assertTrue($accessResult->isNeutral()); $this->assertEquals('I have no opinion', $accessResult->getReason()); @@ -111,22 +109,22 @@ public function testMergeNonGroup($use_set_first) { * @dataProvider providerTestSetFirst */ public function testMergeGroup($use_set_first) { - $orGroup = new AccessGroupOr(); - $orGroup->addDependency($this->forbidden); + $andGroup = new AccessGroupAnd(); + $andGroup->addDependency($this->forbidden); $testRefinable = new RefinableDependentAccessTraitTestClass(); if ($use_set_first) { - $testRefinable->setAccessDependency($orGroup); + $testRefinable->setAccessDependency($andGroup); } else { - $testRefinable->addAccessDependency($orGroup); + $testRefinable->addAccessDependency($andGroup); } $testRefinable->addAccessDependency($this->neutral); - /** @var \Drupal\block_content\Access\AccessGroupOr $dependency */ + /** @var \Drupal\block_content\Access\AccessGroupAnd $dependency */ $dependency = $testRefinable->getAccessDependency(); // Ensure the new dependency is merged with the existing group. - $this->assertTrue($dependency instanceof AccessGroupOr); + $this->assertTrue($dependency instanceof AccessGroupAnd); $dependencies = $dependency->getDependencies(); $accessResultForbidden = $dependencies[0]->access('view', $this->account, TRUE); $this->assertTrue($accessResultForbidden->isForbidden());