diff --git a/apps/user_ldap/lib/GroupPluginManager.php b/apps/user_ldap/lib/GroupPluginManager.php index 5999409cdba34..524cc81ad011a 100644 --- a/apps/user_ldap/lib/GroupPluginManager.php +++ b/apps/user_ldap/lib/GroupPluginManager.php @@ -126,7 +126,7 @@ public function deleteGroup(string $gid): bool { * * Adds a user to a group. */ - public function addToGroup($uid, $gid) { + public function addToGroup(string $uid, string $gid): bool { $plugin = $this->which[GroupInterface::ADD_TO_GROUP]; if ($plugin) { @@ -144,7 +144,7 @@ public function addToGroup($uid, $gid) { * * removes the user from a group. */ - public function removeFromGroup($uid, $gid) { + public function removeFromGroup(string $uid, string $gid): bool { $plugin = $this->which[GroupInterface::REMOVE_FROM_GROUP]; if ($plugin) { @@ -157,14 +157,13 @@ public function removeFromGroup($uid, $gid) { * get the number of all users matching the search string in a group * @param string $gid ID of the group * @param string $search query string - * @return int|false * @throws \Exception */ - public function countUsersInGroup($gid, $search = '') { + public function countUsersInGroup(string $gid, string $search = ''): int { $plugin = $this->which[GroupInterface::COUNT_USERS]; if ($plugin) { - return $plugin->countUsersInGroup($gid,$search); + return (int)$plugin->countUsersInGroup($gid, $search); } throw new \Exception('No plugin implements countUsersInGroup in this LDAP Backend.'); } @@ -172,14 +171,18 @@ public function countUsersInGroup($gid, $search = '') { /** * get an array with group details * @param string $gid - * @return array|false + * @return array * @throws \Exception */ - public function getGroupDetails($gid) { + public function getGroupDetails($gid): array { $plugin = $this->which[GroupInterface::GROUP_DETAILS]; if ($plugin) { - return $plugin->getGroupDetails($gid); + $details = $plugin->getGroupDetails($gid); + if ($details === false) { + return []; + } + return $details; } throw new \Exception('No plugin implements getGroupDetails in this LDAP Backend.'); } diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 8fcb10cb850d0..965000fafa418 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -47,12 +47,19 @@ use OC; use OC\Cache\CappedMemoryCache; use OC\ServerNotAvailableException; +use OCP\Group\Backend\ABackend; +use OCP\Group\Backend\IAddToGroupBackend; +use OCP\Group\Backend\ICountUsersBackend; use OCP\Group\Backend\IGetDisplayNameBackend; use OCP\Group\Backend\IDeleteGroupBackend; +use OCP\Group\Backend\IGroupDetailsBackend; +use OCP\Group\Backend\IRemoveFromGroupBackend; use OCP\GroupInterface; use Psr\Log\LoggerInterface; -class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend { +class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, + IGetDisplayNameBackend, IDeleteGroupBackend, ICountUsersBackend, IAddToGroupBackend, + IRemoveFromGroupBackend { protected $enabled = false; /** @var CappedMemoryCache $cachedGroupMembers array of users with gid as key */ @@ -61,10 +68,9 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I protected CappedMemoryCache $cachedGroupsByMember; /** @var CappedMemoryCache $cachedNestedGroups array of groups with gid (DN) as key */ protected CappedMemoryCache $cachedNestedGroups; - /** @var GroupPluginManager */ - protected $groupPluginManager; - /** @var LoggerInterface */ - protected $logger; + protected GroupPluginManager $groupPluginManager; + protected LoggerInterface $logger; + protected Access $access; /** * @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name @@ -72,7 +78,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I protected $ldapGroupMemberAssocAttr; public function __construct(Access $access, GroupPluginManager $groupPluginManager) { - parent::__construct($access); + $this->access = $access; $filter = $this->access->connection->ldapGroupFilter; $gAssoc = $this->access->connection->ldapGroupMemberAssocAttr; if (!empty($filter) && !empty($gAssoc)) { @@ -1000,22 +1006,19 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { } /** - * returns the number of users in a group, who match the search term + * Returns the number of users in a group, who match the search term * - * @param string $gid the internal group name - * @param string $search optional, a search string - * @return int|bool * @throws Exception * @throws ServerNotAvailableException */ - public function countUsersInGroup($gid, $search = '') { + public function countUsersInGroup(string $gid, string $search = ''): int { if ($this->groupPluginManager->implementsActions(GroupInterface::COUNT_USERS)) { return $this->groupPluginManager->countUsersInGroup($gid, $search); } $cacheKey = 'countUsersInGroup-' . $gid . '-' . $search; if (!$this->enabled || !$this->groupExists($gid)) { - return false; + return 0; } $groupUsers = $this->access->connection->getFromCache($cacheKey); if (!is_null($groupUsers)) { @@ -1025,16 +1028,16 @@ public function countUsersInGroup($gid, $search = '') { $groupDN = $this->access->groupname2dn($gid); if (!$groupDN) { // group couldn't be found, return empty result set - $this->access->connection->writeToCache($cacheKey, false); - return false; + $this->access->connection->writeToCache($cacheKey, 0); + return 0; } $members = $this->_groupMembers($groupDN); $primaryUserCount = $this->countUsersInPrimaryGroup($groupDN, ''); if (!$members && $primaryUserCount === 0) { //in case users could not be retrieved, return empty result set - $this->access->connection->writeToCache($cacheKey, false); - return false; + $this->access->connection->writeToCache($cacheKey, 0); + return 0; } if ($search === '') { @@ -1203,7 +1206,7 @@ protected function filterValidGroups(array $listOfGroups): array { * Returns the supported actions as int to be * compared with GroupInterface::CREATE_GROUP etc. */ - public function implementsActions($actions) { + public function implementsActions($actions): bool { return (bool)((GroupInterface::COUNT_USERS | GroupInterface::DELETE_GROUP | $this->groupPluginManager->getImplementedActions()) & $actions); @@ -1287,7 +1290,7 @@ public function deleteGroup(string $gid): bool { * @return bool * @throws Exception */ - public function addToGroup($uid, $gid) { + public function addToGroup(string $uid, string $gid): bool { if ($this->groupPluginManager->implementsActions(GroupInterface::ADD_TO_GROUP)) { if ($ret = $this->groupPluginManager->addToGroup($uid, $gid)) { $this->access->connection->clearCache(); @@ -1306,7 +1309,7 @@ public function addToGroup($uid, $gid) { * @return bool * @throws Exception */ - public function removeFromGroup($uid, $gid) { + public function removeFromGroup(string $uid, string $gid): bool { if ($this->groupPluginManager->implementsActions(GroupInterface::REMOVE_FROM_GROUP)) { if ($ret = $this->groupPluginManager->removeFromGroup($uid, $gid)) { $this->access->connection->clearCache(); @@ -1321,10 +1324,10 @@ public function removeFromGroup($uid, $gid) { * Gets group details * * @param string $gid Name of the group - * @return array|false + * @return array{displayName?: string} * @throws Exception */ - public function getGroupDetails($gid) { + public function getGroupDetails(string $gid): array { if ($this->groupPluginManager->implementsActions(GroupInterface::GROUP_DETAILS)) { return $this->groupPluginManager->getGroupDetails($gid); } @@ -1370,4 +1373,11 @@ public function getDisplayName(string $gid): string { return ''; } + + public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array { + if (!$this->enabled) { + return []; + } + return parent::searchInGroup($gid, $search, $limit, $offset); + } } diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php index ea2fcce679c4a..b4cf50a2f40e3 100644 --- a/apps/user_ldap/lib/Group_Proxy.php +++ b/apps/user_ldap/lib/Group_Proxy.php @@ -28,11 +28,19 @@ */ namespace OCA\User_LDAP; +use OCP\Group\Backend\ABackend; use OCP\Group\Backend\IDeleteGroupBackend; use OCP\Group\Backend\IGetDisplayNameBackend; +use OCP\Group\Backend\IGroupDetailsBackend; use OCP\Group\Backend\INamedBackend; +use OCP\Group\Backend\IAddToGroupBackend; +use OCP\Group\Backend\ICountUsersBackend; +use OCP\Group\Backend\IRemoveFromGroupBackend; +use OCP\GroupInterface; -class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend { +class Group_Proxy extends Proxy implements GroupInterface, IGroupLDAP, + IGetDisplayNameBackend, IDeleteGroupBackend, ICountUsersBackend, IAddToGroupBackend, + IRemoveFromGroupBackend { private $backends = []; private $refBackend = null; @@ -187,7 +195,7 @@ public function deleteGroup(string $gid): bool { * * Adds a user to a group. */ - public function addToGroup($uid, $gid) { + public function addToGroup(string $uid, string $gid): bool { return $this->handleRequest( $gid, 'addToGroup', [$uid, $gid]); } @@ -201,7 +209,7 @@ public function addToGroup($uid, $gid) { * * removes the user from a group. */ - public function removeFromGroup($uid, $gid) { + public function removeFromGroup(string $uid, string $gid): bool { return $this->handleRequest( $gid, 'removeFromGroup', [$uid, $gid]); } @@ -211,24 +219,32 @@ public function removeFromGroup($uid, $gid) { * * @param string $gid the internal group name * @param string $search optional, a search string - * @return int|bool */ - public function countUsersInGroup($gid, $search = '') { + public function countUsersInGroup(string $gid, string $search = ''): int { return $this->handleRequest( $gid, 'countUsersInGroup', [$gid, $search]); } /** - * get an array with group details - * - * @param string $gid - * @return array|false + * Get an array with group details */ - public function getGroupDetails($gid) { + public function getGroupDetails(string $gid): array { return $this->handleRequest( $gid, 'getGroupDetails', [$gid]); } + public function getGroupsDetails(array $gids): array { + if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) { + throw new \Exception("Should not have been called"); + } + + $groupData = []; + foreach ($gids as $gid) { + $groupData[$gid] = $this->handleRequest($gid, 'getGroupDetails', [$gid]); + } + return $groupData; + } + /** * get a list of all groups * @@ -259,6 +275,17 @@ public function groupExists($gid) { return $this->handleRequest($gid, 'groupExists', [$gid]); } + public function groupsExists(array $gids): array { + $existingGroups = []; + foreach ($gids as $gid) { + $exits = $this->handleRequest($gid, 'groupExists', [$gid]); + if ($exits) { + $existingGroups[] = $gid; + } + } + return $existingGroups; + } + /** * Check if backend implements actions * @@ -306,4 +333,8 @@ public function getDisplayName(string $gid): string { public function getBackendName(): string { return 'LDAP'; } + + public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array { + return $this->handleRequest($gid, 'searchInGroup', [$gid, $search, $limit, $offset]); + } } diff --git a/apps/user_ldap/lib/LDAP.php b/apps/user_ldap/lib/LDAP.php index 3c57959694173..35df0b17225f8 100644 --- a/apps/user_ldap/lib/LDAP.php +++ b/apps/user_ldap/lib/LDAP.php @@ -320,7 +320,15 @@ private function preFunctionCall(string $functionName, array $args): void { $this->curArgs = $args; if ($this->dataCollector !== null) { - $args = array_map(fn ($item) => (!$this->isResource($item) ? $item : '(resource)'), $this->curArgs); + $args = array_map(function ($item) { + if ($this->isResource($item)) { + return '(resource)'; + } + if (isset($item[0]['value']['cookie'])) { + $item[0]['value']['cookie'] = ""; + } + return $item; + }, $this->curArgs); $this->dataCollector->startLdapRequest($this->curFunc, $args); } diff --git a/apps/user_ldap/lib/Proxy.php b/apps/user_ldap/lib/Proxy.php index d9546a163ab89..0a3760cebd60f 100644 --- a/apps/user_ldap/lib/Proxy.php +++ b/apps/user_ldap/lib/Proxy.php @@ -142,7 +142,7 @@ abstract protected function walkBackends($id, $method, $parameters); * @param string $id * @return Access */ - abstract public function getLDAPAccess($id); + abstract public function getLDAPAccess($gid); abstract protected function activeBackends(): int; diff --git a/apps/user_ldap/tests/LDAPGroupPluginDummy.php b/apps/user_ldap/tests/LDAPGroupPluginDummy.php index 25350bfc16a08..364bb701615d2 100644 --- a/apps/user_ldap/tests/LDAPGroupPluginDummy.php +++ b/apps/user_ldap/tests/LDAPGroupPluginDummy.php @@ -37,19 +37,19 @@ public function deleteGroup($gid) { return null; } - public function addToGroup($uid, $gid) { - return null; + public function addToGroup($uid, $gid): bool { + return false; } - public function removeFromGroup($uid, $gid) { - return null; + public function removeFromGroup($uid, $gid): bool { + return false; } - public function countUsersInGroup($gid, $search = '') { - return null; + public function countUsersInGroup($gid, $search = ''): int { + return 0; } - public function getGroupDetails($gid) { - return null; + public function getGroupDetails($gid): array { + return []; } } diff --git a/build/psalm-baseline-ocp.xml b/build/psalm-baseline-ocp.xml index 5490f5058862a..415aa8ae18681 100644 --- a/build/psalm-baseline-ocp.xml +++ b/build/psalm-baseline-ocp.xml @@ -66,18 +66,6 @@ \OC - - - $resource['internalName'] - $resource['resource'] - $resource['size'] - $resource['size'] - $resource['time'] - - - $this->resources - - \OC @@ -169,16 +157,6 @@ \OC - - - IsearchRequest - - - - - public function getUri(): string; - - Color @@ -190,23 +168,6 @@ ContainerExceptionInterface - - - PreconditionNotMetException - - - - - $cursor - - - - - array - array|bool - mixed - - \html_select_options($options, $selected, $params) @@ -219,12 +180,6 @@ - - \OC_Helper::computerFileSize($str) - - - float - \OC \OC diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 1e27a2496c712..dba096a699a08 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -136,16 +136,6 @@ - - '\OCA\DAV\CalDAV\CalDavBackend::createCachedCalendarObject' - '\OCA\DAV\CalDAV\CalDavBackend::createSubscription' - '\OCA\DAV\CalDAV\CalDavBackend::deleteCachedCalendarObject' - '\OCA\DAV\CalDAV\CalDavBackend::deleteSubscription' - '\OCA\DAV\CalDAV\CalDavBackend::publishCalendar' - '\OCA\DAV\CalDAV\CalDavBackend::updateCachedCalendarObject' - '\OCA\DAV\CalDAV\CalDavBackend::updateShares' - '\OCA\DAV\CalDAV\CalDavBackend::updateSubscription' - array array @@ -161,16 +151,6 @@ (int)$calendarId - - dispatch - dispatch - dispatch - dispatch - dispatch - dispatch - dispatch - dispatch - Uri\split($principalUri) Uri\split($row['principaluri']) @@ -341,9 +321,6 @@ - - $id - $this->getKey() $this->getKey() @@ -358,16 +335,6 @@ false - - '\OCA\DAV\CardDAV\CardDavBackend::createCard' - '\OCA\DAV\CardDAV\CardDavBackend::deleteCard' - '\OCA\DAV\CardDAV\CardDavBackend::updateCard' - - - dispatch - dispatch - dispatch - $addressBooks[$row['id']][$readOnlyPropertyName] === 0 @@ -889,7 +856,6 @@ get_class($res) === 'OpenSSLAsymmetricKey' - is_object($res) @@ -1745,6 +1711,14 @@ $lastUpdateCheckTimestamp + + + 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' + + + dispatch + + $ln + 1 @@ -1764,14 +1738,6 @@ string[] - - - 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' - - - dispatch - - @@ -1791,11 +1757,6 @@ is_array($list) - - - $gid - - $i @@ -3366,16 +3327,13 @@ $result $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file, false, $isRename) - + $newUnencryptedSize $result - $stat $this->storage->file_get_contents($path) $this->storage->filesize($path) - $this->storage->getLocalFile($path) - - array + bool int string @@ -3476,40 +3434,16 @@ is_null($this->getContent()) - - - $this->groupCache[$gid]['displayname'] - - - $this->groupCache - $this->groupCache - $this->groupCache - - - + IGroup::class . '::postAddUser' IGroup::class . '::postDelete' IGroup::class . '::postRemoveUser' IGroup::class . '::preAddUser' IGroup::class . '::preDelete' IGroup::class . '::preRemoveUser' - bool - - $hide - - - $user - - - $this->emitter - $this->emitter - $this->emitter - $this->emitter - $this->emitter - $this->emitter - + dispatch dispatch @@ -3518,19 +3452,6 @@ dispatch dispatch - - addToGroup - countUsersInGroup - deleteGroup - removeFromGroup - - - - - createGroup - getGroupDetails - isAdmin - diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1edc39c52f2f3..e9912655a4bbd 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1240,7 +1240,6 @@ 'OC\\FullTextSearch\\Model\\SearchRequestSimpleQuery' => $baseDir . '/lib/private/FullTextSearch/Model/SearchRequestSimpleQuery.php', 'OC\\FullTextSearch\\Model\\SearchTemplate' => $baseDir . '/lib/private/FullTextSearch/Model/SearchTemplate.php', 'OC\\GlobalScale\\Config' => $baseDir . '/lib/private/GlobalScale/Config.php', - 'OC\\Group\\Backend' => $baseDir . '/lib/private/Group/Backend.php', 'OC\\Group\\Database' => $baseDir . '/lib/private/Group/Database.php', 'OC\\Group\\Group' => $baseDir . '/lib/private/Group/Group.php', 'OC\\Group\\Manager' => $baseDir . '/lib/private/Group/Manager.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 2efd6effc9111..01ccdae1eb648 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1273,7 +1273,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\FullTextSearch\\Model\\SearchRequestSimpleQuery' => __DIR__ . '/../../..' . '/lib/private/FullTextSearch/Model/SearchRequestSimpleQuery.php', 'OC\\FullTextSearch\\Model\\SearchTemplate' => __DIR__ . '/../../..' . '/lib/private/FullTextSearch/Model/SearchTemplate.php', 'OC\\GlobalScale\\Config' => __DIR__ . '/../../..' . '/lib/private/GlobalScale/Config.php', - 'OC\\Group\\Backend' => __DIR__ . '/../../..' . '/lib/private/Group/Backend.php', 'OC\\Group\\Database' => __DIR__ . '/../../..' . '/lib/private/Group/Database.php', 'OC\\Group\\Group' => __DIR__ . '/../../..' . '/lib/private/Group/Group.php', 'OC\\Group\\Manager' => __DIR__ . '/../../..' . '/lib/private/Group/Manager.php', diff --git a/lib/private/Group/Backend.php b/lib/private/Group/Backend.php deleted file mode 100644 index 037cdacf4453e..0000000000000 --- a/lib/private/Group/Backend.php +++ /dev/null @@ -1,134 +0,0 @@ - - * @author Knut Ahlers - * @author Roeland Jago Douma - * @author Vincent Petry - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ -namespace OC\Group; - -/** - * Abstract base class for user management - */ -abstract class Backend implements \OCP\GroupInterface { - /** - * error code for functions not provided by the group backend - */ - public const NOT_IMPLEMENTED = -501; - - protected $possibleActions = [ - self::CREATE_GROUP => 'createGroup', - self::DELETE_GROUP => 'deleteGroup', - self::ADD_TO_GROUP => 'addToGroup', - self::REMOVE_FROM_GOUP => 'removeFromGroup', - self::COUNT_USERS => 'countUsersInGroup', - self::GROUP_DETAILS => 'getGroupDetails', - self::IS_ADMIN => 'isAdmin', - ]; - - /** - * Get all supported actions - * @return int bitwise-or'ed actions - * - * Returns the supported actions as int to be - * compared with \OC\Group\Backend::CREATE_GROUP etc. - */ - public function getSupportedActions() { - $actions = 0; - foreach ($this->possibleActions as $action => $methodName) { - if (method_exists($this, $methodName)) { - $actions |= $action; - } - } - - return $actions; - } - - /** - * Check if backend implements actions - * @param int $actions bitwise-or'ed actions - * @return bool - * - * Returns the supported actions as int to be - * compared with \OC\Group\Backend::CREATE_GROUP etc. - */ - public function implementsActions($actions) { - return (bool)($this->getSupportedActions() & $actions); - } - - /** - * is user in group? - * @param string $uid uid of the user - * @param string $gid gid of the group - * @return bool - * - * Checks whether the user is member of a group or not. - */ - public function inGroup($uid, $gid) { - return in_array($gid, $this->getUserGroups($uid)); - } - - /** - * Get all groups a user belongs to - * @param string $uid Name of the user - * @return array an array of group names - * - * This function fetches all groups a user belongs to. It does not check - * if the user exists at all. - */ - public function getUserGroups($uid) { - return []; - } - - /** - * get a list of all groups - * @param string $search - * @param int $limit - * @param int $offset - * @return array an array of group names - * - * Returns a list with all groups - */ - - public function getGroups($search = '', $limit = -1, $offset = 0) { - return []; - } - - /** - * check if a group exists - * @param string $gid - * @return bool - */ - public function groupExists($gid) { - return in_array($gid, $this->getGroups($gid, 1)); - } - - /** - * get a list of all users in a group - * @param string $gid - * @param string $search - * @param int $limit - * @param int $offset - * @return array an array of user ids - */ - public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { - return []; - } -} diff --git a/lib/private/Group/Database.php b/lib/private/Group/Database.php index 7b7ee41def9b2..bab6d27d9fb0e 100644 --- a/lib/private/Group/Database.php +++ b/lib/private/Group/Database.php @@ -30,6 +30,8 @@ namespace OC\Group; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; +use OC\User\DisplayNameCache; +use OC\User\LazyUser; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Group\Backend\ABackend; use OCP\Group\Backend\IAddToGroupBackend; @@ -43,6 +45,7 @@ use OCP\Group\Backend\ISetDisplayNameBackend; use OCP\Group\Backend\INamedBackend; use OCP\IDBConnection; +use OCP\IUserManager; /** * Class for group management in a SQL Database (e.g. MySQL, SQLite) @@ -59,11 +62,9 @@ class Database extends ABackend implements ISetDisplayNameBackend, INamedBackend { - /** @var string[] */ - private $groupCache = []; - - /** @var IDBConnection */ - private $dbConn; + /** @var array */ + private array $groupCache = []; + private ?IDBConnection $dbConn; /** * \OC\Group\Database constructor. @@ -267,7 +268,7 @@ public function getGroups($search = '', $limit = null, $offset = null) { $this->fixDI(); $query = $this->dbConn->getQueryBuilder(); - $query->select('gid') + $query->select('gid', 'displayname') ->from('groups') ->orderBy('gid', 'ASC'); @@ -286,6 +287,10 @@ public function getGroups($search = '', $limit = null, $offset = null) { $groups = []; while ($row = $result->fetch()) { + $this->groupCache[$row['gid']] = [ + 'displayname' => $row['displayname'], + 'gid' => $row['gid'], + ]; $groups[] = $row['gid']; } $result->closeCursor(); @@ -377,6 +382,58 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { return $users; } + public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array { + $this->fixDI(); + + $query = $this->dbConn->getQueryBuilder(); + if ($search !== '') { + $query->select('g.uid', 'u.displayname'); + } else { + $query->select('g.uid'); + } + + $query->from('group_user', 'g') + ->where($query->expr()->eq('gid', $query->createNamedParameter($gid))) + ->orderBy('g.uid', 'ASC'); + + if ($search !== '') { + $query->leftJoin('g', 'users', 'u', $query->expr()->eq('g.uid', 'u.uid')) + ->leftJoin('u', 'preferences', 'p', $query->expr()->andX( + $query->expr()->eq('p.userid', 'u.uid'), + $query->expr()->eq('p.appid', $query->expr()->literal('settings')), + $query->expr()->eq('p.configkey', $query->expr()->literal('email'))) + ) + // sqlite doesn't like re-using a single named parameter here + ->andWhere( + $query->expr()->orX( + $query->expr()->ilike('g.uid', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')), + $query->expr()->ilike('u.displayname', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')), + $query->expr()->ilike('p.configvalue', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')) + ) + ) + ->orderBy('u.uid_lower', 'ASC'); + } + + if ($limit !== -1) { + $query->setMaxResults($limit); + } + if ($offset !== 0) { + $query->setFirstResult($offset); + } + + $result = $query->executeQuery(); + + $users = []; + $userManager = \OC::$server->get(IUserManager::class); + $displayNameCache = \OC::$server->get(DisplayNameCache::class); + while ($row = $result->fetch()) { + $users[$row['uid']] = new LazyUser($row['uid'], $displayNameCache, $userManager, $row['displayname'] ?? null); + } + $result->closeCursor(); + + return $users; + } + /** * get the number of all users matching the search string in a group * @param string $gid @@ -474,6 +531,28 @@ public function getGroupDetails(string $gid): array { return []; } + public function getGroupsDetails(array $gids): array { + $details = []; + foreach (array_chunk($gids, 1000) as $chunk) { + $query = $this->dbConn->getQueryBuilder(); + $query->select('gid', 'displayname') + ->from('groups') + ->where($query->expr()->in('gid', $query->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY))); + + $result = $query->executeQuery(); + while ($row = $result->fetch()) { + $details[$row['gid']] = $row['displayname']; + $this->groupCache[$row['gid']] = [ + 'displayname' => $row['displayname'], + 'gid' => $row['gid'], + ]; + } + $result->closeCursor(); + } + + return $details; + } + public function setDisplayName(string $gid, string $displayName): bool { if (!$this->groupExists($gid)) { return false; diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php index 2ef4d2ee23fd3..6a2936f33fcc1 100644 --- a/lib/private/Group/Group.php +++ b/lib/private/Group/Group.php @@ -33,10 +33,14 @@ namespace OC\Group; use OC\Hooks\PublicEmitter; +use OCP\Group\Backend\IAddToGroupBackend; use OCP\Group\Backend\ICountDisabledInGroup; +use OCP\Group\Backend\ICountUsersBackend; +use OCP\Group\Backend\IDeleteGroupBackend; use OCP\Group\Backend\IGetDisplayNameBackend; use OCP\Group\Backend\IHideFromCollaborationBackend; use OCP\Group\Backend\INamedBackend; +use OCP\Group\Backend\IRemoveFromGroupBackend; use OCP\Group\Backend\ISetDisplayNameBackend; use OCP\GroupInterface; use OCP\IGroup; @@ -46,31 +50,24 @@ use Symfony\Component\EventDispatcher\GenericEvent; class Group implements IGroup { - /** @var null|string */ - protected $displayName; + protected ?string $displayName; + private string $gid; - /** @var string */ - private $gid; + /** @var IUser[] */ + private array $users = []; + private bool $usersLoaded = false; - /** @var \OC\User\User[] */ - private $users = []; - - /** @var bool */ - private $usersLoaded; - - /** @var Backend[] */ - private $backends; - /** @var EventDispatcherInterface */ - private $dispatcher; + /** @var GroupInterface[] */ + private array $backends; + private EventDispatcherInterface $dispatcher; /** @var \OC\User\Manager|IUserManager */ private $userManager; - /** @var PublicEmitter */ - private $emitter; + private ?PublicEmitter $emitter; /** * @param string $gid - * @param Backend[] $backends + * @param list $backends * @param EventDispatcherInterface $dispatcher * @param IUserManager $userManager * @param PublicEmitter $emitter @@ -85,11 +82,11 @@ public function __construct(string $gid, array $backends, EventDispatcherInterfa $this->displayName = $displayName; } - public function getGID() { + public function getGID(): string { return $this->gid; } - public function getDisplayName() { + public function getDisplayName(): string { if (is_null($this->displayName)) { foreach ($this->backends as $backend) { if ($backend instanceof IGetDisplayNameBackend) { @@ -120,11 +117,11 @@ public function setDisplayName(string $displayName): bool { } /** - * get all users in the group + * Get all users in the group * - * @return \OC\User\User[] + * @return IUser[] */ - public function getUsers() { + public function getUsers(): array { if ($this->usersLoaded) { return $this->users; } @@ -151,7 +148,7 @@ public function getUsers() { * @param IUser $user * @return bool */ - public function inGroup(IUser $user) { + public function inGroup(IUser $user): bool { if (isset($this->users[$user->getUID()])) { return true; } @@ -165,11 +162,9 @@ public function inGroup(IUser $user) { } /** - * add a user to the group - * - * @param IUser $user + * Add a user to the group */ - public function addUser(IUser $user) { + public function addUser(IUser $user): void { if ($this->inGroup($user)) { return; } @@ -182,7 +177,8 @@ public function addUser(IUser $user) { $this->emitter->emit('\OC\Group', 'preAddUser', [$this, $user]); } foreach ($this->backends as $backend) { - if ($backend->implementsActions(\OC\Group\Backend::ADD_TO_GROUP)) { + if ($backend instanceof IAddToGroupBackend || $backend->implementsActions(GroupInterface::ADD_TO_GROUP)) { + /** @var IAddToGroupBackend $backend */ $backend->addToGroup($user->getUID(), $this->gid); if ($this->users) { $this->users[$user->getUID()] = $user; @@ -201,11 +197,9 @@ public function addUser(IUser $user) { } /** - * remove a user from the group - * - * @param \OC\User\User $user + * Remove a user from the group */ - public function removeUser($user) { + public function removeUser(IUser $user): void { $result = false; $this->dispatcher->dispatch(IGroup::class . '::preRemoveUser', new GenericEvent($this, [ 'user' => $user, @@ -214,7 +208,9 @@ public function removeUser($user) { $this->emitter->emit('\OC\Group', 'preRemoveUser', [$this, $user]); } foreach ($this->backends as $backend) { - if ($backend->implementsActions(\OC\Group\Backend::REMOVE_FROM_GOUP) and $backend->inGroup($user->getUID(), $this->gid)) { + if (($backend instanceof IRemoveFromGroupBackend || $backend->implementsActions(GroupInterface::REMOVE_FROM_GOUP)) + && $backend->inGroup($user->getUID(), $this->gid)) { + /** @var IRemoveFromGroupBackend $backend */ $backend->removeFromGroup($user->getUID(), $this->gid); $result = true; } @@ -238,18 +234,13 @@ public function removeUser($user) { } /** - * search for users in the group by userid - * - * @param string $search - * @param int $limit - * @param int $offset - * @return \OC\User\User[] + * Search for users in the group by userid or display name + * @return IUser[] */ - public function searchUsers($search, $limit = null, $offset = null) { + public function searchUsers(string $search, ?int $limit = null, ?int $offset = null): array { $users = []; foreach ($this->backends as $backend) { - $userIds = $backend->usersInGroup($this->gid, $search, $limit, $offset); - $users += $this->getVerifiedUsers($userIds); + $users = array_merge($users, $backend->searchInGroup($this->gid, $search, $limit ? $limit : -1, $offset ? $offset : 0)); if (!is_null($limit) and $limit <= 0) { return $users; } @@ -261,12 +252,13 @@ public function searchUsers($search, $limit = null, $offset = null) { * returns the number of users matching the search string * * @param string $search - * @return int|bool + * @return int|false */ - public function count($search = '') { + public function count(string $search = '') { $users = false; foreach ($this->backends as $backend) { - if ($backend->implementsActions(\OC\Group\Backend::COUNT_USERS)) { + if ($backend instanceof ICountUsersBackend || $backend->implementsActions(GroupInterface::COUNT_USERS)) { + /** @var ICountUsersBackend $backend */ if ($users === false) { //we could directly add to a bool variable, but this would //be ugly @@ -281,7 +273,7 @@ public function count($search = '') { /** * returns the number of disabled users * - * @return int|bool + * @return int|false */ public function countDisabled() { $users = false; @@ -299,18 +291,15 @@ public function countDisabled() { } /** - * search for users in the group by displayname + * Search for users in the group by display name * - * @param string $search - * @param int $limit - * @param int $offset - * @return \OC\User\User[] + * @return \OCP\IUser[] + * @depreacted 25.0.0 Use searchUsers instead (same implementation) */ - public function searchDisplayName($search, $limit = null, $offset = null) { + public function searchDisplayName(string $search, ?int $limit = null, ?int $offset = null): array { $users = []; foreach ($this->backends as $backend) { - $userIds = $backend->usersInGroup($this->gid, $search, $limit, $offset); - $users = $this->getVerifiedUsers($userIds); + $users = $backend->searchInGroup($this->gid, $search, $limit, $offset); if (!is_null($limit) and $limit <= 0) { return array_values($users); } @@ -323,7 +312,7 @@ public function searchDisplayName($search, $limit = null, $offset = null) { * * @return string[] */ - public function getBackendNames() { + public function getBackendNames(): array { $backends = []; foreach ($this->backends as $backend) { if ($backend instanceof INamedBackend) { @@ -338,10 +327,8 @@ public function getBackendNames() { /** * delete the group - * - * @return bool */ - public function delete() { + public function delete(): bool { // Prevent users from deleting group admin if ($this->getGID() === 'admin') { return false; @@ -353,7 +340,8 @@ public function delete() { $this->emitter->emit('\OC\Group', 'preDelete', [$this]); } foreach ($this->backends as $backend) { - if ($backend->implementsActions(\OC\Group\Backend::DELETE_GROUP)) { + if ($backend instanceof IDeleteGroupBackend || $backend->implementsActions(GroupInterface::DELETE_GROUP)) { + /** @var IDeleteGroupBackend $backend */ $result = $result || $backend->deleteGroup($this->gid); } } @@ -367,14 +355,11 @@ public function delete() { } /** - * returns all the Users from an array that really exists + * Get all the Users from an array that really exists * @param string[] $userIds an array containing user IDs - * @return \OC\User\User[] an Array with the userId as Key and \OC\User\User as value + * @return \OCP\IUser[] an Array with the userId as Key and \OC\User\User as value */ - private function getVerifiedUsers($userIds) { - if (!is_array($userIds)) { - return []; - } + private function getVerifiedUsers(array $userIds): array { $users = []; foreach ($userIds as $userId) { $user = $this->userManager->get($userId); @@ -385,11 +370,7 @@ private function getVerifiedUsers($userIds) { return $users; } - /** - * @return bool - * @since 14.0.0 - */ - public function canRemoveUser() { + public function canRemoveUser(): bool { foreach ($this->backends as $backend) { if ($backend->implementsActions(GroupInterface::REMOVE_FROM_GOUP)) { return true; @@ -398,11 +379,7 @@ public function canRemoveUser() { return false; } - /** - * @return bool - * @since 14.0.0 - */ - public function canAddUser() { + public function canAddUser(): bool { foreach ($this->backends as $backend) { if ($backend->implementsActions(GroupInterface::ADD_TO_GROUP)) { return true; @@ -411,13 +388,9 @@ public function canAddUser() { return false; } - /** - * @return bool - * @since 16.0.0 - */ public function hideFromCollaboration(): bool { - return array_reduce($this->backends, function (bool $hide, GroupInterface $backend) { - return $hide | ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid)); + return array_reduce($this->backends, function (bool $hide, GroupInterface $backend): bool { + return $hide || ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid)); }, false); } } diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index 28f7a400b41dd..4dd5a2be749c2 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -41,6 +41,9 @@ use OC\Hooks\PublicEmitter; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Backend\ICreateGroupBackend; +use OCP\Group\Backend\IGroupDetailsBackend; +use OCP\Group\Backend\IIsAdminBackend; use OCP\GroupInterface; use OCP\IGroup; use OCP\IGroupManager; @@ -73,10 +76,10 @@ class Manager extends PublicEmitter implements IGroupManager { private $dispatcher; private LoggerInterface $logger; - /** @var \OC\Group\Group[] */ - private $cachedGroups = []; + /** @var array */ + private array $cachedGroups = []; - /** @var (string[])[] */ + /** @var array */ private $cachedUserGroups = []; /** @var \OC\SubAdmin */ @@ -169,17 +172,13 @@ public function get($gid) { return $this->getGroupObject($gid); } - /** - * @param string $gid - * @param string $displayName - * @return \OCP\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)) { + if ($backend instanceof IGroupDetailsBackend || $backend->implementsActions(GroupInterface::GROUP_DETAILS)) { + /** @var IGroupDetailsBackend $backend */ $groupData = $backend->getGroupDetails($gid); - if (is_array($groupData) && !empty($groupData)) { + if (!empty($groupData)) { // take the display name from the first backend that has a non-null one if (is_null($displayName) && isset($groupData['displayName'])) { $displayName = $groupData['displayName']; @@ -193,10 +192,54 @@ protected function getGroupObject($gid, $displayName = null) { if (count($backends) === 0) { return null; } + /** @var GroupInterface[] $backends */ $this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this, $displayName); return $this->cachedGroups[$gid]; } + /** + * @param list $gids List of groupIds for which we want to create a IGroup object + * @param array $displayNames Array containing already know display name for a groupId + * @return array + */ + protected function getGroupsObject(array $gids, array $displayNames = []): array { + $backends = []; + $groups = []; + foreach ($gids as $gid) { + $backends[$gid] = []; + if (!isset($displayNames[$gid])) { + $displayNames[$gid] = null; + } + } + foreach ($this->backends as $backend) { + if ($backend instanceof IGroupDetailsBackend || $backend->implementsActions(GroupInterface::GROUP_DETAILS)) { + $groupDatas = $backend->getGroupsDetails($gids); + foreach ($groupDatas as $gid => $groupData) { + if (!empty($groupData)) { + // take the display name from the first backend that has a non-null one + if (isset($groupData['displayName'])) { + $displayNames[$gid] = $groupData['displayName']; + } + $backends[$gid][] = $backend; + } + } + } else { + $existingGroups = $backend->groupsExists($gids); + foreach ($existingGroups as $group) { + $backends[$group][] = $backend; + } + } + } + foreach ($gids as $gid) { + if (count($backends[$gid]) === 0) { + continue; + } + $this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]); + $groups[$gid] = $this->cachedGroups[$gid]; + } + return $groups; + } + /** * @param string $gid * @return bool @@ -217,7 +260,8 @@ public function createGroup($gid) { } else { $this->emit('\OC\Group', 'preCreate', [$gid]); foreach ($this->backends as $backend) { - if ($backend->implementsActions(Backend::CREATE_GROUP)) { + if ($backend instanceof ICreateGroupBackend || $backend->implementsActions(GroupInterface::CREATE_GROUP)) { + /** @var ICreateGroupBackend $backend */ if ($backend->createGroup($gid)) { $group = $this->getGroupObject($gid); $this->emit('\OC\Group', 'postCreate', [$group]); @@ -239,13 +283,9 @@ public function search($search, $limit = null, $offset = null) { $groups = []; foreach ($this->backends as $backend) { $groupIds = $backend->getGroups($search, $limit, $offset); - foreach ($groupIds as $groupId) { - $aGroup = $this->get($groupId); - if ($aGroup instanceof IGroup) { - $groups[$groupId] = $aGroup; - } else { - $this->logger->debug('Group "' . $groupId . '" was returned by search but not found through direct access', ['app' => 'core']); - } + $newGroups = $this->getGroupsObject($groupIds); + foreach ($newGroups as $groupId => $group) { + $groups[$groupId] = $group; } if (!is_null($limit) and $limit <= 0) { return array_values($groups); @@ -292,8 +332,11 @@ public function getUserIdGroups(string $uid): array { */ public function isAdmin($userId) { foreach ($this->backends as $backend) { - if ($backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) { - return true; + if ($backend instanceof IIsAdminBackend || $backend->implementsActions(GroupInterface::IS_ADMIN)) { + /** @var IIsAdminBackend $backend */ + if ($backend->isAdmin($userId)) { + return true; + } } } return $this->isInGroup($userId, 'admin'); @@ -351,7 +394,7 @@ public function getUserGroupNames(IUser $user) { } /** - * get a list of all display names in a group + * Get a list of all display names in a group * * @param string $gid * @param string $search @@ -360,11 +403,11 @@ public function getUserGroupNames(IUser $user) { * @return array an array of display names (value) and user ids (key) */ public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { + $search = trim($search); $group = $this->get($gid); if (is_null($group)) { return []; } - $search = trim($search); $groupUsers = []; @@ -397,7 +440,7 @@ public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0 $matchingUsers = []; foreach ($groupUsers as $groupUser) { - $matchingUsers[(string) $groupUser->getUID()] = $groupUser->getDisplayName(); + $matchingUsers[$groupUser->getUID()] = $groupUser->getDisplayName(); } return $matchingUsers; } diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php index 8e93d6481ab20..089e6f4330f37 100644 --- a/lib/private/User/LazyUser.php +++ b/lib/private/User/LazyUser.php @@ -31,17 +31,27 @@ class LazyUser implements IUser { private ?IUser $user = null; private DisplayNameCache $displayNameCache; private string $uid; + private ?string $displayName; private IUserManager $userManager; + private ?UserInterface $backend; - public function __construct(string $uid, DisplayNameCache $displayNameCache, IUserManager $userManager) { + public function __construct(string $uid, DisplayNameCache $displayNameCache, IUserManager $userManager, ?string $displayName = null, ?UserInterface $backend = null) { $this->displayNameCache = $displayNameCache; $this->uid = $uid; $this->userManager = $userManager; + $this->displayName = $displayName; + $this->backend = $backend; } private function getUser(): IUser { if ($this->user === null) { - $this->user = $this->userManager->get($this->uid); + if ($this->backend) { + /** @var \OC\User\Manager $manager */ + $manager = $this->userManager; + $this->user = $manager->getUserObject($this->uid, $this->backend); + } else { + $this->user = $this->userManager->get($this->uid); + } } /** @var IUser */ $user = $this->user; @@ -53,6 +63,10 @@ public function getUID() { } public function getDisplayName() { + if ($this->displayName) { + return $this->displayName; + } + return $this->displayNameCache->getDisplayName($this->uid); } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index a6f5658532536..9d38c9410e35c 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -186,14 +186,14 @@ public function get($uid) { } /** - * get or construct the user object + * Get or construct the user object * * @param string $uid * @param \OCP\UserInterface $backend * @param bool $cacheUser If false the newly created user object will not be cached * @return \OC\User\User */ - protected function getUserObject($uid, $backend, $cacheUser = true) { + public function getUserObject($uid, $backend, $cacheUser = true) { if ($backend instanceof IGetRealUIDBackend) { $uid = $backend->getRealUID($uid); } @@ -289,15 +289,16 @@ public function checkPasswordNoLogging($loginName, $password) { * @param string $pattern * @param int $limit * @param int $offset - * @return \OC\User\User[] + * @return IUser[] */ public function search($pattern, $limit = null, $offset = null) { + $displayNameCache = \OC::$server->get(DisplayNameCache::class); $users = []; foreach ($this->backends as $backend) { $backendUsers = $backend->getUsers($pattern, $limit, $offset); if (is_array($backendUsers)) { foreach ($backendUsers as $uid) { - $users[$uid] = $this->getUserObject($uid, $backend); + $users[$uid] = new LazyUser($uid, $displayNameCache, $this, null, $backend); } } } @@ -313,28 +314,29 @@ public function search($pattern, $limit = null, $offset = null) { } /** - * search by displayName + * Search by displayName * * @param string $pattern * @param int $limit * @param int $offset - * @return \OC\User\User[] + * @return IUser[] */ public function searchDisplayName($pattern, $limit = null, $offset = null) { + $displayNameCache = \OC::$server->get(DisplayNameCache::class); $users = []; foreach ($this->backends as $backend) { $backendUsers = $backend->getDisplayNames($pattern, $limit, $offset); if (is_array($backendUsers)) { foreach ($backendUsers as $uid => $displayName) { - $users[] = $this->getUserObject($uid, $backend); + $users[] = new LazyUser($uid, $displayNameCache, $this, $displayName, $backend); } } } usort($users, function ($a, $b) { /** - * @var \OC\User\User $a - * @var \OC\User\User $b + * @var IUser $a + * @var IUser $b */ return strcasecmp($a->getDisplayName(), $b->getDisplayName()); }); diff --git a/lib/public/Group/Backend/ABackend.php b/lib/public/Group/Backend/ABackend.php index 2d611c27b4f38..1c7d62594e10b 100644 --- a/lib/public/Group/Backend/ABackend.php +++ b/lib/public/Group/Backend/ABackend.php @@ -25,7 +25,11 @@ */ namespace OCP\Group\Backend; +use OC\User\DisplayNameCache; +use OC\User\LazyUser; use OCP\GroupInterface; +use OCP\IUserManager; +use OCP\Server; /** * @since 14.0.0 @@ -65,4 +69,38 @@ public function implementsActions($actions): bool { return (bool)($actions & $implements); } + + public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array { + // Default implementation for compatibility reasons + $displayNameCache = Server::get(DisplayNameCache::class); + $userManager = Server::get(IUserManager::class); + $users = []; + foreach ($this->usersInGroup($gid, $search, $limit, $offset) as $userId) { + $users[$userId] = new LazyUser($userId, $displayNameCache, $userManager); + } + return $users; + } + + public function groupsExists(array $gids): array { + $existingGroups = []; + foreach ($gids as $gid) { + $exists = $this->groupExists($gid); + if ($exists) { + $existringGroups[] = $gid; + } + } + return $existingGroups; + } + + public function getGroupsDetails(array $gids): array { + if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) { + throw new \Exception("Should not have been called"); + } + /** @var IGroupDetailsBackend $this */ + $groupData = []; + foreach ($gids as $gid) { + $groupData[$gid] = $this->getGroupDetails($gid); + } + return $groupData; + } } diff --git a/lib/public/Group/Backend/IGroupDetailsBackend.php b/lib/public/Group/Backend/IGroupDetailsBackend.php index 56241d5538b81..520250d4a54e6 100644 --- a/lib/public/Group/Backend/IGroupDetailsBackend.php +++ b/lib/public/Group/Backend/IGroupDetailsBackend.php @@ -6,6 +6,7 @@ * @copyright Copyright (c) 2018 Roeland Jago Douma * * @author Roeland Jago Douma + * @author Carl Schwan * * @license GNU AGPL version 3 or any later version * @@ -26,11 +27,18 @@ namespace OCP\Group\Backend; /** + * @brief Optional interface for group backends * @since 14.0.0 */ interface IGroupDetailsBackend { /** + * @brief Get additional details for a group, for example the display name. + * + * The array returned can be empty when no additional information is available + * for the group. + * + * @return array{displayName?: string} * @since 14.0.0 */ public function getGroupDetails(string $gid): array; diff --git a/lib/public/GroupInterface.php b/lib/public/GroupInterface.php index f1765afed961e..70633aa7006b4 100644 --- a/lib/public/GroupInterface.php +++ b/lib/public/GroupInterface.php @@ -94,7 +94,7 @@ public function getUserGroups($uid); * @return array an array of group names * @since 4.5.0 * - * Returns a list with all groups + * @deprecated since 25.0.0 use getGroupObjects as it is more efficient */ public function getGroups($search = '', $limit = -1, $offset = 0); @@ -107,13 +107,45 @@ public function getGroups($search = '', $limit = -1, $offset = 0); public function groupExists($gid); /** - * get a list of all users in a group + * Check if a list of groups exists + * @param list $gids + * @return list The list of group that exists + * @since 25.0.0 + */ + public function groupsExists(array $gids): array; + + /** + * @brief Get a list of user ids in a group matching the given search parameters. + * * @param string $gid * @param string $search * @param int $limit * @param int $offset * @return array an array of user ids * @since 4.5.0 + * @depreacted 25.0.0 Use searchInGroup instead, for performance reasons */ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0); + + /** + * @brief Get a list of users matching the given search parameters. + * + * Implementations of this method must return lazy evaluated user objects and + * preload if possible the display name. + * + * + * $users = $groupBackend->searchInGroup('admin', 'John', 10, 0); + * + * + * @param string $gid The group id of the user we want to search + * @param string $search The part of the display name or user id of the users we + * want to search. This can be empty to get all the users. + * @param int $limit The limit of results + * @param int $offset The offset of the results + * @return IUser[] + * @since 25.0.0 + */ + public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array; + + public function getGroupsDetails(array $gids): array; } diff --git a/lib/public/IGroup.php b/lib/public/IGroup.php index ba13359c472d9..55277a09f9fb5 100644 --- a/lib/public/IGroup.php +++ b/lib/public/IGroup.php @@ -31,25 +31,40 @@ /** * Interface IGroup * + * This interface makes it possible to interact with groups and abstract the + * various group backends. You should use it directly in your application and + * not subclass it. + * + * + * // Get the admin group + * $adminGroup = $groupManager->get('admin'); + * + * * @since 8.0.0 */ interface IGroup { /** - * @return string + * @brief Get the group id of the group. + * + * The group id is a unique identifier for the group. If multiple group + * backend use the same group identifier, the users of the various backends + * will be considered to be in the same group. + * * @since 8.0.0 */ - public function getGID(); + public function getGID(): string; /** - * Returns the group display name + * @brief Get the display name of the group. + * + * Depending on the internal backend, this might be the same as @see getGID * - * @return string * @since 12.0.0 */ - public function getDisplayName(); + public function getDisplayName(): string; /** - * Set the group display name + * @brief Set the group display name * * @param string $displayName * @return bool @@ -58,76 +73,66 @@ public function getDisplayName(); public function setDisplayName(string $displayName): bool; /** - * get all users in the group + * @brief Get all the users in the group * - * @return \OCP\IUser[] + * @return IUser[] * @since 8.0.0 */ - public function getUsers(); + public function getUsers(): array; /** - * check if a user is in the group + * @brief Check if a user is in the group. * - * @param \OCP\IUser $user - * @return bool * @since 8.0.0 */ - public function inGroup(IUser $user); + public function inGroup(IUser $user): bool; /** - * add a user to the group + * @brief Add a user to this group * - * @param \OCP\IUser $user * @since 8.0.0 */ - public function addUser(IUser $user); + public function addUser(IUser $user): void; /** - * remove a user from the group + * @brief Remove a user from the group * - * @param \OCP\IUser $user * @since 8.0.0 */ - public function removeUser($user); + public function removeUser(IUser $user): void; /** - * search for users in the group by userid + * @brief Search for users in the group by userid or display name * - * @param string $search - * @param int $limit - * @param int $offset - * @return \OCP\IUser[] + * @return IUser[] * @since 8.0.0 */ - public function searchUsers($search, $limit = null, $offset = null); + public function searchUsers(string $search, ?int $limit = null, ?int $offset = null): array; /** - * returns the number of users matching the search string + * @brief Count the number of users matching the search string * - * @param string $search - * @return int|bool + * @return int|false * @since 8.0.0 */ - public function count($search = ''); + public function count(string $search = ''); /** - * returns the number of disabled users + * @brief Get the number of disabled users in the group * - * @return int|bool + * @return int|false * @since 14.0.0 */ public function countDisabled(); /** - * search for users in the group by displayname + * Search for users in the group by display name * - * @param string $search - * @param int $limit - * @param int $offset - * @return \OCP\IUser[] + * @return IUser[] * @since 8.0.0 + * @depreacted 25.0.0 Use searchUsers instead, the implementation is the same */ - public function searchDisplayName($search, $limit = null, $offset = null); + public function searchDisplayName(string $search, ?int $limit = null, ?int $offset = null): array; /** * Get the names of the backends the group is connected to @@ -135,30 +140,34 @@ public function searchDisplayName($search, $limit = null, $offset = null); * @return string[] * @since 22.0.0 */ - public function getBackendNames(); + public function getBackendNames(): array; /** - * delete the group + * @brief Delete the group + * + * This is not possible for the admin group. * - * @return bool * @since 8.0.0 */ - public function delete(); + public function delete(): bool; /** - * @return bool + * @brief Check if the group is allowed to remove users + * * @since 14.0.0 */ - public function canRemoveUser(); + public function canRemoveUser(): bool; /** - * @return bool + * @brief Check if the group is allowed to add users + * * @since 14.0.0 */ - public function canAddUser(); + public function canAddUser(): bool; /** - * @return bool + * @brief Check if the group is hidden from collaboration + * * @since 16.0.0 */ public function hideFromCollaboration(): bool; diff --git a/tests/lib/Group/GroupTest.php b/tests/lib/Group/GroupTest.php index b85496620b83b..526ba0553029a 100644 --- a/tests/lib/Group/GroupTest.php +++ b/tests/lib/Group/GroupTest.php @@ -304,9 +304,9 @@ public function testSearchUsers() { $group = new \OC\Group\Group('group1', [$backend], $this->dispatcher, $userManager); $backend->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', '2') - ->willReturn(['user2']); + ->willReturn(['user2' => new \OC\User\User('user2', null, $this->dispatcher)]); $users = $group->searchUsers('2'); @@ -326,13 +326,13 @@ public function testSearchUsersMultipleBackends() { $group = new \OC\Group\Group('group1', [$backend1, $backend2], $this->dispatcher, $userManager); $backend1->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', '2') - ->willReturn(['user2']); + ->willReturn(['user2' => new \OC\User\User('user2', null, $this->dispatcher)]); $backend2->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', '2') - ->willReturn(['user2']); + ->willReturn(['user2' => new \OC\User\User('user2', null, $this->dispatcher)]); $users = $group->searchUsers('2'); @@ -349,9 +349,9 @@ public function testSearchUsersLimitAndOffset() { $group = new \OC\Group\Group('group1', [$backend], $this->dispatcher, $userManager); $backend->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', 'user', 1, 1) - ->willReturn(['user2']); + ->willReturn(['user2' => new \OC\User\User('user2', null, $this->dispatcher)]); $users = $group->searchUsers('user', 1, 1); @@ -371,13 +371,13 @@ public function testSearchUsersMultipleBackendsLimitAndOffset() { $group = new \OC\Group\Group('group1', [$backend1, $backend2], $this->dispatcher, $userManager); $backend1->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', 'user', 2, 1) - ->willReturn(['user2']); + ->willReturn(['user2' => new \OC\User\User('user2', null, $this->dispatcher)]); $backend2->expects($this->once()) - ->method('usersInGroup') + ->method('searchInGroup') ->with('group1', 'user', 2, 1) - ->willReturn(['user1']); + ->willReturn(['user1' => new \OC\User\User('user1', null, $this->dispatcher)]); $users = $group->searchUsers('user', 2, 1); @@ -441,17 +441,28 @@ public function testCountUsersMultipleBackends() { } public function testCountUsersNoMethod() { - $backend1 = $this->getMockBuilder('OC\Group\Database') - ->disableOriginalConstructor() + $backend1 = $this->getMockBuilder('OCP\Group\GroupInterface') + ->disableOriginalConstructor() + ->setMethods([ + 'getGroupDetails', + 'implementsActions', + 'getUserGroups', + 'inGroup', + 'getGroups', + 'groupExists', + 'usersInGroup', + 'createGroup', + 'addToGroup', + 'removeFromGroup', + 'searchInGroup', + 'countUsersInGroup' + ]) ->getMock(); $userManager = $this->getUserManager(); $group = new \OC\Group\Group('group1', [$backend1], $this->dispatcher, $userManager); $backend1->expects($this->never()) ->method('countUsersInGroup'); - $backend1->expects($this->any()) - ->method('implementsActions') - ->willReturn(false); $users = $group->count('2'); @@ -468,9 +479,6 @@ public function testDelete() { $backend->expects($this->once()) ->method('deleteGroup') ->with('group1'); - $backend->expects($this->any()) - ->method('implementsActions') - ->willReturn(true); $group->delete(); } diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index d689fd4eb7a49..272da55024eb5 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -87,6 +87,7 @@ private function getTestBackend($implementedActions = null) { 'createGroup', 'addToGroup', 'removeFromGroup', + 'searchInGroup', ]) ->getMock(); $backend->expects($this->any()) @@ -352,9 +353,13 @@ public function testSearchResultExistsButGroupDoesNot() { ->with('1') ->willReturn(['group1']); $backend->expects($this->once()) + ->method('getGroupDetails') + ->with('group1') + ->willReturn([]); + $backend->expects($this->any()) ->method('groupExists') ->with('group1') - ->willReturn(false); + ->willReturn(true); /** @var \OC\User\Manager $userManager */ $userManager = $this->createMock(Manager::class); @@ -379,6 +384,10 @@ public function testGetUserGroups() { ->method('groupExists') ->with('group1') ->willReturn(true); + $backend->expects($this->any()) + ->method('getGroupDetails') + ->with('group1') + ->willReturn([]); $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger); $manager->addBackend($backend); @@ -544,19 +553,10 @@ public function testDisplayNamesInGroupWithOneUserBackend() { ->method('groupExists') ->with('testgroup') ->willReturn(true); - - $backend->expects($this->any()) - ->method('inGroup') - ->willReturnCallback(function ($uid, $gid) { - switch ($uid) { - case 'user1': return false; - case 'user2': return true; - case 'user3': return false; - case 'user33': return true; - default: - return null; - } - }); + $backend->expects($this->once()) + ->method('getGroupDetails') + ->with('testgroup') + ->willReturn([]); $this->userManager->expects($this->any()) ->method('searchDisplayName') diff --git a/tests/lib/Util/Group/Dummy.php b/tests/lib/Util/Group/Dummy.php index 3735a5e1167e8..99d4fc1a77505 100644 --- a/tests/lib/Util/Group/Dummy.php +++ b/tests/lib/Util/Group/Dummy.php @@ -29,12 +29,17 @@ namespace Test\Util\Group; -use OC\Group\Backend; +use OCP\Group\Backend\ABackend; +use OCP\Group\Backend\IDeleteGroupBackend; +use OCP\Group\Backend\IAddToGroupBackend; +use OCP\Group\Backend\IRemoveFromGroupBackend; +use OCP\Group\Backend\ICreateGroupBackend; +use OCP\Group\Backend\ICountUsersBackend; /** - * dummy group backend, does not keep state, only for testing use + * Dummy group backend, does not keep state, only for testing use */ -class Dummy extends Backend { +class Dummy extends ABackend implements ICreateGroupBackend, IDeleteGroupBackend, IAddToGroupBackend, IRemoveFromGroupBackend, ICountUsersBackend { private $groups = []; /** * Try to create a new group @@ -44,7 +49,7 @@ class Dummy extends Backend { * Tries to create a new group. If the group name already exists, false will * be returned. */ - public function createGroup($gid) { + public function createGroup(string $gid): bool { if (!isset($this->groups[$gid])) { $this->groups[$gid] = []; return true; @@ -60,7 +65,7 @@ public function createGroup($gid) { * * Deletes a group and removes it from the group_user-table */ - public function deleteGroup($gid) { + public function deleteGroup(string $gid): bool { if (isset($this->groups[$gid])) { unset($this->groups[$gid]); return true; @@ -93,7 +98,7 @@ public function inGroup($uid, $gid) { * * Adds a user to a group. */ - public function addToGroup($uid, $gid) { + public function addToGroup(string $uid, string $gid): bool { if (isset($this->groups[$gid])) { if (array_search($uid, $this->groups[$gid]) === false) { $this->groups[$gid][] = $uid; @@ -114,7 +119,7 @@ public function addToGroup($uid, $gid) { * * removes the user from a group. */ - public function removeFromGroup($uid, $gid) { + public function removeFromGroup(string $uid, string $gid): bool { if (isset($this->groups[$gid])) { if (($index = array_search($uid, $this->groups[$gid])) !== false) { unset($this->groups[$gid][$index]); @@ -200,7 +205,7 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { * @param int $offset * @return int */ - public function countUsersInGroup($gid, $search = '', $limit = -1, $offset = 0) { + public function countUsersInGroup(string $gid, string $search = ''): int { if (isset($this->groups[$gid])) { if (empty($search)) { return count($this->groups[$gid]); @@ -213,5 +218,10 @@ public function countUsersInGroup($gid, $search = '', $limit = -1, $offset = 0) } return $count; } + return 0; + } + + public function groupExists($gid) { + return isset($this->groups[$gid]); } }