aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/user_ldap/lib/Group_LDAP.php6
-rw-r--r--apps/user_ldap/lib/Group_Proxy.php31
-rw-r--r--build/psalm-baseline.xml10
-rw-r--r--lib/private/Group/Database.php85
-rw-r--r--lib/private/Group/Manager.php65
-rw-r--r--lib/public/Group/Backend/ABackend.php30
-rw-r--r--lib/public/Group/Backend/IGroupDetailsBackend.php23
-rw-r--r--lib/public/GroupInterface.php19
-rw-r--r--tests/lib/Group/ManagerTest.php9
9 files changed, 246 insertions, 32 deletions
diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 3a17ba70bb1..b3ff63d3b5c 100644
--- a/apps/user_ldap/lib/Group_LDAP.php
+++ b/apps/user_ldap/lib/Group_LDAP.php
@@ -48,11 +48,12 @@ use Exception;
use OC\ServerNotAvailableException;
use OCP\Cache\CappedMemoryCache;
use OCP\GroupInterface;
+use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\Group\Backend\IGetDisplayNameBackend;
use Psr\Log\LoggerInterface;
-class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
+class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
protected bool $enabled = false;
/** @var CappedMemoryCache<string[]> $cachedGroupMembers array of users with gid as key */
@@ -63,6 +64,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
protected CappedMemoryCache $cachedNestedGroups;
protected GroupPluginManager $groupPluginManager;
protected LoggerInterface $logger;
+ protected Access $access;
/**
* @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name
@@ -70,7 +72,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
protected string $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)) {
diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php
index 9d996b45f43..3556be8d125 100644
--- a/apps/user_ldap/lib/Group_Proxy.php
+++ b/apps/user_ldap/lib/Group_Proxy.php
@@ -31,7 +31,9 @@ namespace OCA\User_LDAP;
use OC\ServerNotAvailableException;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\Group\Backend\IGetDisplayNameBackend;
+use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Backend\INamedBackend;
+use OCP\GroupInterface;
class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend {
private $backends = [];
@@ -257,6 +259,21 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet
}
/**
+ * {@inheritdoc}
+ */
+ 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
*
* @return string[] with group names
@@ -305,6 +322,20 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet
}
/**
+ * {@inheritdoc}
+ */
+ public function groupsExists(array $gids): array {
+ $existingGroups = [];
+ foreach ($gids as $gid) {
+ $exists = $this->handleRequest($gid, 'groupExists', [$gid]);
+ if ($exists) {
+ $existingGroups[$gid] = $gid;
+ }
+ }
+ return $existingGroups;
+ }
+
+ /**
* Check if backend implements actions
*
* @param int $actions bitwise-or'ed actions
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index b97a2962c76..0982147feff 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -2905,16 +2905,6 @@
<code><![CDATA[is_null($this->getContent())]]></code>
</TypeDoesNotContainNull>
</file>
- <file src="lib/private/Group/Database.php">
- <InvalidArrayOffset>
- <code><![CDATA[$this->groupCache[$gid]['displayname']]]></code>
- </InvalidArrayOffset>
- <InvalidPropertyAssignmentValue>
- <code><![CDATA[$this->groupCache]]></code>
- <code><![CDATA[$this->groupCache]]></code>
- <code><![CDATA[$this->groupCache]]></code>
- </InvalidPropertyAssignmentValue>
- </file>
<file src="lib/private/Group/DisplayNameCache.php">
<MissingTemplateParam>
<code>IEventListener</code>
diff --git a/lib/private/Group/Database.php b/lib/private/Group/Database.php
index ef5641d8137..0fcc151363b 100644
--- a/lib/private/Group/Database.php
+++ b/lib/private/Group/Database.php
@@ -62,11 +62,9 @@ class Database extends ABackend implements
ISetDisplayNameBackend,
ISearchableGroupBackend,
INamedBackend {
- /** @var string[] */
+ /** @var array<string, array{gid: string, displayname: string}> */
private $groupCache = [];
-
- /** @var IDBConnection */
- private $dbConn;
+ private ?IDBConnection $dbConn;
/**
* \OC\Group\Database constructor.
@@ -270,7 +268,7 @@ class Database extends ABackend implements
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
- $query->select('gid')
+ $query->select('gid', 'displayname')
->from('groups')
->orderBy('gid', 'ASC');
@@ -293,6 +291,10 @@ class Database extends ABackend implements
$groups = [];
while ($row = $result->fetch()) {
+ $this->groupCache[$row['gid']] = [
+ 'displayname' => $row['displayname'],
+ 'gid' => $row['gid'],
+ ];
$groups[] = $row['gid'];
}
$result->closeCursor();
@@ -332,6 +334,42 @@ class Database extends ABackend implements
}
/**
+ * {@inheritdoc}
+ */
+ public function groupsExists(array $gids): array {
+ $notFoundGids = [];
+ $existingGroups = [];
+
+ // In case the data is already locally accessible, not need to do SQL query
+ // or do a SQL query but with a smaller in clause
+ foreach ($gids as $gid) {
+ if (isset($this->groupCache[$gid])) {
+ $existingGroups[] = $gid;
+ } else {
+ $notFoundGids[] = $gid;
+ }
+ }
+
+ foreach (array_chunk($notFoundGids, 1000) as $chunk) {
+ $qb = $this->dbConn->getQueryBuilder();
+ $result = $qb->select('gid', 'displayname')
+ ->from('groups')
+ ->where($qb->expr()->in('gid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)))
+ ->executeQuery();
+ while ($row = $result->fetch()) {
+ $this->groupCache[$row['gid']] = [
+ 'displayname' => $row['displayname'],
+ 'gid' => $row['gid'],
+ ];
+ $existingGroups[] = $gid;
+ }
+ $result->closeCursor();
+ }
+
+ return $existingGroups;
+ }
+
+ /**
* Get a list of all users in a group
* @param string $gid
* @param string $search
@@ -488,6 +526,43 @@ class Database extends ABackend implements
return [];
}
+ /**
+ * {@inheritdoc}
+ */
+ public function getGroupsDetails(array $gids): array {
+ $notFoundGids = [];
+ $details = [];
+
+ // In case the data is already locally accessible, not need to do SQL query
+ // or do a SQL query but with a smaller in clause
+ foreach ($gids as $gid) {
+ if (isset($this->groupCache[$gid])) {
+ $details[$gid] = ['displayName' => $this->groupCache[$gid]['displayname']];
+ } else {
+ $notFoundGids[] = $gid;
+ }
+ }
+
+ foreach (array_chunk($notFoundGids, 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']] = ['displayName' => $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/Manager.php b/lib/private/Group/Manager.php
index c43b5165a79..924b9a3b6a6 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -21,6 +21,7 @@
* @author Vincent Petry <vincent@nextcloud.com>
* @author Vinicius Cubas Brand <vinicius@eita.org.br>
* @author voxsim "Simon Vocella"
+ * @author Carl Schwan <carl@carlschwan.eu>
*
* @license AGPL-3.0
*
@@ -41,6 +42,7 @@ namespace OC\Group;
use OC\Hooks\PublicEmitter;
use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Events\BeforeGroupCreatedEvent;
use OCP\Group\Events\GroupCreatedEvent;
use OCP\GroupInterface;
@@ -74,10 +76,10 @@ class Manager extends PublicEmitter implements IGroupManager {
private IEventDispatcher $dispatcher;
private LoggerInterface $logger;
- /** @var \OC\Group\Group[] */
+ /** @var array<string, IGroup> */
private $cachedGroups = [];
- /** @var (string[])[] */
+ /** @var array<string, list<string>> */
private $cachedUserGroups = [];
/** @var \OC\SubAdmin */
@@ -185,7 +187,7 @@ class Manager extends PublicEmitter implements IGroupManager {
if ($backend->implementsActions(Backend::GROUP_DETAILS)) {
$groupData = $backend->getGroupDetails($gid);
if (is_array($groupData) && !empty($groupData)) {
- // take the display name from the first backend that has a non-null one
+ // take the display name from the last backend that has a non-null one
if (is_null($displayName) && isset($groupData['displayName'])) {
$displayName = $groupData['displayName'];
}
@@ -198,11 +200,58 @@ class Manager extends PublicEmitter implements IGroupManager {
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];
}
/**
+ * @brief Batch method to create group objects
+ *
+ * @param list<string> $gids List of groupIds for which we want to create a IGroup object
+ * @param array<string, string> $displayNames Array containing already know display name for a groupId
+ * @return array<string, IGroup>
+ */
+ 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)) {
+ /** @var IGroupDetailsBackend $backend */
+ $groupDatas = $backend->getGroupsDetails($gids);
+ foreach ($groupDatas as $gid => $groupData) {
+ if (!empty($groupData)) {
+ // take the display name from the last 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
*/
@@ -246,13 +295,9 @@ class Manager extends PublicEmitter implements IGroupManager {
$groups = [];
foreach ($this->backends as $backend) {
$groupIds = $backend->getGroups($search, $limit ?? -1, $offset ?? 0);
- 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);
diff --git a/lib/public/Group/Backend/ABackend.php b/lib/public/Group/Backend/ABackend.php
index 7f5cf732335..7bf428b1e23 100644
--- a/lib/public/Group/Backend/ABackend.php
+++ b/lib/public/Group/Backend/ABackend.php
@@ -6,6 +6,7 @@ declare(strict_types=1);
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
+ * @author Carl Schwan <carl@carlschwan.eu>
*
* @license GNU AGPL version 3 or any later version
*
@@ -65,4 +66,33 @@ abstract class ABackend implements GroupInterface {
return (bool)($actions & $implements);
}
+
+ /**
+ * @since 26.0.0
+ */
+ public function groupsExists(array $gids): array {
+ $existingGroups = [];
+ foreach ($gids as $gid) {
+ $exists = $this->groupExists($gid);
+ if ($exists) {
+ $existingGroups[$gid] = $gid;
+ }
+ }
+ return $existingGroups;
+ }
+
+ /**
+ * @since 26.0.0
+ */
+ 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 4852f978195..a102c8c2eec 100644
--- a/lib/public/Group/Backend/IGroupDetailsBackend.php
+++ b/lib/public/Group/Backend/IGroupDetailsBackend.php
@@ -6,6 +6,7 @@ declare(strict_types=1);
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
+ * @author Carl Schwan <carl@carlschwan.eu>
*
* @license GNU AGPL version 3 or any later version
*
@@ -26,11 +27,33 @@ declare(strict_types=1);
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;
+
+
+ /**
+ * @brief Batch method to get the group details of a list of groups
+ *
+ * The default implementation in ABackend will just call getGroupDetail in
+ * a loop. But a GroupBackend implementation should provides a more optimized
+ * override this method to provide a more optimized way to execute this operation.
+ *
+ * @throw \RuntimeException if called on a backend that doesn't implements IGroupDetailsBackend
+ *
+ * @return array<string, array{displayName: string}>
+ * @since 26.0.0
+ */
+ public function getGroupsDetails(array $gids): array;
}
diff --git a/lib/public/GroupInterface.php b/lib/public/GroupInterface.php
index a18d38df002..50374e5a949 100644
--- a/lib/public/GroupInterface.php
+++ b/lib/public/GroupInterface.php
@@ -86,7 +86,8 @@ interface GroupInterface {
public function getUserGroups($uid);
/**
- * get a list of all groups
+ * @brief Get a list of all groups
+ *
* @param string $search
* @param int $limit
* @param int $offset
@@ -98,7 +99,8 @@ interface GroupInterface {
public function getGroups(string $search = '', int $limit = -1, int $offset = 0);
/**
- * check if a group exists
+ * @brief Check if a group exists
+ *
* @param string $gid
* @return bool
* @since 4.5.0
@@ -106,6 +108,19 @@ interface GroupInterface {
public function groupExists($gid);
/**
+ * @brief Batch method to check if a list of groups exists
+ *
+ * The default implementation in ABackend will just call groupExists in
+ * a loop. But a GroupBackend implementation should provides a more optimized
+ * override this method to provide a more optimized way to execute this operation.
+ *
+ * @param list<string> $gids
+ * @return list<string> 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
diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 2887d14acaa..06875f86536 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -92,6 +92,7 @@ class ManagerTest extends TestCase {
'inGroup',
'getGroups',
'groupExists',
+ 'groupsExists',
'usersInGroup',
'createGroup',
'addToGroup',
@@ -361,10 +362,12 @@ class ManagerTest extends TestCase {
->method('getGroups')
->with('1')
->willReturn(['group1']);
+ $backend->expects($this->never())
+ ->method('groupExists');
$backend->expects($this->once())
- ->method('groupExists')
- ->with('group1')
- ->willReturn(false);
+ ->method('getGroupsDetails')
+ ->with(['group1'])
+ ->willReturn([]);
/** @var \OC\User\Manager $userManager */
$userManager = $this->createMock(Manager::class);
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-31 14:16:56 +0000