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 DocumentController 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 DocumentController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
47 | class DocumentController extends Controller { |
||
48 | |||
49 | private $uid; |
||
50 | private $l10n; |
||
51 | private $settings; |
||
52 | private $appConfig; |
||
53 | private $cache; |
||
54 | private $logger; |
||
55 | const ODT_TEMPLATE_PATH = '/assets/odttemplate.odt'; |
||
56 | |||
57 | 1 | public function __construct($appName, IRequest $request, IConfig $settings, AppConfig $appConfig, IL10N $l10n, $uid, ICacheFactory $cache, ILogger $logger){ |
|
66 | |||
67 | /** |
||
68 | * @param \SimpleXMLElement $discovery |
||
|
|||
69 | * @param string $mimetype |
||
70 | */ |
||
71 | private function getWopiSrcUrl($discovery_parsed, $mimetype) { |
||
86 | |||
87 | /** |
||
88 | * Log the user with given $userid. |
||
89 | * This function should only be used from public controller methods where no |
||
90 | * existing session exists, for example, when loolwsd is directly calling a |
||
91 | * public method with its own access token. After validating the access |
||
92 | * token, and retrieving the correct user with help of access token, it can |
||
93 | * be set as current user with help of this method. |
||
94 | * |
||
95 | * @param string $userid |
||
96 | */ |
||
97 | private function loginUser($userid) { |
||
98 | \OC_Util::tearDownFS(); |
||
99 | |||
100 | $users = \OC::$server->getUserManager()->search($userid, 1, 0); |
||
101 | if (count($users) > 0) { |
||
102 | $user = array_shift($users); |
||
103 | if (strcasecmp($user->getUID(), $userid) === 0) { |
||
104 | // clear the existing sessions, if any |
||
105 | \OC::$server->getSession()->close(); |
||
106 | |||
107 | // initialize a dummy memory session |
||
108 | $session = new \OC\Session\Memory(''); |
||
109 | // wrap it |
||
110 | $cryptoWrapper = \OC::$server->getSessionCryptoWrapper(); |
||
111 | $session = $cryptoWrapper->wrapSession($session); |
||
112 | // set our session |
||
113 | \OC::$server->setSession($session); |
||
114 | |||
115 | \OC::$server->getUserSession()->setUser($user); |
||
116 | } |
||
117 | } |
||
118 | |||
119 | \OC_Util::setupFS(); |
||
120 | } |
||
121 | |||
122 | /** |
||
123 | * Log out the current user |
||
124 | * This is helpful when we are artifically logged in as someone |
||
125 | */ |
||
126 | private function logoutUser() { |
||
127 | \OC_Util::tearDownFS(); |
||
128 | |||
129 | \OC::$server->getSession()->close(); |
||
130 | } |
||
131 | |||
132 | private function responseError($message, $hint = ''){ |
||
137 | |||
138 | /** |
||
139 | * Return the original wopi url or test wopi url |
||
140 | */ |
||
141 | private function getWopiUrl($tester) { |
||
151 | |||
152 | /** |
||
153 | * Return true if the currently logged in user is a tester. |
||
154 | * This depends on whether current user is the member of one of the groups |
||
155 | * mentioned in settings (test_server_groups) |
||
156 | */ |
||
157 | private function isTester() { |
||
182 | |||
183 | /** Return the content of discovery.xml - either from cache, or download it. |
||
184 | */ |
||
185 | private function getDiscovery(){ |
||
240 | |||
241 | /** Prepare document(s) structure |
||
242 | */ |
||
243 | private function prepareDocuments($rawDocuments){ |
||
306 | |||
307 | /** |
||
308 | * @NoAdminRequired |
||
309 | * @NoCSRFRequired |
||
310 | */ |
||
311 | public function index(){ |
||
357 | |||
358 | /** |
||
359 | * @NoAdminRequired |
||
360 | */ |
||
361 | public function create(){ |
||
456 | |||
457 | /** |
||
458 | * @NoAdminRequired |
||
459 | * Generates and returns an access token for a given fileId. |
||
460 | * Only for authenticated users! |
||
461 | */ |
||
462 | public function wopiGetToken($fileId){ |
||
516 | |||
517 | /** |
||
518 | * @NoAdminRequired |
||
519 | * @NoCSRFRequired |
||
520 | * @PublicPage |
||
521 | * Returns general info about a file. |
||
522 | */ |
||
523 | public function wopiCheckFileInfo($fileId){ |
||
524 | $token = $this->request->getParam('access_token'); |
||
525 | |||
526 | $arr = explode('_', $fileId, 2); |
||
527 | $version = '0'; |
||
528 | View Code Duplication | if (count($arr) == 2) { |
|
529 | $fileId = $arr[0]; |
||
530 | $version = $arr[1]; |
||
531 | } |
||
532 | |||
533 | \OC::$server->getLogger()->debug('Getting info about file {fileId}, version {version} by token {token}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'version' => $version, 'token' => $token ]); |
||
534 | |||
535 | $row = new Db\Wopi(); |
||
536 | $row->loadBy('token', $token); |
||
537 | |||
538 | $res = $row->getPathForToken($fileId, $version, $token); |
||
539 | if ($res == false || http_response_code() != 200) |
||
540 | { |
||
541 | return false; |
||
542 | } |
||
543 | |||
544 | // Login the user to see his mount locations |
||
545 | $this->loginUser($res['owner']); |
||
546 | $view = new \OC\Files\View('/' . $res['owner'] . '/files'); |
||
547 | $info = $view->getFileInfo($res['path']); |
||
548 | $this->logoutUser(); |
||
549 | |||
550 | if (!$info) { |
||
551 | http_response_code(404); |
||
552 | return false; |
||
553 | } |
||
554 | |||
555 | $editorName = \OC::$server->getUserManager()->get($res['editor'])->getDisplayName(); |
||
556 | return array( |
||
557 | 'BaseFileName' => $info['name'], |
||
558 | 'Size' => $info['size'], |
||
559 | 'Version' => $version, |
||
560 | 'UserId' => $res['editor'], |
||
561 | 'UserFriendlyName' => $editorName, |
||
562 | 'UserCanWrite' => $res['canwrite'] ? true : false, |
||
563 | 'PostMessageOrigin' => $res['server_host'] |
||
564 | ); |
||
565 | } |
||
566 | |||
567 | /** |
||
568 | * @NoAdminRequired |
||
569 | * @NoCSRFRequired |
||
570 | * @PublicPage |
||
571 | * Given an access token and a fileId, returns the contents of the file. |
||
572 | * Expects a valid token in access_token parameter. |
||
573 | */ |
||
574 | public function wopiGetFile($fileId){ |
||
575 | $token = $this->request->getParam('access_token'); |
||
576 | |||
577 | $arr = explode('_', $fileId, 2); |
||
578 | $version = '0'; |
||
579 | View Code Duplication | if (count($arr) == 2) { |
|
580 | $fileId = $arr[0]; |
||
581 | $version = $arr[1]; |
||
582 | } |
||
583 | |||
584 | \OC::$server->getLogger()->debug('Getting contents of file {fileId}, version {version} by token {token}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'version' => $version, 'token' => $token ]); |
||
585 | |||
586 | $row = new Db\Wopi(); |
||
587 | $row->loadBy('token', $token); |
||
588 | |||
589 | //TODO: Support X-WOPIMaxExpectedSize header. |
||
590 | $res = $row->getPathForToken($fileId, $version, $token); |
||
591 | $ownerid = $res['owner']; |
||
592 | |||
593 | // Login the user to see his mount locations |
||
594 | $this->loginUser($ownerid); |
||
595 | $view = new \OC\Files\View('/' . $res['owner'] . '/files'); |
||
596 | $info = $view->getFileInfo($res['path']); |
||
597 | |||
598 | if (!$info) { |
||
599 | http_response_code(404); |
||
600 | return false; |
||
601 | } |
||
602 | |||
603 | $filename = ''; |
||
604 | // If some previous version is requested, fetch it from Files_Version app |
||
605 | if ($version !== '0') { |
||
606 | \OCP\JSON::checkAppEnabled('files_versions'); |
||
607 | |||
608 | $filename = '/files_versions/' . $info['name'] . '.v' . $version; |
||
609 | } else { |
||
610 | $filename = '/files' . $res['path']; |
||
611 | } |
||
612 | |||
613 | $this->logoutUser(); |
||
614 | |||
615 | return new DownloadResponse($this->request, $ownerid, $filename); |
||
616 | } |
||
617 | |||
618 | /** |
||
619 | * @NoAdminRequired |
||
620 | * @NoCSRFRequired |
||
621 | * @PublicPage |
||
622 | * Given an access token and a fileId, replaces the files with the request body. |
||
623 | * Expects a valid token in access_token parameter. |
||
624 | */ |
||
625 | public function wopiPutFile($fileId){ |
||
626 | $token = $this->request->getParam('access_token'); |
||
627 | |||
628 | $arr = explode('_', $fileId, 2); |
||
629 | $version = '0'; |
||
630 | View Code Duplication | if (count($arr) == 2) { |
|
631 | $fileId = $arr[0]; |
||
632 | $version = $arr[1]; |
||
633 | } |
||
634 | |||
635 | \OC::$server->getLogger()->debug('Putting contents of file {fileId}, version {version} by token {token}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'version' => $version, 'token' => $token ]); |
||
636 | |||
637 | $row = new Db\Wopi(); |
||
638 | $row->loadBy('token', $token); |
||
639 | |||
640 | $res = $row->getPathForToken($fileId, $version, $token); |
||
641 | if (!$res['canwrite']) { |
||
642 | return array( |
||
643 | 'status' => 'error', |
||
644 | 'message' => 'Permission denied' |
||
645 | ); |
||
646 | } |
||
647 | |||
648 | // Log-in as the user to regiser the change under her name. |
||
649 | $editorid = $res['editor']; |
||
650 | // This call is made from loolwsd, so we need to initialize the |
||
651 | // session before we can make the user who opened the document |
||
652 | // login. This is necessary to make activity app register the |
||
653 | // change made to this file under this user's (editorid) name. |
||
654 | $this->loginUser($editorid); |
||
655 | |||
656 | // Set up the filesystem view for the owner (where the file actually is). |
||
657 | $userid = $res['owner']; |
||
658 | $root = '/' . $userid . '/files'; |
||
659 | $view = new \OC\Files\View($root); |
||
660 | |||
661 | // Read the contents of the file from the POST body and store. |
||
662 | $content = fopen('php://input', 'r'); |
||
663 | \OC::$server->getLogger()->debug('Storing file {fileId} by {editor} owned by {owner}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'editor' => $editorid, 'owner' => $userid ]); |
||
664 | |||
665 | // Setup the FS which is needed to emit hooks (versioning). |
||
666 | \OC_Util::tearDownFS(); |
||
667 | \OC_Util::setupFS($userid); |
||
668 | |||
669 | $view->file_put_contents($res['path'], $content); |
||
670 | |||
671 | $this->logoutUser(); |
||
672 | |||
673 | return array( |
||
674 | 'status' => 'success' |
||
675 | ); |
||
676 | } |
||
677 | |||
678 | /** |
||
679 | * @NoAdminRequired |
||
680 | * @PublicPage |
||
681 | * Process partial/complete file download |
||
682 | */ |
||
683 | public function serve($esId){ |
||
690 | |||
691 | /** |
||
692 | * @NoAdminRequired |
||
693 | */ |
||
694 | public function download($path){ |
||
710 | |||
711 | |||
712 | /** |
||
713 | * @NoAdminRequired |
||
714 | */ |
||
715 | 1 | public function rename($fileId){ |
|
732 | |||
733 | /** |
||
734 | * @NoAdminRequired |
||
735 | * Get file information about single document with fileId |
||
736 | */ |
||
737 | public function get($fileId){ |
||
743 | |||
744 | |||
745 | /** |
||
746 | * @NoAdminRequired |
||
747 | * lists the documents the user has access to (including shared files, once the code in core has been fixed) |
||
748 | * also adds session and member info for these files |
||
749 | */ |
||
750 | public function listAll(){ |
||
753 | } |
||
754 |
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.
Consider the following example. The parameter
$ireland
is not defined by the methodfinale(...)
.The most likely cause is that the parameter was changed, but the annotation was not.