fix: Tidy up code for reshare deletion

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
pull/49927/head
Côme Chilliet 2024-08-22 17:03:07 +07:00 committed by backportbot[bot]
parent dfbaa103f9
commit 786424ef09
2 changed files with 24 additions and 22 deletions

@ -1164,31 +1164,32 @@ class Manager implements IManager {
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 = [];
@ -1197,7 +1198,7 @@ class Manager implements IManager {
IShare::TYPE_USER,
IShare::TYPE_LINK,
IShare::TYPE_REMOTE,
IShare::TYPE_EMAIL
IShare::TYPE_EMAIL,
];
foreach ($userIds as $userId) {
@ -1209,8 +1210,8 @@ class Manager implements IManager {
}
}
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) {
@ -1224,6 +1225,7 @@ class Manager implements IManager {
try {
$this->generalCreateChecks($child);
} catch (GenericShareException $e) {
$this->logger->debug('Delete reshare because of exception '.$e->getMessage(), ['exception' => $e]);
$this->deleteShare($child);
}
}
@ -1252,10 +1254,10 @@ class Manager implements IManager {
$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);
}

@ -242,7 +242,7 @@ class ManagerTest extends \Test\TestCase {
*/
public function testDelete($shareType, $sharedWith) {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren', 'deleteReshare'])
->setMethods(['getShareById', 'deleteChildren', 'deleteReshares'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -260,7 +260,7 @@ class ManagerTest extends \Test\TestCase {
->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())
@ -285,7 +285,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteLazyShare() {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren', 'deleteReshare'])
->setMethods(['getShareById', 'deleteChildren', 'deleteReshares'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -304,7 +304,7 @@ class ManagerTest extends \Test\TestCase {
$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())
@ -329,7 +329,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteNested() {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteReshare'])
->setMethods(['getShareById', 'deleteReshares'])
->getMock();
$path = $this->createMock(File::class);
@ -515,7 +515,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->atLeast(2))->method('deleteShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
$manager->deleteReshare($share);
self::invokePrivate($manager, 'deleteReshares', [$share]);
}
public function testDeleteReshareWhenUserHasAnotherShare(): void {
@ -543,7 +543,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('deleteShare');
$manager->deleteReshare($share);
self::invokePrivate($manager, 'deleteReshares', [$share]);
}
public function testDeleteReshareOfUsersInGroupShare(): void {
@ -592,7 +592,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->exactly(2))->method('deleteShare')->withConsecutive([$reShare1], [$reShare2]);
$manager->deleteReshare($share);
self::invokePrivate($manager, 'deleteReshares', [$share]);
}
public function testGetShareById(): void {