Skip to content

Commit c0c82fd

Browse files
committed
pickup: fix wrong path-caching logic
Signed-off-by: Salvatore Martire <[email protected]>
1 parent 6e43ef9 commit c0c82fd

File tree

2 files changed

+37
-9
lines changed

2 files changed

+37
-9
lines changed

lib/private/Files/Config/UserMountCache.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ private function getChildMounts(string $userId, string $path): array {
672672
)->from('mounts', 'm')->where(
673673
$builder->expr()->like(
674674
'mount_point',
675-
$builder->createNamedParameter($path . '%/')
675+
$builder->createNamedParameter($path . '%')
676676
)
677677
)->andWhere(
678678
$builder->expr()->neq(

lib/private/Files/SetupManager.php

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
use OCP\Constants;
3232
use OCP\Diagnostics\IEventLogger;
3333
use OCP\EventDispatcher\IEventDispatcher;
34-
use OCP\Files\Cache\ICacheEntry;
3534
use OCP\Files\Config\ICachedMountInfo;
3635
use OCP\Files\Config\IHomeMountProvider;
3736
use OCP\Files\Config\IMountProvider;
@@ -57,6 +56,8 @@
5756
use OCP\Lockdown\ILockdownManager;
5857
use OCP\Share\Events\ShareCreatedEvent;
5958
use Psr\Log\LoggerInterface;
59+
use function array_key_exists;
60+
use function dirname;
6061
use function in_array;
6162

6263
class SetupManager {
@@ -74,7 +75,7 @@ class SetupManager {
7475
/**
7576
* An array of paths that have already been set up
7677
*
77-
* @var array<string, class-string<IMountProvider>[]>
78+
* @var array<string, bool>
7879
*/
7980
private array $setupMountProviderPaths = [];
8081
private ICache $cache;
@@ -114,6 +115,28 @@ public function isSetupComplete(IUser $user): bool {
114115
return in_array($user->getUID(), $this->setupUsersComplete, true);
115116
}
116117

118+
/**
119+
* Checks if a path has been cached either directly or through a full setup
120+
* of one of its parents.
121+
*/
122+
private function isPathSetup(string $path, bool $withChildren): bool {
123+
// if the exact path was already setup
124+
$cacheKey = $path . ':' . $withChildren;
125+
if (array_key_exists($cacheKey, $this->setupMountProviderPaths)) {
126+
return true;
127+
}
128+
129+
// or if any of the ancestors was fully setup
130+
while (($path = dirname($path)) !== '/') {
131+
$cacheKey = $path . ':1';
132+
if (array_key_exists($cacheKey, $this->setupMountProviderPaths)) {
133+
return true;
134+
}
135+
}
136+
137+
return false;
138+
}
139+
117140
private function setupBuiltinWrappers() {
118141
if ($this->setupBuiltinWrappersDone) {
119142
return;
@@ -401,7 +424,8 @@ public function setupForPath(string $path, bool $includeChildren = false): void
401424
return;
402425
}
403426

404-
if ($this->isSetupComplete($user) || isset($this->setupMountProviderPaths[$path])) {
427+
// attempt detecting if the path has been already setup
428+
if ($this->isSetupComplete($user) || $this->isPathSetup($path, $includeChildren)) {
405429
return;
406430
}
407431

@@ -417,6 +441,11 @@ public function setupForPath(string $path, bool $includeChildren = false): void
417441

418442
$argsByProvider = [];
419443
foreach ($mountInfos as $mountInfo) {
444+
// attempt detecting if the path was setup, by using its mount point
445+
if ($this->isPathSetup(rtrim($mountInfo->getMountPoint(), '/'), false)) {
446+
continue;
447+
}
448+
420449
$rootMetadata = $rootsMetadata[$mountInfo->getRootId()];
421450
/** @var class-string<IMountProvider>|'' $mountProvider */
422451
$mountProvider = $mountInfo->getMountProvider();
@@ -434,6 +463,9 @@ public function setupForPath(string $path, bool $includeChildren = false): void
434463

435464
$this->eventLogger->start('fs:setup:user:path:authoritative', "Setup $path filesystem for user");
436465

466+
// mark the mount point for $path as cached
467+
$cacheKey = rtrim(array_first($mountInfos)->getMountPoint(), '/') . ':' . $includeChildren;
468+
$this->setupMountProviderPaths[$cacheKey] = true;
437469
$mounts = $this->getMountsFromProviders(
438470
$user,
439471
$path,
@@ -610,14 +642,10 @@ public function getMountsFromProviders(
610642
/** @var class-string<IMountProvider> $providerClass */
611643
foreach ($mountArgsByProvider as $providerClass => $providerArgs) {
612644
if (in_array($providerClass, $this->setupUserMountProviders[$userUid])) {
613-
continue; // skip already setup providers
645+
continue; // skip already fully setup providers
614646
}
615647

616648
if (is_a($providerClass, IPartialMountProvider::class, true)) {
617-
if (isset($this->setupMountProviderPaths[$path])) {
618-
continue; // skip already set up paths
619-
}
620-
$this->setupMountProviderPaths[$path] = true;
621649
// mount provider capable of returning mount-points specific to
622650
// this path
623651
$mounts[] = $this->mountProviderCollection

0 commit comments

Comments
 (0)