Lazy grouping within IAppConfig

apps/provisioning_api/lib/Controller/AppConfigController.php

@@ -27,12 +27,12 @@
  */
 namespace OCA\Provisioning_API\Controller;
 
+use OC\AppConfig;
 use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\IAppConfig;
-use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -42,46 +42,17 @@
 use OCP\Settings\IManager;
 
 class AppConfigController extends OCSController {
-
-	/** @var IConfig */
-	protected $config;
-
-	/** @var IAppConfig */
-	protected $appConfig;
-
-	/** @var IUserSession */
-	private $userSession;
-
-	/** @var IL10N */
-	private $l10n;
-
-	/** @var IGroupManager */
-	private $groupManager;
-
-	/** @var IManager */
-	private $settingManager;
-
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IConfig $config
-	 * @param IAppConfig $appConfig
-	 */
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
-		IConfig $config,
-		IAppConfig $appConfig,
-		IUserSession $userSession,
-		IL10N $l10n,
-		IGroupManager $groupManager,
-		IManager $settingManager) {
+		/** @var AppConfig */
+		private IAppConfig $appConfig,
+		private IUserSession $userSession,
+		private IL10N $l10n,
+		private IGroupManager $groupManager,
+		private IManager $settingManager,
+	) {
 		parent::__construct($appName, $request);
-		$this->config = $config;
-		$this->appConfig = $appConfig;
-		$this->userSession = $userSession;
-		$this->l10n = $l10n;
-		$this->groupManager = $groupManager;
-		$this->settingManager = $settingManager;
 	}
 
 	/**
@@ -113,7 +84,7 @@ public function getKeys(string $app): DataResponse {
 			return new DataResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_FORBIDDEN);
 		}
 		return new DataResponse([
-			'data' => $this->config->getAppKeys($app),
+			'data' => $this->appConfig->getKeys($app),
 		]);
 	}
 
@@ -134,9 +105,10 @@ public function getValue(string $app, string $key, string $defaultValue = ''): D
 		} catch (\InvalidArgumentException $e) {
 			return new DataResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_FORBIDDEN);
 		}
-		return new DataResponse([
-			'data' => $this->config->getAppValue($app, $key, $defaultValue),
-		]);
+
+		/** @psalm-suppress InternalMethod */
+		$value = $this->appConfig->getValueMixed($app, $key, $defaultValue, null);
+		return new DataResponse(['data' => $value]);
 	}
 
 	/**
@@ -171,7 +143,8 @@ public function setValue(string $app, string $key, string $value): DataResponse
 			return new DataResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_FORBIDDEN);
 		}
 
-		$this->config->setAppValue($app, $key, $value);
+		/** @psalm-suppress InternalMethod */
+		$this->appConfig->setValueMixed($app, $key, $value);
 		return new DataResponse();
 	}
 
@@ -195,7 +168,7 @@ public function deleteKey(string $app, string $key): DataResponse {
 			return new DataResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_FORBIDDEN);
 		}
 
-		$this->config->deleteAppValue($app, $key);
+		$this->appConfig->deleteKey($app, $key);
 		return new DataResponse();
 	}
 
@@ -231,7 +204,7 @@ protected function verifyConfigKey(string $app, string $key, string $value) {
 		if ($app === 'files'
 			&& $key === 'default_quota'
 			&& $value === 'none'
-			&& $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '0') {
+			&& $this->appConfig->getValueInt('files', 'allow_unlimited_quota', 1) === 0) {
 			throw new \InvalidArgumentException('The given key can not be set, unlimited quota is forbidden on this instance');
 		}
 	}

apps/provisioning_api/tests/Controller/AppConfigControllerTest.php

@@ -25,11 +25,11 @@
  */
 namespace OCA\Provisioning_API\Tests\Controller;
 
+use OC\AppConfig;
 use OCA\Provisioning_API\Controller\AppConfigController;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IAppConfig;
