diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index 23ef44c25..f6f45e356 100755 --- a/lib/FilesHooks.php +++ b/lib/FilesHooks.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\Activity; use OC\Files\Filesystem; @@ -15,6 +16,7 @@ use OCP\Activity\IManager; use OCP\Constants; use OCP\Files\Config\IUserMountCache; +use OCP\Files\File; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; @@ -178,7 +180,7 @@ protected function addNotificationsForFileAction($filePath, $activityType, $subj $fileId, $path, true, $filteredEmailUsers[$user] ?? false, $filteredNotificationUsers[$user] ?? false, - $activityType + $activityType, ); } } @@ -373,7 +375,7 @@ protected function fileRenaming($oldPath, $newPath) { $fileId, $path . '/' . $fileName, true, $filteredEmailUsers[$user] ?? false, $filteredNotificationUsers[$user] ?? false, - Files::TYPE_FILE_CHANGED + Files::TYPE_FILE_CHANGED, ); } } @@ -475,7 +477,7 @@ protected function generateDeleteActivities($users, $pathMap, $fileId, $oldFileN $fileId, $path . '/' . $oldFileName, true, $filteredEmailUsers[$user] ?? false, $filteredNotificationUsers[$user] ?? false, - Files::TYPE_SHARE_DELETED + Files::TYPE_SHARE_DELETED, ); } $this->commitActivityTransaction($shouldFlush); @@ -511,7 +513,7 @@ protected function generateAddActivities($users, $pathMap, $fileId, $fileName) { $fileId, $path . '/' . $fileName, true, $filteredEmailUsers[$user] ?? false, $filteredNotificationUsers[$user] ?? false, - Files::TYPE_FILE_CHANGED + Files::TYPE_FILE_CHANGED, ); } $this->commitActivityTransaction($shouldFlush); @@ -554,7 +556,7 @@ protected function generateMoveActivities($users, $beforePathMap, $afterPathMap, $fileId, $afterPathMap[$user] . '/' . $fileName, true, $filteredEmailUsers[$user] ?? false, $filteredNotificationUsers[$user] ?? false, - Files::TYPE_FILE_CHANGED + Files::TYPE_FILE_CHANGED, ); } $this->commitActivityTransaction($shouldFlush); @@ -651,25 +653,22 @@ public function share($share) { case IShare::TYPE_USER: $this->shareWithUser( $share->getSharedWith(), - $share->getNodeId(), - $share->getNodeType(), - $share->getTarget() + $share->getNode(), + $share->getTarget(), ); break; case IShare::TYPE_GROUP: $this->shareWithGroup( $share->getSharedWith(), - $share->getNodeId(), - $share->getNodeType(), + $share->getNode(), $share->getTarget(), - (int)$share->getId() + (int)$share->getId(), ); break; case IShare::TYPE_LINK: $this->shareByLink( - $share->getNodeId(), - $share->getNodeType(), - $share->getSharedBy() + $share->getNode(), + $share->getSharedBy(), ); break; default: @@ -682,23 +681,22 @@ public function share($share) { * Sharing a file or folder with a user * * @param string $shareWith - * @param int $fileSource File ID that is being shared - * @param string $itemType File type that is being shared (file or folder) + * @param Node $fileSource File that is being shared * @param string $fileTarget File path */ - protected function shareWithUser($shareWith, $fileSource, $itemType, $fileTarget) { + protected function shareWithUser(string $shareWith, Node $fileSource, string $fileTarget) { // User performing the share - $this->shareNotificationForSharer('shared_user_self', $shareWith, $fileSource, $itemType); + $this->shareNotificationForSharer('shared_user_self', $shareWith, $fileSource); if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'reshared_user_by', $shareWith, $fileSource, $itemType); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'reshared_user_by', $shareWith, $fileSource); } // New shared user $this->addNotificationsForUser( - $shareWith, 'shared_with_by', [[$fileSource => $fileTarget], $this->currentUser->getUserIdentifier()], - (int)$fileSource, $fileTarget, $itemType === 'file', + $shareWith, 'shared_with_by', [[$fileSource->getId() => $fileTarget], $this->currentUser->getUserIdentifier()], + $fileSource->getId(), $fileTarget, $fileSource instanceof File, $this->userSettings->getUserSetting($shareWith, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($shareWith, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($shareWith, 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($shareWith, 'notification', Files_Sharing::TYPE_SHARED), ); } @@ -706,12 +704,11 @@ protected function shareWithUser($shareWith, $fileSource, $itemType, $fileTarget * Sharing a file or folder with a group * * @param string $shareWith - * @param int $fileSource File ID that is being shared - * @param string $itemType File type that is being shared (file or folder) + * @param Node $fileSource File that is being shared * @param string $fileTarget File path * @param int $shareId The Share ID of this share */ - protected function shareWithGroup($shareWith, $fileSource, $itemType, $fileTarget, $shareId) { + protected function shareWithGroup(string $shareWith, Node $fileSource, string $fileTarget, int $shareId) { // Members of the new group $group = $this->groupManager->get($shareWith); if (!($group instanceof IGroup)) { @@ -719,9 +716,9 @@ protected function shareWithGroup($shareWith, $fileSource, $itemType, $fileTarge } // User performing the share - $this->shareNotificationForSharer('shared_group_self', $shareWith, $fileSource, $itemType); + $this->shareNotificationForSharer('shared_group_self', $shareWith, $fileSource); if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'reshared_group_by', $shareWith, $fileSource, $itemType); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'reshared_group_by', $shareWith, $fileSource); } $offset = 0; @@ -736,26 +733,18 @@ protected function shareWithGroup($shareWith, $fileSource, $itemType, $fileTarge /** * Sharing a file or folder via link/public * - * @param int $fileSource File ID that is being shared - * @param string $itemType File type that is being shared (file or folder) - * @param string $linkOwner + * @param Node $fileSource File that is being shared + * @param string $sharedBy */ - protected function shareByLink($fileSource, $itemType, $linkOwner) { - $this->view->chroot('/' . $linkOwner . '/files'); - - try { - $path = $this->view->getPath($fileSource); - } catch (NotFoundException $e) { - return; - } - - $this->shareNotificationForOriginalOwners($linkOwner, 'reshared_link_by', '', $fileSource, $itemType); + protected function shareByLink(Node $fileSource, string $sharedBy) { + $relativePath = $this->getUserRelativePath($sharedBy, $fileSource->getPath()); + $this->shareNotificationForOriginalOwners($sharedBy, 'reshared_link_by', '', $fileSource); $this->addNotificationsForUser( - $linkOwner, 'shared_link_self', [[$fileSource => $path]], - (int)$fileSource, $path, $itemType === 'file', - $this->userSettings->getUserSetting($linkOwner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($linkOwner, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($linkOwner, 'notification', Files_Sharing::TYPE_SHARED) + $sharedBy, 'shared_link_self', [[$fileSource->getId() => $relativePath]], + $fileSource->getId(), $relativePath, $fileSource instanceof File, + $this->userSettings->getUserSetting($sharedBy, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($sharedBy, 'setting', 'batchtime') : false, + (bool)$this->userSettings->getUserSetting($sharedBy, 'notification', Files_Sharing::TYPE_SHARED), ); } @@ -816,11 +805,11 @@ protected function unshareFromUser(IShare $share) { } // User performing the share - $this->shareNotificationForSharer($actionSharer, $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForSharer($actionSharer, $share->getSharedWith(), $share->getNode()); // Owner if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), $actionOwner, $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), $actionOwner, $share->getSharedWith(), $share->getNode()); } // Recipient @@ -828,7 +817,7 @@ protected function unshareFromUser(IShare $share) { $share->getSharedWith(), $actionUser, [[$share->getNodeId() => $share->getTarget()], $this->currentUser->getUserIdentifier()], $share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file', $this->userSettings->getUserSetting($share->getSharedWith(), 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($share->getSharedWith(), 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($share->getSharedWith(), 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($share->getSharedWith(), 'notification', Files_Sharing::TYPE_SHARED), ); } @@ -840,11 +829,11 @@ protected function unshareFromUser(IShare $share) { */ protected function selfUnshareFromUser(IShare $share) { // User performing the share - $this->shareNotificationForSharer('self_unshared', $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForSharer('self_unshared', $share->getSharedWith(), $share->getNode()); // Owner if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'self_unshared_by', $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'self_unshared_by', $share->getSharedWith(), $share->getNode()); } } @@ -872,9 +861,9 @@ protected function unshareFromGroup(IShare $share) { } // User performing the share - $this->shareNotificationForSharer($actionSharer, $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForSharer($actionSharer, $share->getSharedWith(), $share->getNode()); if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), $actionOwner, $share->getSharedWith(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), $actionOwner, $share->getSharedWith(), $share->getNode()); } $offset = 0; @@ -896,11 +885,11 @@ protected function unshareFromGroup(IShare $share) { */ protected function unshareFromSelfGroup(IShare $share) { // User performing the unshare - $this->shareNotificationForSharer('self_unshared', $this->currentUser->getUID(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForSharer('self_unshared', $this->currentUser->getUID(), $share->getNode()); // Owner if ($this->currentUser->getUID() !== null) { - $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'self_unshared_by', $this->currentUser->getUID(), $share->getNodeId(), $share->getNodeType()); + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 'self_unshared_by', $this->currentUser->getUID(), $share->getNode()); } } @@ -926,7 +915,7 @@ protected function unshareLink(IShare $share) { $owner, $actionSharer, [[$share->getNodeId() => $share->getTarget()]], $share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file', $this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED), ); if ($share->getSharedBy() !== $share->getShareOwner()) { @@ -935,7 +924,7 @@ protected function unshareLink(IShare $share) { $owner, $actionOwner, [[$share->getNodeId() => $share->getTarget()], $share->getSharedBy()], $share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file', $this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED), ); } } @@ -943,8 +932,7 @@ protected function unshareLink(IShare $share) { /** * @param IUser[] $usersInGroup * @param string $actionUser - * @param int $fileSource File ID that is being shared - * @param string $itemType File type that is being shared (file or folder) + * @param Node $fileSource File that is being shared * @param string $fileTarget File path * @param int $shareId The Share ID of this share */ @@ -969,12 +957,20 @@ protected function addNotificationsForGroupUsers(array $usersInGroup, $actionUse $affectedUsers = $this->fixPathsForShareExceptions($affectedUsers, $shareId); $shouldFlush = $this->startActivityTransaction(); foreach ($affectedUsers as $user => $path) { - $this->addNotificationsForUser( - $user, $actionUser, [[$fileSource => $path], $this->currentUser->getUserIdentifier()], - $fileSource, $path, ($itemType === 'file'), - $filteredEmailUsersInGroup[$user] ?? false, - $filteredNotificationUsers[$user] ?? false - ); + $emailSetting = $filteredEmailUsersInGroup[$user] ?? false; + $notificationSetting = $filteredNotificationUsers[$user] ?? false; + if ($emailSetting || $notificationSetting) { + $this->addNotificationsForUser( + $user, + $actionUser, + [[$fileSource->getId() => $path], $this->currentUser->getUserIdentifier()], + $fileSource->getId(), + $path, + $fileSource instanceof File, + $emailSetting, + $notificationSetting, + ); + } } $this->commitActivityTransaction($shouldFlush); } @@ -1007,107 +1003,76 @@ protected function fixPathsForShareExceptions(array $affectedUsers, $shareId) { * * @param string $subject * @param string $shareWith - * @param int $fileSource - * @param string $itemType + * @param Node $fileSource */ - protected function shareNotificationForSharer($subject, $shareWith, $fileSource, $itemType) { + protected function shareNotificationForSharer(string $subject, string $shareWith, Node $fileSource) { $sharer = $this->currentUser->getUID(); if ($sharer === null) { return; } - - $this->view->chroot('/' . $sharer . '/files'); - - try { - $path = $this->view->getPath($fileSource); - } catch (NotFoundException $e) { - return; - } + $path = $this->getUserRelativePath($sharer, $fileSource->getPath()); $this->addNotificationsForUser( - $sharer, $subject, [[$fileSource => $path], $shareWith], - $fileSource, $path, ($itemType === 'file'), + $sharer, $subject, [[$fileSource->getId() => $path], $shareWith], + $fileSource->getId(), $path, $fileSource instanceof File, $this->userSettings->getUserSetting($sharer, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($sharer, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($sharer, 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($sharer, 'notification', Files_Sharing::TYPE_SHARED), ); } /** * Add notifications for the user that shares a file/folder - * - * @param string $owner - * @param string $subject - * @param string $shareWith - * @param int $fileSource - * @param string $itemType */ - protected function reshareNotificationForSharer($owner, $subject, $shareWith, $fileSource, $itemType) { - $this->view->chroot('/' . $owner . '/files'); - - try { - $path = $this->view->getPath($fileSource); - } catch (NotFoundException $e) { - return; - } - + protected function reshareNotificationForSharer(string $owner, string $subject, string $shareWith, int $sourceId, string $sourcePath, bool $isFile) { $this->addNotificationsForUser( - $owner, $subject, [[$fileSource => $path], $this->currentUser->getUserIdentifier(), $shareWith], - $fileSource, $path, ($itemType === 'file'), + $owner, $subject, [[$sourceId => $sourcePath], $this->currentUser->getUserIdentifier(), $shareWith], + $sourceId, $sourcePath, $isFile, $this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false, - (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED) + (bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED), ); } /** * Add notifications for the owners whose files have been reshared * - * @param string $currentOwner + * @param string $sharedBy * @param string $subject * @param string $shareWith - * @param int $fileSource - * @param string $itemType + * @param Node $fileSource */ - protected function shareNotificationForOriginalOwners($currentOwner, $subject, $shareWith, $fileSource, $itemType) { - // Get the full path of the current user - $this->view->chroot('/' . $currentOwner . '/files'); - - try { - $path = $this->view->getPath($fileSource); - } catch (NotFoundException $e) { - return; - } - - /** - * Get the original owner and his path - */ - $owner = $this->view->getOwner($path); - if ($owner !== $currentOwner) { - $this->reshareNotificationForSharer($owner, $subject, $shareWith, $fileSource, $itemType); - } - - /** - * Get the sharee who shared the item with the currentUser - */ - $this->view->chroot('/' . $currentOwner . '/files'); + protected function shareNotificationForOriginalOwners(string $sharedBy, string $subject, string $shareWith, Node $fileSource) { + $mount = $fileSource->getMountPoint(); + if ($mount instanceof SharedMount) { + $sourceShare = $mount->getShare(); + try { + $sourceNode = $sourceShare->getNode(); + } catch (NotFoundException) { + return; + } - try { - $mount = $this->view->getMount($path); - } catch (NotFoundException $ex) { - return; - } + if ($sourceShare->getShareOwner() !== $sharedBy) { + $this->reshareNotificationForSharer( + $sourceShare->getShareOwner(), + $subject, + $shareWith, + $sourceNode->getId(), + $this->getUserRelativePath($sourceShare->getShareOwner(), $sourceNode->getPath()), + $sourceNode instanceof File, + ); + } - $storage = $mount->getStorage(); - if (!$storage->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) { - return; - } - /** @var \OCA\Files_Sharing\SharedStorage $storage */ - $shareOwner = $storage->getSharedFrom(); - if ($shareOwner === '' || $shareOwner === null || $shareOwner === $owner || $shareOwner === $currentOwner) { - return; + if ($sourceShare->getSharedBy() && $sourceShare->getSharedBy() !== $sharedBy && $sourceShare->getShareOwner() !== $sourceShare->getSharedBy()) { + $this->reshareNotificationForSharer( + $sourceShare->getSharedBy(), + $subject, + $shareWith, + $sourceNode->getId(), + $sourceShare->getTarget(), + $sourceNode instanceof File, + ); + } } - - $this->reshareNotificationForSharer($shareOwner, $subject, $shareWith, $fileSource, $itemType); } /** @@ -1152,7 +1117,7 @@ protected function addNotificationsForUser($user, $subject, $subjectParams, $fil $e->getMessage(), [ 'app' => 'activity', - 'exception' => $e + 'exception' => $e, ], ); } @@ -1364,4 +1329,12 @@ private function isDeletedNode(string $owner, int $nodeId): bool { return true; } } + + private function getUserRelativePath(string $userId, string $path): string { + if (str_starts_with($path, '/' . $userId . '/files')) { + return substr($path, strlen('/' . $userId . '/files')); + } else { + throw new NotFoundException("$path is not a path for $userId"); + } + } } diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index b307921ae..18347e193 100755 --- a/tests/FilesHooksTest.php +++ b/tests/FilesHooksTest.php @@ -31,13 +31,14 @@ use OCA\Activity\Extension\Files; use OCA\Activity\Extension\Files_Sharing; use OCA\Activity\Tests\TestCase; -use OCA\Files_Sharing\SharedStorage; +use OCA\Files_Sharing\SharedMount; use OCP\Activity\IEvent; use OCP\Activity\IManager; use OCP\Files\Config\IUserMountCache; +use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\IRootFolder; -use OCP\Files\Mount\IMountPoint; -use OCP\Files\NotFoundException; +use OCP\Files\Node; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -176,7 +177,7 @@ protected function getUserMock(string $uid): IUser { return $user; } - public function dataFileCreate(): array { + public static function dataFileCreate(): array { return [ ['user', 'created_self', 'created_by', Files::TYPE_SHARE_CREATED], ['', '', 'created_public', Files_Sharing::TYPE_PUBLIC_UPLOAD], @@ -265,16 +266,16 @@ public function testAddNotificationsForFileActionPartFile(): void { $this->invokePrivate($filesHooks, 'addNotificationsForFileAction', ['/test.txt.part', '', '', '']); } - public function dataAddNotificationsForFileAction(): array { + public static function dataAddNotificationsForFileAction(): array { return [ [ [ 'email' => ['user' => 42], 'notification' => ['user' => true], ], - 'mountcache_used' => false, + 'mountCacheUsed' => false, 'isFavorite' => true, - [ + 'addNotifications' => [ 'user' => [ 'subject' => 'restored_self', 'subject_params' => [[1337 => '/user/files/path']], @@ -296,9 +297,9 @@ public function dataAddNotificationsForFileAction(): array { 'email' => [], 'notification' => ['user1' => false], ], - 'mountcache_used' => false, + 'mountCacheUsed' => false, 'isFavorite' => true, - [ + 'addNotifications' => [ 'user' => [ 'subject' => 'restored_self', 'subject_params' => [[1337 => '/user/files/path']], @@ -320,9 +321,9 @@ public function dataAddNotificationsForFileAction(): array { 'email' => ['user' => 42], 'notification' => ['user' => false], ], - 'mountcache_used' => true, + 'mountCacheUsed' => true, 'isFavorite' => true, - [ + 'addNotifications' => [ 'user' => [ 'subject' => 'restored_self', 'subject_params' => [[1337 => '/path']], @@ -344,9 +345,9 @@ public function dataAddNotificationsForFileAction(): array { 'email' => [], 'notification' => ['user1' => true], ], - 'mountcache_used' => true, + 'mountCacheUsed' => true, 'isFavorite' => true, - [ + 'addNotifications' => [ 'user' => [ 'subject' => 'restored_self', 'subject_params' => [[1337 => '/path']], @@ -468,13 +469,29 @@ public function testAddNotificationsForFileAction(array $filterUsers, bool $moun Files::TYPE_SHARE_RESTORED, ]; } + $receivedActivities = []; $filesHooks->expects($this->any()) ->method('addNotificationsForUser') - ->withConsecutive( - ...$addCalls - ); + ->willReturnCallback(function (...$params) use (&$receivedActivities) { + $receivedActivities[] = $params; + }); self::invokePrivate($filesHooks, 'addNotificationsForFileAction', ['path', Files::TYPE_SHARE_RESTORED, 'restored_self', 'restored_by']); + + $this->assertEquals($addCalls, array_slice($receivedActivities, 0, count($addCalls))); + } + + private function getNodeMock(int $fileId = 1337, string $path = 'path', bool $isFile = true): Node&MockObject { + if ($isFile) { + $node = $this->createMock(File::class); + } else { + $node = $this->createMock(Folder::class); + } + $node->method('getId') + ->willReturn($fileId); + $node->method('getPath') + ->willReturn($path); + return $node; } public function testHookShareWithUser(): void { @@ -482,9 +499,11 @@ public function testHookShareWithUser(): void { 'shareWithUser', ]); + $node = $this->getNodeMock(); + $filesHooks->expects($this->once()) ->method('shareWithUser') - ->with('u1', 1337, 'file', 'path'); + ->with('u1', $node, 'path'); /** @var ShareIManager */ $manager = \OC::$server->get(ShareIManager::class); @@ -492,8 +511,7 @@ public function testHookShareWithUser(): void { $share->setSharedWith('u1'); $share->setShareType(IShare::TYPE_USER); $share->setTarget('path'); - $share->setNodeType('file'); - $share->setNodeId(1337); + $share->setNode($node); $filesHooks->share($share); } @@ -503,9 +521,11 @@ public function testHookShareWithGroup(): void { 'shareWithGroup', ]); + $node = $this->getNodeMock(); + $filesHooks->expects($this->once()) ->method('shareWithGroup') - ->with('g1', 1337, 'file', 'path', 42); + ->with('g1', $node, 'path', 42); /** @var ShareIManager */ $manager = \OC::$server->get(ShareIManager::class); @@ -514,8 +534,7 @@ public function testHookShareWithGroup(): void { $share->setSharedWith('g1'); $share->setShareType(IShare::TYPE_GROUP); $share->setTarget('path'); - $share->setNodeType('file'); - $share->setNodeId(1337); + $share->setNode($node); $filesHooks->share($share); } @@ -525,25 +544,27 @@ public function testShareViaPublicLink(): void { 'shareByLink', ]); + $node = $this->getNodeMock(1337, 'thepath'); + $filesHooks->expects($this->once()) ->method('shareByLink') - ->with(1337, 'file', 'admin'); + ->with($node, 'admin'); /** @var ShareIManager */ $manager = \OC::$server->get(ShareIManager::class); $share = $manager->newShare(); $share->setShareType(IShare::TYPE_LINK); $share->setNodeType('file'); - $share->setNodeId(1337); + $share->setNode($node); $share->setSharedBy('admin'); $filesHooks->share($share); } - public function dataShareWithUser(): array { + public static function dataShareWithUser(): array { return [ - ['file', '/path.txt', true], - ['folder', '/path.txt', false], + ['file', '/path.txt'], + ['folder', '/path.txt'], ]; } @@ -554,7 +575,8 @@ public function dataShareWithUser(): array { * @param string $fileTarget * @param bool $isFile */ - public function testShareWithUser(string $itemType, string $fileTarget, bool $isFile): void { + public function testShareWithUser(string $itemType, string $fileTarget): void { + $isFile = $itemType === 'file'; $filesHooks = $this->getFilesHooks([ 'shareNotificationForSharer', 'addNotificationsForUser', @@ -571,9 +593,10 @@ public function testShareWithUser(string $itemType, string $fileTarget, bool $is ] ); + $node = $this->getNodeMock(1337, 'path.txt', $isFile); $filesHooks->expects($this->once()) ->method('shareNotificationForSharer') - ->with('shared_user_self', 'recipient', 1337, $itemType); + ->with('shared_user_self', 'recipient', $node); $filesHooks->expects($this->once()) ->method('addNotificationsForUser') ->with( @@ -588,7 +611,7 @@ public function testShareWithUser(string $itemType, string $fileTarget, bool $is ); self::invokePrivate($filesHooks, 'shareWithUser', [ - 'recipient', 1337, $itemType, $fileTarget, true + 'recipient', $node, $fileTarget, true ]); } @@ -606,11 +629,11 @@ public function testShareWithGroupNonExisting(): void { ->method('shareNotificationForSharer'); self::invokePrivate($filesHooks, 'shareWithGroup', [ - 'no-group', 0, '', '', 0, true + 'no-group', $this->getNodeMock(), '', 0, true ]); } - public function dataShareWithGroup(): array { + public static function dataShareWithGroup(): array { return [ [ [ @@ -619,13 +642,13 @@ public function dataShareWithGroup(): array { ], [ [ - [$this->getUserMock('user1')], + ['user1'], [], ], 2, 1, [], [], [] ], [ [ - [$this->getUserMock('user1')], + ['user1'], [], ], 2, 1, @@ -646,7 +669,7 @@ public function dataShareWithGroup(): array { ], [ [ - [$this->getUserMock('user1')], + ['user1'], [], ], 2, 1, @@ -659,7 +682,7 @@ public function dataShareWithGroup(): array { ], [ [ - [$this->getUserMock('user')], + ['user'], [], ], 0, 0, @@ -680,6 +703,12 @@ public function dataShareWithGroup(): array { * @param array $addNotifications */ public function testShareWithGroup(array $usersInGroup, int $settingCalls, int $fixCalls, array $settingUsers, array $settingsReturn, array $addNotifications): void { + foreach ($usersInGroup as &$users) { + $users = array_map($this->getUserMock(...), $users); + } + + $node = $this->getNodeMock(42); + $filesHooks = $this->getFilesHooks([ 'shareNotificationForSharer', 'addNotificationsForUser', @@ -705,14 +734,14 @@ public function testShareWithGroup(array $usersInGroup, int $settingCalls, int $ $filesHooks->expects($this->once()) ->method('shareNotificationForSharer') - ->with('shared_group_self', 'group1', 42, 'file'); + ->with('shared_group_self', 'group1', $node); $filesHooks->expects($this->exactly($fixCalls)) ->method('fixPathsForShareExceptions') ->with($this->anything(), 1337) ->willReturnArgument(0); $filesHooks->expects($this->once()) ->method('shareNotificationForOriginalOwners') - ->with('user', 'reshared_group_by', 'group1', 42, 'file') + ->with('user', 'reshared_group_by', 'group1', $node) ->willReturnArgument(0); $addCalls = []; @@ -729,47 +758,25 @@ public function testShareWithGroup(array $usersInGroup, int $settingCalls, int $ Files_Sharing::TYPE_SHARED, ]; } + $receivedActivities = []; $filesHooks->expects($this->any()) ->method('addNotificationsForUser') - ->withConsecutive( - ...$addCalls - ); + ->willReturnCallback(function (...$params) use (&$receivedActivities) { + $receivedActivities[] = $params; + }); self::invokePrivate($filesHooks, 'shareWithGroup', [ - 'group1', 42, 'file', '/file', 1337, true + 'group1', $node, '/file', 1337 ]); + $this->assertEquals($addCalls, $receivedActivities); } - public function dataReshareNotificationForSharer(): array { - return [ - [null], - ['/path'], - ]; - } - - /** - * @dataProvider dataReshareNotificationForSharer - * @param string $path - */ - public function testReshareNotificationForSharer(?string $path): void { + public function testReshareNotificationForSharer(): void { $filesHooks = $this->getFilesHooks([ 'addNotificationsForUser', ]); - $this->view->expects($this->once()) - ->method('chroot') - ->with('/owner/files'); - if ($path === null) { - $this->view->expects($this->once()) - ->method('getPath') - ->willThrowException(new NotFoundException()); - } else { - $this->view->expects($this->once()) - ->method('getPath') - ->willReturn($path); - } - - $this->settings->expects(($path !== null) ? $this->exactly(3) : $this->never()) + $this->settings->expects($this->exactly(3)) ->method('getUserSetting') ->willReturnMap([ ['owner', 'notification', Files_Sharing::TYPE_SHARED, true], @@ -777,7 +784,7 @@ public function testReshareNotificationForSharer(?string $path): void { ['owner', 'setting', 'batchtime', 21], ]); - $filesHooks->expects(($path !== null) ? $this->once() : $this->never()) + $filesHooks->expects($this->once()) ->method('addNotificationsForUser') ->with( 'owner', @@ -790,37 +797,18 @@ public function testReshareNotificationForSharer(?string $path): void { 21 ); - self::invokePrivate($filesHooks, 'reshareNotificationForSharer', ['owner', 'reshared_link_by', '', 42, 'file']); - } - - public function dataShareNotificationForSharer(): array { - return [ - [null], - ['/path'], - ]; + self::invokePrivate($filesHooks, 'reshareNotificationForSharer', ['owner', 'reshared_link_by', '', 42, '/path', true]); } - /** - * @dataProvider dataShareNotificationForSharer - * @param string $path - */ - public function testShare(?string $path): void { + public function testShare(): void { $filesHooks = $this->getFilesHooks([ 'addNotificationsForUser', 'shareNotificationForOriginalOwners', ]); - if ($path === null) { - $this->view->expects($this->once()) - ->method('getPath') - ->willThrowException(new NotFoundException()); - } else { - $this->view->expects($this->once()) - ->method('getPath') - ->willReturn($path); - } + $node = $this->getNodeMock(42, '/user/files/path'); - $this->settings->expects(($path !== null) ? $this->exactly(3) : $this->never()) + $this->settings->expects($this->exactly(3)) ->method('getUserSetting') ->willReturnMap([ ['user', 'notification', Files_Sharing::TYPE_SHARED, true], @@ -828,7 +816,7 @@ public function testShare(?string $path): void { ['user', 'setting', 'batchtime', 21], ]); - $filesHooks->expects(($path !== null) ? $this->once() : $this->never()) + $filesHooks->expects($this->once()) ->method('addNotificationsForUser') ->with( 'user', @@ -840,32 +828,16 @@ public function testShare(?string $path): void { true, 21 ); - $filesHooks->expects(($path !== null) ? $this->once() : $this->never()) + $filesHooks->expects($this->once()) ->method('shareNotificationForOriginalOwners') - ->with('user', 'reshared_link_by', '', 42, 'file'); - - self::invokePrivate($filesHooks, 'shareByLink', [42, 'file', 'user']); - } - - public function testShareNotificationForOriginalOwnersNoPath(): void { - $filesHooks = $this->getFilesHooks([ - 'reshareNotificationForSharer', - ]); - - $this->view->expects($this->once()) - ->method('getPath') - ->with(42) - ->willThrowException(new NotFoundException()); + ->with('user', 'reshared_link_by', '', $node); - $filesHooks->expects($this->never()) - ->method('reshareNotificationForSharer'); - - self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['', '', '', 42, '', '']); + self::invokePrivate($filesHooks, 'shareByLink', [$node, 'user']); } - public function dataShareNotificationForOriginalOwners(): array { + public static function dataShareNotificationForOriginalOwners(): array { return [ - [false, false, 'owner', '', 1], + [false, false, 'owner', '', 0], [true, false, 'owner', '', 1], [true, true, 'owner', '', 1], [true, true, 'owner', 'owner', 1], @@ -885,77 +857,47 @@ public function dataShareNotificationForOriginalOwners(): array { * @param string $shareeUser * @param int $numReshareNotification */ - public function testShareNotificationForOriginalOwners(bool $validMountPoint, bool $validSharedStorage, string $pathOwner, ?string $shareeUser, int $numReshareNotification): void { + public function testShareNotificationForOriginalOwners(bool $validMountPoint, bool $validSharedStorage, string $pathOwner, string $shareeUser, int $numReshareNotification): void { $filesHooks = $this->getFilesHooks([ 'reshareNotificationForSharer', ]); - $this->view->expects($this->atLeastOnce()) - ->method('getPath') - ->willReturn('/path'); - - $this->view->expects($this->once()) - ->method('getOwner') - ->with('/path') + $node = $this->getNodeMock(42, '/path'); + $mount = $this->createMock(SharedMount::class); + $node->method('getMountPoint') + ->willReturn($mount); + $sourceShare = $this->createMock(IShare::class); + $sourceShare->method('getShareOwner') ->willReturn($pathOwner); + $sourceShare->method('getSharedBy') + ->willReturn($shareeUser); + $sourceShare->method('getTarget') + ->willReturn('/source-path'); + $mount->method('getShare') + ->willReturn($sourceShare); $filesHooks->expects($this->exactly($numReshareNotification)) ->method('reshareNotificationForSharer') - ->with($this->anything(), 'subject', 'with', 42, 'type'); + ->with($this->anything(), 'subject', 'with', 42, '/source-path', 'file'); if ($validMountPoint) { - $storage = $this->getMockBuilder(SharedStorage::class) - ->disableOriginalConstructor() - ->onlyMethods([ - 'instanceOfStorage', - 'getSharedFrom', - ]) - ->getMock(); - $storage->expects($this->once()) - ->method('instanceOfStorage') - ->with(SharedStorage::class) - ->willReturn($validSharedStorage); - $storage->expects($validSharedStorage ? $this->once() : $this->never()) - ->method('getSharedFrom') - ->willReturn($shareeUser); - - $mount = $this->createMock(IMountPoint::class); - $mount->expects($this->once()) - ->method('getStorage') - ->willReturn($storage); - - $this->view->expects($this->once()) - ->method('getMount') - ->with('/path') - ->willReturn($mount); + $sourceNode = $this->getNodeMock(42, "/$pathOwner/files/source-path"); + $sourceShare->method('getNode') + ->willReturn($sourceNode); } else { - $this->view->expects($this->once()) - ->method('getMount') - ->with('/path') + $sourceShare->method('getNode') ->willThrowException(new \OCP\Files\NotFoundException()); } - self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['current', 'subject', 'with', 42, 'type']); + self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['current', 'subject', 'with', $node]); } - /** - * @dataProvider dataShareNotificationForSharer - * @param string $path - */ - public function testShareNotificationForSharer(?string $path): void { + public function testShareNotificationForSharer(): void { $filesHooks = $this->getFilesHooks(['addNotificationsForUser']); - if ($path === null) { - $this->view->expects($this->once()) - ->method('getPath') - ->willThrowException(new NotFoundException()); - } else { - $this->view->expects($this->once()) - ->method('getPath') - ->willReturn($path); - } + $node = $this->getNodeMock(42, '/user/files/path'); - $this->settings->expects(($path !== null) ? $this->exactly(3) : $this->never()) + $this->settings->expects($this->exactly(3)) ->method('getUserSetting') ->willReturnMap([ ['user', 'notification', Files_Sharing::TYPE_SHARED, true], @@ -963,7 +905,7 @@ public function testShareNotificationForSharer(?string $path): void { ['user', 'setting', 'batchtime', 21], ]); - $filesHooks->expects(($path !== null) ? $this->once() : $this->never()) + $filesHooks->expects($this->once()) ->method('addNotificationsForUser') ->with( 'user', @@ -976,10 +918,10 @@ public function testShareNotificationForSharer(?string $path): void { 21 ); - self::invokePrivate($filesHooks, 'shareNotificationForSharer', ['subject', 'target', 42, 'file']); + self::invokePrivate($filesHooks, 'shareNotificationForSharer', ['subject', 'target', $node]); } - public function dataAddNotificationsForUser(): array { + public static function dataAddNotificationsForUser(): array { return [ ['user', 'subject', ['parameter'], 42, 'path/subpath', 'path', true, true, false, Files_Sharing::TYPE_SHARED, 'files_sharing', false], ['user', 'subject', ['parameter'], 42, 'path/subpath', 'path', true, true, false, Files_Sharing::TYPE_SHARED, 'files_sharing', false], diff --git a/tests/stubs/oca_files_sharing.php b/tests/stubs/oca_files_sharing.php new file mode 100644 index 000000000..02b90bb5e --- /dev/null +++ b/tests/stubs/oca_files_sharing.php @@ -0,0 +1,7 @@ +