Skip to content

Commit 1829269

Browse files
provokateurinprovokateurin
authored
Merge pull request #56933 from nextcloud/fix/duplicate-mounts
2 parents 5f6e6b3 + 341fd34 commit 1829269

File tree

8 files changed

+91
-31
lines changed

8 files changed

+91
-31
lines changed

‎core/Listener/AddMissingIndicesListener.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,6 @@ public function handle(Event $event): void {
186186
'mounts_class_index',
187187
['mount_provider_class']
188188
);
189-
$event->addMissingIndex(
190-
'mounts',
191-
'mounts_user_root_path_index',
192-
['user_id', 'root_id', 'mount_point'],
193-
['lengths' => [null, null, 128]]
194-
);
195189

196190
$event->addMissingIndex(
197191
'systemtag_object_mapping',

‎core/Migrations/Version33000Date20251209123503.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\Core\Migrations;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\DB\Types;
15+
use OCP\IDBConnection;
16+
use OCP\Migration\IOutput;
17+
use OCP\Migration\SimpleMigrationStep;
18+
use Override;
19+
20+
class Version33000Date20251209123503 extends SimpleMigrationStep {
21+
public function __construct(
22+
private readonly IDBConnection $connection,
23+
) {
24+
}
25+
26+
#[Override]
27+
public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
28+
$this->connection->truncateTable('mounts', false);
29+
}
30+
31+
#[Override]
32+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
33+
/** @var ISchemaWrapper $schema */
34+
$schema = $schemaClosure();
35+
36+
$table = $schema->getTable('mounts');
37+
if (!$table->hasColumn('mount_point_hash')) {
38+
$table->addColumn('mount_point_hash', Types::STRING, [
39+
'notnull' => true,
40+
'length' => 32, // xxh128
41+
]);
42+
$table->dropIndex('mounts_user_root_path_index');
43+
$table->addUniqueIndex(['user_id', 'root_id', 'mount_point_hash'], 'mounts_user_root_path_index');
44+
return $schema;
45+
}
46+
47+
return null;
48+
}
49+
}

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,6 +1547,7 @@
15471547
'OC\\Core\\Migrations\\Version33000Date20251106131209' => $baseDir . '/core/Migrations/Version33000Date20251106131209.php',
15481548
'OC\\Core\\Migrations\\Version33000Date20251124110529' => $baseDir . '/core/Migrations/Version33000Date20251124110529.php',
15491549
'OC\\Core\\Migrations\\Version33000Date20251126152410' => $baseDir . '/core/Migrations/Version33000Date20251126152410.php',
1550+
'OC\\Core\\Migrations\\Version33000Date20251209123503' => $baseDir . '/core/Migrations/Version33000Date20251209123503.php',
15501551
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
15511552
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',
15521553
'OC\\Core\\Service\\CronService' => $baseDir . '/core/Service/CronService.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
15881588
'OC\\Core\\Migrations\\Version33000Date20251106131209' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251106131209.php',
15891589
'OC\\Core\\Migrations\\Version33000Date20251124110529' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251124110529.php',
15901590
'OC\\Core\\Migrations\\Version33000Date20251126152410' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251126152410.php',
1591+
'OC\\Core\\Migrations\\Version33000Date20251209123503' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251209123503.php',
15911592
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
15921593
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',
15931594
'OC\\Core\\Service\\CronService' => __DIR__ . '/../../..' . '/core/Service/CronService.php',

‎lib/private/Files/Config/UserMountCache.php‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use OC\User\LazyUser;
1111
use OCP\Cache\CappedMemoryCache;
12+
use OCP\DB\Exception;
1213
use OCP\DB\QueryBuilder\IQueryBuilder;
1314
use OCP\Diagnostics\IEventLogger;
1415
use OCP\EventDispatcher\IEventDispatcher;
@@ -164,14 +165,25 @@ private function findChangedMounts(array $newMounts, array $cachedMounts): array
164165

