diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index 62859f6a5..11d03bd63 100644 --- a/lib/FilesHooks.php +++ b/lib/FilesHooks.php @@ -128,8 +128,8 @@ private function getFileChangeActivitySettings(int $fileId, array $users, string if (!empty($favoriteUsers)) { $favoriteUsers = array_intersect($users, $favoriteUsers); if (!empty($favoriteUsers)) { - $filteredEmailUsers = array_merge($filteredEmailUsers, $this->userSettings->filterUsersBySetting($favoriteUsers, 'email', Files::TYPE_FAVORITE_CHANGED)); - $filteredNotificationUsers = array_merge($filteredNotificationUsers, $this->userSettings->filterUsersBySetting($favoriteUsers, 'notification', Files::TYPE_FAVORITE_CHANGED)); + $filteredEmailUsers = array_replace($filteredEmailUsers, $this->userSettings->filterUsersBySetting($favoriteUsers, 'email', Files::TYPE_FAVORITE_CHANGED)); + $filteredNotificationUsers = array_replace($filteredNotificationUsers, $this->userSettings->filterUsersBySetting($favoriteUsers, 'notification', Files::TYPE_FAVORITE_CHANGED)); } } @@ -166,7 +166,7 @@ protected function addNotificationsForFileAction(Node $node, string $activityTyp // file can be shared using GroupFolders, including ACL check if ($this->config->getSystemValueBool('activity_use_cached_mountpoints', false)) { - $affectedUsers = array_merge($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); + $affectedUsers = array_replace($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); } [$filteredEmailUsers, $filteredNotificationUsers] @@ -301,7 +301,7 @@ public function fileMove(Node $source, Node $target): void { // file can be shared using GroupFolders, including ACL check if ($this->config->getSystemValueBool('activity_use_cached_mountpoints', false)) { [, , $oldFileId] = $this->getSourcePathAndOwner($source); - $oldAccessList['users'] = array_merge($oldAccessList['users'], $this->getAffectedUsersFromCachedMounts($oldFileId)); + $oldAccessList['users'] = array_replace($oldAccessList['users'], $this->getAffectedUsersFromCachedMounts($oldFileId)); } $this->oldAccessList = $oldAccessList; @@ -374,7 +374,7 @@ protected function fileRenaming(Node $source, Node $target): void { // file can be shared using GroupFolders, including ACL check if ($this->config->getSystemValueBool('activity_use_cached_mountpoints', false)) { - $affectedUsers = array_merge($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); + $affectedUsers = array_replace($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); } [$filteredEmailUsers, $filteredNotificationUsers] = $this->getFileChangeActivitySettings($fileId, array_keys($affectedUsers)); @@ -382,6 +382,8 @@ protected function fileRenaming(Node $source, Node $target): void { $shouldFlush = $this->startActivityTransaction(); try { foreach ($affectedUsers as $user => $path) { + $user = (string)$user; + if ($user === $this->currentUser->getUID()) { $userSubject = 'renamed_self'; $userParams = [ @@ -443,7 +445,7 @@ protected function fileMoving(Node $source, Node $target): void { // file can be shared using GroupFolders, including ACL check if ($this->config->getSystemValueBool('activity_use_cached_mountpoints', false)) { $this->userMountCache->clear(); // clear cache for new data - $affectedUsers = array_merge($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); + $affectedUsers = array_replace($affectedUsers, $this->getAffectedUsersFromCachedMounts($fileId)); } $beforeUsers = array_keys($oldUsers); @@ -509,6 +511,7 @@ protected function generateDeleteActivities($users, array $pathMap, $fileId, $ol $shouldFlush = $this->startActivityTransaction(); try { foreach ($users as $user) { + $user = (string)$user; $path = $pathMap[$user]; if ($user === $this->currentUser->getUID()) { @@ -548,6 +551,7 @@ protected function generateAddActivities($users, array $pathMap, $fileId, $fileN $shouldFlush = $this->startActivityTransaction(); try { foreach ($users as $user) { + $user = (string)$user; $path = $pathMap[$user]; if ($user === $this->currentUser->getUID()) { @@ -590,6 +594,8 @@ protected function generateMoveActivities($users, array $beforePathMap, array $a $shouldFlush = $this->startActivityTransaction(); try { foreach ($users as $user) { + $user = (string)$user; + if ($oldFileName === $fileName) { $userParams = [[$newParentId => $afterPathMap[$user] . '/']]; } else { diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index 570a22f95..f939b20e5 100644 --- a/tests/FilesHooksTest.php +++ b/tests/FilesHooksTest.php @@ -173,6 +173,19 @@ protected function getUserMock(string $uid): IUser { return $user; } + protected function getCachedMountFileInfo(string $uid): CachedMountFileInfo { + return new CachedMountFileInfo( + $this->getUserMock($uid), + 1, + 1, + '/' . $uid . '/files', + null, + '', + 'path', + 'files/path' + ); + } + public static function dataFileCreate(): array { return [ ['user', false, 'created_self', 'created_by', Files::TYPE_SHARE_CREATED], @@ -499,6 +512,191 @@ public function testAddNotificationsForFileAction(array $filterUsers, bool $moun $this->assertEquals($addCalls, array_slice($receivedActivities, 0, count($addCalls))); } + public static function dataAddNotificationsForFileActionNumericUserIds(): array { + return [ + 'access list only' => [ + false, + [ + ['56', 'created_self', false, false], + ['55', 'created_by', 20, true], + ], + ], + 'access list and cached mounts' => [ + true, + [ + ['56', 'created_self', false, false], + ['55', 'created_by', 20, true], + ['232', 'created_by', 30, true], + ], + ], + ]; + } + + /** + * Numeric user IDs become integer array keys, they must not be renumbered + * when the recipient lists and the notification settings are merged. + */ + #[DataProvider('dataAddNotificationsForFileActionNumericUserIds')] + public function testAddNotificationsForFileActionNumericUserIds(bool $mountCacheUsed, array $expected): void { + // User 55 favorited the file, so their settings are merged from two lookups + $this->tagManager = $this->createMock(TagManager::class); + $this->tagManager->method('getUsersFavoritingObject') + ->willReturn(['55']); + $filesHooks = $this->getFilesHooks([ + 'getSourcePathAndOwner', + 'getUserPathsFromPath', + 'addNotificationsForUser', + ], '56'); + + $filesHooks->method('getSourcePathAndOwner') + ->willReturn(['/path', '56', 1337]); + $filesHooks->method('getUserPathsFromPath') + ->willReturn([ + 'ownerPath' => '/path', + 'users' => ['56' => '/path', '55' => '/path'], + 'remotes' => [], + ]); + + $this->config->expects($this->once()) + ->method('getSystemValueBool') + ->with('activity_use_cached_mountpoints', false) + ->willReturn($mountCacheUsed); + $this->userMountCache->method('getMountsForFileId') + ->willReturn([ + $this->getCachedMountFileInfo('56'), + $this->getCachedMountFileInfo('232'), + ]); + + $settings = [ + 'email' => ['55' => 20, '232' => 30], + 'notification' => ['232' => true], + ]; + $favoriteSettings = [ + 'email' => [], + 'notification' => ['55' => true], + ]; + $this->settings->method('filterUsersBySetting') + ->willReturnCallback(fn (array $users, string $method, string $type): array => array_intersect_key( + $type === Files::TYPE_FAVORITE_CHANGED ? $favoriteSettings[$method] : $settings[$method], + array_flip($users), + )); + + $received = []; + $filesHooks->method('addNotificationsForUser') + ->willReturnCallback(function ($user, $subject, $subjectParams, $fileId, $path, $isFile, $email, $notification) use (&$received): void { + $received[] = [$user, $subject, $email, $notification]; + }); + + self::invokePrivate($filesHooks, 'addNotificationsForFileAction', [$this->getNodeMock(1337, '/56/files/path'), Files::TYPE_SHARE_CREATED, 'created_self', 'created_by']); + + $this->assertSame($expected, $received); + } + + public function testFileRenamingNumericUserIdsWithMountCache(): void { + $filesHooks = $this->getFilesHooks([ + 'getSourcePathAndOwner', + 'getUserPathsFromPath', + 'addNotificationsForUser', + ], '56'); + + $filesHooks->method('getSourcePathAndOwner') + ->willReturn(['/folder', '56', 42]); + $filesHooks->method('getUserPathsFromPath') + ->willReturn([ + 'users' => ['56' => '/folder', '55' => '/folder'], + 'remotes' => [], + ]); + + $this->config->expects($this->once()) + ->method('getSystemValueBool') + ->with('activity_use_cached_mountpoints', false) + ->willReturn(true); + $this->userMountCache->method('getMountsForFileId') + ->willReturn([ + $this->getCachedMountFileInfo('56'), + $this->getCachedMountFileInfo('232'), + ]); + $this->settings->method('filterUsersBySetting') + ->willReturn([]); + + $received = []; + $filesHooks->method('addNotificationsForUser') + ->willReturnCallback(function ($user, $subject) use (&$received): void { + $received[] = [$user, $subject]; + }); + + $target = $this->getNodeMock(42, '/56/files/folder/new.txt'); + $target->method('getParent') + ->willReturn($this->getNodeMock(23, '/56/files/folder', false)); + + self::invokePrivate($filesHooks, 'fileRenaming', [$this->getNodeMock(42, '/56/files/folder/old.txt'), $target]); + + $this->assertSame([ + ['56', 'renamed_self'], + ['55', 'renamed_by'], + ['232', 'renamed_by'], + ], $received); + } + + public function testFileMoveNumericUserIdsWithMountCache(): void { + $filesHooks = $this->getFilesHooks([ + 'getSourcePathAndOwner', + 'getUserPathsFromPath', + 'addNotificationsForUser', + ], '56'); + + $sourceParent = $this->getNodeMock(10, '/56/files/a', false); + $targetParent = $this->getNodeMock(11, '/56/files/b', false); + $source = $this->getNodeMock(42, '/56/files/a/file.txt'); + $source->method('getParent') + ->willReturn($sourceParent); + $target = $this->getNodeMock(42, '/56/files/b/file.txt'); + $target->method('getParent') + ->willReturn($targetParent); + + $filesHooks->method('getSourcePathAndOwner') + ->willReturnCallback(fn (Node $node): array => match ($node) { + $sourceParent => ['/a', '56', 10], + $targetParent => ['/b', '56', 11], + $source => ['/a/file.txt', '56', 42], + $target => ['/b/file.txt', '56', 42], + }); + $filesHooks->method('getUserPathsFromPath') + ->willReturnCallback(fn (string $path): array => [ + 'users' => $path === '/a' ? ['56' => '/a', '55' => '/a'] : ['56' => '/b'], + 'remotes' => [], + ]); + + $this->config->method('getSystemValueBool') + ->with('activity_use_cached_mountpoints', false) + ->willReturn(true); + $this->userMountCache->expects($this->exactly(2)) + ->method('getMountsForFileId') + ->with(42) + ->willReturnOnConsecutiveCalls( + [$this->getCachedMountFileInfo('232')], + [$this->getCachedMountFileInfo('77')], + ); + $this->settings->method('filterUsersBySetting') + ->willReturn([]); + + $received = []; + $filesHooks->method('addNotificationsForUser') + ->willReturnCallback(function ($user, $subject) use (&$received): void { + $received[] = [$user, $subject]; + }); + + $filesHooks->fileMove($source, $target); + $filesHooks->fileMovePost($source, $target); + + $this->assertSame([ + ['55', 'deleted_by'], + ['232', 'deleted_by'], + ['77', 'created_by'], + ['56', 'moved_self'], + ], $received); + } + public function testFileMoveCollectsOldAccessList(): void { $filesHooks = $this->getFilesHooks([ 'getSourcePathAndOwner',