From 36662a4f2354f44163ed3ce3b49756e761ae4240 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 8 Oct 2024 11:37:11 +0200 Subject: [PATCH 1/3] fix(FolderManager): Use OCP constant for unlimited space quota value Signed-off-by: provokateurin --- lib/Folder/FolderManager.php | 3 ++- tests/Folder/FolderManagerTest.php | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 396f0e8dc..8397d3a24 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -19,6 +19,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\FileInfo; use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\IConfig; @@ -657,7 +658,7 @@ public function getFoldersFromCircleMemberships(IUser $user, int $rootStorageId * @throws Exception */ public function createFolder(string $mountPoint): int { - $defaultQuota = $this->config->getSystemValueInt('groupfolders.quota.default', -3); + $defaultQuota = $this->config->getSystemValueInt('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED); $query = $this->connection->getQueryBuilder(); diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php index 19b2afd4c..5d89142fa 100644 --- a/tests/Folder/FolderManagerTest.php +++ b/tests/Folder/FolderManagerTest.php @@ -9,6 +9,7 @@ use OCA\GroupFolders\Folder\FolderManager; use OCP\Constants; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\FileInfo; use OCP\Files\IMimeTypeLoader; use OCP\IConfig; use OCP\IDBConnection; @@ -38,8 +39,8 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->config->expects($this->any()) ->method('getSystemValueInt') - ->with('groupfolders.quota.default', -3) - ->willReturn(-3); + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); $this->manager = new FolderManager( \OC::$server->getDatabaseConnection(), $this->groupManager, @@ -73,7 +74,7 @@ private function assertHasFolders($folders) { $folder['size'] = 0; } if (!isset($folder['quota'])) { - $folder['quota'] = -3; + $folder['quota'] = FileInfo::SPACE_UNLIMITED; } if (!isset($folder['acl'])) { $folder['acl'] = false; @@ -324,7 +325,7 @@ public function testGetFoldersForUserSimple() { 'folder_id' => 1, 'mount_point' => 'foo', 'permissions' => 31, - 'quota' => -3 + 'quota' => FileInfo::SPACE_UNLIMITED, ]; $manager->expects($this->once()) From 789437266017d9322375225181f3141b5bd565ad Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 8 Oct 2024 14:39:59 +0200 Subject: [PATCH 2/3] refactor(FolderManager): Add default rootStorageId 0 for getFolder() Signed-off-by: provokateurin --- lib/Folder/FolderManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 8397d3a24..610339ef9 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -259,7 +259,7 @@ private function getManageAcl(array $mappings): array { * @return array{id: mixed, mount_point: mixed, groups: array, quota: int, size: int, acl: bool}|false * @throws Exception */ - public function getFolder(int $id, int $rootStorageId) { + public function getFolder(int $id, int $rootStorageId = 0) { $applicableMap = $this->getAllApplicable(); $query = $this->connection->getQueryBuilder(); From 9568dc167da12d4d91366206eb94ade3b33e455c Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 8 Oct 2024 12:45:38 +0200 Subject: [PATCH 3/3] fix(FolderManager): Use placeholder value for default quota instead of the current value on creation Signed-off-by: provokateurin --- lib/Folder/FolderManager.php | 33 +++++++++---- src/settings/App.tsx | 1 + tests/Folder/FolderManagerTest.php | 74 ++++++++++++++++++++++++++++-- 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 610339ef9..a8181775c 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -35,6 +35,7 @@ class FolderManager { public const ENTITY_GROUP = 1; public const ENTITY_CIRCLE = 2; + public const SPACE_DEFAULT = -4; public function __construct( private IDBConnection $connection, @@ -69,7 +70,7 @@ public function getAllFolders(): array { 'id' => $id, 'mount_point' => $row['mount_point'], 'groups' => $applicableMap[$id] ?? [], - 'quota' => (int)$row['quota'], + 'quota' => $this->getRealQuota((int)$row['quota']), 'size' => 0, 'acl' => (bool)$row['acl'] ]; @@ -127,7 +128,7 @@ public function getAllFoldersWithSize(int $rootStorageId): array { 'id' => $id, 'mount_point' => $row['mount_point'], 'groups' => $applicableMap[$id] ?? [], - 'quota' => (int)$row['quota'], + 'quota' => $this->getRealQuota((int)$row['quota']), 'size' => $row['size'] ? (int)$row['size'] : 0, 'acl' => (bool)$row['acl'], 'manage' => $this->getManageAcl($mappings) @@ -172,7 +173,7 @@ public function getAllFoldersForUserWithSize(int $rootStorageId, IUser $user): a 'id' => $id, 'mount_point' => $row['mount_point'], 'groups' => $applicableMap[$id] ?? [], - 'quota' => (int)$row['quota'], + 'quota' => $this->getRealQuota((int)$row['quota']), 'size' => $row['size'] ? (int)$row['size'] : 0, 'acl' => (bool)$row['acl'], 'manage' => $this->getManageAcl($mappings) @@ -278,7 +279,7 @@ public function getFolder(int $id, int $rootStorageId = 0) { 'id' => $id, 'mount_point' => (string)$row['mount_point'], 'groups' => $applicableMap[$id] ?? [], - 'quota' => (int)$row['quota'], + 'quota' => $this->getRealQuota((int)$row['quota']), 'size' => $row['size'] ? $row['size'] : 0, 'acl' => (bool)$row['acl'], 'manage' => $this->getManageAcl($folderMappings) @@ -525,7 +526,7 @@ public function getFoldersForGroup(string $groupId, int $rootStorageId = 0): arr 'folder_id' => (int)$folder['folder_id'], 'mount_point' => (string)$folder['mount_point'], 'permissions' => (int)$folder['group_permissions'], - 'quota' => (int)$folder['quota'], + 'quota' => $this->getRealQuota((int)$folder['quota']), 'acl' => (bool)$folder['acl'], 'rootCacheEntry' => (isset($folder['fileid'])) ? Cache::cacheEntryFromData($folder, $this->mimeTypeLoader) : null ]; @@ -583,7 +584,7 @@ public function getFoldersForGroups(array $groupIds, int $rootStorageId = 0): ar 'folder_id' => (int)$folder['folder_id'], 'mount_point' => (string)$folder['mount_point'], 'permissions' => (int)$folder['group_permissions'], - 'quota' => (int)$folder['quota'], + 'quota' => $this->getRealQuota((int)$folder['quota']), 'acl' => (bool)$folder['acl'], 'rootCacheEntry' => (isset($folder['fileid'])) ? Cache::cacheEntryFromData($folder, $this->mimeTypeLoader) : null ]; @@ -646,7 +647,7 @@ public function getFoldersFromCircleMemberships(IUser $user, int $rootStorageId 'folder_id' => (int)$folder['folder_id'], 'mount_point' => (string)$folder['mount_point'], 'permissions' => (int)$folder['group_permissions'], - 'quota' => (int)$folder['quota'], + 'quota' => $this->getRealQuota((int)$folder['quota']), 'acl' => (bool)$folder['acl'], 'rootCacheEntry' => (isset($folder['fileid'])) ? Cache::cacheEntryFromData($folder, $this->mimeTypeLoader) : null ]; @@ -658,14 +659,12 @@ public function getFoldersFromCircleMemberships(IUser $user, int $rootStorageId * @throws Exception */ public function createFolder(string $mountPoint): int { - $defaultQuota = $this->config->getSystemValueInt('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED); - $query = $this->connection->getQueryBuilder(); $query->insert('group_folders') ->values([ 'mount_point' => $query->createNamedParameter($mountPoint), - 'quota' => $defaultQuota, + 'quota' => self::SPACE_DEFAULT, ]); $query->executeStatement(); $id = $query->getLastInsertId(); @@ -972,4 +971,18 @@ public function isCirclesAvailable(?CirclesManager &$circlesManager = null): boo return true; } + + private function getRealQuota(int $quota): int { + if ($quota === self::SPACE_DEFAULT) { + $defaultQuota = $this->config->getSystemValueInt('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED); + // Prevent setting the default quota option to be the default quota value creating an unresolvable self reference + if ($defaultQuota <= 0 && $defaultQuota !== FileInfo::SPACE_UNLIMITED) { + throw new \Exception('Default Groupfolder quota value ' . $defaultQuota . ' is not allowed'); + } + + return $defaultQuota; + } + + return $quota; + } } diff --git a/src/settings/App.tsx b/src/settings/App.tsx index d1ccaf419..3a4d33918 100644 --- a/src/settings/App.tsx +++ b/src/settings/App.tsx @@ -20,6 +20,7 @@ import { loadState } from '@nextcloud/initial-state' const bytesInOneGibibyte = Math.pow(1024, 3) const defaultQuotaOptions = { + Default: -4, '1 GB': bytesInOneGibibyte, '5 GB': bytesInOneGibibyte * 5, '10 GB': bytesInOneGibibyte * 10, diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php index 5d89142fa..f28861f26 100644 --- a/tests/Folder/FolderManagerTest.php +++ b/tests/Folder/FolderManagerTest.php @@ -37,10 +37,6 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->config = $this->createMock(IConfig::class); - $this->config->expects($this->any()) - ->method('getSystemValueInt') - ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) - ->willReturn(FileInfo::SPACE_UNLIMITED); $this->manager = new FolderManager( \OC::$server->getDatabaseConnection(), $this->groupManager, @@ -88,6 +84,11 @@ private function assertHasFolders($folders) { } public function testCreateFolder() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $this->manager->createFolder('foo'); $this->assertHasFolders([ @@ -96,6 +97,11 @@ public function testCreateFolder() { } public function testSetMountpoint() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->createFolder('bar'); @@ -108,6 +114,11 @@ public function testSetMountpoint() { } public function testAddApplicable() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $folderId2 = $this->manager->createFolder('bar'); $this->manager->addApplicableGroup($folderId1, 'g1'); @@ -153,6 +164,11 @@ public function testAddApplicable() { } public function testSetPermissions() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->addApplicableGroup($folderId1, 'g1'); $this->manager->addApplicableGroup($folderId1, 'g2'); @@ -181,6 +197,11 @@ public function testSetPermissions() { } public function testRemoveApplicable() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $folderId2 = $this->manager->createFolder('bar'); $this->manager->addApplicableGroup($folderId1, 'g1'); @@ -224,6 +245,11 @@ public function testRemoveApplicable() { } public function testRemoveFolder() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->createFolder('bar'); @@ -235,6 +261,11 @@ public function testRemoveFolder() { } public function testRenameFolder() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->createFolder('other'); @@ -247,6 +278,11 @@ public function testRenameFolder() { } public function testSetACL() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->createFolder('other'); @@ -266,6 +302,11 @@ public function testSetACL() { } public function testGetFoldersForGroup() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->addApplicableGroup($folderId1, 'g1'); $this->manager->addApplicableGroup($folderId1, 'g2'); @@ -279,6 +320,11 @@ public function testGetFoldersForGroup() { } public function testGetFoldersForGroups() { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + $folderId1 = $this->manager->createFolder('foo'); $this->manager->addApplicableGroup($folderId1, 'g1'); $this->manager->addApplicableGroup($folderId1, 'g2'); @@ -403,4 +449,24 @@ public function testGetFolderPermissionsForUserMerge() { $permissions = $manager->getFolderPermissionsForUser($this->getUser(['g1', 'g2', 'g3']), 2); $this->assertEquals(0, $permissions); } + + public function testQuotaDefaultValue(): void { + $folderId1 = $this->manager->createFolder('foo'); + + $exponent = 3; + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturnCallback(function () use (&$exponent) { + return 1024 ** ($exponent++); + }); + + /** @var array $folder */ + $folder = $this->manager->getFolder($folderId1); + $this->assertEquals(1024 ** 3, $folder['quota']); + + /** @var array $folder */ + $folder = $this->manager->getFolder($folderId1); + $this->assertEquals(1024 ** 4, $folder['quota']); + } }