From 6cfd5cbe64595dae33b77ecc0718e60f3f52efad Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 14 Jun 2022 15:57:59 +0200 Subject: [PATCH 1/3] 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 --- .../Version13000Date20170718121200.php | 2 +- .../Version27000Date20220613163520.php | 51 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Config/UserMountCache.php | 36 ++++++------- version.php | 2 +- 6 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 core/Migrations/Version27000Date20220613163520.php diff --git a/core/Migrations/Version13000Date20170718121200.php b/core/Migrations/Version13000Date20170718121200.php index 0924e0590d3..34a249814fb 100644 --- a/core/Migrations/Version13000Date20170718121200.php +++ b/core/Migrations/Version13000Date20170718121200.php @@ -149,7 +149,7 @@ class Version13000Date20170718121200 extends SimpleMigrationStep { $table->addIndex(['storage_id'], 'mounts_storage_index'); $table->addIndex(['root_id'], 'mounts_root_index'); $table->addIndex(['mount_id'], 'mounts_mount_id_index'); - $table->addUniqueIndex(['user_id', 'root_id'], 'mounts_user_root_index'); + $table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]); } else { $table = $schema->getTable('mounts'); $table->addColumn('mount_id', Types::BIGINT, [ diff --git a/core/Migrations/Version27000Date20220613163520.php b/core/Migrations/Version27000Date20220613163520.php new file mode 100644 index 00000000000..5f327e69c96 --- /dev/null +++ b/core/Migrations/Version27000Date20220613163520.php @@ -0,0 +1,51 @@ + + * + * @author Your name + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version27000Date20220613163520 extends SimpleMigrationStep { + public function name(): string { + return "Add mountpoint path to mounts table unique index"; + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('mounts'); + if ($table->hasIndex('mounts_user_root_index')) { + $table->dropIndex('mounts_user_root_index'); + $table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]); + } + + return $schema; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 74920e5548c..cb62da5cf98 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1089,6 +1089,7 @@ return array( 'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => $baseDir . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => $baseDir . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20220613163520' => $baseDir . '/core/Migrations/Version27000Date20220613163520.php', 'OC\\Core\\Migrations\\Version27000Date20230309104325' => $baseDir . '/core/Migrations/Version27000Date20230309104325.php', 'OC\\Core\\Migrations\\Version27000Date20230309104802' => $baseDir . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 67a0911149a..bea129d0d32 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1122,6 +1122,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20220613163520' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20220613163520.php', 'OC\\Core\\Migrations\\Version27000Date20230309104325' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104325.php', 'OC\\Core\\Migrations\\Version27000Date20230309104802' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index fe677c5ea52..aa362b0e970 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -92,38 +92,39 @@ class UserMountCache implements IUserMountCache { } }, $mounts); $newMounts = array_values(array_filter($newMounts)); - $newMountRootIds = array_map(function (ICachedMountInfo $mount) { - return $mount->getRootId(); + $newMountKeys = array_map(function (ICachedMountInfo $mount) { + return $mount->getRootId() . '::' . $mount->getMountPoint(); }, $newMounts); - $newMounts = array_combine($newMountRootIds, $newMounts); + $newMounts = array_combine($newMountKeys, $newMounts); $cachedMounts = $this->getMountsForUser($user); if (is_array($mountProviderClasses)) { $cachedMounts = array_filter($cachedMounts, function (ICachedMountInfo $mountInfo) use ($mountProviderClasses, $newMounts) { // for existing mounts that didn't have a mount provider set // we still want the ones that map to new mounts - if ($mountInfo->getMountProvider() === '' && isset($newMounts[$mountInfo->getRootId()])) { + $mountKey = $mountInfo->getRootId() . '::' . $mountInfo->getMountPoint(); + if ($mountInfo->getMountProvider() === '' && isset($newMounts[$mountKey])) { return true; } return in_array($mountInfo->getMountProvider(), $mountProviderClasses); }); } - $cachedMountRootIds = array_map(function (ICachedMountInfo $mount) { - return $mount->getRootId(); + $cachedRootKeys = array_map(function (ICachedMountInfo $mount) { + return $mount->getRootId() . '::' . $mount->getMountPoint(); }, $cachedMounts); - $cachedMounts = array_combine($cachedMountRootIds, $cachedMounts); + $cachedMounts = array_combine($cachedRootKeys, $cachedMounts); $addedMounts = []; $removedMounts = []; - foreach ($newMounts as $rootId => $newMount) { - if (!isset($cachedMounts[$rootId])) { + foreach ($newMounts as $mountKey => $newMount) { + if (!isset($cachedMounts[$mountKey])) { $addedMounts[] = $newMount; } } - foreach ($cachedMounts as $rootId => $cachedMount) { - if (!isset($newMounts[$rootId])) { + foreach ($cachedMounts as $mountKey => $cachedMount) { + if (!isset($newMounts[$mountKey])) { $removedMounts[] = $cachedMount; } } @@ -154,13 +155,13 @@ class UserMountCache implements IUserMountCache { private function findChangedMounts(array $newMounts, array $cachedMounts) { $new = []; foreach ($newMounts as $mount) { - $new[$mount->getRootId()] = $mount; + $new[$mount->getRootId() . '::' . $mount->getMountPoint()] = $mount; } $changed = []; foreach ($cachedMounts as $cachedMount) { - $rootId = $cachedMount->getRootId(); - if (isset($new[$rootId])) { - $newMount = $new[$rootId]; + $key = $cachedMount->getRootId() . '::' . $cachedMount->getMountPoint(); + if (isset($new[$key])) { + $newMount = $new[$key]; if ( $newMount->getMountPoint() !== $cachedMount->getMountPoint() || $newMount->getStorageId() !== $cachedMount->getStorageId() || @@ -183,7 +184,7 @@ class UserMountCache implements IUserMountCache { 'mount_point' => $mount->getMountPoint(), 'mount_id' => $mount->getMountId(), 'mount_provider_class' => $mount->getMountProvider(), - ], ['root_id', 'user_id']); + ], ['root_id', 'user_id', 'mount_point']); } else { // in some cases this is legitimate, like orphaned shares $this->logger->debug('Could not get storage info for mount at ' . $mount->getMountPoint()); @@ -209,7 +210,8 @@ 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('root_id', $builder->createNamedParameter($mount->getRootId(), IQueryBuilder::PARAM_INT))) + ->andWhere($builder->expr()->eq('mount_point', $builder->createNamedParameter($mount->getMountPoint()))); $query->execute(); } diff --git a/version.php b/version.php index 13eb91b9ec1..4e51a657e3d 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [26, 0, 2, 1]; +$OC_Version = [26, 0, 2, 2]; // The human readable string $OC_VersionString = '26.0.2'; From 957b8b20f96b8752b66d0f5906725b2813cfa3dd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 13 Apr 2023 17:19:26 +0200 Subject: [PATCH 2/3] add new index in repair step instead of on-migrate Signed-off-by: Robin Appelman --- core/Application.php | 3 +++ core/Command/Db/AddMissingIndices.php | 8 ++++++++ core/Migrations/Version27000Date20220613163520.php | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/core/Application.php b/core/Application.php index 4ebbada95e6..2e354610154 100644 --- a/core/Application.php +++ b/core/Application.php @@ -239,6 +239,9 @@ class Application extends App { if (!$table->hasIndex('mounts_class_index')) { $subject->addHintForMissingSubject($table->getName(), 'mounts_class_index'); } + if (!$table->hasIndex('mounts_user_root_path_index')) { + $subject->addHintForMissingSubject($table->getName(), 'mounts_user_root_path_index'); + } } } ); diff --git a/core/Command/Db/AddMissingIndices.php b/core/Command/Db/AddMissingIndices.php index e22d0fddeca..b317f44b499 100644 --- a/core/Command/Db/AddMissingIndices.php +++ b/core/Command/Db/AddMissingIndices.php @@ -465,6 +465,14 @@ class AddMissingIndices extends Command { $updated = true; $output->writeln('oc_mounts table updated successfully.'); } + if (!$table->hasIndex('mounts_user_root_path_index')) { + $output->writeln('Adding mounts_user_root_path_index index to the oc_mounts table, this can take some time...'); + + $table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]); + $this->connection->migrateToSchema($schema->getWrappedSchema()); + $updated = true; + $output->writeln('oc_mounts table updated successfully.'); + } } if (!$updated) { diff --git a/core/Migrations/Version27000Date20220613163520.php b/core/Migrations/Version27000Date20220613163520.php index 5f327e69c96..4217f3b3270 100644 --- a/core/Migrations/Version27000Date20220613163520.php +++ b/core/Migrations/Version27000Date20220613163520.php @@ -43,7 +43,7 @@ class Version27000Date20220613163520 extends SimpleMigrationStep { $table = $schema->getTable('mounts'); if ($table->hasIndex('mounts_user_root_index')) { $table->dropIndex('mounts_user_root_index'); - $table->addIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index', [], ['lengths' => [null, null, 128]]); + // new index gets added with "add missing indexes" } return $schema; From 7c85dfe8f96f206426effde41176be8ab8a5332d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 15 Jun 2023 12:40:34 +0200 Subject: [PATCH 3/3] ci: revert to avoid conflict for release PR Signed-off-by: Arthur Schiwon --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 4e51a657e3d..13eb91b9ec1 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [26, 0, 2, 2]; +$OC_Version = [26, 0, 2, 1]; // The human readable string $OC_VersionString = '26.0.2';