From 9853bc880d68ca5034f93eee3fcde48abcb2fb8e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 25 Feb 2026 16:28:21 +0100 Subject: [PATCH] fix: improve performance of handling delete shares Signed-off-by: Robin Appelman --- .../lib/Listener/SharesUpdatedListener.php | 17 ++++++++++------- .../lib/ShareRecipientUpdater.php | 19 +++++++++++++------ .../tests/ShareRecipientUpdaterTest.php | 18 ++++++++++++++++++ .../tests/SharesUpdatedListenerTest.php | 10 +++------- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php index 880fd9792b8..1ab52a069ba 100644 --- a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -55,11 +55,11 @@ class SharesUpdatedListener implements IEventListener { public function handle(Event $event): void { if ($event instanceof UserShareAccessUpdatedEvent) { foreach ($event->getUsers() as $user) { - $this->updateOrMarkUser($user, true); + $this->updateOrMarkUser($user); } } if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) { - $this->updateOrMarkUser($event->getUser(), true); + $this->updateOrMarkUser($event->getUser()); } if ($event instanceof ShareCreatedEvent || $event instanceof ShareTransferredEvent) { $share = $event->getShare(); @@ -75,8 +75,11 @@ class SharesUpdatedListener implements IEventListener { } } if ($event instanceof BeforeShareDeletedEvent) { - foreach ($this->shareManager->getUsersForShare($event->getShare()) as $user) { - $this->updateOrMarkUser($user, false, [$event->getShare()]); + $share = $event->getShare(); + foreach ($this->shareManager->getUsersForShare($share) as $user) { + $this->markOrRun($user, function () use ($user, $share) { + $this->shareUpdater->updateForDeletedShare($user, $share); + }); } } } @@ -92,9 +95,9 @@ class SharesUpdatedListener implements IEventListener { $this->updatedTime += $end - $start; } - private function updateOrMarkUser(IUser $user, bool $verifyMountPoints, array $ignoreShares = []): void { - $this->markOrRun($user, function () use ($user, $verifyMountPoints, $ignoreShares) { - $this->shareUpdater->updateForUser($user, $verifyMountPoints, $ignoreShares); + private function updateOrMarkUser(IUser $user): void { + $this->markOrRun($user, function () use ($user) { + $this->shareUpdater->updateForUser($user); }); } diff --git a/apps/files_sharing/lib/ShareRecipientUpdater.php b/apps/files_sharing/lib/ShareRecipientUpdater.php index 996c051749a..83cf681344c 100644 --- a/apps/files_sharing/lib/ShareRecipientUpdater.php +++ b/apps/files_sharing/lib/ShareRecipientUpdater.php @@ -28,7 +28,7 @@ class ShareRecipientUpdater { /** * Validate all received shares for a user */ - public function updateForUser(IUser $user, bool $verifyMountPoints = true, array $ignoreShares = []): void { + public function updateForUser(IUser $user): void { // prevent recursion if (isset($this->inUpdate[$user->getUID()])) { return; @@ -40,20 +40,18 @@ class ShareRecipientUpdater { $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); $mountsByPath = array_combine($mountPoints, $cachedMounts); - $shares = $this->shareMountProvider->getSuperSharesForUser($user, $ignoreShares); + $shares = $this->shareMountProvider->getSuperSharesForUser($user); // the share mounts have changed if either the number of shares doesn't matched the number of share mounts // or there is a share for which we don't have a mount yet. $mountsChanged = count($shares) !== count($shareMounts); - foreach ($shares as &$share) { + foreach ($shares as $share) { [$parentShare, $groupedShares] = $share; $mountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/'; $mountKey = $parentShare->getNodeId() . '::' . $mountPoint; if (!isset($cachedMounts[$mountKey])) { $mountsChanged = true; - if ($verifyMountPoints) { - $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares); - } + $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares); } } @@ -78,4 +76,13 @@ class ShareRecipientUpdater { $this->userMountCache->addMount($user, $mountPoint, $share->getNode()->getData(), MountProvider::class); } + + /** + * Process a single deleted share for a user + */ + public function updateForDeletedShare(IUser $user, IShare $share): void { + $mountPoint = '/' . $user->getUID() . '/files/' . trim($share->getTarget(), '/') . '/'; + + $this->userMountCache->removeMount($mountPoint); + } } diff --git a/apps/files_sharing/tests/ShareRecipientUpdaterTest.php b/apps/files_sharing/tests/ShareRecipientUpdaterTest.php index 5ad995c3b1c..2316e6b8b7e 100644 --- a/apps/files_sharing/tests/ShareRecipientUpdaterTest.php +++ b/apps/files_sharing/tests/ShareRecipientUpdaterTest.php @@ -185,4 +185,22 @@ class ShareRecipientUpdaterTest extends \Test\TestCase { $this->updater->updateForUser($user1); } + + public function testDeletedShare() { + $share = $this->createMock(IShare::class); + $share->method('getTarget') + ->willReturn('/target'); + $share->method('getNodeId') + ->willReturn(111); + $user1 = $this->createUser('user1', ''); + + $this->shareTargetValidator->expects($this->never()) + ->method('verifyMountPoint'); + + $this->userMountCache->expects($this->exactly(1)) + ->method('removeMount') + ->with('/user1/files/target/'); + + $this->updater->updateForDeletedShare($user1, $share); + } } diff --git a/apps/files_sharing/tests/SharesUpdatedListenerTest.php b/apps/files_sharing/tests/SharesUpdatedListenerTest.php index ccba9d41c2e..a6ec4ace499 100644 --- a/apps/files_sharing/tests/SharesUpdatedListenerTest.php +++ b/apps/files_sharing/tests/SharesUpdatedListenerTest.php @@ -116,10 +116,8 @@ class SharesUpdatedListenerTest extends \Test\TestCase { $this->shareRecipientUpdater ->expects($this->exactly(2)) ->method('updateForUser') - ->willReturnCallback(function (IUser $user, bool $verifyMountPoints = true, array $ignoreShares = []) use ($user1, $user2) { + ->willReturnCallback(function (IUser $user) use ($user1, $user2) { $this->assertContains($user, [$user1, $user2]); - $this->assertEquals(true, $verifyMountPoints); - $this->assertEquals([], $ignoreShares); }); $this->sharesUpdatedListener->handle($event); @@ -137,11 +135,9 @@ class SharesUpdatedListenerTest extends \Test\TestCase { $this->shareRecipientUpdater ->expects($this->exactly(2)) - ->method('updateForUser') - ->willReturnCallback(function (IUser $user, bool $verifyMountPoints = true, array $ignoreShares = []) use ($user1, $user2, $share) { + ->method('updateForDeletedShare') + ->willReturnCallback(function (IUser $user) use ($user1, $user2, $share) { $this->assertContains($user, [$user1, $user2]); - $this->assertEquals(false, $verifyMountPoints); - $this->assertEquals([$share], $ignoreShares); }); $this->sharesUpdatedListener->handle($event);