From aa83b0b22d26e2d16b7798d3df09f5caf19df1ae Mon Sep 17 00:00:00 2001
From: Robin Appelman <robin@icewind.nl>
Date: Mon, 4 May 2020 17:25:47 +0200
Subject: [PATCH] dont get the group details if we only ask for the id
currenty when getting the groups for a user, the full group object is always created (and cached)
even if only the groupid is required
Signed-off-by: Robin Appelman <robin@icewind.nl>
---
lib/private/Group/Manager.php | 51 +++++++++++++++++++--------------
tests/lib/Group/ManagerTest.php | 20 ++++++-------
2 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index 73c07a6197f..6056bcdb3e3 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -77,7 +77,7 @@ class Manager extends PublicEmitter implements IGroupManager {
/** @var \OC\Group\Group[] */
private $cachedGroups = [];
- /** @var \OC\Group\Group[] */
+ /** @var (string[])[] */
private $cachedUserGroups = [];
/** @var \OC\SubAdmin */
@@ -276,25 +276,18 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return \OC\Group\Group[]
*/
public function getUserIdGroups($uid) {
- if (isset($this->cachedUserGroups[$uid])) {
- return $this->cachedUserGroups[$uid];
- }
$groups = [];
- foreach ($this->backends as $backend) {
- $groupIds = $backend->getUserGroups($uid);
- if (is_array($groupIds)) {
- foreach ($groupIds as $groupId) {
- $aGroup = $this->get($groupId);
- if ($aGroup instanceof IGroup) {
- $groups[$groupId] = $aGroup;
- } else {
- $this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
- }
- }
+
+ foreach ($this->getUserIdGroupIds($uid) as $groupId) {
+ $aGroup = $this->get($groupId);
+ if ($aGroup instanceof IGroup) {
+ $groups[$groupId] = $aGroup;
+ } else {
+ $this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
}
}
- $this->cachedUserGroups[$uid] = $groups;
- return $this->cachedUserGroups[$uid];
+
+ return $groups;
}
/**
@@ -320,7 +313,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return bool if in group
*/
public function isInGroup($userId, $group) {
- return array_key_exists($group, $this->getUserIdGroups($userId));
+ return array_search($group, $this->getUserIdGroupIds($userId)) !== false;
}
/**
@@ -330,9 +323,25 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return array with group ids
*/
public function getUserGroupIds(IUser $user) {
- return array_map(function ($value) {
- return (string)$value;
- }, array_keys($this->getUserGroups($user)));
+ return $this->getUserIdGroupIds($user->getUID());
+ }
+
+ /**
+ * @param string $uid the user id
+ * @return GroupInterface[]
+ */
+ private function getUserIdGroupIds($uid) {
+ if (!isset($this->cachedUserGroups[$uid])) {
+ $groups = [];
+ foreach ($this->backends as $backend) {
+ if ($groupIds = $backend->getUserGroups($uid)) {
+ $groups = array_merge($groups, $groupIds);
+ }
+ }
+ $this->cachedUserGroups[$uid] = $groups;
+ }
+
+ return $this->cachedUserGroups[$uid];
}
/**
diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 05ffa7973c0..ff1d6e641ea 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -390,17 +390,15 @@ class ManagerTest extends TestCase {
}
public function testGetUserGroupIds() {
- /** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Manager $manager */
- $manager = $this->getMockBuilder(\OC\Group\Manager::class)
- ->disableOriginalConstructor()
- ->setMethods(['getUserGroups'])
- ->getMock();
- $manager->expects($this->once())
- ->method('getUserGroups')
- ->willReturn([
- '123' => '123',
- 'abc' => 'abc',
- ]);
+ /**
+ * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
+ */
+ $backend = $this->getTestBackend();
+ $backend->method('getUserGroups')
+ ->willReturn(['123', 'abc']);
+
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger);
+ $manager->addBackend($backend);
/** @var \OC\User\User|\PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
--
GitLab