fix: Replace OC_App::getAllApps with a method in AppManager

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
nextcloud · Sep 13, 2024 · 76f2bc0 · 76f2bc0
1 parent 7ed583c
commit 76f2bc0
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 55 deletions.
diff --git a/core/Command/App/Enable.php b/core/Command/App/Enable.php
@@ -145,7 +145,7 @@ public function completeOptionValues($optionName, CompletionContext $context): a
 	 */
 	public function completeArgumentValues($argumentName, CompletionContext $context): array {
 		if ($argumentName === 'app-id') {
-			$allApps = \OC_App::getAllApps();
+			$allApps = $this->appManager->getAllAppsInAppsFolders();
 			return array_diff($allApps, \OC_App::getEnabledApps(true, true));
 		}
 		return [];

diff --git a/core/Command/App/GetPath.php b/core/Command/App/GetPath.php
@@ -63,7 +63,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 	 */
 	public function completeArgumentValues($argumentName, CompletionContext $context): array {
 		if ($argumentName === 'app') {
-			return \OC_App::getAllApps();
+			return $this->appManager->getAllAppsInAppsFolders();
 		}
 		return [];
 	}

diff --git a/core/Command/App/Update.php b/core/Command/App/Update.php
@@ -69,7 +69,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 				return 1;
 			}
 		} elseif ($input->getOption('all') || $input->getOption('showonly')) {
-			$apps = \OC_App::getAllApps();
+			$apps = $this->manager->getAllAppsInAppsFolders();
 		} else {
 			$output->writeln('<error>Please specify an app to update or "--all" to update all updatable apps"</error>');
 			return 1;

diff --git a/core/Command/Db/Migrations/GenerateCommand.php b/core/Command/Db/Migrations/GenerateCommand.php
@@ -71,13 +71,10 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
 }
 ';
 
