feat(lexicon): fix tests

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

6 files changed, 75 insertions, 81 deletions

diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 8c6f1ce78dc..eb4700020d2 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -84,6 +84,7 @@ class AppManager implements IAppManager {
 		private IEventDispatcher $dispatcher,
 		private LoggerInterface $logger,
 		private ServerVersion $serverVersion,
+		private ConfigManager $configManager,
 	) {
 	}
 
@@ -571,7 +572,7 @@ class AppManager implements IAppManager {
 		));
 		$this->clearAppsCache();
 
-		Server::get(ConfigManager::class)->migrateConfigLexiconKeys($appId);
+		$this->configManager->migrateConfigLexiconKeys($appId);
 	}
 
 	/**
@@ -631,7 +632,7 @@ class AppManager implements IAppManager {
 		));
 		$this->clearAppsCache();
 
-		Server::get(ConfigManager::class)->migrateConfigLexiconKeys($appId);
+		$this->configManager->migrateConfigLexiconKeys($appId);
 	}
 
 	/**
diff --git a/lib/private/Config/ConfigManager.php b/lib/private/Config/ConfigManager.php
index f31b7426dc0..08deb4c84d3 100644
--- a/lib/private/Config/ConfigManager.php
+++ b/lib/private/Config/ConfigManager.php
@@ -16,15 +16,21 @@ use NCU\Config\ValueType;
 use OC\AppConfig;
 use OCP\App\IAppManager;
 use OCP\IAppConfig;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 
+/**
+ * tools to maintains configurations
+ *
+ * @since 32.0.0
+ */
 class ConfigManager {
+	/** @var AppConfig|null $appConfig */
+	private ?IAppConfig $appConfig = null;
+	/** @var UserConfig|null $userConfig */
+	private ?IUserConfig $userConfig = null;
+
 	public function __construct(
-		/** @var AppConfig $appConfig */
-		private readonly IAppConfig $appConfig,
-		/** @var UserConfig $appConfig */
-		private readonly IUserConfig $userConfig,
-		private readonly IAppManager $appManager,
 		private readonly LoggerInterface $logger,
 	) {
 	}
@@ -40,18 +46,22 @@ class ConfigManager {
 	 *
 	 * @see ConfigLexiconEntry
 	 * @internal
+	 * @since 32.0.0
 	 * @param string|null $appId when set to NULL the method will be executed for all enabled apps of the instance
 	 */
 	public function migrateConfigLexiconKeys(?string $appId = null): void {
 		if ($appId === null) {
 			$this->migrateConfigLexiconKeys('core');
-			foreach ($this->appManager->getEnabledApps() as $app) {
+			$appManager = Server::get(IAppManager::class);
+			foreach ($appManager->getEnabledApps() as $app) {
 				$this->migrateConfigLexiconKeys($app);
 			}
 
 			return;
 		}
 
+		$this->loadConfigServices();
+
 		// it is required to ignore aliases when moving config values
 		$this->appConfig->ignoreLexiconAliases(true);
 		$this->userConfig->ignoreLexiconAliases(true);
@@ -65,29 +75,29 @@ class ConfigManager {
 	}
 
 	/**
+	 * config services cannot be load at __construct() or install will fail
+	 */
+	private function loadConfigServices(): void {
+		if ($this->appConfig === null) {
+			$this->appConfig = Server::get(IAppConfig::class);
+		}
+		if ($this->userConfig === null) {
+			$this->userConfig = Server::get(IUserConfig::class);
+		}
+	}
+
+	/**
 	 * Get details from lexicon related to AppConfig and search for entries with rename to initiate
 	 * a migration to new config key
 	 */
 	private function migrateAppConfigKeys(string $appId): void {
 		$lexicon = $this->appConfig->getConfigDetailsFromLexicon($appId);
-
-		// we store a list of config keys to compare with any 'copyFrom'
-		$keys = [];
-		foreach ($lexicon['entries'] as $entry) {
-			$keys[] = $entry->getKey();
-		}
-
 		foreach ($lexicon['entries'] as $entry) {
 			// only interested in entries with rename set
 			if ($entry->getRename() === null) {
 				continue;
 			}
 
-			if (in_array($entry->getRename(), $keys, true)) {
-				$this->logger->error('rename value should not exist as a valid config key within Lexicon');
-				continue;
-			}
-
 			// only migrate if rename config key has a value and the new config key hasn't
 			if ($this->appConfig->hasKey($appId, $entry->getRename())
 				&& !$this->appConfig->hasKey($appId, $entry->getKey())) {
@@ -110,24 +120,12 @@ class ConfigManager {
 	 */
 	private function migrateUserConfigKeys(string $appId): void {
 		$lexicon = $this->userConfig->getConfigDetailsFromLexicon($appId);
-
-		// we store a list of set keys to compare with any 'copyFrom'
-		$keys = [];
-		foreach ($lexicon['entries'] as $entry) {
-			$keys[] = $entry->getKey();
-		}
-
 		foreach ($lexicon['entries'] as $entry) {
 			// only interested in keys with rename set
 			if ($entry->getRename() === null) {
 				continue;
 			}
 
-			if (in_array($entry->getRename(), $keys, true)) {
-				$this->logger->error('rename value should not exist as a valid key within Lexicon');
-				continue;
-			}
-
 			foreach ($this->userConfig->getValuesByUsers($appId, $entry->getRename()) as $userId => $value) {
 				if ($this->userConfig->hasKey($userId, $appId, $entry->getKey())) {
 					continue;
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 6a3a6dfb0a8..c78decd90cb 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -570,9 +570,9 @@ class Server extends ServerContainer implements IServerContainer {
 			return new \OC\SystemConfig($config);
 		});
 
-		$this->registerAlias(IAppManager::class, AppManager::class);
 		$this->registerAlias(IAppConfig::class, \OC\AppConfig::class);
 		$this->registerAlias(IUserConfig::class, \OC\Config\UserConfig::class);
+		$this->registerAlias(IAppManager::class, AppManager::class);
 
 		$this->registerService(IFactory::class, function (Server $c) {
 			return new \OC\L10N\Factory(
diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
index 10aa14803eb..24982ab9e80 100644
--- a/lib/private/legacy/OC_App.php
+++ b/lib/private/legacy/OC_App.php
@@ -212,7 +212,7 @@ class OC_App {
 		array $groups = []) {
 		// Check if app is already downloaded
 		/** @var Installer $installer */
-		$installer = \OCP\Server::get(Installer::class);
+		$installer = Server::get(Installer::class);
 		$isDownloaded = $installer->isDownloaded($appId);
 
 		if (!$isDownloaded) {
@@ -247,7 +247,7 @@ class OC_App {
 			}
 		}
 
-		\OCP\Server::get(LoggerInterface::class)->error('No application directories are marked as writable.', ['app' => 'core']);
+		Server::get(LoggerInterface::class)->error('No application directories are marked as writable.', ['app' => 'core']);
 		return null;
 	}
 
@@ -311,7 +311,7 @@ class OC_App {
 	 * @param string $appId
 	 * @param bool $refreshAppPath should be set to true only during install/upgrade
 	 * @return string|false
-	 * @deprecated 11.0.0 use \OCP\Server::get(IAppManager)->getAppPath()
+	 * @deprecated 11.0.0 use Server::get(IAppManager)->getAppPath()
 	 */
 	public static function getAppPath(string $appId, bool $refreshAppPath = false) {
 		$appId = self::cleanAppId($appId);
@@ -350,7 +350,7 @@ class OC_App {
 	 */
 	public static function getAppVersionByPath(string $path): string {
 		$infoFile = $path . '/appinfo/info.xml';
-		$appData = \OCP\Server::get(IAppManager::class)->getAppInfoByPath($infoFile);
+		$appData = Server::get(IAppManager::class)->getAppInfoByPath($infoFile);
 		return $appData['version'] ?? '';
 	}
 
@@ -392,7 +392,7 @@ class OC_App {
 	 * @deprecated 20.0.0 Please register your alternative login option using the registerAlternativeLogin() on the RegistrationContext in your Application class implementing the OCP\Authentication\IAlternativeLogin interface
 	 */
 	public static function registerLogIn(array $entry) {
-		\OCP\Server::get(LoggerInterface::class)->debug('OC_App::registerLogIn() is deprecated, please register your alternative login option using the registerAlternativeLogin() on the RegistrationContext in your Application class implementing the OCP\Authentication\IAlternativeLogin interface');
+		Server::get(LoggerInterface::class)->debug('OC_App::registerLogIn() is deprecated, please register your alternative login option using the registerAlternativeLogin() on the RegistrationContext in your Application class implementing the OCP\Authentication\IAlternativeLogin interface');
 		self::$altLogin[] = $entry;
 	}
 
@@ -401,11 +401,11 @@ class OC_App {
 	 */
 	public static function getAlternativeLogIns(): array {
 		/** @var Coordinator $bootstrapCoordinator */
-		$bootstrapCoordinator = \OCP\Server::get(Coordinator::class);
+		$bootstrapCoordinator = Server::get(Coordinator::class);
 
 		foreach ($bootstrapCoordinator->getRegistrationContext()->getAlternativeLogins() as $registration) {
 			if (!in_array(IAlternativeLogin::class, class_implements($registration->getService()), true)) {
-				\OCP\Server::get(LoggerInterface::class)->error('Alternative login option {option} does not implement {interface} and is therefore ignored.', [
+				Server::get(LoggerInterface::class)->error('Alternative login option {option} does not implement {interface} and is therefore ignored.', [
 					'option' => $registration->getService(),
 					'interface' => IAlternativeLogin::class,
 					'app' => $registration->getAppId(),
@@ -415,9 +415,9 @@ class OC_App {
 
 			try {
 				/** @var IAlternativeLogin $provider */
-				$provider = \OCP\Server::get($registration->getService());
+				$provider = Server::get($registration->getService());
 			} catch (ContainerExceptionInterface $e) {
-				\OCP\Server::get(LoggerInterface::class)->error('Alternative login option {option} can not be initialized.',
+				Server::get(LoggerInterface::class)->error('Alternative login option {option} can not be initialized.',
 					[
 						'exception' => $e,
 						'option' => $registration->getService(),
@@ -434,7 +434,7 @@ class OC_App {
 					'class' => $provider->getClass(),
 				];
 			} catch (Throwable $e) {
-				\OCP\Server::get(LoggerInterface::class)->error('Alternative login option {option} had an error while loading.',
+				Server::get(LoggerInterface::class)->error('Alternative login option {option} had an error while loading.',
 					[
 						'exception' => $e,
 						'option' => $registration->getService(),
@@ -453,7 +453,7 @@ class OC_App {
 	 * @deprecated 31.0.0 Use IAppManager::getAllAppsInAppsFolders instead
 	 */
 	public static function getAllApps(): array {
-		return \OCP\Server::get(IAppManager::class)->getAllAppsInAppsFolders();
+		return Server::get(IAppManager::class)->getAllAppsInAppsFolders();
 	}
 
 	/**
@@ -462,7 +462,7 @@ class OC_App {
 	 * @deprecated 32.0.0 Use \OCP\Support\Subscription\IRegistry::delegateGetSupportedApps instead
 	 */
 	public function getSupportedApps(): array {
-		$subscriptionRegistry = \OCP\Server::get(\OCP\Support\Subscription\IRegistry::class);
+		$subscriptionRegistry = Server::get(\OCP\Support\Subscription\IRegistry::class);
 		$supportedApps = $subscriptionRegistry->delegateGetSupportedApps();
 		return $supportedApps;
 	}
@@ -487,12 +487,12 @@ class OC_App {
 			if (!in_array($app, $blacklist)) {
 				$info = $appManager->getAppInfo($app, false, $langCode);
 				if (!is_array($info)) {
-					\OCP\Server::get(LoggerInterface::class)->error('Could not read app info file for app "' . $app . '"', ['app' => 'core']);
+					Server::get(LoggerInterface::class)->error('Could not read app info file for app "' . $app . '"', ['app' => 'core']);
 					continue;
 				}
 
 				if (!isset($info['name'])) {
-					\OCP\Server::get(LoggerInterface::class)->error('App id "' . $app . '" has no name in appinfo', ['app' => 'core']);
+					Server::get(LoggerInterface::class)->error('App id "' . $app . '" has no name in appinfo', ['app' => 'core']);
 					continue;
 				}
 
@@ -559,7 +559,7 @@ class OC_App {
 
 	public static function shouldUpgrade(string $app): bool {
 		$versions = self::getAppVersions();
-		$currentVersion = \OCP\Server::get(\OCP\App\IAppManager::class)->getAppVersion($app);
+		$currentVersion = Server::get(\OCP\App\IAppManager::class)->getAppVersion($app);
 		if ($currentVersion && isset($versions[$app])) {
 			$installedVersion = $versions[$app];
 			if (!version_compare($currentVersion, $installedVersion, '=')) {
@@ -648,7 +648,7 @@ class OC_App {
 	 * @deprecated 32.0.0 Use IAppManager::getAppInstalledVersions or IAppConfig::getAppInstalledVersions instead
 	 */
 	public static function getAppVersions(): array {
-		return \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions();
+		return Server::get(IAppConfig::class)->getAppInstalledVersions();
 	}
 
 	/**
@@ -666,13 +666,13 @@ class OC_App {
 		}
 
 		if (is_file($appPath . '/appinfo/database.xml')) {
-			\OCP\Server::get(LoggerInterface::class)->error('The appinfo/database.xml file is not longer supported. Used in ' . $appId);
+			Server::get(LoggerInterface::class)->error('The appinfo/database.xml file is not longer supported. Used in ' . $appId);
 			return false;
 		}
 
 		\OC::$server->getAppManager()->clearAppsCache();
 		$l = \OC::$server->getL10N('core');
-		$appData = \OCP\Server::get(\OCP\App\IAppManager::class)->getAppInfo($appId, false, $l->getLanguageCode());
+		$appData = Server::get(\OCP\App\IAppManager::class)->getAppInfo($appId, false, $l->getLanguageCode());
 
 		$ignoreMaxApps = \OC::$server->getConfig()->getSystemValue('app_install_overwrite', []);
 		$ignoreMax = in_array($appId, $ignoreMaxApps, true);
@@ -712,10 +712,11 @@ class OC_App {
 
 		self::setAppTypes($appId);
 
-		$version = \OCP\Server::get(\OCP\App\IAppManager::class)->getAppVersion($appId);
+		$version = Server::get(\OCP\App\IAppManager::class)->getAppVersion($appId);
 		\OC::$server->getConfig()->setAppValue($appId, 'installed_version', $version);
 
 		// migrate eventual new config keys in the process
+		/** @psalm-suppress InternalMethod */
 		Server::get(ConfigManager::class)->migrateConfigLexiconKeys($appId);
 
 		\OC::$server->get(IEventDispatcher::class)->dispatchTyped(new AppUpdateEvent($appId));
diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php
index 3dd5073731c..5cd141c16a9 100644
--- a/tests/lib/App/AppManagerTest.php
+++ b/tests/lib/App/AppManagerTest.php
@@ -12,6 +12,7 @@ namespace Test\App;
 
 use OC\App\AppManager;
 use OC\AppConfig;
+use OC\Config\ConfigManager;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\Events\AppDisableEvent;
 use OCP\App\Events\AppEnableEvent;
@@ -36,10 +37,7 @@ use Test\TestCase;
  * @package Test\App
  */
 class AppManagerTest extends TestCase {
-	/**
-	 * @return AppConfig|MockObject
-	 */
-	protected function getAppConfig() {
+	protected function getAppConfig(): AppConfig&MockObject {
 		$appConfig = [];
 		$config = $this->createMock(AppConfig::class);
 
@@ -86,33 +84,17 @@ class AppManagerTest extends TestCase {
 		return $config;
 	}
 
-	/** @var IUserSession|MockObject */
-	protected $userSession;
-
-	/** @var IConfig|MockObject */
-	private $config;
-
-	/** @var IGroupManager|MockObject */
-	protected $groupManager;
-
-	/** @var AppConfig|MockObject */
-	protected $appConfig;
-
-	/** @var ICache|MockObject */
-	protected $cache;
-
-	/** @var ICacheFactory|MockObject */
-	protected $cacheFactory;
-
-	/** @var IEventDispatcher|MockObject */
-	protected $eventDispatcher;
-
-	/** @var LoggerInterface|MockObject */
-	protected $logger;
-
+	protected IUserSession&MockObject $userSession;
+	private IConfig&MockObject $config;
+	protected IGroupManager&MockObject $groupManager;
+	protected AppConfig&MockObject $appConfig;
+	protected ICache&MockObject $cache;
+	protected ICacheFactory&MockObject $cacheFactory;
+	protected IEventDispatcher&MockObject $eventDispatcher;
+	protected LoggerInterface&MockObject $logger;
 	protected IURLGenerator&MockObject $urlGenerator;
-
 	protected ServerVersion&MockObject $serverVersion;
+	protected ConfigManager&MockObject $configManager;
 
 	/** @var IAppManager */
 	protected $manager;
@@ -130,6 +112,7 @@ class AppManagerTest extends TestCase {
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->serverVersion = $this->createMock(ServerVersion::class);
+		$this->configManager = $this->createMock(ConfigManager::class);
 
 		$this->overwriteService(AppConfig::class, $this->appConfig);
 		$this->overwriteService(IURLGenerator::class, $this->urlGenerator);
@@ -152,6 +135,7 @@ class AppManagerTest extends TestCase {
 			$this->eventDispatcher,
 			$this->logger,
 			$this->serverVersion,
+			$this->configManager,
 		);
 	}
 
@@ -295,6 +279,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppPath',
@@ -349,6 +334,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppPath',
@@ -411,6 +397,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppPath',
@@ -616,6 +603,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods(['getAppInfo'])
 			->getMock();
@@ -676,6 +664,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods(['getAppInfo'])
 			->getMock();
@@ -817,6 +806,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppInfo',
@@ -848,6 +838,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppInfo',
@@ -878,6 +869,7 @@ class AppManagerTest extends TestCase {
 				$this->eventDispatcher,
 				$this->logger,
 				$this->serverVersion,
+				$this->configManager,
 			])
 			->onlyMethods([
 				'getAppInfo',
diff --git a/tests/lib/AppTest.php b/tests/lib/AppTest.php
index 4813ea8c839..07205c730ce 100644
--- a/tests/lib/AppTest.php
+++ b/tests/lib/AppTest.php
@@ -10,6 +10,7 @@ namespace Test;
 use OC\App\AppManager;
 use OC\App\InfoParser;
 use OC\AppConfig;
+use OC\Config\ConfigManager;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAppConfig;
 use OCP\ICacheFactory;
@@ -573,6 +574,7 @@ class AppTest extends \Test\TestCase {
 			Server::get(IEventDispatcher::class),
 			Server::get(LoggerInterface::class),
 			Server::get(ServerVersion::class),
+			\OCP\Server::get(ConfigManager::class),
 		));
 	}