owncloud /
core
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 AppManager 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 AppManager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 47 | class AppManager implements IAppManager { |
||
| 48 | |||
| 49 | /** |
||
| 50 | * Apps with these types can not be enabled for certain groups only |
||
| 51 | * @var string[] |
||
| 52 | */ |
||
| 53 | protected $protectedAppTypes = [ |
||
| 54 | 'filesystem', |
||
| 55 | 'prelogin', |
||
| 56 | 'authentication', |
||
| 57 | 'logging', |
||
| 58 | 'prevent_group_restriction', |
||
| 59 | ]; |
||
| 60 | |||
| 61 | /** @var \OCP\IUserSession */ |
||
| 62 | private $userSession; |
||
| 63 | /** @var \OCP\IAppConfig */ |
||
| 64 | private $appConfig; |
||
| 65 | /** @var \OCP\IGroupManager */ |
||
| 66 | private $groupManager; |
||
| 67 | /** @var \OCP\ICacheFactory */ |
||
| 68 | private $memCacheFactory; |
||
| 69 | /** @var string[] $appId => $enabled */ |
||
| 70 | private $installedAppsCache; |
||
| 71 | /** @var string[] */ |
||
| 72 | private $shippedApps; |
||
| 73 | /** @var string[] */ |
||
| 74 | private $alwaysEnabled; |
||
| 75 | /** @var EventDispatcherInterface */ |
||
| 76 | private $dispatcher; |
||
| 77 | /** @var IConfig */ |
||
| 78 | private $config; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * Apps as 'appId' => [ |
||
| 82 | * 'path' => '/app/path' |
||
| 83 | * 'url' => '/app/url' |
||
| 84 | * ] |
||
| 85 | * @var string[][] |
||
| 86 | */ |
||
| 87 | private $appDirs = []; |
||
| 88 | |||
| 89 | /** |
||
| 90 | * @param IUserSession $userSession |
||
| 91 | * @param IAppConfig $appConfig |
||
| 92 | * @param IGroupManager $groupManager |
||
| 93 | * @param ICacheFactory $memCacheFactory |
||
| 94 | * @param EventDispatcherInterface $dispatcher |
||
| 95 | * @param IConfig $config |
||
| 96 | */ |
||
| 97 | public function __construct(IUserSession $userSession = null, |
||
| 110 | |||
| 111 | /** |
||
| 112 | * @return string[] $appId => $enabled |
||
| 113 | */ |
||
| 114 | private function getInstalledAppsValues() { |
||
| 130 | |||
| 131 | /** |
||
| 132 | * List all installed apps |
||
| 133 | * |
||
| 134 | * @return string[] |
||
| 135 | */ |
||
| 136 | public function getInstalledApps() { |
||
| 139 | |||
| 140 | /** |
||
| 141 | * List all apps enabled for a user |
||
| 142 | * |
||
| 143 | * @param \OCP\IUser|null $user |
||
| 144 | * @return string[] |
||
| 145 | */ |
||
| 146 | public function getEnabledAppsForUser(IUser $user = null) { |
||
| 153 | |||
| 154 | /** |
||
| 155 | * Check if an app is enabled for user |
||
| 156 | * |
||
| 157 | * @param string $appId |
||
| 158 | * @param \OCP\IUser $user (optional) if not defined, the currently logged in user will be used |
||
| 159 | * @return bool |
||
| 160 | */ |
||
| 161 | public function isEnabledForUser($appId, $user = null) { |
||
| 175 | |||
| 176 | /** |
||
| 177 | * @param string $enabled |
||
| 178 | * @param IUser $user |
||
| 179 | * @return bool |
||
| 180 | */ |
||
| 181 | private function checkAppForUser($enabled, $user) { |
||
| 208 | |||
| 209 | /** |
||
| 210 | * Check if an app is installed in the instance |
||
| 211 | * |
||
| 212 | * @param string $appId |
||
| 213 | * @return bool |
||
| 214 | */ |
||
| 215 | public function isInstalled($appId) { |
||
| 219 | |||
| 220 | /** |
||
| 221 | * Enable an app for every user |
||
| 222 | * |
||
| 223 | * @param string $appId |
||
| 224 | * @throws \Exception |
||
| 225 | */ |
||
| 226 | View Code Duplication | public function enableApp($appId) { |
|
| 227 | if($this->getAppPath($appId) === false) { |
||
| 228 | throw new \Exception("$appId can't be enabled since it is not installed."); |
||
| 229 | } |
||
| 230 | $this->canEnableTheme($appId); |
||
| 231 | |||
| 232 | $this->installedAppsCache[$appId] = 'yes'; |
||
| 233 | $this->appConfig->setValue($appId, 'enabled', 'yes'); |
||
| 234 | $this->dispatcher->dispatch(ManagerEvent::EVENT_APP_ENABLE, new ManagerEvent( |
||
| 235 | ManagerEvent::EVENT_APP_ENABLE, $appId |
||
| 236 | )); |
||
| 237 | $this->clearAppsCache(); |
||
| 238 | } |
||
| 239 | |||
| 240 | /** |
||
| 241 | * Do not allow more than one active app-theme |
||
| 242 | * |
||
| 243 | * @param $appId |
||
| 244 | * @throws AppManagerException |
||
| 245 | */ |
||
| 246 | protected function canEnableTheme($appId) { |
||
| 261 | |||
| 262 | /** |
||
| 263 | * Wrapper for OC_App for easy mocking |
||
| 264 | * |
||
| 265 | * @param string $appId |
||
| 266 | * @return bool |
||
| 267 | */ |
||
| 268 | protected function isTheme($appId) { |
||
| 271 | |||
| 272 | /** |
||
| 273 | * Enable an app only for specific groups |
||
| 274 | * |
||
| 275 | * @param string $appId |
||
| 276 | * @param \OCP\IGroup[] $groups |
||
| 277 | * @throws \Exception if app can't be enabled for groups |
||
| 278 | */ |
||
| 279 | public function enableAppForGroups($appId, $groups) { |
||
| 299 | |||
| 300 | /** |
||
| 301 | * Disable an app for every user |
||
| 302 | * |
||
| 303 | * @param string $appId |
||
| 304 | * @throws \Exception if app can't be disabled |
||
| 305 | */ |
||
| 306 | View Code Duplication | public function disableApp($appId) { |
|
| 307 | if ($this->isAlwaysEnabled($appId)) { |
||
| 308 | throw new \Exception("$appId can't be disabled."); |
||
| 309 | } |
||
| 310 | unset($this->installedAppsCache[$appId]); |
||
| 311 | $this->appConfig->setValue($appId, 'enabled', 'no'); |
||
| 312 | $this->dispatcher->dispatch(ManagerEvent::EVENT_APP_DISABLE, new ManagerEvent( |
||
| 313 | ManagerEvent::EVENT_APP_DISABLE, $appId |
||
| 314 | )); |
||
| 315 | $this->clearAppsCache(); |
||
| 316 | } |
||
| 317 | |||
| 318 | /** |
||
| 319 | * Clear the cached list of apps when enabling/disabling an app |
||
| 320 | */ |
||
| 321 | public function clearAppsCache() { |
||
| 325 | |||
| 326 | /** |
||
| 327 | * Returns a list of apps that need upgrade |
||
| 328 | * |
||
| 329 | * @param array $ocVersion ownCloud version as array of version components |
||
| 330 | * @return array list of app info from apps that need an upgrade |
||
| 331 | * |
||
| 332 | * @internal |
||
| 333 | */ |
||
| 334 | public function getAppsNeedingUpgrade($ocVersion) { |
||
| 351 | |||
| 352 | /** |
||
| 353 | * Returns the app information from "appinfo/info.xml". |
||
| 354 | * |
||
| 355 | * @param string $appId app id |
||
| 356 | * |
||
| 357 | * @return array app info |
||
| 358 | * |
||
| 359 | * @internal |
||
| 360 | */ |
||
| 361 | View Code Duplication | public function getAppInfo($appId) { |
|
| 372 | |||
| 373 | /** |
||
| 374 | * Returns a list of apps incompatible with the given version |
||
| 375 | * |
||
| 376 | * @param array $version ownCloud version as array of version components |
||
| 377 | * |
||
| 378 | * @return array list of app info from incompatible apps |
||
| 379 | * |
||
| 380 | * @internal |
||
| 381 | */ |
||
| 382 | public function getIncompatibleApps($version) { |
||
| 393 | |||
| 394 | /** |
||
| 395 | * @inheritdoc |
||
| 396 | */ |
||
| 397 | public function isShipped($appId) { |
||
| 401 | |||
| 402 | private function isAlwaysEnabled($appId) { |
||
| 406 | |||
| 407 | private function loadShippedJson() { |
||
| 418 | |||
| 419 | /** |
||
| 420 | * @inheritdoc |
||
| 421 | */ |
||
| 422 | public function getAlwaysEnabledApps() { |
||
| 426 | |||
| 427 | /** |
||
| 428 | * @param string $package package path |
||
| 429 | * @param bool $skipMigrations whether to skip migrations, which would only install the code |
||
| 430 | * @return string|false app id or false in case of error |
||
| 431 | * @since 10.0 |
||
| 432 | */ |
||
| 433 | public function installApp($package, $skipMigrations = false) { |
||
| 440 | |||
| 441 | /** |
||
| 442 | * @param string $package |
||
| 443 | * @return mixed |
||
| 444 | * @since 10.0 |
||
| 445 | */ |
||
| 446 | public function updateApp($package) { |
||
| 452 | |||
| 453 | /** |
||
| 454 | * Returns the list of all apps, enabled and disabled |
||
| 455 | * |
||
| 456 | * @return string[] |
||
| 457 | * @since 10.0 |
||
| 458 | */ |
||
| 459 | public function getAllApps() { |
||
| 462 | |||
| 463 | /** |
||
| 464 | * @param string $path |
||
| 465 | * @return string[] app info |
||
| 466 | */ |
||
| 467 | public function readAppPackage($path) { |
||
| 477 | |||
| 478 | /** |
||
| 479 | * Indicates if app installation is supported. Usually it is but in certain |
||
| 480 | * environments it is disallowed because of hardening. In a clustered setup |
||
| 481 | * apps need to be installed on each cluster node which is out of scope of |
||
| 482 | * ownCloud itself. |
||
| 483 | * |
||
| 484 | * @return bool |
||
| 485 | * @since 10.0.3 |
||
| 486 | */ |
||
| 487 | public function canInstall() { |
||
| 495 | |||
| 496 | /** |
||
| 497 | * Get the absolute path to the directory for the given app. |
||
| 498 | * If the app exists in multiple directories, the most recent version is taken. |
||
| 499 | * Returns false if not found |
||
| 500 | * |
||
| 501 | * @param string $appId |
||
| 502 | * @return string|false |
||
| 503 | * @since 10.0.5 |
||
| 504 | */ |
||
| 505 | public function getAppPath($appId) { |
||
| 514 | |||
| 515 | /** |
||
| 516 | * Get the HTTP Web path to the app directory for the given app, relative to the ownCloud webroot. |
||
| 517 | * If the app exists in multiple directories, web path to the most recent version is taken. |
||
| 518 | * Returns false if not found |
||
| 519 | * |
||
| 520 | * @param string $appId |
||
| 521 | * @return string|false |
||
| 522 | * @since 10.0.5 |
||
| 523 | */ |
||
| 524 | public function getAppWebPath($appId) { |
||
| 537 | |||
| 538 | /** |
||
| 539 | * Search for an app in all app directories |
||
| 540 | * Returns an app directory as an array with keys |
||
| 541 | * 'path' - a path to the app with no trailing slash |
||
| 542 | * 'url' - a web path to the app with no trailing slash |
||
| 543 | * both are relative to OC root directory and webroot |
||
| 544 | * |
||
| 545 | * @param string $appId |
||
| 546 | * @return false|string[] |
||
| 547 | */ |
||
| 548 | protected function findAppInDirectories($appId) { |
||
| 579 | |||
| 580 | /** |
||
| 581 | * Save app path and webPath to internal cache |
||
| 582 | * @param string $appId |
||
| 583 | * @param string[] $appData |
||
| 584 | */ |
||
| 585 | protected function saveAppPath($appId, $appData) { |
||
| 588 | |||
| 589 | /** |
||
| 590 | * Get OC web root |
||
| 591 | * Wrapper for easy mocking |
||
| 592 | * @return string |
||
| 593 | */ |
||
| 594 | protected function getOcWebRoot() { |
||
| 597 | |||
| 598 | /** |
||
| 599 | * Get apps roots as an array of path and url |
||
| 600 | * Wrapper for easy mocking |
||
| 601 | * @return string[][] |
||
| 602 | */ |
||
| 603 | protected function getAppRoots(){ |
||
| 606 | |||
| 607 | /** |
||
| 608 | * Get app's version based on it's path |
||
| 609 | * Wrapper for easy mocking |
||
| 610 | * |
||
| 611 | * @param string $path |
||
| 612 | * @return string |
||
| 613 | */ |
||
| 614 | protected function getAppVersionByPath($path) { |
||
| 617 | } |
||
| 618 |
Loading history...
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)or! empty(...)instead.