Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1880,6 +1880,7 @@
'OC\\Group\\Database' => $baseDir . '/lib/private/Group/Database.php',
'OC\\Group\\DisplayNameCache' => $baseDir . '/lib/private/Group/DisplayNameCache.php',
'OC\\Group\\Group' => $baseDir . '/lib/private/Group/Group.php',
'OC\\Group\\LazyGroup' => $baseDir . '/lib/private/Group/LazyGroup.php',
'OC\\Group\\Manager' => $baseDir . '/lib/private/Group/Manager.php',
'OC\\Group\\MetaData' => $baseDir . '/lib/private/Group/MetaData.php',
'OC\\HintException' => $baseDir . '/lib/private/HintException.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Group\\Database' => __DIR__ . '/../../..' . '/lib/private/Group/Database.php',
'OC\\Group\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/Group/DisplayNameCache.php',
'OC\\Group\\Group' => __DIR__ . '/../../..' . '/lib/private/Group/Group.php',
'OC\\Group\\LazyGroup' => __DIR__ . '/../../..' . '/lib/private/Group/LazyGroup.php',
'OC\\Group\\Manager' => __DIR__ . '/../../..' . '/lib/private/Group/Manager.php',
'OC\\Group\\MetaData' => __DIR__ . '/../../..' . '/lib/private/Group/MetaData.php',
'OC\\HintException' => __DIR__ . '/../../..' . '/lib/private/HintException.php',
Expand Down
6 changes: 6 additions & 0 deletions lib/private/Group/Group.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
/** @var User[] */
private array $users = [];
private bool $usersLoaded = false;
private bool $isDeleted = false;

public function __construct(
private string $gid,
Expand Down Expand Up @@ -336,6 +337,7 @@
$this->emitter->emit('\OC\Group', 'postDelete', [$this]);
}
}
$this->isDeleted = $result;
return $result;
}

Expand Down Expand Up @@ -393,4 +395,8 @@
return $hide || ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid));
}, false);
}

public function isDeleted(): bool {

Check failure on line 399 in lib/private/Group/Group.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

MissingOverrideAttribute

lib/private/Group/Group.php:399:2: MissingOverrideAttribute: Method OC\Group\Group::isdeleted should have the "Override" attribute (see https://psalm.dev/358)
return $this->isDeleted;
}
}
132 changes: 132 additions & 0 deletions lib/private/Group/LazyGroup.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php

Comment thread
CarlSchwan marked this conversation as resolved.
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/

namespace OC\Group;

use OCP\EventDispatcher\IEventDispatcher;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Server;
use Psr\Log\LoggerInterface;