-	protected Connection $connection;
-	protected IAppManager $appManager;
-
-	public function __construct(Connection $connection, IAppManager $appManager) {
-		$this->connection = $connection;
-		$this->appManager = $appManager;
-
+	public function __construct(
+		protected Connection $connection,
+		protected IAppManager $appManager,
+	) {
 		parent::__construct();
 	}
 
@@ -155,7 +152,7 @@ public function completeOptionValues($optionName, CompletionContext $context) {
 	 */
 	public function completeArgumentValues($argumentName, CompletionContext $context) {
 		if ($argumentName === 'app') {
-			$allApps = \OC_App::getAllApps();
+			$allApps = $this->appManager->getAllAppsInAppsFolders();
 			return array_diff($allApps, \OC_App::getEnabledApps(true, true));
 		}
 

diff --git a/core/Command/Db/Migrations/GenerateMetadataCommand.php b/core/Command/Db/Migrations/GenerateMetadataCommand.php
@@ -64,7 +64,7 @@ private function extractMigrationMetadataFromCore(): array {
 	 * @throws \Exception
 	 */
 	private function extractMigrationMetadataFromApps(): array {
-		$allApps = \OC_App::getAllApps();
+		$allApps = $this->appManager->getAllAppsInAppsFolders();
 		$metadata = [];
 		foreach ($allApps as $appId) {
 			// We need to load app before being able to extract Migrations

diff --git a/core/Command/L10n/CreateJs.php b/core/Command/L10n/CreateJs.php
@@ -148,7 +148,7 @@ public function completeOptionValues($optionName, CompletionContext $context) {
 	 */
 	public function completeArgumentValues($argumentName, CompletionContext $context) {
 		if ($argumentName === 'app') {
-			return \OC_App::getAllApps();
+			return $this->appManager->getAllAppsInAppsFolders();
 		} elseif ($argumentName === 'lang') {
 			$appName = $context->getWordAtIndex($context->getWordIndex() - 1);
 			try {

diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -155,6 +155,37 @@ public function getInstalledApps() {
 		return array_keys($this->getInstalledAppsValues());
 	}
 
+	/**
+	 * Get a list of all apps in the apps folder
+	 *
+	 * @return list<string> an array of app names (string IDs)
+	 */
+	public function getAllAppsInAppsFolders(): array {
+		$apps = [];
+
+		foreach (\OC::$APPSROOTS as $apps_dir) {
+			if (!is_readable($apps_dir['path'])) {
+				$this->logger->warning('unable to read app folder : ' . $apps_dir['path'], ['app' => 'core']);
+				continue;
+			}
+			$dh = opendir($apps_dir['path']);
+
+			if (is_resource($dh)) {
+				while (($file = readdir($dh)) !== false) {
+					if (
+						$file[0] != '.' &&
+						is_dir($apps_dir['path'] . '/' . $file) &&
+						is_file($apps_dir['path'] . '/' . $file . '/appinfo/info.xml')
+					) {
+						$apps[] = $file;
+					}
+				}
+			}
+		}
+
+		return array_values(array_unique($apps));
+	}
+
 	/**
 	 * List all apps enabled for a user
 	 *

diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php
@@ -46,7 +46,7 @@ public function __construct(
 		private ?IConfig $config,
 		private ?IAppConfig $appConfig,
 		ICacheFactory $cacheFactory,
-		private ?IAppManager $appManager,
+		private IAppManager $appManager,
 		private IMimeTypeDetector $mimeTypeDetector,
 	) {
 		$this->cache = $cacheFactory->createDistributed(self::CACHE_KEY);
@@ -536,7 +536,7 @@ public function verifyCoreSignature(): array {
 	public function runInstanceVerification() {
 		$this->cleanResults();
 		$this->verifyCoreSignature();
-		$appIds = $this->appLocator->getAllApps();
+		$appIds = $this->appManager->getAllAppsInAppsFolders();
 		foreach ($appIds as $appId) {
 			// If an application is shipped a valid signature is required
 			$isShipped = $this->appManager->isShipped($appId);

diff --git a/lib/private/IntegrityCheck/Helpers/AppLocator.php b/lib/private/IntegrityCheck/Helpers/AppLocator.php
@@ -30,13 +30,4 @@ public function getAppPath(string $appId): string {
 		}
 		return $path;
 	}
-
-	/**
-	 * Providers \OC_App::getAllApps()
-	 *
-	 * @return array
-	 */
-	public function getAllApps(): array {
-		return \OC_App::getAllApps();
-	}
 }
diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -954,10 +954,10 @@ public function __construct($webRoot, \OC\Config $config) {
 			if (\OC::$server->get(SystemConfig::class)->getValue('installed', false)) {
 				$config = $c->get(\OCP\IConfig::class);
 				$appConfig = $c->get(\OCP\IAppConfig::class);
-				$appManager = $c->get(IAppManager::class);
 			} else {
 				$config = $appConfig = $appManager = null;
 			}
+			$appManager = $c->get(IAppManager::class);
 
 			return new Checker(
 				new EnvironmentHelper(),

diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
@@ -468,30 +468,10 @@ public static function getAlternativeLogIns(): array {
 	 * get a list of all apps in the apps folder
 	 *
 	 * @return string[] an array of app names (string IDs)
-	 * @todo: change the name of this method to getInstalledApps, which is more accurate
+	 * @deprecated 31.0.0 Use IAppManager::getAllAppsInAppsFolders instead
 	 */
 	public static function getAllApps(): array {
-		$apps = [];
-
-		foreach (OC::$APPSROOTS as $apps_dir) {
-			if (!is_readable($apps_dir['path'])) {
-				\OCP\Server::get(LoggerInterface::class)->warning('unable to read app folder : ' . $apps_dir['path'], ['app' => 'core']);
-				continue;
-			}
-			$dh = opendir($apps_dir['path']);
-
-			if (is_resource($dh)) {
-				while (($file = readdir($dh)) !== false) {
-					if ($file[0] != '.' and is_dir($apps_dir['path'] . '/' . $file) and is_file($apps_dir['path'] . '/' . $file . '/appinfo/info.xml')) {
-						$apps[] = $file;
-					}
-				}
-			}
-		}
-
-		$apps = array_unique($apps);
-
-		return $apps;
+		return \OCP\Server::get(IAppManager::class)->getAllAppsInAppsFolders();
 	}
 
 	/**
@@ -512,9 +492,9 @@ public function getSupportedApps(): array {
 	 * @return array
 	 */
 	public function listAllApps(): array {
-		$installedApps = OC_App::getAllApps();
-
 		$appManager = \OC::$server->getAppManager();
+
+		$installedApps = $appManager->getAllAppsInAppsFolders();
 		//we don't want to show configuration for these
 		$blacklist = $appManager->getAlwaysEnabledApps();
 		$appList = [];

diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
@@ -292,4 +292,12 @@ public function isBackendRequired(string $backend): bool;
 	 * @since 31.0.0
 	 */
 	public function cleanAppId(string $app): string;
+
+	/**
+	 * Get a list of all apps in the apps folder
+	 *
+	 * @return list<string> an array of app names (string IDs)
+	 * @since 31.0.0
+	 */
+	public function getAllAppsInAppsFolders(): array;
 }
diff --git a/tests/lib/IntegrityCheck/CheckerTest.php b/tests/lib/IntegrityCheck/CheckerTest.php
@@ -1030,9 +1030,9 @@ public function testRunInstanceVerification() {
 		$this->checker
 			->expects($this->once())
 			->method('verifyCoreSignature');
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
-			->method('getAllApps')
+			->method('getAllAppsInAppsFolders')
 			->willReturn([
 				'files',
 				'calendar',

diff --git a/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php b/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php
@@ -30,8 +30,4 @@ public function testGetAppPathNotExistentApp() {
 
 		$this->locator->getAppPath('aTotallyNotExistingApp');
 	}
-
-	public function testGetAllApps() {
-		$this->assertSame(\OC_App::getAllApps(), $this->locator->getAllApps());
-	}
 }