Skip to content

Commit ab967de

Browse files
come-nccome-nc
committed
fix: Fix promotion of reshares from subsubfolders
Signed-off-by: Côme Chilliet <[email protected]>
1 parent fdc18a8 commit ab967de

2 files changed

Lines changed: 100 additions & 25 deletions

File tree

‎lib/private/Share20/Manager.php‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,16 +1082,23 @@ protected function promoteReshares(IShare $share): void {
10821082
foreach ($userIds as $userId) {
10831083
foreach ($shareTypes as $shareType) {
10841084
$provider = $this->factory->getProviderForType($shareType);
1085-
$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
1086-
foreach ($shares as $child) {
1087-
$reshareRecords[] = $child;
1088-
}
1089-
}
1085+
if ($node instanceof Folder) {
1086+
/* We need to get all shares by this user to get subshares */
1087+
$shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0);
10901088

1091-
if ($node instanceof Folder) {
1092-
$sharesInFolder = $this->getSharesInFolder($userId, $node, false);
1093-
1094-
foreach ($sharesInFolder as $shares) {
1089+
foreach ($shares as $share) {
1090+
try {
1091+
$path = $share->getNode()->getPath();
1092+
} catch (NotFoundException) {
1093+
/* Ignore share of non-existing node */
1094+
continue;
1095+
}
1096+
if (str_starts_with($path, $node->getPath() . '/') || ($path === $node->getPath())) {
1097+
$reshareRecords[] = $share;
1098+
}
1099+
}
1100+
} else {
1101+
$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
10951102
foreach ($shares as $child) {
10961103
$reshareRecords[] = $child;
10971104
}

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -472,34 +472,92 @@ public function testDeleteChildren(): void {
472472
$this->assertSame($shares, $result);
473473
}
474474

475-
public function testPromoteReshareWhenUserHasOneShare(): void {
475+
public function testPromoteReshareFile(): void {
476+
$manager = $this->createManagerMock()
477+
->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
478+
->getMock();
479+
480+
$file = $this->createMock(File::class);
481+
482+
$share = $this->createMock(IShare::class);
483+
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
484+
$share->method('getNodeType')->willReturn('folder');
485+
$share->method('getSharedWith')->willReturn('userB');
486+
$share->method('getNode')->willReturn($file);
487+
488+
$reShare = $this->createMock(IShare::class);
489+
$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
490+
$reShare->method('getSharedBy')->willReturn('userB');
491+
$reShare->method('getSharedWith')->willReturn('userC');
492+
$reShare->method('getNode')->willReturn($file);
493+
494+
$this->defaultProvider->method('getSharesBy')
495+
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $file) {
496+
$this->assertEquals($file, $node);
497+
if ($shareType === IShare::TYPE_USER) {
498+
return match($userId) {
499+
'userB' => [$reShare],
500+
};
501+
} else {
502+
return [];
503+
}
504+
});
505+
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
506+
507+
$manager->expects($this->exactly(1))->method('updateShare')->with($reShare);
508+
509+
self::invokePrivate($manager, 'promoteReshares', [$share]);
510+
}
511+
512+
public function testPromoteReshare(): void {
476513
$manager = $this->createManagerMock()
477514
->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
478515
->getMock();
479516

480517
$folder = $this->createMock(Folder::class);
518+
$folder->method('getPath')->willReturn('/path/to/folder');
519+
520+
$subFolder = $this->createMock(Folder::class);
521+
$subFolder->method('getPath')->willReturn('/path/to/folder/sub');
522+
523+
$otherFolder = $this->createMock(Folder::class);
524+
$otherFolder->method('getPath')->willReturn('/path/to/otherfolder/');
481525

482526
$share = $this->createMock(IShare::class);
483527
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
484528
$share->method('getNodeType')->willReturn('folder');
485-
$share->method('getSharedWith')->willReturn('UserB');
529+
$share->method('getSharedWith')->willReturn('userB');
486530
$share->method('getNode')->willReturn($folder);
487531

488532
$reShare = $this->createMock(IShare::class);
489-
$reShare->method('getSharedBy')->willReturn('UserB');
490-
$reShare->method('getSharedWith')->willReturn('UserC');
533+
$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
534+
$reShare->method('getSharedBy')->willReturn('userB');
535+
$reShare->method('getSharedWith')->willReturn('userC');
491536
$reShare->method('getNode')->willReturn($folder);
492537

493538
$reShareInSubFolder = $this->createMock(IShare::class);
494-
$reShareInSubFolder->method('getSharedBy')->willReturn('UserB');
539+
$reShareInSubFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
540+
$reShareInSubFolder->method('getSharedBy')->willReturn('userB');
541+
$reShareInSubFolder->method('getNode')->willReturn($subFolder);
495542

496-
$manager->method('getSharesInFolder')->willReturn([$reShareInSubFolder]);
497-
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
543+
$reShareInOtherFolder = $this->createMock(IShare::class);
544+
$reShareInOtherFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
545+
$reShareInOtherFolder->method('getSharedBy')->willReturn('userB');
546+
$reShareInOtherFolder->method('getNode')->willReturn($otherFolder);
498547

499548
$this->defaultProvider->method('getSharesBy')
500-
->willReturn([$reShare]);
549+
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $reShareInSubFolder, $reShareInOtherFolder) {
550+
if ($shareType === IShare::TYPE_USER) {
551+
return match($userId) {
552+
'userB' => [$reShare,$reShareInSubFolder,$reShareInOtherFolder],
553+
};
554+
} else {
555+
return [];
556+
}
557+
});
558+
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
501559

502-
$manager->expects($this->atLeast(2))->method('updateShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
560+
$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
503561

504562
self::invokePrivate($manager, 'promoteReshares', [$share]);
505563
}
@@ -510,23 +568,24 @@ public function testPromoteReshareWhenUserHasAnotherShare(): void {
510568
->getMock();
511569

512570
$folder = $this->createMock(Folder::class);
571+
$folder->method('getPath')->willReturn('/path/to/folder');
513572

514573
$share = $this->createMock(IShare::class);
515574
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
516575
$share->method('getNodeType')->willReturn('folder');
517-
$share->method('getSharedWith')->willReturn('UserB');
576+
$share->method('getSharedWith')->willReturn('userB');
518577
$share->method('getNode')->willReturn($folder);
519578

520579
$reShare = $this->createMock(IShare::class);
521580
$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
522581
$reShare->method('getNodeType')->willReturn('folder');
523-
$reShare->method('getSharedBy')->willReturn('UserB');
582+
$reShare->method('getSharedBy')->willReturn('userB');
524583
$reShare->method('getNode')->willReturn($folder);
525584

526585
$this->defaultProvider->method('getSharesBy')->willReturn([$reShare]);
527-
$manager->method('getSharesInFolder')->willReturn([]);
528586
$manager->method('generalCreateChecks')->willReturn(true);
529587

588+
/* No share is promoted because generalCreateChecks does not throw */
530589
$manager->expects($this->never())->method('updateShare');
531590

532591
self::invokePrivate($manager, 'promoteReshares', [$share]);
@@ -538,6 +597,7 @@ public function testPromoteReshareOfUsersInGroupShare(): void {
538597
->getMock();
539598

540599
$folder = $this->createMock(Folder::class);
600+
$folder->method('getPath')->willReturn('/path/to/folder');
541601

542602
$userA = $this->createMock(IUser::class);
543603
$userA->method('getUID')->willReturn('userA');
@@ -552,13 +612,13 @@ public function testPromoteReshareOfUsersInGroupShare(): void {
552612
$reShare1 = $this->createMock(IShare::class);
553613
$reShare1->method('getShareType')->willReturn(IShare::TYPE_USER);
554614
$reShare1->method('getNodeType')->willReturn('folder');
555-
$reShare1->method('getSharedBy')->willReturn('UserB');
615+
$reShare1->method('getSharedBy')->willReturn('userB');
556616
$reShare1->method('getNode')->willReturn($folder);
557617

558618
$reShare2 = $this->createMock(IShare::class);
559619
$reShare2->method('getShareType')->willReturn(IShare::TYPE_USER);
560620
$reShare2->method('getNodeType')->willReturn('folder');
561-
$reShare2->method('getSharedBy')->willReturn('UserC');
621+
$reShare2->method('getSharedBy')->willReturn('userC');
562622
$reShare2->method('getNode')->willReturn($folder);
563623

564624
$userB = $this->createMock(IUser::class);
@@ -570,11 +630,19 @@ public function testPromoteReshareOfUsersInGroupShare(): void {
570630
$this->groupManager->method('get')->with('Group')->willReturn($group);
571631

572632
$this->defaultProvider->method('getSharesBy')
573-
->willReturn([]);
633+
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare1, $reShare2) {
634+
if ($shareType === IShare::TYPE_USER) {
635+
return match($userId) {
636+
'userB' => [$reShare1],
637+
'userC' => [$reShare2],
638+
};
639+
} else {
640+
return [];
641+
}
642+
});
574643
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
575644

576645
$manager->method('getSharedWith')->willReturn([]);
577-
$manager->expects($this->exactly(2))->method('getSharesInFolder')->willReturnOnConsecutiveCalls([[$reShare1]], [[$reShare2]]);
578646

579647
$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare1], [$reShare2]);
580648

0 commit comments

Comments
 (0)