diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 924b231fd0c67..ef83043a070d7 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -8,8 +8,6 @@ namespace OCA\Files_Sharing\AppInfo; -use OC\Group\DisplayNameCache as GroupDisplayNameCache; -use OC\User\DisplayNameCache; use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Files\Event\LoadSidebar; use OCA\Files_Sharing\Capabilities; @@ -50,7 +48,6 @@ use OCP\Files\Events\Node\BeforeNodeReadEvent; use OCP\Files\Events\UserHomeSetupEvent; use OCP\Group\Events\BeforeGroupDeletedEvent; -use OCP\Group\Events\GroupChangedEvent; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; @@ -61,8 +58,6 @@ use OCP\Share\Events\ShareCreatedEvent; use OCP\Share\Events\ShareMovedEvent; use OCP\Share\Events\ShareTransferredEvent; -use OCP\User\Events\UserChangedEvent; -use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Psr\Container\ContainerInterface; @@ -96,10 +91,6 @@ function () use ($c) { $context->registerCapability(Capabilities::class); $context->registerNotifierService(Notifier::class); - $context->registerEventListener(UserChangedEvent::class, DisplayNameCache::class); - $context->registerEventListener(UserDeletedEvent::class, DisplayNameCache::class); - $context->registerEventListener(GroupChangedEvent::class, GroupDisplayNameCache::class); - $context->registerEventListener(GroupDeletedEvent::class, GroupDisplayNameCache::class); // Sidebar and files scripts $context->registerEventListener(LoadAdditionalScriptsEvent::class, LoadAdditionalListener::class); diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 88656772b825c..3e0450c6c8d04 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -130,7 +130,7 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA } else { if ($this->appManager->isEnabledForUser('user_ldap')) { $isLDAPUsed - = $this->groupManager->isBackendUsed('\OCA\User_LDAP\Group_Proxy'); + = $this->groupManager->isBackendUsed(\OCA\User_LDAP\Group_Proxy::class); if ($isLDAPUsed) { // LDAP user count can be slow, so we sort by group name here $sortGroupsBy = MetaData::SORT_GROUPNAME; diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index a0e94f456c5cf..2655d18a97c9f 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2673,6 +2673,11 @@ + + + + + diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index 31b79d4e85213..930d73fd1dd84 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -9,6 +9,7 @@ namespace OC\Core\AppInfo; use NCU\Sharing\ISharingRegistry; +use OC\App\Listeners\RemoveGroupRestrictionsListener; use OC\Authentication\Events\RemoteWipeFinished; use OC\Authentication\Events\RemoteWipeStarted; use OC\Authentication\Listeners\RemoteWipeActivityListener; @@ -41,8 +42,11 @@ use OC\Core\Sharing\Recipient\UserShareRecipientType; use OC\DirectEditing\Listeners\UserDeletedTokenCleanupListener as UserDeletedDirectEditingTokenCleanupListener; use OC\DirectEditing\Listeners\UserDisabledTokenCleanupListener as UserDisabledDirectEditingTokenCleanupListener; +use OC\Group\DisplayNameCache as GroupDisplayNameCache; +use OC\Group\Listeners\MembershipCacheListener; use OC\OCM\OCMDiscoveryHandler; use OC\TagManager; +use OC\User\DisplayNameCache; use OCP\Accounts\UserUpdatedEvent; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; @@ -52,6 +56,13 @@ use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; use OCP\DB\Events\AddMissingIndicesEvent; use OCP\DB\Events\AddMissingPrimaryKeyEvent; +use OCP\Group\Events\BeforeGroupDeletedEvent; +use OCP\Group\Events\BeforeUserAddedEvent; +use OCP\Group\Events\BeforeUserRemovedEvent; +use OCP\Group\Events\GroupChangedEvent; +use OCP\Group\Events\GroupDeletedEvent; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; use OCP\IAppConfig; use OCP\Interaction\RestrictInteractionEvent; use OCP\Navigation\Events\LoadAdditionalEntriesEvent; @@ -109,6 +120,18 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(UserUpdatedEvent::class, AvatarVersionListener::class); $context->registerEventListener(UserChangedEvent::class, AvatarVersionListener::class); + $context->registerEventListener(UserChangedEvent::class, DisplayNameCache::class); + $context->registerEventListener(UserDeletedEvent::class, DisplayNameCache::class); + $context->registerEventListener(GroupChangedEvent::class, GroupDisplayNameCache::class); + $context->registerEventListener(GroupDeletedEvent::class, GroupDisplayNameCache::class); + $context->registerEventListener(BeforeUserAddedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(UserAddedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(BeforeUserRemovedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(UserRemovedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(BeforeGroupDeletedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(GroupDeletedEvent::class, MembershipCacheListener::class); + $context->registerEventListener(GroupDeletedEvent::class, RemoveGroupRestrictionsListener::class); + // Tags $context->registerEventListener(UserDeletedEvent::class, TagManager::class); diff --git a/lib/OC.php b/lib/OC.php index d68c206068eff..06ac993c6864f 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -913,7 +913,6 @@ public static function initForRequest(): void { self::registerResourceCollectionHooks(); self::registerFileReferenceEventListener(); self::registerRenderReferenceEventListener(); - self::registerAppRestrictionsHooks(); // Make sure that the application class is not loaded before the database is setup if ($systemConfig->getValue('installed', false)) { @@ -1063,29 +1062,6 @@ private static function registerAccountHooks(): void { $dispatcher->addServiceListener(UserChangedEvent::class, \OC\Accounts\Hooks::class); } - private static function registerAppRestrictionsHooks(): void { - $eventDispatcher = Server::get(IEventDispatcher::class); - $eventDispatcher->addListener(GroupDeletedEvent::class, function (GroupDeletedEvent $event) { - $group = $event->getGroup(); - $appManager = Server::get(\OCP\App\IAppManager::class); - $apps = $appManager->getEnabledAppsForGroup($group); - foreach ($apps as $appId) { - $restrictions = $appManager->getAppRestriction($appId); - if (empty($restrictions)) { - continue; - } - $key = array_search($group->getGID(), $restrictions, true); - unset($restrictions[$key]); - $restrictions = array_values($restrictions); - if (empty($restrictions)) { - $appManager->disableApp($appId); - } else { - $appManager->enableAppForGroups($appId, $restrictions); - } - } - }); - } - private static function registerResourceCollectionHooks(): void { \OC\Collaboration\Resources\Listener::register(Server::get(IEventDispatcher::class)); } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a641ddddb350b..d35bfa00f5916 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1284,6 +1284,7 @@ 'OC\\App\\CompareVersion' => $baseDir . '/lib/private/App/CompareVersion.php', 'OC\\App\\DependencyAnalyzer' => $baseDir . '/lib/private/App/DependencyAnalyzer.php', 'OC\\App\\InfoParser' => $baseDir . '/lib/private/App/InfoParser.php', + 'OC\\App\\Listeners\\RemoveGroupRestrictionsListener' => $baseDir . '/lib/private/App/Listeners/RemoveGroupRestrictionsListener.php', 'OC\\App\\Platform' => $baseDir . '/lib/private/App/Platform.php', 'OC\\App\\PlatformRepository' => $baseDir . '/lib/private/App/PlatformRepository.php', 'OC\\Archive\\Archive' => $baseDir . '/lib/private/Archive/Archive.php', @@ -2023,6 +2024,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\\Listeners\\MembershipCacheListener' => $baseDir . '/lib/private/Group/Listeners/MembershipCacheListener.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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 1931ad3bff8a3..f6f6c7ec91b55 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1330,6 +1330,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\App\\CompareVersion' => __DIR__ . '/../../..' . '/lib/private/App/CompareVersion.php', 'OC\\App\\DependencyAnalyzer' => __DIR__ . '/../../..' . '/lib/private/App/DependencyAnalyzer.php', 'OC\\App\\InfoParser' => __DIR__ . '/../../..' . '/lib/private/App/InfoParser.php', + 'OC\\App\\Listeners\\RemoveGroupRestrictionsListener' => __DIR__ . '/../../..' . '/lib/private/App/Listeners/RemoveGroupRestrictionsListener.php', 'OC\\App\\Platform' => __DIR__ . '/../../..' . '/lib/private/App/Platform.php', 'OC\\App\\PlatformRepository' => __DIR__ . '/../../..' . '/lib/private/App/PlatformRepository.php', 'OC\\Archive\\Archive' => __DIR__ . '/../../..' . '/lib/private/Archive/Archive.php', @@ -2069,6 +2070,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\\Listeners\\MembershipCacheListener' => __DIR__ . '/../../..' . '/lib/private/Group/Listeners/MembershipCacheListener.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', diff --git a/lib/private/App/Listeners/RemoveGroupRestrictionsListener.php b/lib/private/App/Listeners/RemoveGroupRestrictionsListener.php new file mode 100644 index 0000000000000..49475a201ffac --- /dev/null +++ b/lib/private/App/Listeners/RemoveGroupRestrictionsListener.php @@ -0,0 +1,49 @@ + + */ +class RemoveGroupRestrictionsListener implements IEventListener { + public function __construct( + private readonly IAppManager $appManager, + ) { + } + + #[\Override] + public function handle(Event $event): void { + if (!$event instanceof GroupDeletedEvent) { + return; + } + + $group = $event->getGroup(); + $apps = $this->appManager->getEnabledAppsForGroup($group); + foreach ($apps as $appId) { + $restrictions = $this->appManager->getAppRestriction($appId); + if (empty($restrictions)) { + continue; + } + $key = array_search($group->getGID(), $restrictions, true); + unset($restrictions[$key]); + $restrictions = array_values($restrictions); + if (empty($restrictions)) { + $this->appManager->disableApp($appId); + } else { + $this->appManager->enableAppForGroups($appId, $restrictions); + } + } + } +} diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php index a27c4ef89c536..e63f993da97b9 100644 --- a/lib/private/Group/Group.php +++ b/lib/private/Group/Group.php @@ -8,7 +8,6 @@ namespace OC\Group; -use OC\Hooks\PublicEmitter; use OC\User\LazyUser; use OC\User\User; use OCP\EventDispatcher\IEventDispatcher; @@ -48,7 +47,6 @@ public function __construct( private array $backends, private IEventDispatcher $dispatcher, private IUserManager $userManager, - private ?PublicEmitter $emitter = null, /** @var ?non-empty-string $displayName */ protected ?string $displayName = null, ) { @@ -153,9 +151,6 @@ public function addUser(IUser $user): void { $this->dispatcher->dispatchTyped(new BeforeUserAddedEvent($this, $user)); - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'preAddUser', [$this, $user]); - } foreach ($this->backends as $backend) { if ($backend->implementsActions(\OC\Group\Backend::ADD_TO_GROUP)) { /** @var IAddToGroupBackend $backend */ @@ -163,10 +158,6 @@ public function addUser(IUser $user): void { $this->users[$user->getUID()] = $user; $this->dispatcher->dispatchTyped(new UserAddedEvent($this, $user)); - - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'postAddUser', [$this, $user]); - } return; } } @@ -179,9 +170,6 @@ public function addUser(IUser $user): void { public function removeUser(IUser $user): void { $result = false; $this->dispatcher->dispatchTyped(new BeforeUserRemovedEvent($this, $user)); - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'preRemoveUser', [$this, $user]); - } foreach ($this->backends as $backend) { if ($backend->implementsActions(\OC\Group\Backend::REMOVE_FROM_GOUP) && $backend->inGroup($user->getUID(), $this->gid)) { /** @var IRemoveFromGroupBackend $backend */ @@ -191,9 +179,6 @@ public function removeUser(IUser $user): void { } if ($result) { $this->dispatcher->dispatchTyped(new UserRemovedEvent($this, $user)); - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'postRemoveUser', [$this, $user]); - } if ($this->users) { foreach ($this->users as $index => $groupUser) { if ($groupUser->getUID() === $user->getUID()) { @@ -322,9 +307,6 @@ public function delete(): bool { $result = false; $this->dispatcher->dispatchTyped(new BeforeGroupDeletedEvent($this)); - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'preDelete', [$this]); - } foreach ($this->backends as $backend) { if ($backend->implementsActions(\OC\Group\Backend::DELETE_GROUP)) { /** @var IDeleteGroupBackend $backend */ @@ -333,9 +315,6 @@ public function delete(): bool { } if ($result) { $this->dispatcher->dispatchTyped(new GroupDeletedEvent($this)); - if ($this->emitter) { - $this->emitter->emit('\OC\Group', 'postDelete', [$this]); - } } return $result; } diff --git a/lib/private/Group/Listeners/MembershipCacheListener.php b/lib/private/Group/Listeners/MembershipCacheListener.php new file mode 100644 index 0000000000000..f3d9e063b6e63 --- /dev/null +++ b/lib/private/Group/Listeners/MembershipCacheListener.php @@ -0,0 +1,45 @@ + + */ +class MembershipCacheListener implements IEventListener { + public function __construct( + private readonly Manager $groupManager, + ) { + } + + #[\Override] + public function handle(Event $event): void { + if ($event instanceof BeforeUserAddedEvent + || $event instanceof UserAddedEvent + || $event instanceof BeforeUserRemovedEvent + || $event instanceof UserRemovedEvent) { + $this->groupManager->invalidateUserGroups($event->getUser()->getUID()); + } elseif ($event instanceof BeforeGroupDeletedEvent || $event instanceof GroupDeletedEvent) { + $this->groupManager->invalidateGroup($event->getGroup()->getGID()); + } + } +} diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index ccde43705aebe..e9f6d59052e2a 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -8,7 +8,6 @@ namespace OC\Group; -use OC\Hooks\PublicEmitter; use OC\Settings\AuthorizedGroupMapper; use OC\SubAdmin; use OCA\Settings\Settings\Admin\Users; @@ -18,7 +17,9 @@ use OCP\Group\Backend\IGroupDetailsBackend; use OCP\Group\Events\BeforeGroupCreatedEvent; use OCP\Group\Events\GroupCreatedEvent; +use OCP\Group\ISubAdmin; use OCP\GroupInterface; +use OCP\ICache; use OCP\ICacheFactory; use OCP\IDBConnection; use OCP\IGroup; @@ -26,31 +27,17 @@ use OCP\IUser; use OCP\Security\Ip\IRemoteAddress; use OCP\Server; -use Psr\Log\LoggerInterface; -use function is_string; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; -/** - * Class Manager - * - * Hooks available in scope \OC\Group: - * - preAddUser(\OC\Group\Group $group, \OC\User\User $user) - * - postAddUser(\OC\Group\Group $group, \OC\User\User $user) - * - preRemoveUser(\OC\Group\Group $group, \OC\User\User $user) - * - postRemoveUser(\OC\Group\Group $group, \OC\User\User $user) - * - preDelete(\OC\Group\Group $group) - * - postDelete(\OC\Group\Group $group) - * - preCreate(string $groupId) - * - postCreate(\OC\Group\Group $group) - * - * @package OC\Group - */ -class Manager extends PublicEmitter implements IGroupManager { - /** @var GroupInterface[] */ +class Manager implements IGroupManager { + /** @var list */ private array $backends = []; /** @var array */ private array $cachedGroups = []; /** @var array> */ - private array $cachedUserGroups = []; + private array $cachedUserGroupsLocal = []; + private ICache $cachedUserGroups; private ?SubAdmin $subAdmin = null; private DisplayNameCache $displayNameCache; private const int MAX_GROUP_LENGTH = 255; @@ -58,48 +45,20 @@ class Manager extends PublicEmitter implements IGroupManager { public function __construct( private \OC\User\Manager $userManager, private IEventDispatcher $dispatcher, - private LoggerInterface $logger, ICacheFactory $cacheFactory, private IRemoteAddress $remoteAddress, ) { $this->displayNameCache = new DisplayNameCache($cacheFactory, $this); - - $this->listen('\OC\Group', 'preDelete', function (IGroup $group): void { - unset($this->cachedGroups[$group->getGID()]); - $this->cachedUserGroups = []; - }); - $this->listen('\OC\Group', 'preAddUser', function (IGroup $group): void { - $this->cachedUserGroups = []; - }); - $this->listen('\OC\Group', 'preRemoveUser', function (IGroup $group): void { - $this->cachedUserGroups = []; - }); + $this->cachedUserGroups = $cacheFactory->createDistributed('user_groups_membership'); } - /** - * Checks whether a given backend is used - * - * @param string $backendClass Full classname including complete namespace - * @return bool - */ #[\Override] - public function isBackendUsed($backendClass) { - $backendClass = strtolower(ltrim($backendClass, '\\')); - - foreach ($this->backends as $backend) { - if (strtolower(get_class($backend)) === $backendClass) { - return true; - } - } - - return false; + public function isBackendUsed(string $backendClass): bool { + return array_any($this->backends, fn (GroupInterface $backend): bool => $backend::class === $backendClass); } - /** - * @param GroupInterface $backend - */ #[\Override] - public function addBackend($backend) { + public function addBackend(GroupInterface $backend): void { $this->backends[] = $backend; $this->clearCaches(); } @@ -113,44 +72,31 @@ public function removeBackend(GroupInterface $backend): void { } #[\Override] - public function clearBackends() { + public function clearBackends(): void { $this->backends = []; $this->clearCaches(); } - /** - * Get the active backends - * - * @return GroupInterface[] - */ #[\Override] - public function getBackends() { + public function getBackends(): array { return $this->backends; } - protected function clearCaches() { + protected function clearCaches(): void { $this->cachedGroups = []; - $this->cachedUserGroups = []; + $this->cachedUserGroups->clear(); + $this->cachedUserGroupsLocal = []; } - /** - * @param string $gid - * @return IGroup|null - */ #[\Override] - public function get($gid) { + public function get(string $gid): ?IGroup { if (isset($this->cachedGroups[$gid])) { return $this->cachedGroups[$gid]; } return $this->getGroupObject($gid); } - /** - * @param string $gid - * @param string $displayName - * @return IGroup|null - */ - protected function getGroupObject($gid, $displayName = null) { + protected function getGroupObject(string $gid, ?string $displayName = null): ?IGroup { $backends = []; foreach ($this->backends as $backend) { if ($backend->implementsActions(Backend::GROUP_DETAILS)) { @@ -170,7 +116,7 @@ protected function getGroupObject($gid, $displayName = null) { return null; } /** @var GroupInterface[] $backends */ - $this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this, $displayName); + $this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $displayName); return $this->cachedGroups[$gid]; } @@ -225,28 +171,20 @@ public function getGroupsObjects(array $gids, array $displayNames = []): array { if (count($backends[$gid]) === 0) { continue; } - $this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]); + $this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $displayNames[$gid]); $groups[$gid] = $this->cachedGroups[$gid]; } return $groups; } - /** - * @param string $gid - * @return bool - */ #[\Override] - public function groupExists($gid) { + public function groupExists(string $gid): bool { return $this->get($gid) instanceof IGroup; } - /** - * @param string $gid - * @return IGroup|null - */ #[\Override] - public function createGroup($gid) { - if ($gid === '' || $gid === null) { + public function createGroup(string $gid): ?IGroup { + if ($gid === '') { return null; } elseif ($group = $this->get($gid)) { return $group; @@ -254,7 +192,6 @@ public function createGroup($gid) { throw new \Exception('Group name is limited to ' . self::MAX_GROUP_LENGTH . ' characters'); } else { $this->dispatcher->dispatchTyped(new BeforeGroupCreatedEvent($gid)); - $this->emit('\OC\Group', 'preCreate', [$gid]); foreach ($this->backends as $backend) { if ($backend->implementsActions(Backend::CREATE_GROUP)) { if ($backend instanceof ICreateNamedGroupBackend) { @@ -262,13 +199,11 @@ public function createGroup($gid) { if (($gid = $backend->createGroup($groupName)) !== null) { $group = $this->getGroupObject($gid); $this->dispatcher->dispatchTyped(new GroupCreatedEvent($group)); - $this->emit('\OC\Group', 'postCreate', [$group]); return $group; } } elseif ($backend->createGroup($gid)) { $group = $this->getGroupObject($gid); $this->dispatcher->dispatchTyped(new GroupCreatedEvent($group)); - $this->emit('\OC\Group', 'postCreate', [$group]); return $group; } } @@ -278,7 +213,7 @@ public function createGroup($gid) { } #[\Override] - public function search(string $search, ?int $limit = null, ?int $offset = 0) { + public function search(string $search, ?int $limit = null, ?int $offset = 0): array { $groups = []; foreach ($this->backends as $backend) { $groupIds = $backend->getGroups($search, $limit ?? -1, $offset ?? 0); @@ -293,10 +228,6 @@ public function search(string $search, ?int $limit = null, ?int $offset = 0) { return array_values($groups); } - /** - * @param IUser|null $user - * @return array - */ #[\Override] public function getUserGroups(?IUser $user = null): array { if (!$user instanceof IUser) { @@ -316,18 +247,15 @@ public function getUserIdGroups(string $uid): array { /** * Checks if a userId is in the admin group - * - * @param string $userId - * @return bool if admin */ #[\Override] - public function isAdmin($userId) { + public function isAdmin(string $userId): bool { if (!$this->remoteAddress->allowsAdminActions()) { return false; } foreach ($this->backends as $backend) { - if (is_string($userId) && $backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) { + if ($backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) { return true; } } @@ -347,15 +275,8 @@ public function isDelegatedAdmin(string $userId): bool { return in_array(Users::class, $authorizedClasses, true); } - /** - * Checks if a userId is in a group - * - * @param string $userId - * @param string $group - * @return bool if in group - */ #[\Override] - public function isInGroup($userId, $group) { + public function isInGroup(string $userId, string $group): bool { return in_array($group, $this->getUserIdGroupIds($userId), true); } @@ -369,42 +290,31 @@ public function getUserGroupIds(IUser $user): array { * @return list */ private function getUserIdGroupIds(string $uid): array { - if (!isset($this->cachedUserGroups[$uid])) { + if (isset($this->cachedUserGroupsLocal[$uid])) { + return $this->cachedUserGroupsLocal[$uid]; + } + $groups = $this->cachedUserGroups->get($uid); + if ($groups === null) { $groups = []; foreach ($this->backends as $backend) { if ($groupIds = $backend->getUserGroups($uid)) { $groups = array_merge($groups, $groupIds); } } - $this->cachedUserGroups[$uid] = $groups; + $this->cachedUserGroups->set($uid, $groups, 60 * 2); // 2min + $this->cachedUserGroupsLocal[$uid] = $groups; } - return $this->cachedUserGroups[$uid]; + return $groups; } - /** - * @param string $groupId - * @return ?string - */ #[\Override] 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) { + public function displayNamesInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array { $group = $this->get($gid); if (is_null($group)) { return []; @@ -448,9 +358,10 @@ public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0 } /** - * @return SubAdmin + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface */ - public function getSubAdmin() { + public function getSubAdmin(): ISubAdmin { if (!$this->subAdmin) { $this->subAdmin = new SubAdmin( $this->userManager, @@ -462,4 +373,15 @@ public function getSubAdmin() { return $this->subAdmin; } + + public function invalidateUserGroups(string $uid): void { + $this->cachedUserGroups->remove($uid); + unset($this->cachedUserGroupsLocal[$uid]); + } + + public function invalidateGroup(string $gid): void { + unset($this->cachedGroups[$gid]); + $this->cachedUserGroups->clear(); + $this->cachedUserGroupsLocal = []; + } } diff --git a/lib/public/Group/ISubAdmin.php b/lib/public/Group/ISubAdmin.php index e70ed3b31d615..81511ce10104f 100644 --- a/lib/public/Group/ISubAdmin.php +++ b/lib/public/Group/ISubAdmin.php @@ -9,12 +9,14 @@ namespace OCP\Group; +use OCP\AppFramework\Attribute\Consumable; use OCP\IGroup; use OCP\IUser; /** * @since 16.0.0 */ +#[Consumable(since: '16.0.0')] interface ISubAdmin { /** * add a SubAdmin diff --git a/lib/public/IGroupManager.php b/lib/public/IGroupManager.php index 5f2fd33475909..7b1308c9c81aa 100644 --- a/lib/public/IGroupManager.php +++ b/lib/public/IGroupManager.php @@ -8,36 +8,27 @@ namespace OCP; +use OCP\AppFramework\Attribute\Consumable; + /** - * Class Manager - * - * Hooks available in scope \OC\Group: - * - preAddUser(\OC\Group\Group $group, \OC\User\User $user) - * - postAddUser(\OC\Group\Group $group, \OC\User\User $user) - * - preRemoveUser(\OC\Group\Group $group, \OC\User\User $user) - * - postRemoveUser(\OC\Group\Group $group, \OC\User\User $user) - * - preDelete(\OC\Group\Group $group) - * - postDelete(\OC\Group\Group $group) - * - preCreate(string $groupId) - * - postCreate(\OC\Group\Group $group) + * Group manager interface. * * @since 8.0.0 */ +#[Consumable(since: '8.0.0')] interface IGroupManager { /** * Checks whether a given backend is used * - * @param string $backendClass Full classname including complete namespace - * @return bool + * @param class-string $backendClass Full classname including complete namespace * @since 8.1.0 */ - public function isBackendUsed($backendClass); + public function isBackendUsed(string $backendClass): bool; /** - * @param \OCP\GroupInterface $backend * @since 8.0.0 */ - public function addBackend($backend); + public function addBackend(GroupInterface $backend): void; /** * @since 34.0.0 @@ -47,35 +38,35 @@ public function removeBackend(GroupInterface $backend): void; /** * @since 8.0.0 */ - public function clearBackends(); + public function clearBackends(): void; /** * Get the active backends - * @return \OCP\GroupInterface[] + * @return list<\OCP\GroupInterface> * @since 13.0.0 */ - public function getBackends(); + public function getBackends(): array; /** * @param string $gid * @return \OCP\IGroup|null * @since 8.0.0 */ - public function get($gid); + public function get(string $gid): ?IGroup; /** * @param string $gid * @return bool * @since 8.0.0 */ - public function groupExists($gid); + public function groupExists(string $gid): bool; /** * @param string $gid * @return \OCP\IGroup|null * @since 8.0.0 */ - public function createGroup($gid); + public function createGroup(string $gid): ?IGroup; /** * @param string $search @@ -84,14 +75,14 @@ public function createGroup($gid); * @return list * @since 8.0.0 */ - public function search(string $search, ?int $limit = null, ?int $offset = 0); + public function search(string $search, ?int $limit = null, ?int $offset = 0): array; /** * @param \OCP\IUser|null $user * @return \OCP\IGroup[] * @since 8.0.0 */ - public function getUserGroups(?IUser $user = null); + public function getUserGroups(?IUser $user = null): array; /** * @param \OCP\IUser $user @@ -110,7 +101,7 @@ public function getUserGroupIds(IUser $user): array; * @return array ['user id' => 'display name'] * @since 8.0.0 */ - public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0); + public function displayNamesInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array; /** * Checks if a userId is in the admin group @@ -118,7 +109,7 @@ public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0 * @return bool if admin * @since 8.0.0 */ - public function isAdmin($userId); + public function isAdmin(string $userId): bool; /** * Checks if a userId is eligible to users administration delegation @@ -135,7 +126,7 @@ public function isDelegatedAdmin(string $userId): bool; * @return bool if in group * @since 8.0.0 */ - public function isInGroup($userId, $group); + public function isInGroup(string $userId, string $group): bool; /** * Get the display name of a Nextcloud group diff --git a/tests/lib/Group/GroupTest.php b/tests/lib/Group/GroupTest.php index 50181a3ea2a31..1bd9137a486b3 100644 --- a/tests/lib/Group/GroupTest.php +++ b/tests/lib/Group/GroupTest.php @@ -491,7 +491,7 @@ public function testSetDisplayNameDispatchesOldValue(): void { ->with('group1', 'New Name') ->willReturn(true); - $group = new Group('group1', [$backend], $dispatcher, $userManager, null, 'Old Name'); + $group = new Group('group1', [$backend], $dispatcher, $userManager, 'Old Name'); $this->assertTrue($group->setDisplayName('New Name')); } diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 6d369be102b11..29a13b2dcdb86 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -9,6 +9,7 @@ namespace Test\Group; use OC\Group\Database; +use OC\Group\Listeners\MembershipCacheListener; use OC\User\Manager; use OC\User\User; use OCP\EventDispatcher\IEventDispatcher; @@ -19,11 +20,11 @@ use OCP\Group\Backend\IRemoveFromGroupBackend; use OCP\Group\Backend\ISearchableGroupBackend; use OCP\GroupInterface; +use OCP\ICache; use OCP\ICacheFactory; use OCP\IUser; use OCP\Security\Ip\IRemoteAddress; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Log\LoggerInterface; use Test\TestCase; abstract class TestBackend extends ABackend implements ISearchableGroupBackend, IAddToGroupBackend, ICreateGroupBackend, IGroupDetailsBackend, IRemoveFromGroupBackend, GroupInterface { @@ -31,16 +32,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; + private ICacheFactory&MockObject $cache; + private ICache&MockObject $userGroupsCache; + private IRemoteAddress&MockObject $remoteIpAddress; #[\Override] protected function setUp(): void { @@ -48,14 +44,26 @@ protected function setUp(): void { $this->userManager = $this->createMock(Manager::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); - $this->logger = $this->createMock(LoggerInterface::class); $this->cache = $this->createMock(ICacheFactory::class); + $this->userGroupsCache = $this->createMock(ICache::class); + $this->cache->method('createDistributed') + ->willReturnCallback(fn (string $prefix): ICache => $prefix === 'user_groups_membership' + ? $this->userGroupsCache + : $this->createMock(ICache::class)); $this->remoteIpAddress = $this->createMock(IRemoteAddress::class); $this->remoteIpAddress->method('allowsAdminActions')->willReturn(true); } - private function getTestUser($userId) { + private function wireCacheInvalidation(\OC\Group\Manager $manager): void { + $listener = new MembershipCacheListener($manager); + $this->dispatcher->method('dispatchTyped') + ->willReturnCallback(function ($event) use ($listener): void { + $listener->handle($event); + }); + } + + private function getTestUser(string $userId): IUser&MockObject { $mockUser = $this->createMock(IUser::class); $mockUser->expects($this->any()) ->method('getUID') @@ -66,11 +74,7 @@ private function getTestUser($userId) { return $mockUser; } - /** - * @param null|int $implementedActions - * @return \PHPUnit\Framework\MockObject\MockObject - */ - private function getTestBackend($implementedActions = null) { + private function getTestBackend(?int $implementedActions = null): TestBackend&MockObject { if ($implementedActions === null) { $implementedActions = GroupInterface::ADD_TO_GROUP @@ -107,16 +111,13 @@ private function getTestBackend($implementedActions = null) { } public function testGet(): void { - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend - */ $backend = $this->getTestBackend(); $backend->expects($this->any()) ->method('groupExists') ->with('group1') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $group = $manager->get('group1'); @@ -125,22 +126,19 @@ public function testGet(): void { } public function testGetNoBackend(): void { - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $this->assertNull($manager->get('group1')); } public function testGetNotExists(): void { - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend - */ $backend = $this->getTestBackend(); $backend->expects($this->once()) ->method('groupExists') ->with('group1') ->willReturn(false); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $this->assertNull($manager->get('group1')); @@ -150,8 +148,9 @@ public function testGetDeleted(): void { $backend = new \Test\Util\Group\Dummy(); $backend->createGroup('group1'); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); + $this->wireCacheInvalidation($manager); $group = $manager->get('group1'); $group->delete(); @@ -159,25 +158,19 @@ public function testGetDeleted(): void { } public function testGetMultipleBackends(): void { - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend1 - */ $backend1 = $this->getTestBackend(); $backend1->expects($this->any()) ->method('groupExists') ->with('group1') ->willReturn(false); - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend2 - */ $backend2 = $this->getTestBackend(); $backend2->expects($this->any()) ->method('groupExists') ->with('group1') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -187,7 +180,6 @@ public function testGetMultipleBackends(): void { } public function testCreate(): void { - /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */ $backendGroupCreated = false; $backend = $this->getTestBackend(); $backend->expects($this->any()) @@ -203,7 +195,7 @@ public function testCreate(): void { return true; }); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $group = $manager->createGroup('group1'); @@ -211,7 +203,6 @@ public function testCreate(): void { } public function testCreateFailure(): void { - /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */ $backendGroupCreated = false; $backend = $this->getTestBackend( GroupInterface::ADD_TO_GROUP @@ -232,7 +223,7 @@ public function testCreateFailure(): void { ->method('getGroupDetails') ->willReturn([]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $group = $manager->createGroup('group1'); @@ -240,8 +231,6 @@ public function testCreateFailure(): void { } public function testCreateTooLong(): void { - /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */ - $backendGroupCreated = false; $backend = $this->getTestBackend( GroupInterface::ADD_TO_GROUP | GroupInterface::REMOVE_FROM_GOUP @@ -256,7 +245,7 @@ public function testCreateTooLong(): void { ->with($groupName) ->willReturn(false); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $this->expectException(\Exception::class); @@ -264,7 +253,6 @@ public function testCreateTooLong(): void { } public function testCreateExists(): void { - /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */ $backend = $this->getTestBackend(); $backend->expects($this->any()) ->method('groupExists') @@ -273,7 +261,7 @@ public function testCreateExists(): void { $backend->expects($this->never()) ->method('createGroup'); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $group = $manager->createGroup('group1'); @@ -281,9 +269,6 @@ public function testCreateExists(): void { } public function testSearch(): void { - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend - */ $backend = $this->getTestBackend(); $backend->expects($this->once()) ->method('getGroups') @@ -295,7 +280,7 @@ public function testSearch(): void { ['group1', ['displayName' => 'group1']], ]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $groups = $manager->search('1'); @@ -305,9 +290,6 @@ public function testSearch(): void { } public function testSearchMultipleBackends(): void { - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend1 - */ $backend1 = $this->getTestBackend(); $backend1->expects($this->once()) ->method('getGroups') @@ -320,9 +302,6 @@ public function testSearchMultipleBackends(): void { ['group12', []], ]); - /** - * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend2 - */ $backend2 = $this->getTestBackend(); $backend2->expects($this->once()) ->method('getGroups') @@ -335,7 +314,7 @@ public function testSearchMultipleBackends(): void { ['group1', ['displayName' => 'group1']], ]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -382,7 +361,7 @@ public function testSearchMultipleBackendsLimitAndOffset(): void { ['group12', ['displayName' => 'group12']], ]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -411,7 +390,7 @@ public function testSearchResultExistsButGroupDoesNot(): void { /** @var \OC\User\Manager $userManager */ $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $groups = $manager->search('1'); @@ -432,7 +411,7 @@ public function testGetUserGroups(): void { ->with('group1') ->willReturn(['displayName' => 'group1']); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $groups = $manager->getUserGroups($this->getTestUser('user1')); @@ -450,7 +429,7 @@ public function testGetUserGroupIds(): void { ->with('myUID') ->willReturn(['123', 'abc']); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); /** @var User|\PHPUnit\Framework\MockObject\MockObject $user */ @@ -480,7 +459,7 @@ public function testGetUserGroupsWithDeletedGroup(): void { ->with(['group1']) ->willReturn(['group1' => []]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); /** @var User|\PHPUnit\Framework\MockObject\MockObject $user */ @@ -506,7 +485,7 @@ public function testInGroup(): void { ->method('groupExists') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $this->assertTrue($manager->isInGroup('user1', 'group1')); @@ -525,7 +504,7 @@ public function testIsAdmin(): void { ->method('groupExists') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $this->assertTrue($manager->isAdmin('user1')); @@ -544,7 +523,7 @@ public function testNotAdmin(): void { ->method('groupExists') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $this->assertFalse($manager->isAdmin('user1')); @@ -575,7 +554,7 @@ public function testGetUserGroupsMultipleBackends(): void { ->method('getGroupDetails') ->willReturnCallback(fn ($gid) => ['displayName' => $gid]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -634,7 +613,7 @@ public function testDisplayNamesInGroupWithOneUserBackend(): void { } }); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3'); @@ -694,7 +673,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitSpecified(): v } }); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1); @@ -758,7 +737,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitAndOffsetSpeci } }); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1); @@ -787,7 +766,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmpty(): void $this->userManager->expects($this->never())->method('get'); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', ''); @@ -815,7 +794,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitS $this->userManager->expects($this->never())->method('get'); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', '', 1); @@ -843,7 +822,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitA $this->userManager->expects($this->never())->method('get'); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', '', 1, 1); @@ -875,8 +854,9 @@ public function testGetUserGroupsWithAddUser(): void { ->with('group1') ->willReturn(['displayName' => 'group1']); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); + $this->wireCacheInvalidation($manager); // prime cache $user1 = $this->getTestUser('user1'); @@ -918,8 +898,9 @@ public function testGetUserGroupsWithRemoveUser(): void { ->method('removeFromGroup') ->willReturn(true); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); + $this->wireCacheInvalidation($manager); // prime cache $user1 = $this->getTestUser('user1'); @@ -948,7 +929,7 @@ public function testGetUserIdGroups(): void { ->with('user1') ->willReturn(null); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); $groups = $manager->getUserIdGroups('user1'); @@ -973,7 +954,7 @@ public function testGroupDisplayName(): void { ['group1', ['gid' => 'group1', 'displayName' => 'Group One']], ['group2', ['gid' => 'group2']], ]); - $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress); + $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->cache, $this->remoteIpAddress); $manager->addBackend($backend); // group with display name diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 1d62c7301c519..2a942548628ae 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1288,9 +1288,8 @@ public function testGeneralChecks(array $shareParams, ?string $exceptionMessage, ['user1', $this->createMock(IUser::class)], ]); - $this->groupManager->method('groupExists')->willReturnMap([ - ['group0', true], - ]); + $this->groupManager->method('groupExists') + ->willReturnCallback(fn (string $group): bool => $group === 'group0'); $userFolder = $this->createMock(IUserFolder::class); diff --git a/tests/lib/SystemTag/SystemTagManagerTest.php b/tests/lib/SystemTag/SystemTagManagerTest.php index 80f327fafb9ba..cfa4fd649f97f 100644 --- a/tests/lib/SystemTag/SystemTagManagerTest.php +++ b/tests/lib/SystemTag/SystemTagManagerTest.php @@ -525,7 +525,7 @@ public function testAllowedToCreateTag(bool $isCli, ?bool $isAdmin, bool $isRest $this->groupManager->expects($this->any()) ->method('isAdmin') ->with('test') - ->willReturn($isAdmin); + ->willReturn($isAdmin ?? false); $this->appConfig->expects($this->any()) ->method('getValueBool') ->with('systemtags', 'restrict_creation_to_admin') @@ -539,15 +539,7 @@ public function testAllowedToCreateTag(bool $isCli, ?bool $isAdmin, bool $isRest \OC::$CLI = $oldCli; } - public static function disallowedToCreateProvider(): array { - return [ - [false], - [null], - ]; - } - - #[\PHPUnit\Framework\Attributes\DataProvider('disallowedToCreateProvider')] - public function testDisallowedToCreateTag(?bool $isAdmin): void { + public function testDisallowedToCreateTag(): void { $oldCli = \OC::$CLI; \OC::$CLI = false; @@ -557,11 +549,11 @@ public function testDisallowedToCreateTag(?bool $isAdmin): void { ->willReturn('test'); $this->userSession->expects($this->any()) ->method('getUser') - ->willReturn($isAdmin === null ? null : $user); + ->willReturn($user); $this->groupManager->expects($this->any()) ->method('isAdmin') ->with('test') - ->willReturn($isAdmin); + ->willReturn(false); $this->appConfig->expects($this->any()) ->method('getValueBool') ->with('systemtags', 'restrict_creation_to_admin') @@ -573,11 +565,7 @@ public function testDisallowedToCreateTag(?bool $isAdmin): void { \OC::$CLI = $oldCli; } - /** - * @param ISystemTag $tag1 - * @param ISystemTag $tag2 - */ - private function assertSameTag($tag1, $tag2) { + private function assertSameTag(ISystemTag $tag1, ISystemTag $tag2): void { $this->assertEquals($tag1->getId(), $tag2->getId()); $this->assertEquals($tag1->getName(), $tag2->getName()); $this->assertEquals($tag1->isUserVisible(), $tag2->isUserVisible());