Skip to content

Commit ed66a2b

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #62222 from nextcloud/backport/61904/stable33
[stable33] perf: Replace more calls from getUserGroups to getUserGroupIds
2 parents d0e7a5a + 441fdec commit ed66a2b

15 files changed

Lines changed: 402 additions & 48 deletions

‎apps/provisioning_api/lib/Controller/AUserDataOCSController.php‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99
namespace OCA\Provisioning_API\Controller;
1010

11+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1112
use OC\Group\Manager as GroupManager;
1213
use OC\User\Backend;
1314
use OC\User\NoUserException;
@@ -35,6 +36,7 @@
3536

3637
/**
3738
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
39+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
3840
* @psalm-import-type Provisioning_APIUserDetailsQuota from ResponseDefinitions
3941
*/
4042
abstract class AUserDataOCSController extends OCSController {
@@ -61,6 +63,7 @@ public function __construct(
6163
protected ISubAdmin $subAdminManager,
6264
protected IFactory $l10nFactory,
6365
protected IRootFolder $rootFolder,
66+
private GroupDisplayNameCache $groupDisplayNameCache,
6467
) {
6568
parent::__construct($appName, $request);
6669
}
@@ -102,11 +105,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
102105

103106
// Get groups data
104107
$userAccount = $this->accountManager->getAccount($targetUserObject);
105-
$groups = $this->groupManager->getUserGroups($targetUserObject);
106-
$gids = [];
107-
foreach ($groups as $group) {
108-
$gids[] = $group->getGID();
109-
}
108+
$gids = $this->groupManager->getUserGroupIds($targetUserObject);
110109

111110
if ($isAdmin || $isDelegatedAdmin) {
112111
try {
@@ -252,6 +251,36 @@ protected function getUserSubAdminGroupsData(string $userId): array {
252251
return $groups;
253252
}
254253

254+
/**
255+
* A full group has id, name, usercount, disabled, canAdd and canRemove. Only
256+
* the displayname is cached; usercount/disabled are not cached. So this only
257+
* returns an {id, displayname} skeleton instead of the full group.
258+
*
259+
* @param array<string, Provisioning_APIUserDetails|array{id: string}> $userDetails
260+
* @return list<Provisioning_APIUserDetailsGroupDisplayname>
261+
*/
262+
protected function findGroupsWithDisplayname(array $userDetails): array {
263+
$groupIds = [];
264+
265+
foreach ($userDetails as $userDetail) {
266+
if (isset($userDetail['groups'])) {
267+
array_push($groupIds, ...array_values($userDetail['groups']));
268+
}
269+
if (isset($userDetail['subadmin'])) {
270+
array_push($groupIds, ...array_values($userDetail['subadmin']));
271+
}
272+
}
273+
274+
$groupIds = array_unique($groupIds);
275+
sort($groupIds);
276+
277+
$info = [];
278+
foreach ($this->groupDisplayNameCache->getDisplayNames($groupIds) as $groupId => $displayName) {
279+
$info[] = ['id' => $groupId, 'displayname' => $displayName ?? $groupId];
280+
}
281+
return $info;
282+
}
283+
255284
/**
256285
* @param IUser $user
257286
* @return Provisioning_APIUserDetailsQuota

‎apps/provisioning_api/lib/Controller/GroupsController.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99
namespace OCA\Provisioning_API\Controller;
1010

11+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1112
use OCA\Provisioning_API\ResponseDefinitions;
1213
use OCA\Settings\Settings\Admin\Sharing;
1314
use OCA\Settings\Settings\Admin\Users;
@@ -36,6 +37,7 @@
3637
/**
3738
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
3839
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
40+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
3941
*/
4042
class GroupsController extends AUserDataOCSController {
4143

@@ -51,6 +53,7 @@ public function __construct(
5153
IFactory $l10nFactory,
5254
IRootFolder $rootFolder,
5355
private LoggerInterface $logger,
56+
GroupDisplayNameCache $groupDisplayNameCache,
5457
) {
5558
parent::__construct($appName,
5659
$request,
@@ -62,6 +65,7 @@ public function __construct(
6265
$subAdminManager,
6366
$l10nFactory,
6467
$rootFolder,
68+
$groupDisplayNameCache,
6569
);
6670
}
6771

@@ -186,7 +190,7 @@ public function getGroupUsers(string $groupId): DataResponse {
186190
* @param int|null $limit Limit the amount of groups returned
187191
* @param int $offset Offset for searching for groups
188192
*
189-
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
193+
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
190194
* @throws OCSException
191195
*
192196
* 200: Group users details returned
@@ -229,7 +233,10 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
229233
// continue if a users ceased to exist.
230234
}
231235
}
232-
return new DataResponse(['users' => $usersDetails]);
236+
return new DataResponse([
237+
'users' => $usersDetails,
238+
'groups' => $this->findGroupsWithDisplayname($usersDetails),
239+
]);
233240
}
234241

235242
throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND);

‎apps/provisioning_api/lib/Controller/UsersController.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
use InvalidArgumentException;
1414
use OC\Authentication\Token\RemoteWipe;
15+
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
1516
use OC\Group\Group;
1617
use OC\KnownUser\KnownUserService;
1718
use OC\User\Backend;
@@ -56,6 +57,7 @@
5657
/**
5758
* @psalm-import-type Provisioning_APIGroupDetails from ResponseDefinitions
5859
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
60+
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
5961
*/
6062
class UsersController extends AUserDataOCSController {
6163

@@ -81,6 +83,7 @@ public function __construct(
8183
private IEventDispatcher $eventDispatcher,
8284
private IPhoneNumberUtil $phoneNumberUtil,
8385
private IAppManager $appManager,
86+
GroupDisplayNameCache $groupDisplayNameCache,
8487
) {
8588
parent::__construct(
8689
$appName,
@@ -93,6 +96,7 @@ public function __construct(
9396
$subAdminManager,
9497
$l10nFactory,
9598
$rootFolder,
99+
$groupDisplayNameCache,
96100
);
97101

98102
$this->l10n = $l10nFactory->get($appName);
@@ -146,7 +150,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset =
146150
* @param string $search Text to search for
147151
* @param int|null $limit Limit the amount of groups returned
148152
* @param int $offset Offset for searching for groups
149-
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
153+
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, array{}>
150154
*
151155
* 200: Users details returned
152156
*/
@@ -198,7 +202,8 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
198202
}
199203

200204
return new DataResponse([
201-
'users' => $usersDetails
205+
'users' => $usersDetails,
206+
'groups' => $this->findGroupsWithDisplayname($usersDetails),
202207
]);
203208
}
204209

‎apps/provisioning_api/lib/ResponseDefinitions.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@
2020
*
2121
* @psalm-type Provisioning_APIUserDetailsScope = 'v2-private'|'v2-local'|'v2-federated'|'v2-published'
2222
*
23+
* @psalm-type Provisioning_APIUserDetailsGroupDisplayname = array{
24+
* id: string,
25+
* displayname: string,
26+
* }
27+
*
2328
* @psalm-type Provisioning_APIUserDetails = array{
2429
* additional_mail: list<string>,
2530
* additional_mailScope?: list<Provisioning_APIUserDetailsScope>,

‎apps/provisioning_api/openapi-full.json‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,21 @@
333333
}
334334
}
335335
},
336+
"UserDetailsGroupDisplayname": {
337+
"type": "object",
338+
"required": [
339+
"id",
340+
"displayname"
341+
],
342+
"properties": {
343+
"id": {
344+
"type": "string"
345+
},
346+
"displayname": {
347+
"type": "string"
348+
}
349+
}
350+
},
336351
"UserDetailsQuota": {
337352
"type": "object",
338353
"properties": {
@@ -3526,7 +3541,8 @@
35263541
"data": {
35273542
"type": "object",
35283543
"required": [
3529-
"users"
3544+
"users",
3545+
"groups"
35303546
],
35313547
"properties": {
35323548
"users": {
@@ -3549,6 +3565,12 @@
35493565
}
35503566
]
35513567
}
3568+
},
3569+
"groups": {
3570+
"type": "array",
3571+
"items": {
3572+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
3573+
}
35523574
}
35533575
}
35543576
}
@@ -3991,7 +4013,8 @@
39914013
"data": {
39924014
"type": "object",
39934015
"required": [
3994-
"users"
4016+
"users",
4017+
"groups"
39954018
],
39964019
"properties": {
39974020
"users": {
@@ -4014,6 +4037,12 @@
40144037
}
40154038
]
40164039
}
4040+
},
4041+
"groups": {
4042+
"type": "array",
4043+
"items": {
4044+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
4045+
}
40174046
}
40184047
}
40194048
}

