Skip to content

Commit f5e2b0b

Browse files
committed
allow storing multiple mounts for the same rootid in the mount cache
currently `[$userId, $rootId]` is used as the unique key for storing mounts in the mount cache, however there are cases where the same rootid is mounted in multiple places for a user which currently leads to not all of those mounts being added to the cache. Previously this didn't matter as the mount cache was only used to list users with access to a specific file, so a user having access to the file multiple times didn' change anything. With 24 the mount cache is used for more cases and multiple mounts for the same id becomes relevant. While I think there isn't a real negative effect atm besides missing the optimized path we should ensure that the mounts are properly listed Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 74f31ba commit f5e2b0b

6 files changed

Lines changed: 74 additions & 19 deletions

File tree

core/Migrations/Version13000Date20170718121200.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public function changeSchema(IOutput $output, \Closure $schemaClosure, array $op
149149
$table->addIndex(['storage_id'], 'mounts_storage_index');
150150
$table->addIndex(['root_id'], 'mounts_root_index');
151151
$table->addIndex(['mount_id'], 'mounts_mount_id_index');
152-
$table->addUniqueIndex(['user_id', 'root_id'], 'mounts_user_root_index');
152+
$table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]);
153153
} else {
154154
$table = $schema->getTable('mounts');
155155
$table->addColumn('mount_id', Types::BIGINT, [
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2022 Your name <your@email.com>
7+
*
8+
* @author Your name <your@email.com>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
27+
namespace OC\Core\Migrations;
28+
29+
use Closure;
30+
use OCP\DB\ISchemaWrapper;
31+
use OCP\Migration\IOutput;
32+
use OCP\Migration\SimpleMigrationStep;
33+
34+
class Version27000Date20220613163520 extends SimpleMigrationStep {
35+
public function name(): string {
36+
return "Add mountpoint path to mounts table unique index";
37+
}
38+
39+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
40+
/** @var ISchemaWrapper $schema */
41+
$schema = $schemaClosure();
42+
43+
$table = $schema->getTable('mounts');
44+
if ($table->hasIndex('mounts_user_root_index')) {
45+
$table->dropIndex('mounts_user_root_index');
46+
$table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]);
47+
}
48+
49+
return $schema;
50+
}
51+
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,7 @@
11081108
'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php',
11091109
'OC\\Core\\Migrations\\Version25000Date20220905140840' => $baseDir . '/core/Migrations/Version25000Date20220905140840.php',
11101110
'OC\\Core\\Migrations\\Version25000Date20221007010957' => $baseDir . '/core/Migrations/Version25000Date20221007010957.php',
1111+
'OC\\Core\\Migrations\\Version27000Date20220613163520' => $baseDir . '/core/Migrations/Version27000Date20220613163520.php',
11111112
'OC\\Core\\Migrations\\Version27000Date20230309104325' => $baseDir . '/core/Migrations/Version27000Date20230309104325.php',
11121113
'OC\\Core\\Migrations\\Version27000Date20230309104802' => $baseDir . '/core/Migrations/Version27000Date20230309104802.php',
11131114
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
11411141
'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php',
11421142
'OC\\Core\\Migrations\\Version25000Date20220905140840' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220905140840.php',
11431143
'OC\\Core\\Migrations\\Version25000Date20221007010957' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20221007010957.php',
1144+
'OC\\Core\\Migrations\\Version27000Date20220613163520' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20220613163520.php',
11441145
'OC\\Core\\Migrations\\Version27000Date20230309104325' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104325.php',
11451146
'OC\\Core\\Migrations\\Version27000Date20230309104802' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104802.php',
11461147
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',

lib/private/Files/Config/UserMountCache.php

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,38 +92,39 @@ public function registerMounts(IUser $user, array $mounts, array $mountProviderC
9292
}
9393
}, $mounts);
9494
$newMounts = array_values(array_filter($newMounts));
95-
$newMountRootIds = array_map(function (ICachedMountInfo $mount) {
96-
return $mount->getRootId();
95+
$newMountKeys = array_map(function (ICachedMountInfo $mount) {
96+
return $mount->getRootId() . '::' . $mount->getMountPoint();
9797
}, $newMounts);
98-
$newMounts = array_combine($newMountRootIds, $newMounts);
98+
$newMounts = array_combine($newMountKeys, $newMounts);
9999

100100
$cachedMounts = $this->getMountsForUser($user);
101101
if (is_array($mountProviderClasses)) {
102102
$cachedMounts = array_filter($cachedMounts, function (ICachedMountInfo $mountInfo) use ($mountProviderClasses, $newMounts) {
103103
// for existing mounts that didn't have a mount provider set
104104
// we still want the ones that map to new mounts
105-
if ($mountInfo->getMountProvider() === '' && isset($newMounts[$mountInfo->getRootId()])) {
105+
$mountKey = $mountInfo->getRootId() . '::' . $mountInfo->getMountPoint();
106+
if ($mountInfo->getMountProvider() === '' && isset($newMounts[$mountKey])) {
106107
return true;
107108
}
108109
return in_array($mountInfo->getMountProvider(), $mountProviderClasses);
109110
});
110111
}
111-
$cachedMountRootIds = array_map(function (ICachedMountInfo $mount) {
112-
return $mount->getRootId();
112+
$cachedRootKeys = array_map(function (ICachedMountInfo $mount) {
113+
return $mount->getRootId() . '::' . $mount->getMountPoint();
113114
}, $cachedMounts);
114-
$cachedMounts = array_combine($cachedMountRootIds, $cachedMounts);
115+
$cachedMounts = array_combine($cachedRootKeys, $cachedMounts);
115116

116117
$addedMounts = [];
117118
$removedMounts = [];
118119

119-
foreach ($newMounts as $rootId => $newMount) {
120-
if (!isset($cachedMounts[$rootId])) {
120+
foreach ($newMounts as $mountKey => $newMount) {
121+
if (!isset($cachedMounts[$mountKey])) {
121122
$addedMounts[] = $newMount;
122123
}
123124
}
124125

125-
foreach ($cachedMounts as $rootId => $cachedMount) {
126-
if (!isset($newMounts[$rootId])) {
126+
foreach ($cachedMounts as $mountKey => $cachedMount) {
127+
if (!isset($newMounts[$mountKey])) {
127128
$removedMounts[] = $cachedMount;
128129
}
129130
}
@@ -161,13 +162,13 @@ public function registerMounts(IUser $user, array $mounts, array $mountProviderC
161162
private function findChangedMounts(array $newMounts, array $cachedMounts) {
162163
$new = [];
163164
foreach ($newMounts as $mount) {
164-
$new[$mount->getRootId()] = $mount;
165+
$new[$mount->getRootId() . '::' . $mount->getMountPoint()] = $mount;
165166
}
166167
$changed = [];
167168
foreach ($cachedMounts as $cachedMount) {
168-
$rootId = $cachedMount->getRootId();
169-
if (isset($new[$rootId])) {
170-
$newMount = $new[$rootId];
169+
$key = $cachedMount->getRootId() . '::' . $cachedMount->getMountPoint();
170+
if (isset($new[$key])) {
171+
$newMount = $new[$key];
171172
if (
172173
$newMount->getMountPoint() !== $cachedMount->getMountPoint() ||
173174
$newMount->getStorageId() !== $cachedMount->getStorageId() ||
@@ -190,7 +191,7 @@ private function addToCache(ICachedMountInfo $mount) {
190191
'mount_point' => $mount->getMountPoint(),
191192
'mount_id' => $mount->getMountId(),
192193
'mount_provider_class' => $mount->getMountProvider(),
193-
], ['root_id', 'user_id']);
194+
], ['root_id', 'user_id', 'mount_point']);
194195
} else {
195196
// in some cases this is legitimate, like orphaned shares
196197
$this->logger->debug('Could not get storage info for mount at ' . $mount->getMountPoint());
@@ -216,7 +217,8 @@ private function removeFromCache(ICachedMountInfo $mount) {
216217

217218
$query = $builder->delete('mounts')
218219
->where($builder->expr()->eq('user_id', $builder->createNamedParameter($mount->getUser()->getUID())))
219-
->andWhere($builder->expr()->eq('root_id', $builder->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT)));
220+
->andWhere($builder->expr()->eq('root_id', $builder->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT)))
221+
->andWhere($builder->expr()->eq('mount_point', $builder->createNamedParameter($mount->getMountPoint())));
220222
$query->execute();
221223
}
222224

version.php

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

33-
$OC_Version = [27, 0, 0, 0];
33+
$OC_Version = [27, 0, 0, 1];
3434

3535
// The human-readable string
3636
$OC_VersionString = '27.0.0 dev';

0 commit comments

Comments
 (0)