Merge pull request #51676 from nextcloud/fix/add-getappversions-replacement

Add OC_App::getAppVersions replacement in IAppManager

12 files changed, 101 insertions, 54 deletions

diff --git a/apps/updatenotification/lib/Notification/Notifier.php b/apps/updatenotification/lib/Notification/Notifier.php
index 8cee44144b2..8ce6bfc2d3d 100644
--- a/apps/updatenotification/lib/Notification/Notifier.php
+++ b/apps/updatenotification/lib/Notification/Notifier.php
@@ -20,8 +20,7 @@ use OCP\Notification\IManager;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
 use OCP\Notification\UnknownNotificationException;
-use OCP\Server;
-use OCP\Util;
+use OCP\ServerVersion;
 
 class Notifier implements INotifier {
 	/** @var string[] */
@@ -44,8 +43,10 @@ class Notifier implements INotifier {
 		protected IFactory $l10NFactory,
 		protected IUserSession $userSession,
 		protected IGroupManager $groupManager,
+		protected IAppManager $appManager,
+		protected ServerVersion $serverVersion,
 	) {
-		$this->appVersions = $this->getAppVersions();
+		$this->appVersions = $this->appManager->getAppInstalledVersions();
 	}
 
 	/**
@@ -144,15 +145,12 @@ class Notifier implements INotifier {
 	 * @param string $installedVersion
 	 * @throws AlreadyProcessedException When the update is already installed
 	 */
-	protected function updateAlreadyInstalledCheck(INotification $notification, $installedVersion) {
+	protected function updateAlreadyInstalledCheck(INotification $notification, $installedVersion): void {
 		if (version_compare($notification->getObjectId(), $installedVersion, '<=')) {
 			throw new AlreadyProcessedException();
 		}
 	}
 
-	/**
-	 * @return bool
-	 */
 	protected function isAdmin(): bool {
 		$user = $this->userSession->getUser();
 
@@ -164,14 +162,10 @@ class Notifier implements INotifier {
 	}
 
 	protected function getCoreVersions(): string {
-		return implode('.', Util::getVersion());
-	}
-
-	protected function getAppVersions(): array {
-		return \OC_App::getAppVersions();
+		return implode('.', $this->serverVersion->getVersion());
 	}
 
-	protected function getAppInfo($appId, $languageCode) {
-		return Server::get(IAppManager::class)->getAppInfo($appId, false, $languageCode);
+	protected function getAppInfo(string $appId, ?string $languageCode): ?array {
+		return $this->appManager->getAppInfo($appId, false, $languageCode);
 	}
 }
diff --git a/apps/updatenotification/tests/Notification/NotifierTest.php b/apps/updatenotification/tests/Notification/NotifierTest.php
index 1e53b8d4aea..45c4fc9960f 100644
--- a/apps/updatenotification/tests/Notification/NotifierTest.php
+++ b/apps/updatenotification/tests/Notification/NotifierTest.php
@@ -9,6 +9,7 @@ declare(strict_types=1);
 namespace OCA\UpdateNotification\Tests\Notification;
 
 use OCA\UpdateNotification\Notification\Notifier;
+use OCP\App\IAppManager;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -17,22 +18,20 @@ use OCP\L10N\IFactory;
 use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
+use OCP\ServerVersion;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class NotifierTest extends TestCase {
 
-	/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
-	protected $urlGenerator;
-	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
-	protected $config;
-	/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
-	protected $notificationManager;
-	/** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */
-	protected $l10nFactory;
-	/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
-	protected $userSession;
-	/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
-	protected $groupManager;
+	protected IURLGenerator&MockObject $urlGenerator;
+	protected IConfig&MockObject $config;
+	protected IManager&MockObject $notificationManager;
+	protected IFactory&MockObject $l10nFactory;
+	protected IUserSession&MockObject $userSession;
+	protected IGroupManager&MockObject $groupManager;
+	protected IAppManager&MockObject $appManager;
+	protected ServerVersion&MockObject $serverVersion;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -43,6 +42,8 @@ class NotifierTest extends TestCase {
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->appManager = $this->createMock(IAppManager::class);
+		$this->serverVersion = $this->createMock(ServerVersion::class);
 	}
 
 	/**
@@ -57,7 +58,9 @@ class NotifierTest extends TestCase {
 				$this->notificationManager,
 				$this->l10nFactory,
 				$this->userSession,
-				$this->groupManager
+				$this->groupManager,
+				$this->appManager,
+				$this->serverVersion,
 			);
 		}
 		{
@@ -69,6 +72,8 @@ class NotifierTest extends TestCase {
 					$this->l10nFactory,
 					$this->userSession,
 					$this->groupManager,
+					$this->appManager,
+					$this->serverVersion,
 				])
 				->onlyMethods($methods)
 				->getMock();
diff --git a/core/Command/App/ListApps.php b/core/Command/App/ListApps.php
index 5244d66e0d8..bb59e441119 100644
--- a/core/Command/App/ListApps.php
+++ b/core/Command/App/ListApps.php
@@ -14,7 +14,7 @@ use Symfony\Component\Console\Output\OutputInterface;
 
 class ListApps extends Base {
 	public function __construct(
-		protected IAppManager $manager,
+		protected IAppManager $appManager,
 	) {
 		parent::__construct();
 	}
@@ -56,16 +56,16 @@ class ListApps extends Base {
 		$showEnabledApps = $input->getOption('enabled') || !$input->getOption('disabled');
 		$showDisabledApps = $input->getOption('disabled') || !$input->getOption('enabled');
 
-		$apps = \OC_App::getAllApps();
+		$apps = $this->appManager->getAllAppsInAppsFolders();
 		$enabledApps = $disabledApps = [];
-		$versions = \OC_App::getAppVersions();
+		$versions = $this->appManager->getAppInstalledVersions();
 
 		//sort enabled apps above disabled apps
 		foreach ($apps as $app) {
-			if ($shippedFilter !== null && $this->manager->isShipped($app) !== $shippedFilter) {
+			if ($shippedFilter !== null && $this->appManager->isShipped($app) !== $shippedFilter) {
 				continue;
 			}
-			if ($this->manager->isEnabledForAnyone($app)) {
+			if ($this->appManager->isEnabledForAnyone($app)) {
 				$enabledApps[] = $app;
 			} else {
 				$disabledApps[] = $app;
@@ -88,7 +88,7 @@ class ListApps extends Base {
 
 			sort($disabledApps);
 			foreach ($disabledApps as $app) {
-				$apps['disabled'][$app] = $this->manager->getAppVersion($app) . (isset($versions[$app]) ? ' (installed ' . $versions[$app] . ')' : '');
+				$apps['disabled'][$app] = $this->appManager->getAppVersion($app) . (isset($versions[$app]) ? ' (installed ' . $versions[$app] . ')' : '');
 			}
 		}
 
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 067cc89d674..740da31770d 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -812,6 +812,15 @@ class AppManager implements IAppManager {
 	}
 
 	/**
+	 * Returns the installed versions of all apps
+	 *
+	 * @return array<string, string>
+	 */
+	public function getAppInstalledVersions(): array {
+		return $this->getAppConfig()->getAppInstalledVersions();
+	}
+
+	/**
 	 * Returns a list of apps incompatible with the given version
 	 *
 	 * @param string $version Nextcloud version as array of version components
diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php
index ad9299f01fd..1228b9c20e3 100644
--- a/lib/private/AppConfig.php
+++ b/lib/private/AppConfig.php
@@ -62,6 +62,9 @@ class AppConfig implements IAppConfig {
 	/** @var array<array-key, array{entries: array<array-key, ConfigLexiconEntry>, strictness: ConfigLexiconStrictness}> ['app_id' => ['strictness' => ConfigLexiconStrictness, 'entries' => ['config_key' => ConfigLexiconEntry[]]] */
 	private array $configLexiconDetails = [];
 
+	/** @var ?array<string, string> */
+	private ?array $appVersionsCache = null;
+
 	public function __construct(
 		protected IDBConnection $connection,
 		protected LoggerInterface $logger,
@@ -1647,4 +1650,17 @@ class AppConfig implements IAppConfig {
 
 		return $this->configLexiconDetails[$appId];
 	}
+
+	/**
+	 * Returns the installed versions of all apps
+	 *
+	 * @return array<string, string>
+	 */
+	public function getAppInstalledVersions(): array {
+		if ($this->appVersionsCache === null) {
+			/** @var array<string, string> */
+			$this->appVersionsCache = $this->searchValues('installed_version', false, IAppConfig::VALUE_STRING);
+		}
+		return $this->appVersionsCache;
+	}
 }
diff --git a/lib/private/AppFramework/Services/AppConfig.php b/lib/private/AppFramework/Services/AppConfig.php
index 423a9eb5814..77c5ea4de0c 100644
--- a/lib/private/AppFramework/Services/AppConfig.php
+++ b/lib/private/AppFramework/Services/AppConfig.php
@@ -337,4 +337,13 @@ class AppConfig implements IAppConfig {
 	public function deleteUserValue(string $userId, string $key): void {
 		$this->config->deleteUserValue($userId, $this->appName, $key);
 	}
+
+	/**
+	 * Returns the installed versions of all apps
+	 *
+	 * @return array<string, string>
+	 */
+	public function getAppInstalledVersions(): array {
+		return $this->appConfig->getAppInstalledVersions();
+	}
 }
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 523fc7b9914..4d79fefd261 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -607,10 +607,10 @@ class Server extends ServerContainer implements IServerContainer {
 
 			if ($config->getValue('installed', false) && !(defined('PHPUNIT_RUN') && PHPUNIT_RUN)) {
 				$logQuery = $config->getValue('log_query');
-				$prefixClosure = function () use ($logQuery, $serverVersion) {
+				$prefixClosure = function () use ($logQuery, $serverVersion): ?string {
 					if (!$logQuery) {
 						try {
-							$v = \OC_App::getAppVersions();
+							$v = \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions();
 						} catch (\Doctrine\DBAL\Exception $e) {
 							// Database service probably unavailable
 							// Probably related to https://github.com/nextcloud/server/issues/37424
@@ -867,15 +867,15 @@ class Server extends ServerContainer implements IServerContainer {
 
 		$this->registerDeprecatedAlias('IntegrityCodeChecker', Checker::class);
 		$this->registerService(Checker::class, function (ContainerInterface $c) {
-			// IConfig and IAppManager requires a working database. This code
-			// might however be called when ownCloud is not yet setup.
+			// IConfig requires a working database. This code
+			// might however be called when Nextcloud is not yet setup.
 			if (\OC::$server->get(SystemConfig::class)->getValue('installed', false)) {
 				$config = $c->get(\OCP\IConfig::class);
 				$appConfig = $c->get(\OCP\IAppConfig::class);
 			} else {
-				$config = $appConfig = $appManager = null;
+				$config = null;
+				$appConfig = null;
 			}
-			$appManager = $c->get(IAppManager::class);
 
 			return new Checker(
 				$c->get(ServerVersion::class),
@@ -885,7 +885,7 @@ class Server extends ServerContainer implements IServerContainer {
 				$config,
 				$appConfig,
 				$c->get(ICacheFactory::class),
-				$appManager,
+				$c->get(IAppManager::class),
 				$c->get(IMimeTypeDetector::class)
 			);
 		});
diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index 9c1d7ca1d2c..01d22edc0fb 100644
--- a/lib/private/TemplateLayout.php
+++ b/lib/private/TemplateLayout.php
@@ -48,6 +48,7 @@ class TemplateLayout {
 		private InitialStateService $initialState,
 		private INavigationManager $navigationManager,
 		private ITemplateManager $templateManager,
+		private ServerVersion $serverVersion,
 	) {
 	}
 
@@ -204,8 +205,8 @@ class TemplateLayout {
 
 		if ($this->config->getSystemValueBool('installed', false)) {
 			if (empty(self::$versionHash)) {
-				$v = \OC_App::getAppVersions();
-				$v['core'] = implode('.', \OCP\Util::getVersion());
+				$v = $this->appManager->getAppInstalledVersions();
+				$v['core'] = implode('.', $this->serverVersion->getVersion());
 				self::$versionHash = substr(md5(implode(',', $v)), 0, 8);
 			}
 		} else {
@@ -220,7 +221,7 @@ class TemplateLayout {
 			// this is on purpose outside of the if statement below so that the initial state is prefilled (done in the getConfig() call)
 			// see https://github.com/nextcloud/server/pull/22636 for details
 			$jsConfigHelper = new JSConfigHelper(
-				\OCP\Server::get(ServerVersion::class),
+				$this->serverVersion,
 				\OCP\Util::getL10N('lib'),
 				\OCP\Server::get(Defaults::class),
 				$this->appManager,
diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
index 7fee946b776..ecceafa65b3 100644
--- a/lib/private/legacy/OC_App.php
+++ b/lib/private/legacy/OC_App.php
@@ -456,10 +456,9 @@ class OC_App {
 	/**
 	 * List all supported apps
 	 *
-	 * @return array
+	 * @deprecated 32.0.0 Use \OCP\Support\Subscription\IRegistry::delegateGetSupportedApps instead
 	 */
 	public function getSupportedApps(): array {
-		/** @var \OCP\Support\Subscription\IRegistry $subscriptionRegistry */
 		$subscriptionRegistry = \OCP\Server::get(\OCP\Support\Subscription\IRegistry::class);
 		$supportedApps = $subscriptionRegistry->delegateGetSupportedApps();
 		return $supportedApps;
@@ -643,16 +642,10 @@ class OC_App {
 
 	/**
 	 * get the installed version of all apps
+	 * @deprecated 32.0.0 Use IAppManager::getAppInstalledVersions or IAppConfig::getAppInstalledVersions instead
 	 */
-	public static function getAppVersions() {
-		static $versions;
-
-		if (!$versions) {
-			/** @var IAppConfig $appConfig */
-			$appConfig = \OCP\Server::get(IAppConfig::class);
-			$versions = $appConfig->searchValues('installed_version');
-		}
-		return $versions;
+	public static function getAppVersions(): array {
+		return \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions();
 	}
 
 	/**
diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
index fa35819b779..67ef2d796be 100644
--- a/lib/public/App/IAppManager.php
+++ b/lib/public/App/IAppManager.php
@@ -52,6 +52,14 @@ interface IAppManager {
 	public function getAppVersion(string $appId, bool $useCache = true): string;
 
 	/**
+	 * Returns the installed version of all apps
+	 *
+	 * @return array<string, string>
+	 * @since 32.0.0
+	 */
+	public function getAppInstalledVersions(): array;
+
+	/**
 	 * Returns the app icon or null if none is found
 	 *
 	 * @param string $appId
diff --git a/lib/public/IAppConfig.php b/lib/public/IAppConfig.php
index d4d5c1c09c7..f4210793476 100644
--- a/lib/public/IAppConfig.php
+++ b/lib/public/IAppConfig.php
@@ -507,4 +507,12 @@ interface IAppConfig {
 	 * @deprecated 29.0.0 Use {@see getAllValues()} or {@see searchValues()}
 	 */
 	public function getFilteredValues($app);
+
+	/**
+	 * Returns the installed version of all apps
+	 *
+	 * @return array<string, string>
+	 * @since 32.0.0
+	 */
+	public function getAppInstalledVersions(): array;
 }
diff --git a/tests/lib/TemplateLayoutTest.php b/tests/lib/TemplateLayoutTest.php
index 29e31b3f391..b3895525408 100644
--- a/tests/lib/TemplateLayoutTest.php
+++ b/tests/lib/TemplateLayoutTest.php
@@ -15,6 +15,7 @@ use OCP\App\IAppManager;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
 use OCP\INavigationManager;
+use OCP\ServerVersion;
 use OCP\Template\ITemplateManager;
 use PHPUnit\Framework\MockObject\MockObject;
 
@@ -24,6 +25,7 @@ class TemplateLayoutTest extends \Test\TestCase {
 	private InitialStateService&MockObject $initialState;
 	private INavigationManager&MockObject $navigationManager;
 	private ITemplateManager&MockObject $templateManager;
+	private ServerVersion&MockObject $serverVersion;
 
 	private TemplateLayout $templateLayout;
 
@@ -35,6 +37,7 @@ class TemplateLayoutTest extends \Test\TestCase {
 		$this->initialState = $this->createMock(InitialStateService::class);
 		$this->navigationManager = $this->createMock(INavigationManager::class);
 		$this->templateManager = $this->createMock(ITemplateManager::class);
+		$this->serverVersion = $this->createMock(ServerVersion::class);
 	}
 
 	/** @dataProvider dataVersionHash */
@@ -69,6 +72,7 @@ class TemplateLayoutTest extends \Test\TestCase {
 				$this->initialState,
 				$this->navigationManager,
 				$this->templateManager,
+				$this->serverVersion,
 			])
 			->getMock();