From 6e9ba894a26114f3f0a3e0d1016cb5ec2aeec8c1 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Mon, 29 Dec 2025 18:06:12 +0100 Subject: [PATCH] fix: add $forChildren parameter to IPartialMountProvider Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../Files/Config/MountProviderCollection.php | 2 ++ lib/private/Files/SetupManager.php | 2 ++ .../Files/Config/IPartialMountProvider.php | 2 ++ tests/lib/Files/SetupManagerTest.php | 33 +++++++++++++++++-- 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Config/MountProviderCollection.php b/lib/private/Files/Config/MountProviderCollection.php index 24d2bf54a09..9f4c52bd540 100644 --- a/lib/private/Files/Config/MountProviderCollection.php +++ b/lib/private/Files/Config/MountProviderCollection.php @@ -91,6 +91,7 @@ class MountProviderCollection implements IMountProviderCollection, Emitter { public function getUserMountsFromProviderByPath( string $providerClass, string $path, + bool $forChildren, array $mountProviderArgs, ): array { $provider = $this->providers[$providerClass] ?? null; @@ -107,6 +108,7 @@ class MountProviderCollection implements IMountProviderCollection, Emitter { /** @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 aebd99c76df..06096375136 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -486,6 +486,7 @@ class SetupManager { $this->mountProviderCollection->getUserMountsFromProviderByPath( $mountProvider, $path, + false, [$providerArgs] ) ); @@ -573,6 +574,7 @@ class SetupManager { = $this->mountProviderCollection->getUserMountsFromProviderByPath( $providerClass, $path, + true, $providerArgs, ); } diff --git a/lib/public/Files/Config/IPartialMountProvider.php b/lib/public/Files/Config/IPartialMountProvider.php index 2f933622669..0ebb3413e1a 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 40d81616236..2e3bd475e66 100644 --- a/tests/lib/Files/SetupManagerTest.php +++ b/tests/lib/Files/SetupManagerTest.php @@ -118,6 +118,10 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { ->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 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { ->willReturnCallback(function ( string $providerClass, string $pathArg, + bool $forChildren, array $mountProviderArgs, ) use ( $cachedChildMount, @@ -335,14 +354,17 @@ class SetupManagerTest extends TestCase { // 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 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { $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 @@ class SetupManagerTest extends TestCase { // 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 @@ class SetupManagerTestPartialMountProvider implements IPartialMountProvider { return []; } - public function getMountsForPath(string $path, array $mountProviderArgs, IStorageFactory $loader): array { + public function getMountsForPath(string $path, bool $forChildren, array $mountProviderArgs, IStorageFactory $loader): array { return []; } }