Skip to content

fix(performance): move array_merge outside of loop #55915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miaulalala wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(performance): move array_merge outside of loop #55915

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
188874e
fix(performance): move array_merge outside of loop
miaulalala Oct 22, 2025
96d7f17
fixup! fix(performance): move array_merge outside of loop
miaulalala Oct 28, 2025
d13c31a
fixup! fix(performance): move array_merge outside of loop
miaulalala Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 93 additions & 95 deletions apps/provisioning_api/lib/Controller/UsersController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
use OCP\Files\IRootFolder;
use OCP\Group\ISubAdmin;
use OCP\HintException;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
Expand All @@ -60,6 +62,9 @@
class UsersController extends AUserDataOCSController {

private IL10N $l10n;
private ICache $cache;

private const CACHE_PREFIX = 'provisioning_usercache';

public function __construct(
string $appName,
Expand All @@ -81,6 +86,7 @@ public function __construct(
private IEventDispatcher $eventDispatcher,
private IPhoneNumberUtil $phoneNumberUtil,
private IAppManager $appManager,
private ICacheFactory $cacheFactory,
) {
parent::__construct(
$appName,
Expand All @@ -96,6 +102,7 @@ public function __construct(
);

$this->l10n = $l10nFactory->get($appName);
$this->cache = $this->cacheFactory->createDistributed(self::CACHE_PREFIX);
}

/**
Expand All @@ -111,30 +118,46 @@ public function __construct(
#[NoAdminRequired]
public function getUsers(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
$user = $this->userSession->getUser();
$users = [];

// Admin? Or SubAdmin?
$uid = $user->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();

$users = $this->cache->get($uid . '_' . $search);
if (!empty($users)) {
return new DataResponse([
'users' => $users
]);
}

$isAdmin = $this->groupManager->isAdmin($uid);
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid);
if ($isAdmin || $isDelegatedAdmin) {
$users = $this->userManager->search($search, $limit, $offset);
} elseif ($subAdminManager->isSubAdmin($user)) {
$subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user);
foreach ($subAdminOfGroups as $key => $group) {
$subAdminOfGroups[$key] = $group->getGID();
}
$users = array_keys($users);
$this->cache->set($uid . '_' . $search, $users, 10);
return new DataResponse([
'users' => $users
]);
}

$users = [];
foreach ($subAdminOfGroups as $group) {
$users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search, $limit, $offset));
}
$users = [];
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$subAdminManager->isSubAdmin($user)) {
return new DataResponse([
'users' => $users
]);
}

/** @var list<string> $users */
$users = array_keys($users);
$subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user);
foreach ($subAdminOfGroups as $key => $group) {
$subAdminOfGroups[$key] = $group->getGID();
}

foreach ($subAdminOfGroups as $group) {
foreach ($this->groupManager->displayNamesInGroup($group, $search, $limit, $offset) as $uid => $user) {
$users[] = $uid;
}
}

$this->cache->set($uid . '_' . $search, $users, 10);
return new DataResponse([
'users' => $users
]);
Expand All @@ -152,29 +175,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset =
*/
#[NoAdminRequired]
public function getUsersDetails(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
$currentUser = $this->userSession->getUser();
$users = [];

// Admin? Or SubAdmin?
$uid = $currentUser->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();
$isAdmin = $this->groupManager->isAdmin($uid);
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid);
if ($isAdmin || $isDelegatedAdmin) {
$users = $this->userManager->search($search, $limit, $offset);
$users = array_keys($users);
} elseif ($subAdminManager->isSubAdmin($currentUser)) {
$subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser);
foreach ($subAdminOfGroups as $key => $group) {
$subAdminOfGroups[$key] = $group->getGID();
}

$users = [];
foreach ($subAdminOfGroups as $group) {
$users[] = array_keys($this->groupManager->displayNamesInGroup($group, $search, $limit, $offset));
}
$users = array_merge(...$users);
}
$users = $this->getUsers($search, $limit, $offset)->getData()['users'];

$usersDetails = [];
foreach ($users as $userId) {
Expand All @@ -187,14 +188,9 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
$userData = null;
$this->logger->warning('Found one enabled account that is removed from its backend, but still exists in Nextcloud database', ['accountId' => $userId]);
}
// Do not insert empty entry
if ($userData !== null) {
$usersDetails[$userId] = $userData;
} else {
// Logged user does not have permissions to see this user
// only showing its id
$usersDetails[$userId] = ['id' => $userId];
}

// $userdata === null means logged user does not have permissions to see this user
$usersDetails[$userId] = $userData ?? ['id' => $userId];
}

