perf: use more optimized way to get user storage info in ocs user info when possible

Signed-off-by: Robin Appelman <robin@icewind.nl>
2024-11-25 17:30:32 +07:00 · 2024-11-25 17:30:32 +07:00 · 118ff883b0
parent e686b93d73
commit 118ff883b0
5 changed files with 78 additions and 31 deletions
--- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php
+++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php
@ -11,7 +11,6 @@ namespace OCA\Provisioning_API\Controller;
 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\Http;
 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\IUserSession;
 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 @@ abstract class AUserDataOCSController extends OCSController {
 		protected IAccountManager $accountManager,
 		protected ISubAdmin $subAdminManager,
 		protected IFactory $l10nFactory,
+		protected IRootFolder $rootFolder,
 	) {
 		parent::__construct($appName, $request);
 	}
@ -119,7 +122,7 @@ abstract class AUserDataOCSController extends OCSController {
 		$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 @@ abstract class AUserDataOCSController extends OCSController {
 	}

 	/**
-	 * @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 = FileInfo::SPACE_UNLIMITED;
+				}
+
+				$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 @@ abstract class AUserDataOCSController extends OCSController {
 					'exception' => $e,
 				]
 			);
-			/* In case the Exception left things in a bad state */
-			\OC_Util::tearDownFS();
 			return [];
 		}
 		return $data;
--- a/apps/provisioning_api/lib/Controller/GroupsController.php
+++ b/apps/provisioning_api/lib/Controller/GroupsController.php
@ -21,6 +21,7 @@ use OCP\AppFramework\OCS\OCSException;
 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 @@ class GroupsController extends AUserDataOCSController {
 		IAccountManager $accountManager,
 		ISubAdmin $subAdminManager,
 		IFactory $l10nFactory,
+		IRootFolder $rootFolder,
 		private LoggerInterface $logger,
 	) {
 		parent::__construct($appName,
@ -58,7 +60,8 @@ class GroupsController extends AUserDataOCSController {
 			$userSession,
 			$accountManager,
 			$subAdminManager,
-			$l10nFactory
+			$l10nFactory,
+			$rootFolder,
 		);
 	}

--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@ -31,6 +31,7 @@ use OCP\AppFramework\OCS\OCSForbiddenException;
 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 @@ class UsersController extends AUserDataOCSController {
 		IAccountManager $accountManager,
 		ISubAdmin $subAdminManager,
 		IFactory $l10nFactory,
+		IRootFolder $rootFolder,
 		private IURLGenerator $urlGenerator,
 		private LoggerInterface $logger,
 		private NewUserMailHelper $newUserMailHelper,
@ -85,7 +87,8 @@ class UsersController extends AUserDataOCSController {
 			$userSession,
 			$accountManager,
 			$subAdminManager,
-			$l10nFactory
+			$l10nFactory,
+			$rootFolder,
 		);

 		$this->l10n = $l10nFactory->get($appName);
--- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
@ -12,6 +12,7 @@ use OC\User\NoUserException;
 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 @@ class GroupsControllerTest extends \Test\TestCase {
 		$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 @@ class GroupsControllerTest extends \Test\TestCase {
 				$this->accountManager,
 				$this->subAdminManager,
 				$this->l10nFactory,
+				$this->rootFolder,
 				$this->logger
 			])
 			->setMethods(['fillStorageInfo'])
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@ -23,6 +23,7 @@ use OCP\Accounts\IAccountPropertyCollection;
 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 @@ class UsersControllerTest extends TestCase {
 		$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 @@ class UsersControllerTest extends TestCase {
 				$this->accountManager,
 				$this->subAdminManager,
 				$this->l10nFactory,
+				$this->rootFolder,
 				$this->urlGenerator,
 				$this->logger,
 				$this->newUserMailHelper,
@ -508,6 +512,7 @@ class UsersControllerTest extends TestCase {
 				$this->accountManager,
 				$this->subAdminManager,
 				$this->l10nFactory,
+				$this->rootFolder,
 				$this->urlGenerator,
 				$this->logger,
 				$this->newUserMailHelper,
@ -1127,7 +1132,7 @@ class UsersControllerTest extends TestCase {
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('UID')
+			->with($targetUser)
 			->willReturn(['DummyValue']);

 		$backend = $this->createMock(UserInterface::class);
@ -1254,7 +1259,7 @@ class UsersControllerTest extends TestCase {
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('UID')
+			->with($targetUser)
 			->willReturn(['DummyValue']);

 		$backend = $this->createMock(UserInterface::class);
@ -1429,7 +1434,7 @@ class UsersControllerTest extends TestCase {
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('UID')
+			->with($targetUser)
 			->willReturn(['DummyValue']);

 		$backend = $this->createMock(UserInterface::class);
@ -3788,6 +3793,7 @@ class UsersControllerTest extends TestCase {
 				$this->accountManager,
 				$this->subAdminManager,
 				$this->l10nFactory,
+				$this->rootFolder,
 				$this->urlGenerator,
 				$this->logger,
 				$this->newUserMailHelper,
@ -3878,6 +3884,7 @@ class UsersControllerTest extends TestCase {
 				$this->accountManager,
 				$this->subAdminManager,
 				$this->l10nFactory,
+				$this->rootFolder,
 				$this->urlGenerator,
 				$this->logger,
 				$this->newUserMailHelper,