From 5958fccf90bbe8631a1802adc040b900cf0cd394 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Nov 2024 17:30:32 +0100 Subject: [PATCH] perf: use more optimized way to get user storage info in ocs user info when possible Signed-off-by: Robin Appelman --- .../lib/Controller/AUserData.php | 81 +++++++++++++------ .../lib/Controller/GroupsController.php | 5 +- .../lib/Controller/UsersController.php | 5 +- .../tests/Controller/GroupsControllerTest.php | 5 ++ .../tests/Controller/UsersControllerTest.php | 13 ++- 5 files changed, 78 insertions(+), 31 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index 61459e40b8b4b..0cd13c8ff4b09 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -11,7 +11,6 @@ use OC\Group\Manager as GroupManager; use OC\User\Backend; use OC\User\NoUserException; -use OC_Helper; use OCA\Provisioning_API\ResponseDefinitions; use OCP\Accounts\IAccountManager; use OCP\Accounts\PropertyDoesNotExistException; @@ -19,6 +18,8 @@ use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\Files\FileInfo; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Group\ISubAdmin; use OCP\IConfig; @@ -29,6 +30,7 @@ use OCP\L10N\IFactory; use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Backend\ISetPasswordBackend; +use OCP\Util; /** * @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions @@ -56,6 +58,7 @@ public function __construct( protected IAccountManager $accountManager, protected ISubAdmin $subAdminManager, protected IFactory $l10nFactory, + protected IRootFolder $rootFolder, ) { parent::__construct($appName, $request); } @@ -119,7 +122,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; $data['backend'] = $targetUserObject->getBackendClassName(); $data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); - $data[self::USER_FIELD_QUOTA] = $this->fillStorageInfo($targetUserObject->getUID()); + $data[self::USER_FIELD_QUOTA] = $this->fillStorageInfo($targetUserObject); $managers = $this->getManagers($targetUserObject); $data[self::USER_FIELD_MANAGER] = empty($managers) ? '' : $managers[0]; @@ -243,34 +246,62 @@ protected function getUserSubAdminGroupsData(string $userId): array { } /** - * @param string $userId + * @param IUser $user * @return Provisioning_APIUserDetailsQuota * @throws OCSException */ - protected function fillStorageInfo(string $userId): array { - try { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($userId); - $storage = OC_Helper::getStorageInfo('/', null, true, false); - $data = [ - 'free' => $storage['free'], - 'used' => $storage['used'], - 'total' => $storage['total'], - 'relative' => $storage['relative'], - self::USER_FIELD_QUOTA => $storage['quota'], - ]; - } catch (NotFoundException $ex) { - // User fs is not setup yet - $user = $this->userManager->get($userId); - if ($user === null) { - throw new OCSException('User does not exist', 101); + protected function fillStorageInfo(IUser $user): array { + $includeExternal = $this->config->getSystemValueBool('quota_include_external_storage'); + $userId = $user->getUID(); + + $quota = $user->getQuota(); + if ($quota === 'none') { + $quota = FileInfo::SPACE_UNLIMITED; + } else { + $quota = Util::computerFileSize($quota); + if ($quota === false) { + $quota = FileInfo::SPACE_UNLIMITED; } - $quota = $user->getQuota(); - if ($quota !== 'none') { - $quota = OC_Helper::computerFileSize($quota); + } + + try { + if ($includeExternal) { + \OC_Util::tearDownFS(); + \OC_Util::setupFS($user->getUID()); + $storage = \OC_Helper::getStorageInfo('/', null, true, false); + $data = [ + 'free' => $storage['free'], + 'used' => $storage['used'], + 'total' => $storage['total'], + 'relative' => $storage['relative'], + self::USER_FIELD_QUOTA => $storage['quota'], + ]; + } else { + $userFileInfo = $this->rootFolder->getUserFolder($userId)->getStorage()->getCache()->get(''); + $used = $userFileInfo->getSize(); + + if ($quota > 0) { + // prevent division by zero or error codes (negative values) + $relative = round(($used / $quota) * 10000) / 100; + $free = $quota - $used; + $total = $quota; + } else { + $relative = 0; + $free = FileInfo::SPACE_UNLIMITED; + $total = $used; + } + + $data = [ + 'free' => $free, + 'used' => $used, + 'total' => $total, + 'relative' => $relative, + self::USER_FIELD_QUOTA => $quota, + ]; } + } catch (NotFoundException $ex) { $data = [ - self::USER_FIELD_QUOTA => $quota !== false ? $quota : 'none', + self::USER_FIELD_QUOTA => $quota >= 0 ? $quota : 'none', 'used' => 0 ]; } catch (\Exception $e) { @@ -282,8 +313,6 @@ protected function fillStorageInfo(string $userId): array { 'exception' => $e, ] ); - /* In case the Exception left things in a bad state */ - \OC_Util::tearDownFS(); return []; } return $data; diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 87544cc899274..e2aac4fea198d 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -21,6 +21,7 @@ use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; use OCP\IConfig; use OCP\IGroup; @@ -48,6 +49,7 @@ public function __construct( IAccountManager $accountManager, ISubAdmin $subAdminManager, IFactory $l10nFactory, + IRootFolder $rootFolder, private LoggerInterface $logger, ) { parent::__construct($appName, @@ -58,7 +60,8 @@ public function __construct( $userSession, $accountManager, $subAdminManager, - $l10nFactory + $l10nFactory, + $rootFolder, ); } diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 9363b4ca9352c..2e276071fb9fd 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -31,6 +31,7 @@ use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; use OCP\HintException; use OCP\IConfig; @@ -67,6 +68,7 @@ public function __construct( IAccountManager $accountManager, ISubAdmin $subAdminManager, IFactory $l10nFactory, + IRootFolder $rootFolder, private IURLGenerator $urlGenerator, private LoggerInterface $logger, private NewUserMailHelper $newUserMailHelper, @@ -85,7 +87,8 @@ public function __construct( $userSession, $accountManager, $subAdminManager, - $l10nFactory + $l10nFactory, + $rootFolder, ); $this->l10n = $l10nFactory->get($appName); diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index b1a708b9aa2fd..29b098429e89a 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -12,6 +12,7 @@ use OCA\Provisioning_API\Controller\GroupsController; use OCP\Accounts\IAccountManager; use OCP\AppFramework\OCS\OCSException; +use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; use OCP\IConfig; use OCP\IGroup; @@ -46,6 +47,8 @@ class GroupsControllerTest extends \Test\TestCase { /** @var GroupsController|\PHPUnit\Framework\MockObject\MockObject */ protected $api; + private IRootFolder $rootFolder; + protected function setUp(): void { parent::setUp(); @@ -59,6 +62,7 @@ protected function setUp(): void { $this->subAdminManager = $this->createMock(ISubAdmin::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->rootFolder = $this->createMock(IRootFolder::class); $this->groupManager ->method('getSubAdmin') @@ -75,6 +79,7 @@ protected function setUp(): void { $this->accountManager, $this->subAdminManager, $this->l10nFactory, + $this->rootFolder, $this->logger ]) ->setMethods(['fillStorageInfo']) diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index d5931ef0eba75..3b27016ce9c62 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -23,6 +23,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; use OCP\IConfig; use OCP\IGroup; @@ -76,6 +77,7 @@ class UsersControllerTest extends TestCase { private $knownUserService; /** @var IEventDispatcher|MockObject */ private $eventDispatcher; + private IRootFolder $rootFolder; /** @var IPhoneNumberUtil */ private $phoneNumberUtil; @@ -98,6 +100,7 @@ protected function setUp(): void { $this->knownUserService = $this->createMock(KnownUserService::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->phoneNumberUtil = new PhoneNumberUtil(); + $this->rootFolder = $this->createMock(IRootFolder::class); $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement)); @@ -114,6 +117,7 @@ protected function setUp(): void { $this->accountManager, $this->subAdminManager, $this->l10nFactory, + $this->rootFolder, $this->urlGenerator, $this->logger, $this->newUserMailHelper, @@ -508,6 +512,7 @@ public function testAddUserSuccessfulWithDisplayName(): void { $this->accountManager, $this->subAdminManager, $this->l10nFactory, + $this->rootFolder, $this->urlGenerator, $this->logger, $this->newUserMailHelper, @@ -1127,7 +1132,7 @@ public function testGetUserDataAsAdmin(): void { $this->api ->expects($this->once()) ->method('fillStorageInfo') - ->with('UID') + ->with($targetUser) ->willReturn(['DummyValue']); $backend = $this->createMock(UserInterface::class); @@ -1254,7 +1259,7 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible(): void { $this->api ->expects($this->once()) ->method('fillStorageInfo') - ->with('UID') + ->with($targetUser) ->willReturn(['DummyValue']); $backend = $this->createMock(UserInterface::class); @@ -1429,7 +1434,7 @@ public function testGetUserDataAsSubAdminSelfLookup(): void { $this->api ->expects($this->once()) ->method('fillStorageInfo') - ->with('UID') + ->with($targetUser) ->willReturn(['DummyValue']); $backend = $this->createMock(UserInterface::class); @@ -3788,6 +3793,7 @@ public function testGetCurrentUserLoggedIn(): void { $this->accountManager, $this->subAdminManager, $this->l10nFactory, + $this->rootFolder, $this->urlGenerator, $this->logger, $this->newUserMailHelper, @@ -3878,6 +3884,7 @@ public function testGetUser(): void { $this->accountManager, $this->subAdminManager, $this->l10nFactory, + $this->rootFolder, $this->urlGenerator, $this->logger, $this->newUserMailHelper,