Skip to content

Commit

Permalink
feat(files/user-config): make configs admin-configurable
Browse files Browse the repository at this point in the history
Signed-off-by: Kai Henseler <kai.henseler@strato.de>
  • Loading branch information
bromiesTM committed Dec 13, 2024
1 parent 8869c73 commit 85edc56
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 9 deletions.
11 changes: 8 additions & 3 deletions apps/files/lib/Service/UserConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace OCA\Files\Service;

use OCA\Files\AppInfo\Application;
use OCP\AppFramework\Services\IAppConfig;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserSession;
Expand Down Expand Up @@ -53,7 +54,7 @@ class UserConfig {
protected IConfig $config;
protected ?IUser $user = null;

public function __construct(IConfig $config, IUserSession $userSession) {
public function __construct(IConfig $config, IUserSession $userSession, protected IAppConfig $appConfig) {
$this->config = $config;
$this->user = $userSession->getUser();
}
Expand Down Expand Up @@ -143,8 +144,12 @@ public function getConfigs(): array {

$userId = $this->user->getUID();
$userConfigs = array_map(function (string $key) use ($userId) {
$value = $this->config->getUserValue($userId, Application::APP_ID, $key, $this->getDefaultConfigValue($key));
// If the default is expected to be a boolean, we need to cast the value
$value = $this->config->getUserValue($userId, Application::APP_ID, $key, null);
// If the default value is expected to be a boolean, we need to cast the value
if ($value === null) {
$value = $this->appConfig->getAppValueBool($key, $this->getDefaultConfigValue($key));
}

if (is_bool($this->getDefaultConfigValue($key)) && is_string($value)) {
return $value === '1';
}
Expand Down
85 changes: 79 additions & 6 deletions apps/files/tests/Service/UserConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use OCA\Files\AppInfo\Application;
use OCA\Files\Service\UserConfig;
use OCP\AppFramework\Services\IAppConfig;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserSession;
Expand All @@ -34,6 +35,9 @@ class UserConfigTest extends \Test\TestCase {
/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
private $userSessionMock;

/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
private $appConfigMock;

/**
* @var UserConfig|\PHPUnit\Framework\MockObject\MockObject
*/
Expand All @@ -42,6 +46,7 @@ class UserConfigTest extends \Test\TestCase {
protected function setUp(): void {
parent::setUp();
$this->configMock = $this->createMock(IConfig::class);
$this->appConfigMock = $this->createMock(IAppConfig::class);

$this->userUID = static::getUniqueID('user_id-');
\OC::$server->getUserManager()->createUser($this->userUID, 'test');
Expand All @@ -67,6 +72,7 @@ protected function getUserConfigService(array $methods = []) {
->setConstructorArgs([
$this->configMock,
$this->userSessionMock,
$this->appConfigMock,
])
->setMethods($methods)
->getMock();
Expand Down Expand Up @@ -100,23 +106,23 @@ public function testThrowsExceptionWhenNoUserLoggedInForSetConfig(): void {
$this->expectException(\Exception::class);
$this->expectExceptionMessage('No user logged in');

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$userConfig->setConfig('crop_image_previews', true);
}

public function testThrowsInvalidArgumentExceptionForUnknownConfigKey(): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Unknown config key');

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$userConfig->setConfig('unknown_key', true);
}

public function testThrowsInvalidArgumentExceptionForInvalidConfigValue(): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Invalid config value');

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$userConfig->setConfig('crop_image_previews', 'foo');
}

Expand All @@ -141,7 +147,7 @@ public function testSetsConfigWithBooleanValuesSuccessfully($boolValue, $expecte
->method('setUserValue')
->with($this->userUID, Application::APP_ID, 'crop_image_previews', $expectedValue);

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$userConfig->setConfig('crop_image_previews', $boolValue);
}

Expand All @@ -152,7 +158,13 @@ public function testGetsConfigsWithDefaultValuesSuccessfully(): void {
return $default;
});

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
// pass the default app settings unchanged
$this->appConfigMock->method('getAppValueBool')
->willReturnCallback(function ($key, $default) {
return $default;
});

$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$configs = $userConfig->getConfigs();
$this->assertEquals([
'crop_image_previews' => true,
Expand Down Expand Up @@ -188,7 +200,13 @@ public function testGetsConfigsOverrideWithUserValuesSuccessfully(): void {
return $default;
});

$userConfig = new UserConfig($this->configMock, $this->userSessionMock);
// pass the default app settings unchanged
$this->appConfigMock->method('getAppValueBool')
->willReturnCallback(function ($key, $default) {
return $default;
});

$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$configs = $userConfig->getConfigs();
$this->assertEquals([
'crop_image_previews' => false,
Expand All @@ -199,4 +217,59 @@ public function testGetsConfigsOverrideWithUserValuesSuccessfully(): void {
'folder_tree' => true,
], $configs);
}

public static function overrideConfigValues(): array {
// app config key, default value, user value, admin value, expected value, test description
return [
// default value is set due to no user and no admin values
['show_hidden', false, null, null, false, "user has no setting, admin has no setting - user gets default setting"],

// admin value is set due to no user value
['show_hidden', false, null, true, true, "user has no setting, admin value is set - user gets admin setting"],
['show_hidden', false, null, false, false, "user has no setting, admin value is set - user gets admin setting"],

// admin value is ignored in favor of user value
['show_hidden', false, true, null, true, "user overrides default setting, admin has no setting - user gets user setting"],
['show_hidden', false, true, true, true, "user overrides default setting, admin value is set - user gets user setting"],
['show_hidden', false, true, false, true, "user overrides default setting, admin value is set - user gets user setting"],

['show_hidden', false, false, null, false, "user sets default setting, admin has no setting - user gets user setting"],
['show_hidden', false, false, true, false, "user sets default setting, admin value is set - user gets user setting"],
['show_hidden', false, false, false, false, "user sets default setting, admin value is set - user gets user setting"],
];
}

/**
* @dataProvider overrideConfigValues
*/
public function testGetsConfigsOverrideWithAppsValuesSuccessfully(string $appConfigKey, bool $defaultValue, $userValue, $adminValue, $expectedValue, $testDescription): void {
$this->userSessionMock->method('getUser')->willReturn($this->userMock);

// emulate override by the user config values
$this->configMock->method('getUserValue')
->willReturnCallback(function ($userId, $appId, $key, $default) use ($appConfigKey, $userValue) {
if ($key === $appConfigKey) {
return $userValue === null ? $default : $userValue;
}
return $default;
});

// emulate override by the app config values
$this->appConfigMock->method('getAppValueBool')
->willReturnCallback(function ($key, $default) use ($adminValue, $appConfigKey) {
if ($key === $appConfigKey) {
return $adminValue === null ? $default : $adminValue;
}
return $default;
});

$userConfig = new UserConfig($this->configMock, $this->userSessionMock, $this->appConfigMock);
$configs = $userConfig->getConfigs();

// ony the expected value should be different from the default
$expectedConfigs = $configs;
$expectedConfigs[$appConfigKey] = $expectedValue;

$this->assertEquals($expectedConfigs, $configs, $testDescription);
}
}

0 comments on commit 85edc56

Please sign in to comment.