-use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -45,8 +45,6 @@
  */
 class AppConfigControllerTest extends TestCase {
 
-	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
-	private $config;
 	/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
 	private $appConfig;
 	/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
@@ -61,8 +59,7 @@ class AppConfigControllerTest extends TestCase {
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->config = $this->createMock(IConfig::class);
-		$this->appConfig = $this->createMock(IAppConfig::class);
+		$this->appConfig = $this->createMock(AppConfig::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
@@ -80,7 +77,6 @@ protected function getInstance(array $methods = []) {
 			return new AppConfigController(
 				'provisioning_api',
 				$request,
-				$this->config,
 				$this->appConfig,
 				$this->userSession,
 				$this->l10n,
@@ -92,7 +88,6 @@ protected function getInstance(array $methods = []) {
 				->setConstructorArgs([
 					'provisioning_api',
 					$request,
-					$this->config,
 					$this->appConfig,
 					$this->userSession,
 					$this->l10n,
@@ -137,15 +132,15 @@ public function testGetKeys($app, $keys, $throws, $status) {
 				->with($app)
 				->willThrowException($throws);
 
-			$this->config->expects($this->never())
-				->method('getAppKeys');
+			$this->appConfig->expects($this->never())
+				->method('getKeys');
 		} else {
 			$api->expects($this->once())
 				->method('verifyAppId')
 				->with($app);
 
-			$this->config->expects($this->once())
-				->method('getAppKeys')
+			$this->appConfig->expects($this->once())
+				->method('getKeys')
 				->with($app)
 				->willReturn($keys);
 		}
@@ -183,16 +178,13 @@ public function testGetValue($app, $key, $default, $return, $throws, $status) {
 				->method('verifyAppId')
 				->with($app)
 				->willThrowException($throws);
-
-			$this->config->expects($this->never())
-				->method('getAppValue');
 		} else {
 			$api->expects($this->once())
 				->method('verifyAppId')
 				->with($app);
 
-			$this->config->expects($this->once())
-				->method('getAppValue')
+			$this->appConfig->expects($this->once())
+				->method('getValueMixed')
 				->with($app, $key, $default)
 				->willReturn($return);
 		}
@@ -246,8 +238,8 @@ public function testSetValue($app, $key, $value, $appThrows, $keyThrows, $status
 
 			$api->expects($this->never())
 				->method('verifyConfigKey');
-			$this->config->expects($this->never())
-				->method('setAppValue');
+			$this->appConfig->expects($this->never())
+				->method('setValueMixed');
 		} elseif ($keyThrows instanceof  \Exception) {
 			$api->expects($this->once())
 				->method('verifyAppId')
@@ -257,8 +249,8 @@ public function testSetValue($app, $key, $value, $appThrows, $keyThrows, $status
 				->with($app, $key)
 				->willThrowException($keyThrows);
 
-			$this->config->expects($this->never())
-				->method('setAppValue');
+			$this->appConfig->expects($this->never())
+				->method('setValueMixed');
 		} else {
 			$api->expects($this->once())
 				->method('verifyAppId')
@@ -267,8 +259,8 @@ public function testSetValue($app, $key, $value, $appThrows, $keyThrows, $status
 				->method('verifyConfigKey')
 				->with($app, $key);
 
-			$this->config->expects($this->once())
-				->method('setAppValue')
+			$this->appConfig->expects($this->once())
+				->method('setValueMixed')
 				->with($app, $key, $value);
 		}
 
@@ -310,8 +302,8 @@ public function testDeleteValue($app, $key, $appThrows, $keyThrows, $status) {
 
 			$api->expects($this->never())
 				->method('verifyConfigKey');
-			$this->config->expects($this->never())
-				->method('deleteAppValue');
+			$this->appConfig->expects($this->never())
+				->method('deleteKey');
 		} elseif ($keyThrows instanceof  \Exception) {
 			$api->expects($this->once())
 				->method('verifyAppId')
@@ -321,8 +313,8 @@ public function testDeleteValue($app, $key, $appThrows, $keyThrows, $status) {
 				->with($app, $key)
 				->willThrowException($keyThrows);
 
-			$this->config->expects($this->never())
-				->method('deleteAppValue');
+			$this->appConfig->expects($this->never())
+				->method('deleteKey');
 		} else {
 			$api->expects($this->once())
 				->method('verifyAppId')
@@ -331,8 +323,8 @@ public function testDeleteValue($app, $key, $appThrows, $keyThrows, $status) {
 				->method('verifyConfigKey')
 				->with($app, $key);
 
-			$this->config->expects($this->once())
-				->method('deleteAppValue')
+			$this->appConfig->expects($this->once())
+				->method('deleteKey')
 				->with($app, $key);
 		}
 

core/Command/Config/App/GetConfig.php

@@ -1,8 +1,11 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
  * @author Joas Schilling <[email protected]>
+ * @author Maxence Lange <[email protected]>
  *
  * @license AGPL-3.0
  *
@@ -21,15 +24,16 @@
  */
 namespace OC\Core\Command\Config\App;
 
-use OCP\IConfig;
+use OCP\Exceptions\AppConfigUnknownKeyException;
+use OCP\IAppConfig;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class GetConfig extends Base {
 	public function __construct(
-		protected IConfig $config,
+		protected IAppConfig $appConfig,
 	) {
 		parent::__construct();
 	}
@@ -50,6 +54,12 @@ protected function configure() {
 				InputArgument::REQUIRED,
 				'Name of the config to get'
 			)
+			->addOption(
+				'details',
+				null,
+				InputOption::VALUE_NONE,
+				'returns complete details about the app config value'
+			)
 			->addOption(
 				'default-value',
 				null,
@@ -71,14 +81,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$configName = $input->getArgument('name');
 		$defaultValue = $input->getOption('default-value');
 
-		if (!in_array($configName, $this->config->getAppKeys($appName)) && !$input->hasParameterOption('--default-value')) {
-			return 1;
+		if ($input->getOption('details')) {
+			$details = $this->appConfig->getDetails($appName, $configName);
+			$details['type'] = $details['typeString'];
+			unset($details['typeString']);
+			$this->writeArrayInOutputFormat($input, $output, $details);
+			return 0;
 		}
 
-		if (!in_array($configName, $this->config->getAppKeys($appName))) {
+		try {
+			$configValue = $this->appConfig->getDetails($appName, $configName)['value'];
+		} catch (AppConfigUnknownKeyException $e) {
+			if (!$input->hasParameterOption('--default-value')) {
+				return 1;
+			}
 			$configValue = $defaultValue;
-		} else {
-			$configValue = $this->config->getAppValue($appName, $configName);
 		}
 
 		$this->writeMixedInOutputFormat($input, $output, $configValue);