From f15414c8e0993ec951619ef02164e89ce4ffdf55 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 14 Jun 2022 15:57:59 +0200 Subject: [PATCH] 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 +- .../Version25000Date20220613163520.php | 51 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Config/UserMountCache.php | 39 +++++++------- version.php | 2 +- 6 files changed, 76 insertions(+), 20 deletions(-) create mode 100644 core/Migrations/Version25000Date20220613163520.php diff --git a/core/Migrations/Version13000Date20170718121200.php b/core/Migrations/Version13000Date20170718121200.php index 3e14b4af47adb..646eec8b4cc13 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->addUniqueIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index'); } else { $table = $schema->getTable('mounts'); $table->addColumn('mount_id', Types::BIGINT, [ diff --git a/core/Migrations/Version25000Date20220613163520.php b/core/Migrations/Version25000Date20220613163520.php new file mode 100644 index 0000000000000..724e680ca0629 --- /dev/null +++ b/core/Migrations/Version25000Date20220613163520.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 Version25000Date20220613163520 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->addUniqueIndex(['user_id', 'root_id', 'mount_point'], 'mounts_user_root_path_index'); + } + + return $schema; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5b8e057636c75..d5e9d77449d03 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1047,6 +1047,7 @@ 'OC\\Core\\Migrations\\Version24000Date20220425072957' => $baseDir . '/core/Migrations/Version24000Date20220425072957.php', 'OC\\Core\\Migrations\\Version25000Date20220515204012' => $baseDir . '/core/Migrations/Version25000Date20220515204012.php', 'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php', + 'OC\\Core\\Migrations\\Version25000Date20220613163520' => $baseDir . '/core/Migrations/Version25000Date20220613163520.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 dd3031d3a9bc3..25db830c971d0 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1080,6 +1080,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version24000Date20220425072957' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20220425072957.php', 'OC\\Core\\Migrations\\Version25000Date20220515204012' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220515204012.php', 'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php', + 'OC\\Core\\Migrations\\Version25000Date20220613163520' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220613163520.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..15a8e2085c83d 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() || @@ -166,14 +167,15 @@ private function findChangedMounts(array $newMounts, array $cachedMounts) { private function addToCache(ICachedMountInfo $mount) { if ($mount->getStorageId() !== -1) { - $this->connection->insertIfNotExist('*PREFIX*mounts', [ + $a = $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']); + ], ['root_id', 'user_id', 'mount_point']); + $b = 1; } 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 +201,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 75578ed166857..ff5b165c82b18 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, 0, 7]; +$OC_Version = [25, 0, 0, 8]; // The human readable string $OC_VersionString = '25.0.0 beta 3';