class LazyGroup implements IGroup {
private ?IGroup $group = null;
private ?bool $isDeleted = null;

public function __construct(
private string $gid,
private IGroupManager $groupManager,
) {
}

#[\Override]
public function getGID(): string {
return $this->gid;
}

private function getGroup(): IGroup {
if ($this->group === null) {
$this->group = $this->groupManager->get($this->gid);
;
Comment thread
CarlSchwan marked this conversation as resolved.
Outdated
}
if ($this->group === null) {
Server::get(LoggerInterface::class)->debug('Trying to use the deleted group: "' . $this->gid . '"', ['app' => 'core']);
$this->isDeleted = true;
$this->group = new Group($this->gid, [], Server::get(IEventDispatcher::class), Server::get(IUserManager::class));
} else {
$this->isDeleted = false;
}
return $this->group;
}

#[\Override]
public function getDisplayName(): string {

Check failure on line 50 in lib/private/Group/LazyGroup.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidNullableReturnType

lib/private/Group/LazyGroup.php:50:36: InvalidNullableReturnType: The declared return type 'string' for OC\Group\LazyGroup::getDisplayName is not nullable, but 'null|string' contains null (see https://psalm.dev/144)
// Use display name cache from IGroupManager
return $this->groupManager->getDisplayName($this->gid);

Check failure on line 52 in lib/private/Group/LazyGroup.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

NullableReturnStatement

lib/private/Group/LazyGroup.php:52:10: NullableReturnStatement: The declared return type 'string' for OC\Group\LazyGroup::getDisplayName is not nullable, but the function returns 'null|string' (see https://psalm.dev/139)
}

#[\Override]
public function setDisplayName(string $displayName): bool {
return $this->group->setDisplayName($displayName);
Comment thread
CarlSchwan marked this conversation as resolved.
Outdated
}

#[\Override]
public function getUsers(): array {
return $this->getGroup()->getUsers();
}

#[\Override]
public function inGroup(IUser $user): bool {
return $this->getGroup()->inGroup($user);
}

#[\Override]
public function addUser(IUser $user): void {
$this->getGroup()->addUser($user);
}

#[\Override]
public function removeUser(IUser $user): void {
$this->getGroup()->removeUser($user);
}

#[\Override]
public function searchUsers(string $search, ?int $limit = null, ?int $offset = null): array {
return $this->getGroup()->searchUsers($search, $limit, $offset);
}

#[\Override]
public function count($search = ''): int|bool {
return $this->getGroup()->count($search);
}

#[\Override]
public function countDisabled(): int|bool {
return $this->getGroup()->countDisabled();
}

#[\Override]
public function searchDisplayName(string $search, ?int $limit = null, ?int $offset = null): array {
return $this->getGroup()->searchDisplayName($search, $limit, $offset);
}

#[\Override]
public function getBackendNames(): array {
return $this->getGroup()->getBackendNames();
}

#[\Override]
public function delete(): bool {
return $this->getGroup()->delete();
}

#[\Override]
public function canRemoveUser(): bool {
return $this->getGroup()->canRemoveUser();
}

#[\Override]
public function canAddUser(): bool {
return $this->getGroup()->canAddUser();
}

#[\Override]
public function hideFromCollaboration(): bool {
return $this->getGroup()->hideFromCollaboration();
}

#[\Override]
public function isDeleted(): bool {
if ($this->isDeleted === null) {
$this->getGroup();
}
Comment thread
CarlSchwan marked this conversation as resolved.
Outdated
return $this->isDeleted === true ? true : $this->getGroup()->isDeleted();
}
}
19 changes: 1 addition & 18 deletions lib/private/Group/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,7 @@ public function getUserIdGroups(string $uid): array {
$groups = [];

foreach ($this->getUserIdGroupIds($uid) as $groupId) {
$aGroup = $this->get($groupId);
if ($aGroup instanceof IGroup) {
$groups[$groupId] = $aGroup;
} else {
$this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
Comment thread
CarlSchwan marked this conversation as resolved.
}
$groups[$groupId] = new LazyGroup($groupId, $this);
}

return $groups;
Expand Down Expand Up @@ -401,18 +396,6 @@ public function getDisplayName(string $groupId): ?string {
return $this->displayNameCache->getDisplayName($groupId);
}

/**
* get an array of groupid and displayName for a user
*
* @param IUser $user
* @return array ['displayName' => displayname]
*/
public function getUserGroupNames(IUser $user) {
return array_map(function ($group) {
return ['displayName' => $this->displayNameCache->getDisplayName($group->getGID())];
}, $this->getUserGroups($user));
}

#[\Override]
public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) {
$group = $this->get($gid);
Expand Down
7 changes: 7 additions & 0 deletions lib/public/IGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,11 @@ public function canAddUser(): bool;
* @since 16.0.0
*/
public function hideFromCollaboration(): bool;

/**
* Return whether the group is deleted.
*
* @since 35.0.0
*/
public function isDeleted(): bool;
}
30 changes: 9 additions & 21 deletions tests/lib/Group/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,11 @@ abstract class TestBackend extends ABackend implements ISearchableGroupBackend,
}

class ManagerTest extends TestCase {
/** @var Manager|MockObject */
protected $userManager;
/** @var IEventDispatcher|MockObject */
protected $dispatcher;
/** @var LoggerInterface|MockObject */
protected $logger;
/** @var ICacheFactory|MockObject */
private $cache;
/** @var IRemoteAddress|MockObject */
private $remoteIpAddress;
protected Manager&MockObject $userManager;
protected IEventDispatcher&MockObject $dispatcher;
protected LoggerInterface&MockObject $logger;
private ICacheFactory&MockObject $cache;
private IRemoteAddress&MockObject $remoteIpAddress;

#[\Override]
protected function setUp(): void {
Expand All @@ -55,7 +50,7 @@ protected function setUp(): void {
$this->remoteIpAddress->method('allowsAdminActions')->willReturn(true);
}

private function getTestUser($userId) {
private function getTestUser(string $userId): IUser&MockObject {
$mockUser = $this->createMock(IUser::class);
$mockUser->expects($this->any())
->method('getUID')
Expand All @@ -68,9 +63,8 @@ private function getTestUser($userId) {

/**
* @param null|int $implementedActions
* @return \PHPUnit\Framework\MockObject\MockObject
*/
private function getTestBackend($implementedActions = null) {
private function getTestBackend(?int $implementedActions = null): MockObject&TestBackend {
if ($implementedActions === null) {
$implementedActions
= GroupInterface::ADD_TO_GROUP
Expand Down Expand Up @@ -467,9 +461,6 @@ public function testGetUserGroupIds(): void {
}

public function testGetUserGroupsWithDeletedGroup(): void {
/**
* @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend
*/
$backend = $this->createMock(Database::class);
$backend->expects($this->once())
->method('getUserGroups')
Expand All @@ -483,20 +474,17 @@ public function testGetUserGroupsWithDeletedGroup(): void {
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);

/** @var User|\PHPUnit\Framework\MockObject\MockObject $user */
$user = $this->createMock(IUser::class);
$user->expects($this->atLeastOnce())
->method('getUID')
->willReturn('user1');

$groups = $manager->getUserGroups($user);
$this->assertEmpty($groups);
$this->assertCount(1, $groups);
$this->assertTrue($groups['group1']->isDeleted());
}

public function testInGroup(): void {
/**
* @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend
*/
$backend = $this->getTestBackend();
$backend->expects($this->once())
->method('getUserGroups')
Expand Down
Loading