‎apps/provisioning_api/openapi.json‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,21 @@
333333
}
334334
}
335335
},
336+
"UserDetailsGroupDisplayname": {
337+
"type": "object",
338+
"required": [
339+
"id",
340+
"displayname"
341+
],
342+
"properties": {
343+
"id": {
344+
"type": "string"
345+
},
346+
"displayname": {
347+
"type": "string"
348+
}
349+
}
350+
},
336351
"UserDetailsQuota": {
337352
"type": "object",
338353
"properties": {
@@ -930,7 +945,8 @@
930945
"data": {
931946
"type": "object",
932947
"required": [
933-
"users"
948+
"users",
949+
"groups"
934950
],
935951
"properties": {
936952
"users": {
@@ -953,6 +969,12 @@
953969
}
954970
]
955971
}
972+
},
973+
"groups": {
974+
"type": "array",
975+
"items": {
976+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
977+
}
956978
}
957979
}
958980
}
@@ -1506,7 +1528,8 @@
15061528
"data": {
15071529
"type": "object",
15081530
"required": [
1509-
"users"
1531+
"users",
1532+
"groups"
15101533
],
15111534
"properties": {
15121535
"users": {
@@ -1529,6 +1552,12 @@
15291552
}
15301553
]
15311554
}
1555+
},
1556+
"groups": {
1557+
"type": "array",
1558+
"items": {
1559+
"$ref": "#/components/schemas/UserDetailsGroupDisplayname"
1560+
}
15321561
}
15331562
}
15341563
}

0 commit comments

Comments
 (0)