Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Maxence Lange <[email protected]>
d

Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
@ArtificialOwl
ArtificialOwl committed Jan 15, 2024
1 parent 1da9fa0 commit b93d9c7
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 289 deletions.
40 changes: 16 additions & 24 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 @@ -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
17 changes: 3 additions & 14 deletions core/Command/Config/App/GetConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int

if ($input->getOption('details')) {
$details = $this->appConfig->getDetails($appName, $configName);
$format = $input->getOption('output') ?? 'plain';
if ($format === 'json') {
$output->writeln(json_encode($details));
} elseif ($format === 'json_pretty') {
$output->writeln(json_encode($details, JSON_PRETTY_PRINT));
} else {
$output->writeln('App: ' . $details['app'] ?? '');
$output->writeln('Config Key: ' . $details['key'] ?? '');
$output->writeln('Config Value: ' . $details['value'] ?? '');
$output->writeln('Value type: ' . $details['typeString'] ?? '');
$output->writeln('Lazy loaded: ' . (($details['lazy'] ?? false) ? 'Yes' : 'No'));
$output->writeln('Sensitive: ' . (($details['sensitive'] ?? false) ? 'Yes' : 'No'));
}

$details['type'] = $details['typeString'];
unset($details['typeString']);
$this->writeArrayInOutputFormat($input, $output, $details);
return 0;
}

Expand Down
41 changes: 18 additions & 23 deletions core/Command/Config/App/SetConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
use Symfony\Component\Console\Question\Question;

class SetConfig extends Base {
private InputInterface $input;
private OutputInterface $output;

public function __construct(
protected IAppConfig $appConfig,
) {
Expand Down Expand Up @@ -97,8 +94,6 @@ protected function configure() {
protected function execute(InputInterface $input, OutputInterface $output): int {
$appName = $input->getArgument('app');
$configName = $input->getArgument('name');
$this->input = $input;
$this->output = $output;

if (!($this->appConfig instanceof AppConfig)) {
throw new \Exception('Only compatible with OC\AppConfig as it uses internal methods');
Expand Down Expand Up @@ -126,19 +121,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
*/
$updated = false;
if (!$input->hasParameterOption('--value')) {
if (!$input->getOption('lazy') && $this->appConfig->isLazy($appName, $configName) && $this->ask('NOT LAZY')) {
if (!$input->getOption('lazy') && $this->appConfig->isLazy($appName, $configName) && $this->ask($input, $output, 'NOT LAZY')) {
$updated = $this->appConfig->updateLazy($appName, $configName, false);
}
if ($input->getOption('lazy') && !$this->appConfig->isLazy($appName, $configName) && $this->ask('LAZY')) {
if ($input->getOption('lazy') && !$this->appConfig->isLazy($appName, $configName) && $this->ask($input, $output, 'LAZY')) {
$updated = $this->appConfig->updateLazy($appName, $configName, true) || $updated;
}
if (!$input->getOption('sensitive') && $this->appConfig->isSensitive($appName, $configName) && $this->ask('NOT SENSITIVE')) {
if (!$input->getOption('sensitive') && $this->appConfig->isSensitive($appName, $configName) && $this->ask($input, $output, 'NOT SENSITIVE')) {
$updated = $this->appConfig->updateSensitive($appName, $configName, false) || $updated;
}
if ($input->getOption('sensitive') && !$this->appConfig->isSensitive($appName, $configName) && $this->ask('SENSITIVE')) {
if ($input->getOption('sensitive') && !$this->appConfig->isSensitive($appName, $configName) && $this->ask($input, $output, 'SENSITIVE')) {
$updated = $this->appConfig->updateSensitive($appName, $configName, true) || $updated;
}
if ($typeString !== null && $type !== $this->appConfig->getValueType($appName, $configName) && $this->ask($typeString)) {
if ($type !== null && $type !== $this->appConfig->getValueType($appName, $configName) && $typeString !== null && $this->ask($input, $output, $typeString)) {
$updated = $this->appConfig->updateType($appName, $configName, $type) || $updated;
}
} else {
Expand All @@ -149,7 +144,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
*/
try {
$currType = $this->appConfig->getValueType($appName, $configName);
if ($type === null || $type === $currType || !$this->ask($typeString)) {
if ($typeString === null || $type === $currType || !$this->ask($input, $output, $typeString)) {
$type = $currType;
} else {
$updated = $this->appConfig->updateType($appName, $configName, $type);

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 3 of OC\AppConfig::updateType cannot be null, possibly null value provided
Expand All @@ -166,7 +161,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$lazy = $input->getOption('lazy');
try {
$currLazy = $this->appConfig->isLazy($appName, $configName);
if ($lazy === null || $lazy === $currLazy || !$this->ask(($lazy) ? 'LAZY' : 'NOT LAZY')) {
if ($lazy === null || $lazy === $currLazy || !$this->ask($input, $output, ($lazy) ? 'LAZY' : 'NOT LAZY')) {
$lazy = $currLazy;
}
} catch (AppConfigUnknownKeyException) {
Expand All @@ -179,7 +174,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$sensitive = $input->getOption('sensitive');
try {
$currSensitive = $this->appConfig->isLazy($appName, $configName);
if ($sensitive === null || $sensitive === $currSensitive || !$this->ask(($sensitive) ? 'LAZY' : 'NOT LAZY')) {
if ($sensitive === null || $sensitive === $currSensitive || !$this->ask($input, $output, ($sensitive) ? 'LAZY' : 'NOT LAZY')) {
$sensitive = $currSensitive;
}
} catch (AppConfigUnknownKeyException) {
Expand Down Expand Up @@ -249,26 +244,26 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 0;
}

private function ask(string $request): bool {
private function ask(InputInterface $input, OutputInterface $output, string $request): bool {
$helper = $this->getHelper('question');
if ($this->input->getOption('no-interaction')) {
if ($input->getOption('no-interaction')) {
return true;
}

$this->output->writeln(sprintf('You are about to set config value %s as <info>%s</info>',
'<info>' . $this->input->getArgument('app') . '</info>/<info>' . $this->input->getArgument('name') . '</info>',
$output->writeln(sprintf('You are about to set config value %s as <info>%s</info>',
'<info>' . $input->getArgument('app') . '</info>/<info>' . $input->getArgument('name') . '</info>',
strtoupper($request)
));
$this->output->writeln('');
$this->output->writeln('<comment>This might break thing, affect performance on your instance or its security!</comment>');
$output->writeln('');
$output->writeln('<comment>This might break thing, affect performance on your instance or its security!</comment>');

$result = (strtolower((string)$helper->ask(
$this->input,
$this->output,
$input,
$output,
new Question('<comment>Confirm this action by typing \'yes\'</comment>: '))) === 'yes');

$this->output->writeln(($result) ? 'done' : 'cancelled');
$this->output->writeln('');
$output->writeln(($result) ? 'done' : 'cancelled');
$output->writeln('');

return $result;
}
Expand Down
4 changes: 3 additions & 1 deletion core/Migrations/Version29000Date20231126110901.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

// Create new field in appconfig for the new IAppConfig API, including lazy grouping.
/**
* Create new fields for type and lazy loading in appconfig for the new IAppConfig API.
*/
class Version29000Date20231126110901 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
Expand Down
Loading

0 comments on commit b93d9c7

Please sign in to comment.