Merge pull request #8506 from nextcloud/use-appmanager

Use isInstalled of AppManger instead of reimplement it
nextcloud · Mar 22, 2018 · f843b7e · f843b7e
2 parents be35b54 + 514de5d
commit f843b7e
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 20 deletions.
diff --git a/core/Command/App/Install.php b/core/Command/App/Install.php
@@ -58,6 +58,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		}
 
 		try {
+			/** @var Installer $installer */
 			$installer = \OC::$server->query(Installer::class);
 			$installer->downloadApp($appId);
 			$result = $installer->installApp($appId);

diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -204,7 +204,9 @@ private function checkAppForUser($enabled, $user) {
 	}
 
 	/**
-	 * Check if an app is installed in the instance
+	 * Check if an app is enabled in the instance
+	 *
+	 * Notice: This actually checks if the app is enabled and not only if it is installed.
 	 *
 	 * @param string $appId
 	 * @return bool

diff --git a/lib/private/Installer.php b/lib/private/Installer.php
@@ -165,17 +165,6 @@ public function installApp($appId) {
 		return $info['id'];
 	}
 
-	/**
-	 * @brief checks whether or not an app is installed
-	 * @param string $app app
-	 * @returns bool
-	 *
-	 * Checks whether or not an app is installed, i.e. registered in apps table.
-	 */
-	public static function isInstalled( $app ) {
-		return (\OC::$server->getConfig()->getAppValue($app, "installed_version", null) !== null);
-	}
-
 	/**
 	 * Updates the specified app from the appstore
 	 *
@@ -510,17 +499,19 @@ public function installAppBundle(Bundle $bundle) {
 	 * @return array Array of error messages (appid => Exception)
 	 */
 	public static function installShippedApps($softErrors = false) {
+		$appManager = \OC::$server->getAppManager();
+		$config = \OC::$server->getConfig();
 		$errors = [];
 		foreach(\OC::$APPSROOTS as $app_dir) {
 			if($dir = opendir( $app_dir['path'] )) {
 				while( false !== ( $filename = readdir( $dir ))) {
 					if( $filename[0] !== '.' and is_dir($app_dir['path']."/$filename") ) {
 						if( file_exists( $app_dir['path']."/$filename/appinfo/info.xml" )) {
-							if(!Installer::isInstalled($filename)) {
+							if($config->getAppValue($filename, "installed_version", null) === null) {
 								$info=OC_App::getAppInfo($filename);
 								$enabled = isset($info['default_enable']);
-								if (($enabled || in_array($filename, \OC::$server->getAppManager()->getAlwaysEnabledApps()))
-									  && \OC::$server->getConfig()->getAppValue($filename, 'enabled') !== 'no') {
+								if (($enabled || in_array($filename, $appManager->getAlwaysEnabledApps()))
+									  && $config->getAppValue($filename, 'enabled') !== 'no') {
 									if ($softErrors) {
 										try {
 											Installer::installShippedApp($filename);
@@ -534,7 +525,7 @@ public static function installShippedApps($softErrors = false) {
 									} else {
 										Installer::installShippedApp($filename);
 									}
-									\OC::$server->getConfig()->setAppValue($filename, 'enabled', 'yes');
+									$config->setAppValue($filename, 'enabled', 'yes');
 								}
 							}
 						}

diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php
@@ -383,6 +383,7 @@ public function enable(string $appId,
 						   array $groups = []) {
 
 		// Check if app is already downloaded
+		/** @var Installer $installer */
 		$installer = \OC::$server->query(Installer::class);
 		$isDownloaded = $installer->isDownloaded($appId);
 

diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
@@ -67,7 +67,9 @@ public function getAppVersion(string $appId, bool $useCache = true): string;
 	public function isEnabledForUser($appId, $user = null);
 
 	/**
-	 * Check if an app is installed in the instance
+	 * Check if an app is enabled in the instance
+	 *
+	 * Notice: This actually checks if the app is enabled and not only if it is installed.
 	 *
 	 * @param string $appId
 	 * @return bool

diff --git a/tests/lib/InstallerTest.php b/tests/lib/InstallerTest.php
@@ -103,9 +103,9 @@ public function testInstallApp() {
 			\OC::$server->getLogger(),
 			\OC::$server->getConfig()
 		);
-		$installer->installApp(self::$appid);
-		$isInstalled = Installer::isInstalled(self::$appid);
-		$this->assertTrue($isInstalled);
+		$this->assertNull(\OC::$server->getConfig()->getAppValue('testapp', 'enabled', null), 'Check that the app is not listed before installation');
+		$this->assertSame('testapp', $installer->installApp(self::$appid));
+		$this->assertSame('no', \OC::$server->getConfig()->getAppValue('testapp', 'enabled', null), 'Check that the app is listed after installation');
 		$this->assertSame('0.9', \OC::$server->getConfig()->getAppValue('testapp', 'installed_version'));
 		$installer->removeApp(self::$appid);
 	}