return new DataResponse([
Expand Down Expand Up @@ -833,6 +829,7 @@ public function editUserMultiValue(
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

$this->cache->clear(self::CACHE_PREFIX);
$subAdminManager = $this->groupManager->getSubAdmin();
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
$isAdminOrSubadmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID())
Expand Down Expand Up @@ -931,6 +928,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

$this->cache->clear(self::CACHE_PREFIX);
$permittedFields = [];
if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
$allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true);
Expand Down Expand Up @@ -1337,6 +1335,7 @@ public function deleteUser(string $userId): DataResponse {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

$this->cache->clear(self::CACHE_PREFIX);
// Go ahead with the delete
if ($targetUser->delete()) {
return new DataResponse();
Expand Down Expand Up @@ -1383,7 +1382,6 @@ public function enableUser(string $userId): DataResponse {
*/
private function setEnabled(string $userId, bool $value): DataResponse {
$currentLoggedInUser = $this->userSession->getUser();

$targetUser = $this->userManager->get($userId);
if ($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) {
throw new OCSException('', 101);
Expand All @@ -1397,6 +1395,7 @@ private function setEnabled(string $userId, bool $value): DataResponse {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

$this->cache->clear(self::CACHE_PREFIX);
// enable/disable the user now
$targetUser->setEnabled($value);
return new DataResponse();
Expand Down Expand Up @@ -1429,22 +1428,21 @@ public function getUsersGroups(string $userId): DataResponse {
return new DataResponse([
'groups' => $this->groupManager->getUserGroupIds($targetUser)
]);
} else {
$subAdminManager = $this->groupManager->getSubAdmin();
}

// Looking up someone else
if ($subAdminManager->isUserAccessible($loggedInUser, $targetUser)) {
// Return the group that the method caller is subadmin of for the user in question
$groups = array_values(array_intersect(
array_map(static fn (IGroup $group) => $group->getGID(), $subAdminManager->getSubAdminsGroups($loggedInUser)),
$this->groupManager->getUserGroupIds($targetUser)
));
return new DataResponse(['groups' => $groups]);
} else {
// Not permitted
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}
$subAdminManager = $this->groupManager->getSubAdmin();

// Looking up someone else
if (!$subAdminManager->isUserAccessible($loggedInUser, $targetUser)) {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

// Return the group that the method caller is subadmin of for the user in question
$groups = array_values(array_intersect(
array_map(static fn (IGroup $group) => $group->getGID(), $subAdminManager->getSubAdminsGroups($loggedInUser)),
$this->groupManager->getUserGroupIds($targetUser)
));
return new DataResponse(['groups' => $groups]);
}

/**
Expand Down Expand Up @@ -1487,41 +1485,41 @@ function (Group $group) {
return new DataResponse([
'groups' => $groups,
]);
} else {
$subAdminManager = $this->groupManager->getSubAdmin();
}

// Looking up someone else
if ($subAdminManager->isUserAccessible($loggedInUser, $targetUser)) {
// Return the group that the method caller is subadmin of for the user in question
$gids = array_values(array_intersect(
array_map(
static fn (IGroup $group) => $group->getGID(),
$subAdminManager->getSubAdminsGroups($loggedInUser),
),
$this->groupManager->getUserGroupIds($targetUser)
));
$groups = array_map(
function (string $gid) {
$group = $this->groupManager->get($gid);
return [
'id' => $group->getGID(),
'displayname' => $group->getDisplayName(),
'usercount' => $group->count(),
'disabled' => $group->countDisabled(),
'canAdd' => $group->canAddUser(),
'canRemove' => $group->canRemoveUser(),
];
},
$gids,
);
return new DataResponse([
'groups' => $groups,
]);
} else {
// Not permitted
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}
$subAdminManager = $this->groupManager->getSubAdmin();

// Looking up someone else
if (!$subAdminManager->isUserAccessible($loggedInUser, $targetUser)) {
// Not permitted
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

// Return the group that the method caller is subadmin of for the user in question
$gids = array_values(array_intersect(
array_map(
static fn (IGroup $group) => $group->getGID(),
$subAdminManager->getSubAdminsGroups($loggedInUser),
),
$this->groupManager->getUserGroupIds($targetUser)
));
$groups = array_map(
function (string $gid) {
$group = $this->groupManager->get($gid);
return [
'id' => $group->getGID(),
'displayname' => $group->getDisplayName(),
'usercount' => $group->count(),
'disabled' => $group->countDisabled(),
'canAdd' => $group->canAddUser(),
'canRemove' => $group->canRemoveUser(),
];
},
$gids,
);
return new DataResponse([
'groups' => $groups,
]);
}

/**
Expand Down Expand Up @@ -1623,7 +1621,7 @@ public function addToGroup(string $userId, string $groupid = ''): DataResponse {
public function removeFromGroup(string $userId, string $groupid): DataResponse {
$loggedInUser = $this->userSession->getUser();

if ($groupid === null || trim($groupid) === '') {
if (trim($groupid) === '') {
throw new OCSException('', 101);
}

Expand Down
Loading