From 0b40b11289e47dc73a34605d70bb5df0ceccbe3c 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 832e7050c6ad2..41f53cd54cf20 100644 --- a/core/Migrations/Version13000Date20170718121200.php +++ b/core/Migrations/Version13000Date20170718121200.php @@ -150,7 +150,7 @@ public function changeSchema(IOutput $output, \Closure $schemaClosure, array $op $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 0000000000000..5f327e69c964a --- /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 bd2e96a177f8a..cf9e20f4cc8fe 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1071,6 +1071,7 @@ 'OC\\Core\\Migrations\\Version25000Date20220515204012' => $baseDir . '/core/Migrations/Version25000Date20220515204012.php', 'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => $baseDir . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20220613163520' => $baseDir . '/core/Migrations/Version27000Date20220613163520.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index ffddd7723527d..b03276772a971 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1104,6 +1104,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version25000Date20220515204012' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220515204012.php', 'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20220613163520' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20220613163520.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index 3540b56374219..b4104a14cb650 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -83,38 +83,39 @@ public function registerMounts(IUser $user, array $mounts, array $mountProviderC } }, $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; } } @@ -144,13 +145,13 @@ public function registerMounts(IUser $user, array $mounts, array $mountProviderC 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() || @@ -173,7 +174,7 @@ private function addToCache(ICachedMountInfo $mount) { '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()); @@ -199,7 +200,8 @@ 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('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 40d6d804d306c..e662e0fbd0e06 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 = [25, 0, 7, 1]; +$OC_Version = [25, 0, 7, 2]; // The human readable string $OC_VersionString = '25.0.7'; From b9f47232ca939308373e92e7b4ea872fd96d0f59 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 add4c0d7793e5..cb961d3986804 100644 --- a/core/Application.php +++ b/core/Application.php @@ -235,6 +235,9 @@ function (GenericEvent $event) use ($container) { 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 e22d0fddeca34..b317f44b49930 100644 --- a/core/Command/Db/AddMissingIndices.php +++ b/core/Command/Db/AddMissingIndices.php @@ -465,6 +465,14 @@ private function addCoreIndexes(OutputInterface $output, bool $dryRun): void { $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 5f327e69c964a..4217f3b3270aa 100644 --- a/core/Migrations/Version27000Date20220613163520.php +++ b/core/Migrations/Version27000Date20220613163520.php @@ -43,7 +43,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 370cfb2b560d3a1e0b153e57310cbb1b0355a37c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 15 Jun 2023 11:47:29 +0200 Subject: [PATCH 3/3] ci: undo version.php change to avoid conflict in 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 e662e0fbd0e06..40d6d804d306c 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 = [25, 0, 7, 2]; +$OC_Version = [25, 0, 7, 1]; // The human readable string $OC_VersionString = '25.0.7';