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 File 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 File, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 64 | class File extends Node implements IFile, IFileNode { |
||
| 65 | |||
| 66 | use EventEmitterTrait; |
||
| 67 | protected $request; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * Sets up the node, expects a full path name |
||
| 71 | * |
||
| 72 | * @param \OC\Files\View $view |
||
| 73 | * @param \OCP\Files\FileInfo $info |
||
| 74 | * @param \OCP\Share\IManager $shareManager |
||
| 75 | */ |
||
| 76 | public function __construct($view, $info, $shareManager = null, Request $request = null) { |
||
| 77 | if (isset($request)) { |
||
| 78 | $this->request = $request; |
||
| 79 | } else { |
||
| 80 | $this->request = \OC::$server->getRequest(); |
||
| 81 | } |
||
| 82 | parent::__construct($view, $info, $shareManager); |
||
| 83 | } |
||
| 84 | |||
| 85 | /** |
||
| 86 | * Updates the data |
||
| 87 | * |
||
| 88 | * The data argument is a readable stream resource. |
||
| 89 | * |
||
| 90 | * After a successful put operation, you may choose to return an ETag. The |
||
| 91 | * etag must always be surrounded by double-quotes. These quotes must |
||
| 92 | * appear in the actual string you're returning. |
||
| 93 | * |
||
| 94 | * Clients may use the ETag from a PUT request to later on make sure that |
||
| 95 | * when they update the file, the contents haven't changed in the mean |
||
| 96 | * time. |
||
| 97 | * |
||
| 98 | * If you don't plan to store the file byte-by-byte, and you return a |
||
| 99 | * different object on a subsequent GET you are strongly recommended to not |
||
| 100 | * return an ETag, and just return null. |
||
| 101 | * |
||
| 102 | * @param resource|string $data |
||
| 103 | * |
||
| 104 | * @throws Forbidden |
||
| 105 | * @throws UnsupportedMediaType |
||
| 106 | * @throws BadRequest |
||
| 107 | * @throws Exception |
||
| 108 | * @throws EntityTooLarge |
||
| 109 | * @throws ServiceUnavailable |
||
| 110 | * @throws FileLocked |
||
| 111 | * @return string|null |
||
| 112 | */ |
||
| 113 | public function put($data) { |
||
| 114 | $path = $this->fileView->getAbsolutePath($this->path); |
||
| 115 | return $this->emittingCall(function () use (&$data) { |
||
| 116 | try { |
||
| 117 | $exists = $this->fileView->file_exists($this->path); |
||
| 118 | if ($this->info && $exists && !$this->info->isUpdateable()) { |
||
| 119 | throw new Forbidden(); |
||
| 120 | } |
||
| 121 | } catch (StorageNotAvailableException $e) { |
||
| 122 | throw new ServiceUnavailable("File is not updatable: " . $e->getMessage()); |
||
| 123 | } |
||
| 124 | |||
| 125 | // verify path of the target |
||
| 126 | $this->verifyPath(); |
||
| 127 | |||
| 128 | // chunked handling |
||
| 129 | if (\OC_FileChunking::isWebdavChunk()) { |
||
| 130 | try { |
||
| 131 | return $this->createFileChunked($data); |
||
|
|
|||
| 132 | } catch (\Exception $e) { |
||
| 133 | $this->convertToSabreException($e); |
||
| 134 | } |
||
| 135 | } |
||
| 136 | |||
| 137 | list($partStorage) = $this->fileView->resolvePath($this->path); |
||
| 138 | $needsPartFile = $this->needsPartFile($partStorage) && (strlen($this->path) > 1); |
||
| 139 | |||
| 140 | if ($needsPartFile) { |
||
| 141 | // mark file as partial while uploading (ignored by the scanner) |
||
| 142 | $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . rand() . '.part'; |
||
| 143 | } else { |
||
| 144 | // upload file directly as the final path |
||
| 145 | $partFilePath = $this->path; |
||
| 146 | } |
||
| 147 | |||
| 148 | // the part file and target file might be on a different storage in case of a single file storage (e.g. single file share) |
||
| 149 | /** @var \OC\Files\Storage\Storage $partStorage */ |
||
| 150 | list($partStorage, $internalPartPath) = $this->fileView->resolvePath($partFilePath); |
||
| 151 | /** @var \OC\Files\Storage\Storage $storage */ |
||
| 152 | list($storage, $internalPath) = $this->fileView->resolvePath($this->path); |
||
| 153 | try { |
||
| 154 | $target = $partStorage->fopen($internalPartPath, 'wb'); |
||
| 155 | if ($target === false) { |
||
| 156 | \OCP\Util::writeLog('webdav', '\OC\Files\Filesystem::fopen() failed', \OCP\Util::ERROR); |
||
| 157 | // because we have no clue about the cause we can only throw back a 500/Internal Server Error |
||
| 158 | throw new Exception('Could not write file contents'); |
||
| 159 | } |
||
| 160 | list($count, $result) = \OC_Helper::streamCopy($data, $target); |
||
| 161 | fclose($target); |
||
| 162 | |||
| 163 | if (!self::isChecksumValid($partStorage, $internalPartPath)) { |
||
| 164 | throw new BadRequest('The computed checksum does not match the one received from the client.'); |
||
| 165 | } |
||
| 166 | |||
| 167 | if ($result === false) { |
||
| 168 | $expected = -1; |
||
| 169 | if (isset($_SERVER['CONTENT_LENGTH'])) { |
||
| 170 | $expected = $_SERVER['CONTENT_LENGTH']; |
||
| 171 | } |
||
| 172 | throw new Exception('Error while copying file to target location (copied bytes: ' . $count . ', expected filesize: ' . $expected . ' )'); |
||
| 173 | } |
||
| 174 | |||
| 175 | // if content length is sent by client: |
||
| 176 | // double check if the file was fully received |
||
| 177 | // compare expected and actual size |
||
| 178 | if (isset($_SERVER['CONTENT_LENGTH']) && $_SERVER['REQUEST_METHOD'] === 'PUT') { |
||
| 179 | $expected = $_SERVER['CONTENT_LENGTH']; |
||
| 180 | if ($count != $expected) { |
||
| 181 | throw new BadRequest('expected filesize ' . $expected . ' got ' . $count); |
||
| 182 | } |
||
| 183 | } |
||
| 184 | |||
| 185 | } catch (\Exception $e) { |
||
| 186 | if ($needsPartFile) { |
||
| 187 | $partStorage->unlink($internalPartPath); |
||
| 188 | } |
||
| 189 | $this->convertToSabreException($e); |
||
| 190 | } |
||
| 191 | |||
| 192 | try { |
||
| 193 | $view = \OC\Files\Filesystem::getView(); |
||
| 194 | if ($view) { |
||
| 195 | $run = $this->emitPreHooks($exists); |
||
| 196 | } else { |
||
| 197 | $run = true; |
||
| 198 | } |
||
| 199 | |||
| 200 | try { |
||
| 201 | $this->changeLock(ILockingProvider::LOCK_EXCLUSIVE); |
||
| 202 | } catch (LockedException $e) { |
||
| 203 | if ($needsPartFile) { |
||
| 204 | $partStorage->unlink($internalPartPath); |
||
| 205 | } |
||
| 206 | throw new FileLocked($e->getMessage(), $e->getCode(), $e); |
||
| 207 | } |
||
| 208 | |||
| 209 | if ($needsPartFile) { |
||
| 210 | // rename to correct path |
||
| 211 | try { |
||
| 212 | if ($run) { |
||
| 213 | $renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath); |
||
| 214 | $fileExists = $storage->file_exists($internalPath); |
||
| 215 | } |
||
| 216 | if (!$run || $renameOkay === false || $fileExists === false) { |
||
| 217 | \OCP\Util::writeLog('webdav', 'renaming part file to final file failed', \OCP\Util::ERROR); |
||
| 218 | throw new Exception('Could not rename part file to final file'); |
||
| 219 | } |
||
| 220 | } catch (ForbiddenException $ex) { |
||
| 221 | throw new DAVForbiddenException($ex->getMessage(), $ex->getRetry()); |
||
| 222 | } catch (\Exception $e) { |
||
| 223 | $partStorage->unlink($internalPartPath); |
||
| 224 | $this->convertToSabreException($e); |
||
| 225 | } |
||
| 226 | } |
||
| 227 | |||
| 228 | // since we skipped the view we need to scan and emit the hooks ourselves |
||
| 229 | $storage->getUpdater()->update($internalPath); |
||
| 230 | |||
| 231 | try { |
||
| 232 | $this->changeLock(ILockingProvider::LOCK_SHARED); |
||
| 233 | } catch (LockedException $e) { |
||
| 234 | throw new FileLocked($e->getMessage(), $e->getCode(), $e); |
||
| 235 | } |
||
| 236 | |||
| 237 | // allow sync clients to send the mtime along in a header |
||
| 238 | if (isset($this->request->server['HTTP_X_OC_MTIME'])) { |
||
| 239 | $mtime = $this->sanitizeMtime($this->request->server ['HTTP_X_OC_MTIME']); |
||
| 240 | if ($this->fileView->touch($this->path, $mtime)) { |
||
| 241 | $this->header('X-OC-MTime: accepted'); |
||
| 242 | } |
||
| 243 | } |
||
| 244 | |||
| 245 | if ($view) { |
||
| 246 | $this->emitPostHooks($exists); |
||
| 247 | } |
||
| 248 | |||
| 249 | $this->refreshInfo(); |
||
| 250 | } catch (StorageNotAvailableException $e) { |
||
| 251 | throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); |
||
| 252 | } |
||
| 253 | |||
| 254 | return '"' . $this->info->getEtag() . '"'; |
||
| 255 | }, [ |
||
| 256 | 'before' => ['path' => $path], |
||
| 257 | 'after' => ['path' => $path]], |
||
| 258 | 'file', 'create'); |
||
| 259 | } |
||
| 260 | |||
| 261 | private function getPartFileBasePath($path) { |
||
| 269 | |||
| 270 | /** |
||
| 271 | * @param string $path |
||
| 272 | */ |
||
| 273 | private function emitPreHooks($exists, $path = null) { |
||
| 313 | |||
| 314 | /** |
||
| 315 | * @param string $path |
||
| 316 | */ |
||
| 317 | private function emitPostHooks($exists, $path = null) { |
||
| 335 | |||
| 336 | /** |
||
| 337 | * Returns the data |
||
| 338 | * |
||
| 339 | * @return resource |
||
| 340 | * @throws Forbidden |
||
| 341 | * @throws ServiceUnavailable |
||
| 342 | */ |
||
| 343 | public function get() { |
||
| 367 | |||
| 368 | /** |
||
| 369 | * Delete the current file |
||
| 370 | * |
||
| 371 | * @throws Forbidden |
||
| 372 | * @throws ServiceUnavailable |
||
| 373 | */ |
||
| 374 | public function delete() { |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Returns the mime-type for a file |
||
| 395 | * |
||
| 396 | * If null is returned, we'll assume application/octet-stream |
||
| 397 | * |
||
| 398 | * @return string |
||
| 399 | */ |
||
| 400 | public function getContentType() { |
||
| 409 | |||
| 410 | /** |
||
| 411 | * @return array|false |
||
| 412 | */ |
||
| 413 | public function getDirectDownload() { |
||
| 425 | |||
| 426 | /** |
||
| 427 | * @param resource $data |
||
| 428 | * @return null|string |
||
| 429 | * @throws Exception |
||
| 430 | * @throws BadRequest |
||
| 431 | * @throws NotImplemented |
||
| 432 | * @throws ServiceUnavailable |
||
| 433 | */ |
||
| 434 | private function createFileChunked($data) { |
||
| 556 | |||
| 557 | /** |
||
| 558 | * will return true if checksum was not provided in request |
||
| 559 | * |
||
| 560 | * @param Storage $storage |
||
| 561 | * @param $path |
||
| 562 | * @return bool |
||
| 563 | */ |
||
| 564 | private static function isChecksumValid(Storage $storage, $path) { |
||
| 579 | |||
| 580 | /** |
||
| 581 | * Returns whether a part file is needed for the given storage |
||
| 582 | * or whether the file can be assembled/uploaded directly on the |
||
| 583 | * target storage. |
||
| 584 | * |
||
| 585 | * @param \OCP\Files\Storage $storage |
||
| 586 | * @return bool true if the storage needs part file handling |
||
| 587 | */ |
||
| 588 | private function needsPartFile($storage) { |
||
| 595 | |||
| 596 | /** |
||
| 597 | * Convert the given exception to a SabreException instance |
||
| 598 | * |
||
| 599 | * @param \Exception $e |
||
| 600 | * |
||
| 601 | * @throws \Sabre\DAV\Exception |
||
| 602 | */ |
||
| 603 | private function convertToSabreException(\Exception $e) { |
||
| 642 | |||
| 643 | /** |
||
| 644 | * Set $algo to get a specific checksum, leave null to get all checksums |
||
| 645 | * (space seperated) |
||
| 646 | * @param null $algo |
||
| 647 | * @return string |
||
| 648 | */ |
||
| 649 | public function getChecksum($algo = null) { |
||
| 668 | |||
| 669 | protected function header($string) { |
||
| 672 | |||
| 673 | /** |
||
| 674 | * @return \OCP\Files\Node |
||
| 675 | */ |
||
| 676 | public function getNode() { |
||
| 679 | } |
||
| 680 |
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.