165166
private function addToCache(ICachedMountInfo $mount) {
166167
if ($mount->getStorageId() !== -1) {
167-
$this->connection->insertIfNotExist('*PREFIX*mounts', [
168-
'storage_id' => $mount->getStorageId(),
169-
'root_id' => $mount->getRootId(),
170-
'user_id' => $mount->getUser()->getUID(),
171-
'mount_point' => $mount->getMountPoint(),
172-
'mount_id' => $mount->getMountId(),
173-
'mount_provider_class' => $mount->getMountProvider(),
174-
], ['root_id', 'user_id', 'mount_point']);
168+
$qb = $this->connection->getQueryBuilder();
169+
$qb
170+
->insert('mounts')
171+
->values([
172+
'storage_id' => $qb->createNamedParameter($mount->getStorageId(), IQueryBuilder::PARAM_INT),
173+
'root_id' => $qb->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT),
174+
'user_id' => $qb->createNamedParameter($mount->getUser()->getUID()),
175+
'mount_point' => $qb->createNamedParameter($mount->getMountPoint()),
176+
'mount_point_hash' => $qb->createNamedParameter(hash('xxh128', $mount->getMountPoint())),
177+
'mount_id' => $qb->createNamedParameter($mount->getMountId(), IQueryBuilder::PARAM_INT),
178+
'mount_provider_class' => $qb->createNamedParameter($mount->getMountProvider()),
179+
]);
180+
try {
181+
$qb->executeStatement();
182+
} catch (Exception $e) {
183+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
184+
throw $e;
185+
}
186+
}
175187
} else {
176188
// in some cases this is legitimate, like orphaned shares
177189
$this->logger->debug('Could not get storage info for mount at ' . $mount->getMountPoint());
@@ -184,6 +196,7 @@ private function updateCachedMount(ICachedMountInfo $mount) {
184196
$query = $builder->update('mounts')
185197
->set('storage_id', $builder->createNamedParameter($mount->getStorageId()))
186198
->set('mount_point', $builder->createNamedParameter($mount->getMountPoint()))
199+
->set('mount_point_hash', $builder->createNamedParameter(hash('xxh128', $mount->getMountPoint())))
187200
->set('mount_id', $builder->createNamedParameter($mount->getMountId(), IQueryBuilder::PARAM_INT))
188201
->set('mount_provider_class', $builder->createNamedParameter($mount->getMountProvider()))
189202
->where($builder->expr()->eq('user_id', $builder->createNamedParameter($mount->getUser()->getUID())))
@@ -198,7 +211,7 @@ private function removeFromCache(ICachedMountInfo $mount) {
198211
$query = $builder->delete('mounts')
199212
->where($builder->expr()->eq('user_id', $builder->createNamedParameter($mount->getUser()->getUID())))
200213
->andWhere($builder->expr()->eq('root_id', $builder->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT)))
201-
->andWhere($builder->expr()->eq('mount_point', $builder->createNamedParameter($mount->getMountPoint())));
214+
->andWhere($builder->expr()->eq('mount_point_hash', $builder->createNamedParameter(hash('xxh128', $mount->getMountPoint()))));
202215
$query->executeStatement();
203216
}
204217

@@ -449,16 +462,8 @@ public function remoteStorageMounts($storageId) {
449462
public function getUsedSpaceForUsers(array $users) {
450463
$builder = $this->connection->getQueryBuilder();
451464

452-
$slash = $builder->createNamedParameter('/');
453-
454-
$mountPoint = $builder->func()->concat(
455-
$builder->func()->concat($slash, 'user_id'),
456-
$slash
457-
);
458-
459-
$userIds = array_map(function (IUser $user) {
460-
return $user->getUID();
461-
}, $users);
465+
$mountPointHashes = array_map(static fn (IUser $user) => hash('xxh128', '/' . $user->getUID() . '/'), $users);
466+
$userIds = array_map(static fn (IUser $user) => $user->getUID(), $users);
462467

463468
$query = $builder->select('m.user_id', 'f.size')
464469
->from('mounts', 'm')
@@ -467,7 +472,7 @@ public function getUsedSpaceForUsers(array $users) {
467472
$builder->expr()->eq('m.storage_id', 'f.storage'),
468473
$builder->expr()->eq('f.path_hash', $builder->createNamedParameter(md5('files')))
469474
))
470-
->where($builder->expr()->eq('m.mount_point', $mountPoint))
475+
->where($builder->expr()->in('m.mount_point_hash', $builder->createNamedParameter($mountPointHashes, IQueryBuilder::PARAM_STR_ARRAY)))
471476
->andWhere($builder->expr()->in('m.user_id', $builder->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)));
472477

473478
$result = $query->executeQuery();

‎tests/lib/DB/QueryBuilder/Partitioned/PartitionedQueryBuilderTest.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ private function setupFileCache(): void {
9090
'storage_id' => $query->createNamedParameter(1001001, IQueryBuilder::PARAM_INT),
9191
'user_id' => $query->createNamedParameter('partitioned_test'),
9292
'mount_point' => $query->createNamedParameter('/mount/point'),
93+
'mount_point_hash' => $query->createNamedParameter(hash('xxh128', '/mount/point')),
9394
'mount_provider_class' => $query->createNamedParameter('test'),
9495
'root_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT),
9596
]);
@@ -138,7 +139,7 @@ public function testSimplePartitionedQuery(): void {
138139
$builder->addPartition(new PartitionSplit('filecache', ['filecache']));
139140

140141
// query borrowed from UserMountCache
141-
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
142+
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class')
142143
->from('mounts', 'm')
143144
->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
144145
->where($builder->expr()->eq('storage_id', $builder->createNamedParameter(1001001, IQueryBuilder::PARAM_INT)));
@@ -151,6 +152,7 @@ public function testSimplePartitionedQuery(): void {
151152
$this->assertCount(1, $results);
152153
$this->assertEquals($results[0]['user_id'], 'partitioned_test');
153154
$this->assertEquals($results[0]['mount_point'], '/mount/point');
155+
$this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point'));
154156
$this->assertEquals($results[0]['mount_provider_class'], 'test');
155157
$this->assertEquals($results[0]['path'], 'file1');
156158
}
@@ -159,7 +161,7 @@ public function testMultiTablePartitionedQuery(): void {
159161
$builder = $this->getQueryBuilder();
160162
$builder->addPartition(new PartitionSplit('filecache', ['filecache', 'filecache_extended']));
161163

162-
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class', 'fe.upload_time')
164+
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class', 'fe.upload_time')
163165
->from('mounts', 'm')
164166
->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
165167
->innerJoin('f', 'filecache_extended', 'fe', $builder->expr()->eq('f.fileid', 'fe.fileid'))
@@ -173,6 +175,7 @@ public function testMultiTablePartitionedQuery(): void {
173175
$this->assertCount(1, $results);
174176
$this->assertEquals($results[0]['user_id'], 'partitioned_test');
175177
$this->assertEquals($results[0]['mount_point'], '/mount/point');
178+
$this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point'));
176179
$this->assertEquals($results[0]['mount_provider_class'], 'test');
177180
$this->assertEquals($results[0]['path'], 'file1');
178181
$this->assertEquals($results[0]['upload_time'], 1234);
@@ -182,7 +185,7 @@ public function testPartitionedQueryFromSplit(): void {
182185
$builder = $this->getQueryBuilder();
183186
$builder->addPartition(new PartitionSplit('filecache', ['filecache']));
184187

185-
$query = $builder->select('storage', 'm.root_id', 'm.user_id', 'm.mount_point', 'm.mount_id', 'path', 'm.mount_provider_class')
188+
$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')
186189
->from('filecache', 'f')
187190
->innerJoin('f', 'mounts', 'm', $builder->expr()->eq('m.root_id', 'f.fileid'));
188191
$query->where($builder->expr()->eq('storage', $builder->createNamedParameter(1001001, IQueryBuilder::PARAM_INT)));
@@ -195,6 +198,7 @@ public function testPartitionedQueryFromSplit(): void {
195198
$this->assertCount(1, $results);
196199
$this->assertEquals($results[0]['user_id'], 'partitioned_test');
197200
$this->assertEquals($results[0]['mount_point'], '/mount/point');
201+
$this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point'));
198202
$this->assertEquals($results[0]['mount_provider_class'], 'test');
199203
$this->assertEquals($results[0]['path'], 'file1');
200204
}
@@ -204,7 +208,7 @@ public function testMultiJoinPartitionedQuery(): void {
204208
$builder->addPartition(new PartitionSplit('filecache', ['filecache']));
205209

206210
// query borrowed from UserMountCache
207-
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
211+
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_point_hash', 'mount_id', 'f.path', 'mount_provider_class')
208212
->selectAlias('s.id', 'storage_string_id')
209213
->from('mounts', 'm')
210214
->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
@@ -219,6 +223,7 @@ public function testMultiJoinPartitionedQuery(): void {
219223
$this->assertCount(1, $results);
220224
$this->assertEquals($results[0]['user_id'], 'partitioned_test');
221225
$this->assertEquals($results[0]['mount_point'], '/mount/point');
226+
$this->assertEquals($results[0]['mount_point_hash'], hash('xxh128', '/mount/point'));
222227
$this->assertEquals($results[0]['mount_provider_class'], 'test');
223228
$this->assertEquals($results[0]['path'], 'file1');
224229
$this->assertEquals($results[0]['storage_string_id'], 'test1');

‎tests/lib/Files/Cache/FileAccessTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void {
6262
'root_id' => $queryBuilder->createNamedParameter(10, IQueryBuilder::PARAM_INT),
6363
'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'),
6464
'mount_point' => $queryBuilder->createNamedParameter('/files'),
65+
'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/files')),
6566
'user_id' => $queryBuilder->createNamedParameter('test'),
6667
])
6768
->executeStatement();
@@ -72,6 +73,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void {
7273
'root_id' => $queryBuilder->createNamedParameter(30, IQueryBuilder::PARAM_INT),
7374
'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass1'),
7475
'mount_point' => $queryBuilder->createNamedParameter('/documents'),
76+
'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/documents')),
7577
'user_id' => $queryBuilder->createNamedParameter('test'),
7678
])
7779
->executeStatement();
@@ -82,6 +84,7 @@ private function setUpTestDatabaseForGetDistinctMounts(): void {
8284
'root_id' => $queryBuilder->createNamedParameter(31, IQueryBuilder::PARAM_INT),
8385
'mount_provider_class' => $queryBuilder->createNamedParameter('TestProviderClass2'),
8486
'mount_point' => $queryBuilder->createNamedParameter('/foobar'),
87+
'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/foobar')),
8588
'user_id' => $queryBuilder->createNamedParameter('test'),
8689
])
8790
->executeStatement();
@@ -147,6 +150,7 @@ public function testGetDistinctMountsWithRewriteHomeDirectories(): void {
147150
'root_id' => $queryBuilder->createNamedParameter(40, IQueryBuilder::PARAM_INT),
148151
'mount_provider_class' => $queryBuilder->createNamedParameter(LocalHomeMountProvider::class),
149152
'mount_point' => $queryBuilder->createNamedParameter('/home/user'),
153+
'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/home/user')),
150154
'user_id' => $queryBuilder->createNamedParameter('test'),
151155
])
152156
->executeStatement();
@@ -159,6 +163,7 @@ public function testGetDistinctMountsWithRewriteHomeDirectories(): void {
159163
'root_id' => $queryBuilder->createNamedParameter(41, IQueryBuilder::PARAM_INT),
160164
'mount_provider_class' => $queryBuilder->createNamedParameter('TestMountProvider3'),
161165
'mount_point' => $queryBuilder->createNamedParameter('/test/files/foobar'),
166+
'mount_point_hash' => $queryBuilder->createNamedParameter(hash('xxh128', '/test/files/foobar')),
162167
'user_id' => $queryBuilder->createNamedParameter('test'),
163168
])
164169
->executeStatement();

‎version.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level
1010
// when updating major/minor version number.
1111

12-
$OC_Version = [33, 0, 0, 5];
12+
$OC_Version = [33, 0, 0, 6];
1313

1414
// The human-readable string
1515
$OC_VersionString = '33.0.0 dev';

0 commit comments

Comments
 (0)