fix(UserMountCache): Add back unique index for oc_mounts and use normal insert

Signed-off-by: provokateurin <kate@provokateurin.de>
2025-12-18 15:56:14 -05:00 · 2025-12-09 13:53:12 +01:00 · 2025-12-09 13:53:12 +01:00 · 341fd348e6
commit 341fd348e6
parent db530d1eae
8 changed files with 91 additions and 31 deletions
--- a/core/Listener/AddMissingIndicesListener.php
+++ b/core/Listener/AddMissingIndicesListener.php
@ -186,12 +186,6 @@ class AddMissingIndicesListener implements IEventListener {
 			'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',
--- a/core/Migrations/Version33000Date20251209123503.php
+++ b/core/Migrations/Version33000Date20251209123503.php
@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Core\Migrations;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+use Override;
+
+class Version33000Date20251209123503 extends SimpleMigrationStep {
+	public function __construct(
+		private readonly IDBConnection $connection,
+	) {
+	}
+
+	#[Override]
+	public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+		$this->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;
+	}
+}
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@ -1547,6 +1547,7 @@ return array(
    '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',
--- 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',
--- a/lib/private/Files/Config/UserMountCache.php
+++ b/lib/private/Files/Config/UserMountCache.php
@ -9,6 +9,7 @@ namespace OC\Files\Config;

 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 @@ class UserMountCache implements IUserMountCache {

 	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 @@ class UserMountCache implements IUserMountCache {
 		$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 @@ class UserMountCache implements IUserMountCache {
 		$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 @@ class UserMountCache implements IUserMountCache {
 	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 @@ class UserMountCache implements IUserMountCache {
 					$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();
--- a/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php
+++ b/tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php
@ -90,6 +90,7 @@ class PartitionedQueryBuilderTest extends TestCase {
 				'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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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 @@ class PartitionedQueryBuilderTest extends TestCase {
 		$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');
--- a/tests/lib/Files/Cache/FileAccessTest.php
+++ b/tests/lib/Files/Cache/FileAccessTest.php
@ -62,6 +62,7 @@ class FileAccessTest extends TestCase {
 				'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 @@ class FileAccessTest extends TestCase {
 				'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 @@ class FileAccessTest extends TestCase {
 				'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 @@ class FileAccessTest extends TestCase {
 				'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 @@ class FileAccessTest extends TestCase {
 				'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();
--- 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';