From 341fd348e6ce47e62610f26912570f488eed1179 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 9 Dec 2025 13:53:12 +0100 Subject: [PATCH] fix(UserMountCache): Add back unique index for oc_mounts and use normal insert Signed-off-by: provokateurin --- core/Listener/AddMissingIndicesListener.php | 6 --- .../Version33000Date20251209123503.php | 49 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Config/UserMountCache.php | 45 +++++++++-------- .../PartitionedQueryBuilderTest.php | 13 +++-- tests/lib/Files/Cache/FileAccessTest.php | 5 ++ version.php | 2 +- 8 files changed, 91 insertions(+), 31 deletions(-) create mode 100644 core/Migrations/Version33000Date20251209123503.php diff --git a/core/Listener/AddMissingIndicesListener.php b/core/Listener/AddMissingIndicesListener.php index 89c96648b9937..1130165d870d7 100644 --- a/core/Listener/AddMissingIndicesListener.php +++ b/core/Listener/AddMissingIndicesListener.php @@ -186,12 +186,6 @@ public function handle(Event $event): void { 'mounts_class_index', ['mount_provider_class'] ); - $event->addMissingIndex( - 'mounts', - 'mounts_user_root_path_index', - ['user_id', 'root_id', 'mount_point'], - ['lengths' => [null, null, 128]] - ); $event->addMissingIndex( 'systemtag_object_mapping', diff --git a/core/Migrations/Version33000Date20251209123503.php b/core/Migrations/Version33000Date20251209123503.php new file mode 100644 index 0000000000000..daa3470850d40 --- /dev/null +++ b/core/Migrations/Version33000Date20251209123503.php @@ -0,0 +1,49 @@ +connection->truncateTable('mounts', false); + } + + #[Override] + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('mounts'); + if (!$table->hasColumn('mount_point_hash')) { + $table->addColumn('mount_point_hash', Types::STRING, [ + 'notnull' => true, + 'length' => 32, // xxh128 + ]); + $table->dropIndex('mounts_user_root_path_index'); + $table->addUniqueIndex(['user_id', 'root_id', 'mount_point_hash'], 'mounts_user_root_path_index'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 98f4d5a9dcf15..0b6dd29f2040b 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1547,6 +1547,7 @@ 'OC\\Core\\Migrations\\Version33000Date20251106131209' => $baseDir . '/core/Migrations/Version33000Date20251106131209.php', 'OC\\Core\\Migrations\\Version33000Date20251124110529' => $baseDir . '/core/Migrations/Version33000Date20251124110529.php', 'OC\\Core\\Migrations\\Version33000Date20251126152410' => $baseDir . '/core/Migrations/Version33000Date20251126152410.php', + 'OC\\Core\\Migrations\\Version33000Date20251209123503' => $baseDir . '/core/Migrations/Version33000Date20251209123503.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => $baseDir . '/core/Service/CronService.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c463a93e9a3a3..9c34d2cd4debd 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1588,6 +1588,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version33000Date20251106131209' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251106131209.php', 'OC\\Core\\Migrations\\Version33000Date20251124110529' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251124110529.php', 'OC\\Core\\Migrations\\Version33000Date20251126152410' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251126152410.php', + 'OC\\Core\\Migrations\\Version33000Date20251209123503' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251209123503.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => __DIR__ . '/../../..' . '/core/Service/CronService.php', diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index 7cffc01b6f281..7079c8a295730 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -9,6 +9,7 @@ use OC\User\LazyUser; use OCP\Cache\CappedMemoryCache; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -164,14 +165,25 @@ private function findChangedMounts(array $newMounts, array $cachedMounts): array private function addToCache(ICachedMountInfo $mount) { if ($mount->getStorageId() !== -1) { - $this->connection->insertIfNotExist('*PREFIX*mounts', [ - 'storage_id' => $mount->getStorageId(), - 'root_id' => $mount->getRootId(), - 'user_id' => $mount->getUser()->getUID(), - 'mount_point' => $mount->getMountPoint(), - 'mount_id' => $mount->getMountId(), - 'mount_provider_class' => $mount->getMountProvider(), - ], ['root_id', 'user_id', 'mount_point']); + $qb = $this->connection->getQueryBuilder(); + $qb + ->insert('mounts') + ->values([ + 'storage_id' => $qb->createNamedParameter($mount->getStorageId(), IQueryBuilder::PARAM_INT), + 'root_id' => $qb->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT), + 'user_id' => $qb->createNamedParameter($mount->getUser()->getUID()), + 'mount_point' => $qb->createNamedParameter($mount->getMountPoint()), + 'mount_point_hash' => $qb->createNamedParameter(hash('xxh128', $mount->getMountPoint())), + 'mount_id' => $qb->createNamedParameter($mount->getMountId(), IQueryBuilder::PARAM_INT), + 'mount_provider_class' => $qb->createNamedParameter($mount->getMountProvider()), + ]); + try { + $qb->executeStatement(); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + } } else { // in some cases this is legitimate, like orphaned shares $this->logger->debug('Could not get storage info for mount at ' . $mount->getMountPoint()); @@ -184,6 +196,7 @@ private function updateCachedMount(ICachedMountInfo $mount) { $query = $builder->update('mounts') ->set('storage_id', $builder->createNamedParameter($mount->getStorageId())) ->set('mount_point', $builder->createNamedParameter($mount->getMountPoint())) + ->set('mount_point_hash', $builder->createNamedParameter(hash('xxh128', $mount->getMountPoint()))) ->set('mount_id', $builder->createNamedParameter($mount->getMountId(), IQueryBuilder::PARAM_INT)) ->set('mount_provider_class', $builder->createNamedParameter($mount->getMountProvider())) ->where($builder->expr()->eq('user_id', $builder->createNamedParameter($mount->getUser()->getUID()))) @@ -198,7 +211,7 @@ private function removeFromCache(ICachedMountInfo $mount) { $query = $builder->delete('mounts') ->where($builder->expr()->eq('user_id', $builder->createNamedParameter($mount->getUser()->getUID()))) ->andWhere($builder->expr()->eq('root_id', $builder->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT))) - ->andWhere($builder->expr()->eq('mount_point', $builder->createNamedParameter($mount->getMountPoint()))); + ->andWhere($builder->expr()->eq('mount_point_hash', $builder->createNamedParameter(hash('xxh128', $mount->getMountPoint())))); $query->executeStatement(); } @@ -449,16 +462,8 @@ public function remoteStorageMounts($storageId) { public function getUsedSpaceForUsers(array $users) { $builder = $this->connection->getQueryBuilder(); - $slash = $builder->createNamedParameter('/'); - - $mountPoint = $builder->func()->concat( - $builder->func()->concat($slash, 'user_id'), - $slash - ); - - $userIds = array_map(function (IUser $user) { - return $user->getUID(); - }, $users); + $mountPointHashes = array_map(static fn (IUser $user) => hash('xxh128', '/' . $user->getUID() . '/'), $users); + $userIds = array_map(static fn (IUser $user) => $user->getUID(), $users); $query = $builder->select('m.user_id', 'f.size') ->from('mounts', 'm') @@ -467,7 +472,7 @@ public function getUsedSpaceForUsers(array $users) { $builder->expr()->eq('m.storage_id', 'f.storage'), $builder->expr()->eq('f.path_hash', $builder->createNamedParameter(md5('files'))) )) - ->where($builder->expr()->eq('m.mount_point', $mountPoint)) + ->where($builder->expr()->in('m.mount_point_hash', $builder->createNamedParameter($mountPointHashes, IQueryBuilder::PARAM_STR_ARRAY))) ->andWhere($builder->expr()->in('m.user_id', $builder->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY))); $result = $query->executeQuery(); diff --git a/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php b/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php index ae4bb832f27b2..8d58ed9779b82 100644 --- a/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php @@ -90,6 +90,7 @@ private function setupFileCache(): void { 'storage_id' => $query->createNamedParameter(1001001, IQueryBuilder::PARAM_INT), 'user_id' => $query->createNamedParameter('partitioned_test'), 'mount_point' => $query->createNamedParameter('/mount/point'), + 'mount_point_hash' => $query->createNamedParameter(hash('xxh128', '/mount/point')), 'mount_provider_class' => $query->createNamedParameter('test'), 'root_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT), ]); @@ -138,7 +139,7 @@ public function testSimplePartitionedQuery(): void { $builder->addPartition(new PartitionSplit('filecache', ['filecache'])); // query borrowed from UserMountCache - $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class') + $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class') ->from('mounts', 'm') ->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid')) ->where($builder->expr()->eq('storage_id', $builder->createNamedParameter(1001001, IQueryBuilder::PARAM_INT))); @@ -151,6 +152,7 @@ public function testSimplePartitionedQuery(): void { $this->assertCount(1, $results); $this->assertEquals($results[0]['user_id'], 'partitioned_test'); $this->assertEquals($results[0]['mount_point'], '/mount/point'); + $this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point')); $this->assertEquals($results[0]['mount_provider_class'], 'test'); $this->assertEquals($results[0]['path'], 'file1'); } @@ -159,7 +161,7 @@ public function testMultiTablePartitionedQuery(): void { $builder = $this->getQueryBuilder(); $builder->addPartition(new PartitionSplit('filecache', ['filecache', 'filecache_extended'])); - $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class', 'fe.upload_time') + $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class', 'fe.upload_time') ->from('mounts', 'm') ->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid')) ->innerJoin('f', 'filecache_extended', 'fe', $builder->expr()->eq('f.fileid', 'fe.fileid')) @@ -173,6 +175,7 @@ public function testMultiTablePartitionedQuery(): void { $this->assertCount(1, $results); $this->assertEquals($results[0]['user_id'], 'partitioned_test'); $this->assertEquals($results[0]['mount_point'], '/mount/point'); + $this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point')); $this->assertEquals($results[0]['mount_provider_class'], 'test'); $this->assertEquals($results[0]['path'], 'file1'); $this->assertEquals($results[0]['upload_time'], 1234); @@ -182,7 +185,7 @@ public function testPartitionedQueryFromSplit(): void { $builder = $this->getQueryBuilder(); $builder->addPartition(new PartitionSplit('filecache', ['filecache'])); - $query = $builder->select('storage', 'm.root_id', 'm.user_id', 'm.mount_point', 'm.mount_id', 'path', 'm.mount_provider_class') + $query = $builder->select('storage', 'm.root_id', 'm.user_id', 'm.mount_point', 'm.mount_point_hash', 'm.mount_id', 'path', 'm.mount_provider_class') ->from('filecache', 'f') ->innerJoin('f', 'mounts', 'm', $builder->expr()->eq('m.root_id', 'f.fileid')); $query->where($builder->expr()->eq('storage', $builder->createNamedParameter(1001001, IQueryBuilder::PARAM_INT))); @@ -195,6 +198,7 @@ public function testPartitionedQueryFromSplit(): void { $this->assertCount(1, $results); $this->assertEquals($results[0]['user_id'], 'partitioned_test'); $this->assertEquals($results[0]['mount_point'], '/mount/point'); + $this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point')); $this->assertEquals($results[0]['mount_provider_class'], 'test'); $this->assertEquals($results[0]['path'], 'file1'); } @@ -204,7 +208,7 @@ public function testMultiJoinPartitionedQuery(): void { $builder->addPartition(new PartitionSplit('filecache', ['filecache'])); // query borrowed from UserMountCache - $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class') + $query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class') ->selectAlias('s.id', 'storage_string_id') ->from('mounts', 'm') ->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid')) @@ -219,6 +223,7 @@ public function testMultiJoinPartitionedQuery(): void { $this->assertCount(1, $results); $this->assertEquals($results[0]['user_id'], 'partitioned_test'); $this->assertEquals($results[0]['mount_point'], '/mount/point'); + $this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point')); $this->assertEquals($results[0]['mount_provider_class'], 'test'); $this->assertEquals($results[0]['path'], 'file1'); $this->assertEquals($results[0]['storage_string_id'], 'test1'); diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 36658c5a72102..a8ad3c6e7e121 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -62,6 +62,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void { 'root_id' => $queryBuilder->createNamedParameter(10, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'), 'mount_point' => $queryBuilder->createNamedParameter('/files'), + 'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/files')), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -72,6 +73,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void { 'root_id' => $queryBuilder->createNamedParameter(30, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'), 'mount_point' => $queryBuilder->createNamedParameter('/documents'), + 'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/documents')), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -82,6 +84,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void { 'root_id' => $queryBuilder->createNamedParameter(31, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'), 'mount_point' => $queryBuilder->createNamedParameter('/foobar'), + 'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/foobar')), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -147,6 +150,7 @@ public function testGetDistinctMountsWithRewriteHomeDirectories(): void { 'root_id' => $queryBuilder->createNamedParameter(40, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter(LocalHomeMountProvider::class), 'mount_point' => $queryBuilder->createNamedParameter('/home/user'), + 'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/home/user')), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); @@ -159,6 +163,7 @@ public function testGetDistinctMountsWithRewriteHomeDirectories(): void { 'root_id' => $queryBuilder->createNamedParameter(41, IQueryBuilder::PARAM_INT), 'mount_provider_class' => $queryBuilder->createNamedParameter('TestMountProvider3'), 'mount_point' => $queryBuilder->createNamedParameter('/test/files/foobar'), + 'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/test/files/foobar')), 'user_id' => $queryBuilder->createNamedParameter('test'), ]) ->executeStatement(); diff --git a/version.php b/version.php index 984c72a4b1852..9d9c837ef04a6 100644 --- a/version.php +++ b/version.php @@ -9,7 +9,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level // when updating major/minor version number. -$OC_Version = [33, 0, 0, 5]; +$OC_Version = [33, 0, 0, 6]; // The human-readable string $OC_VersionString = '33.0.0 dev';