diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index cbf2a8457..3cc79efc4 100644 --- a/lib/FilesHooks.php +++ b/lib/FilesHooks.php @@ -1097,32 +1097,51 @@ protected function shareNotificationForOriginalOwners(string $sharedBy, string $ $mount = $fileSource->getMountPoint(); if ($mount instanceof SharedMount) { $sourceShare = $mount->getShare(); - try { - $sourceNode = $sourceShare->getNode(); - } catch (NotFoundException) { - return; - } + + $fileId = $fileSource->getId(); if ($sourceShare->getShareOwner() !== $sharedBy) { + $owner = $sourceShare->getShareOwner(); + try { + $ownerNodes = $this->rootFolder->getUserFolder($owner)->getById($fileId); + $ownerNode = $ownerNodes[0] ?? null; + if ($ownerNode === null) { + return; + } + } catch (NotFoundException) { + return; + } + $this->reshareNotificationForSharer( - $sourceShare->getShareOwner(), + $owner, $subject, $shareWith, - $sourceNode->getId(), - $this->getUserRelativePath($sourceShare->getShareOwner(), $sourceNode->getPath()), - $sourceNode instanceof File, + $fileId, + $this->getUserRelativePath($owner, $ownerNode->getPath()), + $fileSource instanceof File, ); } if ($sourceShare->getSharedBy() && $sourceShare->getSharedBy() !== $sharedBy && $sourceShare->getShareOwner() !== $sourceShare->getSharedBy()) { + $sharer = $sourceShare->getSharedBy(); + try { + $sharerNodes = $this->rootFolder->getUserFolder($sharer)->getById($fileId); + $sharerNode = $sharerNodes[0] ?? null; + if ($sharerNode === null) { + return; + } + } catch (NotFoundException) { + return; + } + $this->reshareNotificationForSharer( - $sourceShare->getSharedBy(), + $sharer, $subject, $shareWith, - $sourceNode->getId(), - $sourceShare->getTarget(), - $sourceNode instanceof File, + $fileId, + $this->getUserRelativePath($sharer, $sharerNode->getPath()), + $fileSource instanceof File, ); } } diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index 90b4dece7..405e893d4 100644 --- a/tests/FilesHooksTest.php +++ b/tests/FilesHooksTest.php @@ -801,19 +801,18 @@ public function testShare(): void { public static function dataShareNotificationForOriginalOwners(): array { return [ - [false, false, 'owner', '', 0], - [true, false, 'owner', '', 1], - [true, true, 'owner', '', 1], - [true, true, 'owner', 'owner', 1], - [true, true, 'owner', 'sharee', 2], - [true, true, 'current', 'sharee', 1], - [true, true, 'owner', 'current', 1], - [true, true, 'current', 'current', 0], + [true, 'owner', '', 1], + [true, 'owner', 'owner', 1], + [true, 'owner', 'sharee', 2], + [true, 'current', 'sharee', 1], + [true, 'owner', 'current', 1], + [true, 'current', 'current', 0], + [false, 'owner', '', 0], ]; } #[DataProvider('dataShareNotificationForOriginalOwners')] - public function testShareNotificationForOriginalOwners(bool $validMountPoint, bool $validSharedStorage, string $pathOwner, string $shareeUser, int $numReshareNotification): void { + public function testShareNotificationForOriginalOwners(bool $nodeFound, string $pathOwner, string $shareeUser, int $numReshareNotification): void { $filesHooks = $this->getFilesHooks([ 'reshareNotificationForSharer', ]); @@ -834,20 +833,69 @@ public function testShareNotificationForOriginalOwners(bool $validMountPoint, bo $filesHooks->expects($this->exactly($numReshareNotification)) ->method('reshareNotificationForSharer') - ->with($this->anything(), 'subject', 'with', 42, '/source-path', 'file'); - - if ($validMountPoint) { - $sourceNode = $this->getNodeMock(42, "/$pathOwner/files/source-path"); - $sourceShare->method('getNode') - ->willReturn($sourceNode); + ->with($this->anything(), 'subject', 'with', 42, '/source-path', true); + + // Mock rootFolder->getUserFolder()->getById() for each relevant user + if ($nodeFound) { + $this->rootFolder->method('getUserFolder') + ->willReturnCallback(function (string $userId) { + $userFolder = $this->createMock(Folder::class); + $resolvedNode = $this->getNodeMock(42, "/$userId/files/source-path"); + $userFolder->method('getById') + ->with(42) + ->willReturn([$resolvedNode]); + return $userFolder; + }); } else { - $sourceShare->method('getNode') + $this->rootFolder->method('getUserFolder') ->willThrowException(new \OCP\Files\NotFoundException()); } self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['current', 'subject', 'with', $node]); } + /** + * Test that resharing a subfolder notifies the owner about the subfolder, + * not the parent folder (issue #2277). + */ + public function testShareNotificationForOriginalOwnersUsesActualFileNotShareRoot(): void { + $filesHooks = $this->getFilesHooks([ + 'reshareNotificationForSharer', + ]); + + // The actual subfolder being reshared (file ID 99, inside a shared parent) + $subfolder = $this->getNodeMock(99, '/userA/files/ParentFolder/Subfolder', false); + $mount = $this->createMock(SharedMount::class); + $subfolder->method('getMountPoint') + ->willReturn($mount); + + // The original share is for the parent folder (file ID 50) + $sourceShare = $this->createMock(IShare::class); + $sourceShare->method('getShareOwner') + ->willReturn('admin'); + $sourceShare->method('getSharedBy') + ->willReturn(''); + $mount->method('getShare') + ->willReturn($sourceShare); + + // The owner's view of the subfolder (resolved by file ID 99) + $ownerSubfolder = $this->getNodeMock(99, '/admin/files/ParentFolder/Subfolder', false); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(99) + ->willReturn([$ownerSubfolder]); + $this->rootFolder->method('getUserFolder') + ->with('admin') + ->willReturn($userFolder); + + // The notification must reference file ID 99 and /ParentFolder/Subfolder, NOT the parent + $filesHooks->expects($this->once()) + ->method('reshareNotificationForSharer') + ->with('admin', 'reshared_user_by', 'userB', 99, '/ParentFolder/Subfolder', false); + + self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['userA', 'reshared_user_by', 'userB', $subfolder]); + } + public function testShareNotificationForSharer(): void { $filesHooks = $this->getFilesHooks(['addNotificationsForUser']);