owncloud /
activity
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like FilesHooks often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use FilesHooks, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 43 | class FilesHooks { |
||
| 44 | const USER_BATCH_SIZE = 50; |
||
| 45 | |||
| 46 | /** @var \OCP\Activity\IManager */ |
||
| 47 | protected $manager; |
||
| 48 | |||
| 49 | /** @var \OCA\Activity\Data */ |
||
| 50 | protected $activityData; |
||
| 51 | |||
| 52 | /** @var \OCA\Activity\UserSettings */ |
||
| 53 | protected $userSettings; |
||
| 54 | |||
| 55 | /** @var \OCP\IGroupManager */ |
||
| 56 | protected $groupManager; |
||
| 57 | |||
| 58 | /** @var \OCP\IDBConnection */ |
||
| 59 | protected $connection; |
||
| 60 | |||
| 61 | /** @var \OC\Files\View */ |
||
| 62 | protected $view; |
||
| 63 | |||
| 64 | /** @var string|false */ |
||
| 65 | protected $currentUser; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * Constructor |
||
| 69 | * |
||
| 70 | * @param IManager $manager |
||
| 71 | * @param Data $activityData |
||
| 72 | * @param UserSettings $userSettings |
||
| 73 | * @param IGroupManager $groupManager |
||
| 74 | * @param View $view |
||
| 75 | * @param IDBConnection $connection |
||
| 76 | * @param string|false $currentUser |
||
| 77 | */ |
||
| 78 | 47 | public function __construct(IManager $manager, Data $activityData, UserSettings $userSettings, IGroupManager $groupManager, View $view, IDBConnection $connection, $currentUser) { |
|
| 87 | |||
| 88 | /** |
||
| 89 | * @return string|false Current UserID if logged in, false otherwise |
||
| 90 | */ |
||
| 91 | 2 | protected function getCurrentUser() { |
|
| 94 | |||
| 95 | /** |
||
| 96 | * Store the create hook events |
||
| 97 | * @param string $path Path of the file that has been created |
||
| 98 | */ |
||
| 99 | 2 | public function fileCreate($path) { |
|
| 100 | 2 | if ($this->getCurrentUser() !== false) { |
|
| 101 | 1 | $this->addNotificationsForFileAction($path, Files::TYPE_SHARE_CREATED, 'created_self', 'created_by'); |
|
| 102 | 1 | } else { |
|
| 103 | 1 | $this->addNotificationsForFileAction($path, Files::TYPE_SHARE_CREATED, '', 'created_public'); |
|
| 104 | } |
||
| 105 | 2 | } |
|
| 106 | |||
| 107 | /** |
||
| 108 | * Store the update hook events |
||
| 109 | * @param string $path Path of the file that has been modified |
||
| 110 | */ |
||
| 111 | 1 | public function fileUpdate($path) { |
|
| 114 | |||
| 115 | /** |
||
| 116 | * Store the delete hook events |
||
| 117 | * @param string $path Path of the file that has been deleted |
||
| 118 | */ |
||
| 119 | 1 | public function fileDelete($path) { |
|
| 122 | |||
| 123 | /** |
||
| 124 | * Store the restore hook events |
||
| 125 | * @param string $path Path of the file that has been restored |
||
| 126 | */ |
||
| 127 | 1 | public function fileRestore($path) { |
|
| 130 | |||
| 131 | /** |
||
| 132 | * Creates the entries for file actions on $file_path |
||
| 133 | * |
||
| 134 | * @param string $filePath The file that is being changed |
||
| 135 | * @param int $activityType The activity type |
||
| 136 | * @param string $subject The subject for the actor |
||
| 137 | * @param string $subjectBy The subject for other users (with "by $actor") |
||
| 138 | */ |
||
| 139 | 3 | protected function addNotificationsForFileAction($filePath, $activityType, $subject, $subjectBy) { |
|
| 140 | // Do not add activities for .part-files |
||
| 141 | 3 | if (substr($filePath, -5) === '.part') { |
|
| 142 | 1 | return; |
|
| 143 | } |
||
| 144 | |||
| 145 | 2 | list($filePath, $uidOwner, $fileId) = $this->getSourcePathAndOwner($filePath); |
|
| 146 | 2 | $affectedUsers = $this->getUserPathsFromPath($filePath, $uidOwner); |
|
| 147 | 2 | $filteredStreamUsers = $this->userSettings->filterUsersBySetting(array_keys($affectedUsers), 'stream', $activityType); |
|
| 148 | 2 | $filteredEmailUsers = $this->userSettings->filterUsersBySetting(array_keys($affectedUsers), 'email', $activityType); |
|
| 149 | |||
| 150 | 2 | foreach ($affectedUsers as $user => $path) { |
|
| 151 | 2 | if (empty($filteredStreamUsers[$user]) && empty($filteredEmailUsers[$user])) { |
|
| 152 | 2 | continue; |
|
| 153 | } |
||
| 154 | |||
| 155 | 2 | if ($user === $this->currentUser) { |
|
| 156 | 1 | $userSubject = $subject; |
|
| 157 | 1 | $userParams = [[$fileId => $path]]; |
|
| 158 | 1 | } else { |
|
| 159 | 1 | $userSubject = $subjectBy; |
|
| 160 | 1 | $userParams = [[$fileId => $path], $this->currentUser]; |
|
| 161 | } |
||
| 162 | |||
| 163 | 2 | $this->addNotificationsForUser( |
|
| 164 | 2 | $user, $userSubject, $userParams, |
|
| 165 | 2 | $fileId, $path, true, |
|
| 166 | 2 | !empty($filteredStreamUsers[$user]), |
|
| 167 | 2 | !empty($filteredEmailUsers[$user]) ? $filteredEmailUsers[$user] : 0, |
|
| 168 | $activityType |
||
| 169 | 2 | ); |
|
| 170 | 2 | } |
|
| 171 | 2 | } |
|
| 172 | |||
| 173 | /** |
||
| 174 | * Returns a "username => path" map for all affected users |
||
| 175 | * |
||
| 176 | * @param string $path |
||
| 177 | * @param string $uidOwner |
||
| 178 | * @return array |
||
| 179 | */ |
||
| 180 | protected function getUserPathsFromPath($path, $uidOwner) { |
||
| 183 | |||
| 184 | /** |
||
| 185 | * Return the source |
||
| 186 | * |
||
| 187 | * @param string $path |
||
| 188 | * @return array |
||
| 189 | */ |
||
| 190 | protected function getSourcePathAndOwner($path) { |
||
| 206 | |||
| 207 | /** |
||
| 208 | * Manage sharing events |
||
| 209 | * @param array $params The hook params |
||
| 210 | */ |
||
| 211 | 3 | View Code Duplication | public function share($params) { |
| 222 | |||
| 223 | /** |
||
| 224 | * Manage sharing events |
||
| 225 | * @param array $params The hook params |
||
| 226 | */ |
||
| 227 | View Code Duplication | public function unShare($params) { |
|
| 238 | |||
| 239 | /** |
||
| 240 | * Sharing a file or folder with a user |
||
| 241 | * |
||
| 242 | * @param string $shareWith |
||
| 243 | * @param int $fileSource File ID that is being shared |
||
| 244 | * @param string $itemType File type that is being shared (file or folder) |
||
| 245 | * @param string $fileTarget File path |
||
| 246 | * @param bool $isSharing True if sharing, false if unsharing |
||
| 247 | */ |
||
| 248 | 2 | protected function shareFileOrFolderWithUser($shareWith, $fileSource, $itemType, $fileTarget, $isSharing) { |
|
| 271 | |||
| 272 | /** |
||
| 273 | * Sharing a file or folder with a group |
||
| 274 | * |
||
| 275 | * @param string $shareWith |
||
| 276 | * @param int $fileSource File ID that is being shared |
||
| 277 | * @param string $itemType File type that is being shared (file or folder) |
||
| 278 | * @param string $fileTarget File path |
||
| 279 | * @param int $shareId The Share ID of this share |
||
| 280 | * @param bool $isSharing True if sharing, false if unsharing |
||
| 281 | */ |
||
| 282 | 6 | protected function shareFileOrFolderWithGroup($shareWith, $fileSource, $itemType, $fileTarget, $shareId, $isSharing) { |
|
| 311 | |||
| 312 | /** |
||
| 313 | * @param IUser[] $usersInGroup |
||
| 314 | * @param string $actionUser |
||
| 315 | * @param int $fileSource File ID that is being shared |
||
| 316 | * @param string $itemType File type that is being shared (file or folder) |
||
| 317 | * @param string $fileTarget File path |
||
| 318 | * @param int $shareId The Share ID of this share |
||
| 319 | */ |
||
| 320 | 4 | protected function addNotificationsForGroupUsers(array $usersInGroup, $actionUser, $fileSource, $itemType, $fileTarget, $shareId) { |
|
| 352 | |||
| 353 | /** |
||
| 354 | * Check when there was a naming conflict and the target is different |
||
| 355 | * for some of the users |
||
| 356 | * |
||
| 357 | * @param array $affectedUsers |
||
| 358 | * @param int $shareId |
||
| 359 | * @return mixed |
||
| 360 | */ |
||
| 361 | protected function fixPathsForShareExceptions(array $affectedUsers, $shareId) { |
||
| 375 | |||
| 376 | /** |
||
| 377 | * Sharing a file or folder via link/public |
||
| 378 | * |
||
| 379 | * @param int $fileSource File ID that is being shared |
||
| 380 | * @param string $itemType File type that is being shared (file or folder) |
||
| 381 | * @param string $linkOwner |
||
| 382 | * @param bool $isSharing True if sharing, false if unsharing |
||
| 383 | */ |
||
| 384 | 2 | protected function shareFileOrFolderByLink($fileSource, $itemType, $linkOwner, $isSharing) { |
|
| 416 | |||
| 417 | /** |
||
| 418 | * Add notifications for the user that shares a file/folder |
||
| 419 | * |
||
| 420 | * @param string $subject |
||
| 421 | * @param string $shareWith |
||
| 422 | * @param int $fileSource |
||
| 423 | * @param string $itemType |
||
| 424 | */ |
||
| 425 | 2 | protected function shareNotificationForSharer($subject, $shareWith, $fileSource, $itemType) { |
|
| 441 | |||
| 442 | /** |
||
| 443 | * Add notifications for the user that shares a file/folder |
||
| 444 | * |
||
| 445 | * @param string $owner |
||
| 446 | * @param string $subject |
||
| 447 | * @param string $shareWith |
||
| 448 | * @param int $fileSource |
||
| 449 | * @param string $itemType |
||
| 450 | */ |
||
| 451 | 2 | protected function reshareNotificationForSharer($owner, $subject, $shareWith, $fileSource, $itemType) { |
|
| 467 | |||
| 468 | /** |
||
| 469 | * Add notifications for the owners whose files have been reshared |
||
| 470 | * |
||
| 471 | * @param string $currentOwner |
||
| 472 | * @param string $subject |
||
| 473 | * @param string $shareWith |
||
| 474 | * @param int $fileSource |
||
| 475 | * @param string $itemType |
||
| 476 | */ |
||
| 477 | 10 | protected function shareNotificationForOriginalOwners($currentOwner, $subject, $shareWith, $fileSource, $itemType) { |
|
| 517 | |||
| 518 | /** |
||
| 519 | * Adds the activity and email for a user when the settings require it |
||
| 520 | * |
||
| 521 | * @param string $user |
||
| 522 | * @param string $subject |
||
| 523 | * @param array $subjectParams |
||
| 524 | * @param int $fileId |
||
| 525 | * @param string $path |
||
| 526 | * @param bool $isFile If the item is a file, we link to the parent directory |
||
| 527 | * @param bool $streamSetting |
||
| 528 | * @param int $emailSetting |
||
| 529 | * @param string $type |
||
| 530 | */ |
||
| 531 | 9 | protected function addNotificationsForUser($user, $subject, $subjectParams, $fileId, $path, $isFile, $streamSetting, $emailSetting, $type = Files_Sharing::TYPE_SHARED) { |
|
| 565 | } |
||
| 566 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.