Skip to content

Commit

Permalink
lazy AppConfig
Browse files Browse the repository at this point in the history 
Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
@ArtificialOwl
ArtificialOwl committed Jan 15, 2024
1 parent 32bf74a commit c1ad25a
Show file tree
Hide file tree
Showing 24 changed files with 2,304 additions and 474 deletions.
65 changes: 19 additions & 46 deletions apps/provisioning_api/lib/Controller/AppConfigController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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),
]);
}

Expand All @@ -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]);
}

/**
Expand Down Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down Expand Up @@ -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');
}
}
Expand Down
48 changes: 20 additions & 28 deletions apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 */
Expand All @@ -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);
Expand All @@ -80,7 +77,6 @@ protected function getInstance(array $methods = []) {
return new AppConfigController(
'provisioning_api',
$request,
$this->config,
$this->appConfig,
$this->userSession,
$this->l10n,
Expand All @@ -92,7 +88,6 @@ protected function getInstance(array $methods = []) {
->setConstructorArgs([
'provisioning_api',
$request,
$this->config,
$this->appConfig,
$this->userSession,
$this->l10n,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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);
}

Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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);
}

Expand Down
31 changes: 24 additions & 7 deletions core/Command/Config/App/GetConfig.php
View file
Original file line number Diff line number Diff line change
@@ -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
*
Expand All @@ -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();
}
Expand All @@ -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,
Expand All @@ -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);
Expand Down
Loading

0 comments on commit c1ad25a

Please sign in to comment.