diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php index a27c4ef89c536..0b041e1abc81a 100644 --- a/lib/private/Group/Group.php +++ b/lib/private/Group/Group.php @@ -104,6 +104,24 @@ public function getUsers(): array { return $this->users; } + $this->users = $this->getVerifiedUsers($this->getBackendUserIds()); + $this->usersLoaded = true; + return $this->users; + } + + #[\Override] + public function getUserIds(): array { + if ($this->usersLoaded) { + return array_keys($this->users); + } + + return $this->getBackendUserIds(); + } + + /** + * @return list + */ + private function getBackendUserIds(): array { $userIds = []; foreach ($this->backends as $backend) { $diff = array_diff( @@ -114,10 +132,7 @@ public function getUsers(): array { $userIds = array_merge($userIds, $diff); } } - - $this->users = $this->getVerifiedUsers($userIds); - $this->usersLoaded = true; - return $this->users; + return array_values($userIds); } /** diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 0fff3fd0644bc..17391898b5a8d 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -1472,9 +1472,7 @@ public function getAccessList($nodes, $currentAccess) { continue; } - $userList = $group->getUsers(); - foreach ($userList as $user) { - $uid = $user->getUID(); + foreach ($group->getUserIds() as $uid) { $users[$uid] = $users[$uid] ?? []; $users[$uid][$row['id']] = $row; } diff --git a/lib/public/IGroup.php b/lib/public/IGroup.php index 33da553d1ee38..00f4595dccbe1 100644 --- a/lib/public/IGroup.php +++ b/lib/public/IGroup.php @@ -46,6 +46,17 @@ public function setDisplayName(string $displayName): bool; */ public function getUsers(): array; + /** + * Get the ids of all users in the group + * + * Unlike {@see self::getUsers()} the ids are not checked against the user + * backends, so they can contain users that no longer exist. + * + * @return list + * @since 36.0.0 + */ + public function getUserIds(): array; + /** * check if a user is in the group * diff --git a/tests/lib/Group/GroupTest.php b/tests/lib/Group/GroupTest.php index 50181a3ea2a31..fc413358c9ed5 100644 --- a/tests/lib/Group/GroupTest.php +++ b/tests/lib/Group/GroupTest.php @@ -8,7 +8,9 @@ namespace Test\Group; +use OC\Group\Database; use OC\Group\Group; +use OC\User\Manager; use OC\User\User; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\BeforeGroupChangedEvent; @@ -116,6 +118,40 @@ public function testGetUsersMultipleBackends(): void { $this->assertEquals('user3', $user3->getUID()); } + public function testGetUserIdsMultipleBackends(): void { + $backend1 = $this->createMock(Database::class); + $backend2 = $this->createMock(Database::class); + $userManager = $this->createMock(Manager::class); + $userManager->expects($this->never()) + ->method('get'); + $group = new Group('group1', [$backend1, $backend2], $this->dispatcher, $userManager); + + $backend1->expects($this->once()) + ->method('usersInGroup') + ->with('group1') + ->willReturn(['user1', 'user2']); + $backend2->expects($this->once()) + ->method('usersInGroup') + ->with('group1') + ->willReturn(['user2', '3']); + + $this->assertSame(['user1', 'user2', '3'], $group->getUserIds()); + } + + public function testGetUserIdsReusesLoadedUsers(): void { + $backend = $this->createMock(Database::class); + $userManager = $this->getUserManager(); + $group = new Group('group1', [$backend], $this->dispatcher, $userManager); + + $backend->expects($this->once()) + ->method('usersInGroup') + ->with('group1') + ->willReturn(['user1', 'user2']); + + $group->getUsers(); + $this->assertSame(['user1', 'user2'], $group->getUserIds()); + } + public function testInGroupSingleBackend(): void { $backend = $this->getMockBuilder('OC\Group\Database') ->disableOriginalConstructor()