diff --git a/lib/private/Files/Config/MountProviderCollection.php b/lib/private/Files/Config/MountProviderCollection.php index 24d2bf54a0960..9f4c52bd54026 100644 --- a/lib/private/Files/Config/MountProviderCollection.php +++ b/lib/private/Files/Config/MountProviderCollection.php @@ -91,6 +91,7 @@ public function getMountsForUser(IUser $user): array { public function getUserMountsFromProviderByPath( string $providerClass, string $path, + bool $forChildren, array $mountProviderArgs, ): array { $provider = $this->providers[$providerClass] ?? null; @@ -107,6 +108,7 @@ public function getUserMountsFromProviderByPath( /** @var IPartialMountProvider $provider */ return $provider->getMountsForPath( $path, + $forChildren, $mountProviderArgs, $this->loader, ); diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php index aebd99c76dfdd..0609637513627 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -486,6 +486,7 @@ public function setupForPath(string $path, bool $includeChildren = false): void $this->mountProviderCollection->getUserMountsFromProviderByPath( $mountProvider, $path, + false, [$providerArgs] ) ); @@ -573,6 +574,7 @@ static function (ICachedMountInfo $info) use ($rootsMetadata) { = $this->mountProviderCollection->getUserMountsFromProviderByPath( $providerClass, $path, + true, $providerArgs, ); } diff --git a/lib/public/Files/Config/IPartialMountProvider.php b/lib/public/Files/Config/IPartialMountProvider.php index 2f933622669e1..0ebb3413e1a66 100644 --- a/lib/public/Files/Config/IPartialMountProvider.php +++ b/lib/public/Files/Config/IPartialMountProvider.php @@ -30,6 +30,7 @@ interface IPartialMountProvider extends IMountProvider { * corresponding IMountPoint instances. * * @param string $path path for which the mounts are set up + * @param bool $forChildren when true, only child mounts for path should be returned * @param IMountProviderArgs[] $mountProviderArgs * @param IStorageFactory $loader * @return array IMountPoint instances, indexed by @@ -37,6 +38,7 @@ interface IPartialMountProvider extends IMountProvider { */ public function getMountsForPath( string $path, + bool $forChildren, array $mountProviderArgs, IStorageFactory $loader, ): array; diff --git a/tests/lib/Files/SetupManagerTest.php b/tests/lib/Files/SetupManagerTest.php index 40d8161623661..2e3bd475e665d 100644 --- a/tests/lib/Files/SetupManagerTest.php +++ b/tests/lib/Files/SetupManagerTest.php @@ -118,6 +118,10 @@ public function testTearDown(): void { $this->setupManager->tearDown(); } + /** + * Tests that a path is not set up twice for providers implementing + * IPartialMountProvider in setupForPath. + */ public function testSetupForPathWithPartialProviderSkipsAlreadySetupPath(): void { $cachedMount = $this->getCachedMountInfo($this->mountPoint, 42); @@ -140,6 +144,7 @@ public function testSetupForPathWithPartialProviderSkipsAlreadySetupPath(): void ->with( SetupManagerTestPartialMountProvider::class, $this->path, + false, $this->callback(function (array $args) use ($cachedMount) { $this->assertCount(1, $args); $this->assertInstanceOf(IMountProviderArgs::class, $args[0]); @@ -172,6 +177,10 @@ public function testSetupForPathWithPartialProviderSkipsAlreadySetupPath(): void $this->setupManager->setupForPath($this->path, false); } + /** + * Tests that providers that are not implementing IPartialMountProvider are + * not set up more than once by setupForPath. + */ public function testSetupForPathWithNonPartialProviderSkipsAlreadySetupProvider(): void { $cachedMount = $this->getCachedMountInfo($this->mountPoint, 42, IMountProvider::class); @@ -211,6 +220,11 @@ public function testSetupForPathWithNonPartialProviderSkipsAlreadySetupProvider( $this->setupManager->setupForPath($this->path, false); } + /** + * Tests that setupForPath does not instantiate already set up providers + * when called for the same path first with $withChildren set to true + * and then set to false. + */ public function testSetupForPathWithChildrenAndNonPartialProviderSkipsAlreadySetupProvider(): void { $cachedMount = $this->getCachedMountInfo($this->mountPoint, 42, IMountProvider::class); $additionalCachedMount = $this->getCachedMountInfo($this->mountPoint . 'additional/', 43, SetupManagerTestFullMountProvider::class); @@ -269,6 +283,10 @@ public function testSetupForPathWithChildrenAndNonPartialProviderSkipsAlreadySet $this->setupManager->setupForPath($this->path, false); } + /** + * Tests that setupForPath does not set up child mounts again if a parent + * was set up with $withChildren set to true. + */ public function testSetupForPathWithChildrenAndPartialProviderSkipsIfParentAlreadySetup(): void { $childPath = "{$this->path}/child"; $childMountPoint = "{$childPath}/"; @@ -322,6 +340,7 @@ public function testSetupForPathWithChildrenAndPartialProviderSkipsIfParentAlrea ->willReturnCallback(function ( string $providerClass, string $pathArg, + bool $forChildren, array $mountProviderArgs, ) use ( $cachedChildMount, @@ -335,14 +354,17 @@ public function testSetupForPathWithChildrenAndPartialProviderSkipsIfParentAlrea // call for the parent $expectedCachedMount = $cachedMount; $mountPoints = [$partialMount]; + $expectedForChildren = false; } else { // call for the children $expectedCachedMount = $cachedChildMount; $mountPoints = [$partialChildMount]; + $expectedForChildren = true; } $this->assertSame(SetupManagerTestPartialMountProvider::class, $providerClass); $this->assertSame($expectedPath, $pathArg); + $this->assertSame($expectedForChildren, $forChildren); $this->assertCount(1, $mountProviderArgs); $this->assertInstanceOf(IMountProviderArgs::class, $mountProviderArgs[0]); $this->assertSame($expectedCachedMount, $mountProviderArgs[0]->mountInfo); @@ -376,6 +398,10 @@ public function testSetupForPathWithChildrenAndPartialProviderSkipsIfParentAlrea $this->setupManager->setupForPath($childPath, true); } + /** + * Tests that when called twice setupForPath does not set up mounts from + * providers implementing IPartialMountProviders or IMountProvider. + */ public function testSetupForPathHandlesPartialAndFullProvidersWithChildren(): void { $parentPartialCachedMount = $this->getCachedMountInfo($this->mountPoint, 42); $childCachedPartialMount = $this->getCachedMountInfo("{$this->mountPoint}partial/", 43); @@ -419,7 +445,7 @@ public function testSetupForPathHandlesPartialAndFullProvidersWithChildren(): vo $invokedCount = $this->exactly(2); $this->mountProviderCollection->expects($invokedCount) ->method('getUserMountsFromProviderByPath') - ->willReturnCallback(function (string $providerClass, string $pathArg, array $mountProviderArgs) use ( + ->willReturnCallback(function (string $providerClass, string $pathArg, bool $forChildren, array $mountProviderArgs) use ( $childCachedPartialMount, $childPartialMount, $parentPartialMount, @@ -430,14 +456,17 @@ public function testSetupForPathHandlesPartialAndFullProvidersWithChildren(): vo // call for the parent $expectedCachedMount = $parentPartialCachedMount; $mountPoints = [$parentPartialMount]; + $expectedForChildren = false; } else { // call for the children $expectedCachedMount = $childCachedPartialMount; $mountPoints = [$childPartialMount]; + $expectedForChildren = true; } $this->assertSame(SetupManagerTestPartialMountProvider::class, $providerClass); $this->assertSame($expectedPath, $pathArg); + $this->assertSame($expectedForChildren, $forChildren); $this->assertCount(1, $mountProviderArgs); $this->assertInstanceOf(IMountProviderArgs::class, $mountProviderArgs[0]); $this->assertSame($expectedCachedMount, $mountProviderArgs[0]->mountInfo); @@ -488,7 +517,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader): array { return []; } - public function getMountsForPath(string $path, array $mountProviderArgs, IStorageFactory $loader): array { + public function getMountsForPath(string $path, bool $forChildren, array $mountProviderArgs, IStorageFactory $loader): array { return []; } }