diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index bfac92f935c8a..4b7b261887297 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1011,31 +1011,32 @@ protected function deleteChildren(IShare $share) { return $deletedShares; } - public function deleteReshare(IShare $share) { - // Skip if node not found + protected function deleteReshares(IShare $share): void { try { $node = $share->getNode(); } catch (NotFoundException) { + /* Skip if node not found */ return; } $userIds = []; - if ($share->getShareType() === IShare::TYPE_USER) { + if ($share->getShareType() === IShare::TYPE_USER) { $userIds[] = $share->getSharedWith(); - } - - if ($share->getShareType() === IShare::TYPE_GROUP) { + } elseif ($share->getShareType() === IShare::TYPE_GROUP) { $group = $this->groupManager->get($share->getSharedWith()); - $users = $group->getUsers(); + $users = $group?->getUsers() ?? []; foreach ($users as $user) { - // Skip if share owner is member of shared group + /* Skip share owner */ if ($user->getUID() === $share->getShareOwner()) { continue; } $userIds[] = $user->getUID(); } + } else { + /* We only support user and group shares */ + return; } $reshareRecords = []; @@ -1044,7 +1045,7 @@ public function deleteReshare(IShare $share) { IShare::TYPE_USER, IShare::TYPE_LINK, IShare::TYPE_REMOTE, - IShare::TYPE_EMAIL + IShare::TYPE_EMAIL, ]; foreach ($userIds as $userId) { @@ -1056,8 +1057,8 @@ public function deleteReshare(IShare $share) { } } - if ($share->getNodeType() === 'folder') { - $sharesInFolder = $this->getSharesInFolder($userId, $node, true); + if ($node instanceof Folder) { + $sharesInFolder = $this->getSharesInFolder($userId, $node, false); foreach ($sharesInFolder as $shares) { foreach ($shares as $child) { @@ -1071,6 +1072,7 @@ public function deleteReshare(IShare $share) { try { $this->generalCreateChecks($child); } catch (GenericShareException $e) { + $this->logger->debug('Delete reshare because of exception '.$e->getMessage(), ['exception' => $e]); $this->deleteShare($child); } } @@ -1099,10 +1101,10 @@ public function deleteShare(IShare $share) { $provider = $this->factory->getProviderForType($share->getShareType()); $provider->delete($share); - // Delete shares that shared by the "share with user/group" - $this->deleteReshare($share); - $this->dispatcher->dispatchTyped(new ShareDeletedEvent($share)); + + // Delete reshares of the deleted share + $this->deleteReshares($share); } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 7a6e9cb3c4074..d5819e6abf6b0 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -228,7 +228,7 @@ public function dataTestDelete() { */ public function testDelete($shareType, $sharedWith) { $manager = $this->createManagerMock() - ->setMethods(['getShareById', 'deleteChildren', 'deleteReshare']) + ->setMethods(['getShareById', 'deleteChildren', 'deleteReshares']) ->getMock(); $manager->method('deleteChildren')->willReturn([]); @@ -246,7 +246,7 @@ public function testDelete($shareType, $sharedWith) { ->setTarget('myTarget'); $manager->expects($this->once())->method('deleteChildren')->with($share); - $manager->expects($this->once())->method('deleteReshare')->with($share); + $manager->expects($this->once())->method('deleteReshares')->with($share); $this->defaultProvider ->expects($this->once()) @@ -271,7 +271,7 @@ public function testDelete($shareType, $sharedWith) { public function testDeleteLazyShare() { $manager = $this->createManagerMock() - ->setMethods(['getShareById', 'deleteChildren', 'deleteReshare']) + ->setMethods(['getShareById', 'deleteChildren', 'deleteReshares']) ->getMock(); $manager->method('deleteChildren')->willReturn([]); @@ -290,7 +290,7 @@ public function testDeleteLazyShare() { $this->rootFolder->expects($this->never())->method($this->anything()); $manager->expects($this->once())->method('deleteChildren')->with($share); - $manager->expects($this->once())->method('deleteReshare')->with($share); + $manager->expects($this->once())->method('deleteReshares')->with($share); $this->defaultProvider ->expects($this->once()) @@ -315,7 +315,7 @@ public function testDeleteLazyShare() { public function testDeleteNested() { $manager = $this->createManagerMock() - ->setMethods(['getShareById', 'deleteReshare']) + ->setMethods(['getShareById', 'deleteReshares']) ->getMock(); $path = $this->createMock(File::class); @@ -501,7 +501,7 @@ public function testDeleteReshareWhenUserHasOneShare() { $manager->expects($this->atLeast(2))->method('deleteShare')->withConsecutive([$reShare], [$reShareInSubFolder]); - $manager->deleteReshare($share); + self::invokePrivate($manager, 'deleteReshares', [$share]); } public function testDeleteReshareWhenUserHasAnotherShare() { @@ -529,7 +529,7 @@ public function testDeleteReshareWhenUserHasAnotherShare() { $manager->expects($this->never())->method('deleteShare'); - $manager->deleteReshare($share); + self::invokePrivate($manager, 'deleteReshares', [$share]); } public function testDeleteReshareOfUsersInGroupShare() { @@ -578,7 +578,7 @@ public function testDeleteReshareOfUsersInGroupShare() { $manager->expects($this->exactly(2))->method('deleteShare')->withConsecutive([$reShare1], [$reShare2]); - $manager->deleteReshare($share); + self::invokePrivate($manager, 'deleteReshares', [$share]); } public function testGetShareById() {