Skip to content

Commit

Permalink
allow storing multiple mounts for the same rootid in the mount cache
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
icewind1991 committed Aug 31, 2022
1 parent f56ecf9 commit f15414c
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 20 deletions.
2 changes: 1 addition & 1 deletion core/Migrations/Version13000Date20170718121200.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down
51 changes: 51 additions & 0 deletions core/Migrations/Version25000Date20220613163520.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2022 Your name <[email protected]>
*
* @author Your name <[email protected]>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/

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;
}
}
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
39 changes: 21 additions & 18 deletions lib/private/Files/Config/UserMountCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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() ||
Expand All @@ -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());
Expand All @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit f15414c

Please sign in to comment.