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

provokateurin · provokateurin · commit fb3398077503 · 2025-12-16T12:51:46.000+01:00
Signed-off-by: provokateurin &lt;kate@provokateurin.de&gt;
diff --git a/core/Application.php b/core/Application.php
@@ -218,12 +218,6 @@ public function __construct() {
 				'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
@@ -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;
+	}
+}
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -1471,6 +1471,7 @@
     'OC\\Core\\Migrations\\Version31000Date20250213102442' => $baseDir . '/core/Migrations/Version31000Date20250213102442.php',
     'OC\\Core\\Migrations\\Version31000Date20250731062008' => $baseDir . '/core/Migrations/Version31000Date20250731062008.php',
     'OC\\Core\\Migrations\\Version32000Date20250620081925' => $baseDir . '/core/Migrations/Version32000Date20250620081925.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\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -17,11 +17,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
             'OC\\' => 3,
             'OCP\\' => 4,
         ),
-        'N' => 
+        'N' =>
         array (
             'NCU\\' => 4,
         ),
-        'B' => 
+        'B' =>
         array (
             'Bamarni\\Composer\\Bin\\' => 21,
         ),
@@ -40,11 +40,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         array (
             0 => __DIR__ . '/../../..' . '/lib/public',
         ),
-        'NCU\\' => 
+        'NCU\\' =>
         array (
             0 => __DIR__ . '/../../..' . '/lib/unstable',
         ),
-        'Bamarni\\Composer\\Bin\\' => 
+        'Bamarni\\Composer\\Bin\\' =>
         array (
             0 => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src',
         ),
@@ -1520,6 +1520,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Core\\Migrations\\Version31000Date20250213102442' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20250213102442.php',
         'OC\\Core\\Migrations\\Version31000Date20250731062008' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20250731062008.php',
         'OC\\Core\\Migrations\\Version32000Date20250620081925' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250620081925.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\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
diff --git 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;
@@ -160,14 +161,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());
@@ -180,6 +192,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())))
@@ -194,7 +207,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();
 	}
 
@@ -426,16 +439,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')
@@ -444,7 +449,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
@@ -86,6 +86,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),
 			]);
@@ -134,7 +135,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)));
@@ -147,6 +148,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');
 	}
@@ -155,7 +157,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'))
@@ -169,6 +171,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);
@@ -178,7 +181,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)));
@@ -191,6 +194,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');
 	}
@@ -200,7 +204,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'))
@@ -215,6 +219,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/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 = [31, 0, 12, 3];
+$OC_Version = [31, 0, 12, 4];
 
 // The human-readable string
 $OC_VersionString = '31.0.12';
