nextcloud /
server
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 ApiController 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 ApiController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 55 | class ApiController extends Controller { |
||
| 56 | /** @var TagService */ |
||
| 57 | private $tagService; |
||
| 58 | /** @var IManager * */ |
||
| 59 | private $shareManager; |
||
| 60 | /** @var IPreview */ |
||
| 61 | private $previewManager; |
||
| 62 | /** IUserSession */ |
||
| 63 | private $userSession; |
||
| 64 | /** IConfig */ |
||
| 65 | private $config; |
||
| 66 | /** @var Folder */ |
||
| 67 | private $userFolder; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * @param string $appName |
||
| 71 | * @param IRequest $request |
||
| 72 | * @param IUserSession $userSession |
||
| 73 | * @param TagService $tagService |
||
| 74 | * @param IPreview $previewManager |
||
| 75 | * @param IManager $shareManager |
||
| 76 | * @param IConfig $config |
||
| 77 | * @param Folder $userFolder |
||
| 78 | */ |
||
| 79 | public function __construct($appName, |
||
| 95 | |||
| 96 | /** |
||
| 97 | * Gets a thumbnail of the specified file |
||
| 98 | * |
||
| 99 | * @since API version 1.0 |
||
| 100 | * |
||
| 101 | * @NoAdminRequired |
||
| 102 | * @NoCSRFRequired |
||
| 103 | * @StrictCookieRequired |
||
| 104 | * |
||
| 105 | * @param int $x |
||
| 106 | * @param int $y |
||
| 107 | * @param string $file URL-encoded filename |
||
| 108 | * @return DataResponse|FileDisplayResponse |
||
| 109 | */ |
||
| 110 | public function getThumbnail($x, $y, $file) { |
||
| 131 | |||
| 132 | /** |
||
| 133 | * Updates the info of the specified file path |
||
| 134 | * The passed tags are absolute, which means they will |
||
| 135 | * replace the actual tag selection. |
||
| 136 | * |
||
| 137 | * @NoAdminRequired |
||
| 138 | * |
||
| 139 | * @param string $path path |
||
| 140 | * @param array|string $tags array of tags |
||
| 141 | * @return DataResponse |
||
| 142 | */ |
||
| 143 | public function updateFileTags($path, $tags = null) { |
||
| 166 | |||
| 167 | /** |
||
| 168 | * @param \OCP\Files\Node[] $nodes |
||
| 169 | * @return array |
||
| 170 | */ |
||
| 171 | private function formatNodes(array $nodes) { |
||
| 188 | |||
| 189 | /** |
||
| 190 | * Returns a list of recently modifed files. |
||
| 191 | * |
||
| 192 | * @NoAdminRequired |
||
| 193 | * |
||
| 194 | * @return DataResponse |
||
| 195 | */ |
||
| 196 | public function getRecentFiles() { |
||
| 201 | |||
| 202 | /** |
||
| 203 | * Returns a list of favorites modifed folder. |
||
| 204 | * |
||
| 205 | * @NoAdminRequired |
||
| 206 | * |
||
| 207 | * @return DataResponse |
||
| 208 | */ |
||
| 209 | public function getFavoritesFolder() { |
||
| 210 | $nodes = $this->userFolder->searchByTag('_$!<Favorite>!$_', $this->userSession->getUser()->getUID()); |
||
| 211 | |||
| 212 | $favorites = []; |
||
| 213 | $i = 0; |
||
| 214 | foreach ($nodes as &$node) { |
||
| 215 | |||
| 216 | $favorites[$i]['id'] = $node->getId(); |
||
| 217 | $favorites[$i]['name'] = $node->getName(); |
||
| 218 | $favorites[$i]['path'] = $node->getInternalPath(); |
||
| 219 | $favorites[$i]['mtime'] = $node->getMTime(); |
||
| 220 | $i++; |
||
| 221 | } |
||
| 222 | |||
| 223 | return new DataResponse(['favoriteFolders' => $favorites]); |
||
| 224 | } |
||
| 225 | |||
| 226 | /** |
||
| 227 | * Return a list of share types for outgoing shares |
||
| 228 | * |
||
| 229 | * @param Node $node file node |
||
| 230 | * |
||
| 231 | * @return int[] array of share types |
||
| 232 | */ |
||
| 233 | View Code Duplication | private function getShareTypes(Node $node) { |
|
| 234 | $userId = $this->userSession->getUser()->getUID(); |
||
| 235 | $shareTypes = []; |
||
| 236 | $requestedShareTypes = [ |
||
| 237 | \OCP\Share::SHARE_TYPE_USER, |
||
| 238 | \OCP\Share::SHARE_TYPE_GROUP, |
||
| 239 | \OCP\Share::SHARE_TYPE_LINK, |
||
| 240 | \OCP\Share::SHARE_TYPE_REMOTE, |
||
| 241 | \OCP\Share::SHARE_TYPE_EMAIL |
||
| 242 | ]; |
||
| 243 | foreach ($requestedShareTypes as $requestedShareType) { |
||
| 244 | // one of each type is enough to find out about the types |
||
| 245 | $shares = $this->shareManager->getSharesBy( |
||
| 246 | $userId, |
||
| 247 | $requestedShareType, |
||
| 248 | $node, |
||
| 249 | false, |
||
| 250 | 1 |
||
| 251 | ); |
||
| 252 | if (!empty($shares)) { |
||
| 253 | $shareTypes[] = $requestedShareType; |
||
| 254 | } |
||
| 255 | } |
||
| 256 | return $shareTypes; |
||
| 257 | } |
||
| 258 | |||
| 259 | /** |
||
| 260 | * Change the default sort mode |
||
| 261 | * |
||
| 262 | * @NoAdminRequired |
||
| 263 | * |
||
| 264 | * @param string $mode |
||
| 265 | * @param string $direction |
||
| 266 | * @return Response |
||
| 267 | */ |
||
| 268 | public function updateFileSorting($mode, $direction) { |
||
| 269 | $allowedMode = ['name', 'size', 'mtime']; |
||
| 270 | $allowedDirection = ['asc', 'desc']; |
||
| 271 | if (!in_array($mode, $allowedMode) || !in_array($direction, $allowedDirection)) { |
||
| 272 | $response = new Response(); |
||
| 273 | $response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); |
||
| 274 | return $response; |
||
| 275 | } |
||
| 276 | $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting', $mode); |
||
| 277 | $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'file_sorting_direction', $direction); |
||
| 278 | return new Response(); |
||
| 279 | } |
||
| 280 | |||
| 281 | /** |
||
| 282 | * Toggle default for showing/hiding hidden files |
||
| 283 | * |
||
| 284 | * @NoAdminRequired |
||
| 285 | * |
||
| 286 | * @param bool $show |
||
| 287 | */ |
||
| 288 | public function showHiddenFiles($show) { |
||
| 289 | $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'show_hidden', (int)$show); |
||
| 290 | return new Response(); |
||
| 291 | } |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Toggle default for showing/hiding xxx folder |
||
| 295 | * |
||
| 296 | * @NoAdminRequired |
||
| 297 | * |
||
| 298 | * @param bool $show |
||
| 299 | * @param bool $key the key of the folder |
||
| 300 | * |
||
| 301 | * @return Response |
||
| 302 | */ |
||
| 303 | public function toggleShowFolder(int $show, string $key) { |
||
| 304 | // ensure the edited key exists |
||
| 305 | $navItems = \OCA\Files\App::getNavigationManager()->getAll(); |
||
| 306 | foreach ($navItems as $item) { |
||
| 307 | // check if data is valid |
||
| 308 | if (($show === 0 || $show === 1) && isset($item['expandedState']) && $key === $item['expandedState']) { |
||
| 309 | $this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', $key, (int)$show); |
||
| 310 | return new Response(); |
||
| 311 | } |
||
| 312 | } |
||
| 313 | $response = new Response(); |
||
| 314 | $response->setStatus(Http::STATUS_FORBIDDEN); |
||
| 315 | return $response; |
||
| 316 | } |
||
| 317 | |||
| 318 | /** |
||
| 319 | * quickaccess-sorting-strategy |
||
| 320 | * |
||
| 321 | * @NoAdminRequired |
||
| 322 | * |
||
| 323 | * @param string $strategy |
||
| 324 | * @return Response |
||
| 325 | */ |
||
| 326 | public function setSortingStrategy($strategy) { |
||
| 330 | |||
| 331 | /** |
||
| 332 | * Get reverse-state for quickaccess-list |
||
| 333 | * |
||
| 334 | * @NoAdminRequired |
||
| 335 | * |
||
| 336 | * @return String |
||
| 337 | */ |
||
| 338 | public function getSortingStrategy() { |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Toggle for reverse quickaccess-list |
||
| 344 | * |
||
| 345 | * @NoAdminRequired |
||
| 346 | * |
||
| 347 | * @param bool $reverse |
||
| 348 | * @return Response |
||
| 349 | */ |
||
| 350 | public function setReverseQuickaccess($reverse) { |
||
| 354 | |||
| 355 | /** |
||
| 356 | * Get reverse-state for quickaccess-list |
||
| 357 | * |
||
| 358 | * @NoAdminRequired |
||
| 359 | * |
||
| 360 | * @return bool |
||
| 361 | */ |
||
| 362 | public function getReverseQuickaccess() { |
||
| 368 | |||
| 369 | /** |
||
| 370 | * Set state for show sorting menu |
||
| 371 | * |
||
| 372 | * @NoAdminRequired |
||
| 373 | * |
||
| 374 | * @param bool $show |
||
| 375 | * @return Response |
||
| 376 | */ |
||
| 377 | public function setShowQuickaccessSettings($show) { |
||
| 381 | |||
| 382 | /** |
||
| 383 | * Get state for show sorting menu |
||
| 384 | * |
||
| 385 | * @NoAdminRequired |
||
| 386 | * |
||
| 387 | * @return bool |
||
| 388 | */ |
||
| 389 | public function getShowQuickaccessSettings() { |
||
| 395 | |||
| 396 | /** |
||
| 397 | * Set sorting-order for custom sorting |
||
| 398 | * |
||
| 399 | * @NoAdminRequired |
||
| 400 | * |
||
| 401 | * @param String $order |
||
| 402 | * @return Response |
||
| 403 | */ |
||
| 404 | public function setSortingOrder($order) { |
||
| 408 | |||
| 409 | /** |
||
| 410 | * Get sorting-order for custom sorting |
||
| 411 | * |
||
| 412 | * @NoAdminRequired |
||
| 413 | * |
||
| 414 | * @return String |
||
| 415 | */ |
||
| 416 | public function getSortingOrder() { |
||
| 419 | |||
| 420 | /** |
||
| 421 | * Get sorting-order for custom sorting |
||
| 422 | * |
||
| 423 | * @NoAdminRequired |
||
| 424 | * |
||
| 425 | * @param String |
||
| 426 | * @return String |
||
| 427 | */ |
||
| 428 | public function getNodeType($folderpath) { |
||
| 432 | |||
| 433 | |||
| 434 | } |
||
| 435 |
This check looks at variables that have been passed in as parameters and are passed out again to other methods.
If the outgoing method call has stricter type requirements than the method itself, an issue is raised.
An additional type check may prevent trouble.