Skip to content

Commit

Permalink
fix(lexicon): convert system entry to string
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
ArtificialOwl committed Dec 16, 2024
1 parent 7675793 commit 444ad75
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 20 deletions.
1 change: 1 addition & 0 deletions core/Command/Config/App/GetConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

// get the default value defined from lexicon
/** @psalm-suppress UndefinedInterfaceMethod */
$configValue = $this->appConfig->getValueMixed($appName, $configName, $defaultValue ?? '');
}

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,7 @@
'OC\\Comments\\Manager' => $baseDir . '/lib/private/Comments/Manager.php',
'OC\\Comments\\ManagerFactory' => $baseDir . '/lib/private/Comments/ManagerFactory.php',
'OC\\Config' => $baseDir . '/lib/private/Config.php',
'OC\\Config\\Lexicon\\CoreConfigLexicon' => $baseDir . '/lib/private/Config/Lexicon/CoreConfigLexicon.php',
'OC\\Config\\UserConfig' => $baseDir . '/lib/private/Config/UserConfig.php',
'OC\\Console\\Application' => $baseDir . '/lib/private/Console/Application.php',
'OC\\Console\\TimestampFormatter' => $baseDir . '/lib/private/Console/TimestampFormatter.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Comments\\Manager' => __DIR__ . '/../../..' . '/lib/private/Comments/Manager.php',
'OC\\Comments\\ManagerFactory' => __DIR__ . '/../../..' . '/lib/private/Comments/ManagerFactory.php',
'OC\\Config' => __DIR__ . '/../../..' . '/lib/private/Config.php',
'OC\\Config\\Lexicon\\CoreConfigLexicon' => __DIR__ . '/../../..' . '/lib/private/Config/Lexicon/CoreConfigLexicon.php',
'OC\\Config\\UserConfig' => __DIR__ . '/../../..' . '/lib/private/Config/UserConfig.php',
'OC\\Console\\Application' => __DIR__ . '/../../..' . '/lib/private/Console/Application.php',
'OC\\Console\\TimestampFormatter' => __DIR__ . '/../../..' . '/lib/private/Console/TimestampFormatter.php',
Expand Down
41 changes: 35 additions & 6 deletions lib/private/Config/UserConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ private function getTypedValue(
ValueType $type,
): string {
$this->assertParams($userId, $app, $key);
if (!$this->matchAndApplyLexiconDefinition($app, $key, $lazy, $type, default: $default)) {
if (!$this->matchAndApplyLexiconDefinition($userId, $app, $key, $lazy, $type, default: $default)) {
return $default; // returns default if strictness of lexicon is set to WARNING (block and report)
}
$this->loadConfig($userId, $lazy);
Expand Down Expand Up @@ -1047,7 +1047,7 @@ private function setTypedValue(
ValueType $type,
): bool {
$this->assertParams($userId, $app, $key);
if (!$this->matchAndApplyLexiconDefinition($app, $key, $lazy, $type, $flags)) {
if (!$this->matchAndApplyLexiconDefinition($userId, $app, $key, $lazy, $type, $flags)) {
return false; // returns false as database is not updated
}
$this->loadConfig($userId, $lazy);
Expand Down Expand Up @@ -1823,6 +1823,7 @@ private function decryptSensitiveValue(string $userId, string $app, string $key,
* @throws TypeConflictException
*/
private function matchAndApplyLexiconDefinition(
string $userId,
string $app,
string $key,
bool &$lazy,
Expand All @@ -1844,18 +1845,46 @@ private function matchAndApplyLexiconDefinition(
}

$lazy = $configValue->isLazy();
// default from Lexicon got priority but it can still be overwritten by admin
// with 'lexicon.default.userconfig' => [<app>.<key> => 'my value'], in config.php
$default = $this->config->getSystemValue('lexicon.default.userconfig', [])[$app . '.' . $key] ?? $configValue->getDefault() ?? $default;
$flags = $configValue->getFlags();

if ($configValue->isDeprecated()) {
$this->logger->notice('User config key ' . $app . '/' . $key . ' is set as deprecated.');
}

if ($this->hasKey($userId, $app, $key, $lazy)) {
return true; // if key exists there should be no need to extract default
}

// default from Lexicon got priority but it can still be overwritten by admin
$default = $this->getSystemDefault($app, $configValue) ?? $configValue->getDefault() ?? $default;

return true;
}

/**
* get default value set in config/config.php if stored in key:
*
* 'lexicon.default.userconfig' => [
* <appId> => [
* <configKey> => 'my value',
* ]
* ],
*
* The entry is converted to string to fit the expected type when managing default value
*
* @param string $appId
* @param ConfigLexiconEntry $configValue
*
* @return string|null
*/
private function getSystemDefault(string $appId, ConfigLexiconEntry $configValue): ?string {
$default = $this->config->getSystemValue('lexicon.default.userconfig', [])[$appId][$configValue->getKey()] ?? null;
if ($default === null) {
return null; // no system default, using default default.
}

return $configValue->convertToString($default);
}

/**
* manage ConfigLexicon behavior based on strictness set in IConfigLexicon
*
Expand Down
39 changes: 25 additions & 14 deletions lib/unstable/Config/Lexicon/ConfigLexiconEntry.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,12 @@ class ConfigLexiconEntry {
public function __construct(
private readonly string $key,
private readonly ValueType $type,
null|string|int|float|bool|array $default = null,
private null|string|int|float|bool|array $defaultRaw = null,
string $definition = '',
private readonly bool $lazy = false,
private readonly int $flags = 0,
private readonly bool $deprecated = false,
) {
if ($default !== null) {
// in case $default is array but is not expected to be an array...
$default = ($type !== ValueType::ARRAY && is_array($default)) ? json_encode($default) : $default;
$this->default = match ($type) {
ValueType::MIXED => (string)$default,
ValueType::STRING => $this->convertFromString((string)$default),
ValueType::INT => $this->convertFromInt((int)$default),
ValueType::FLOAT => $this->convertFromFloat((float)$default),
ValueType::BOOL => $this->convertFromBool((bool)$default),
ValueType::ARRAY => $this->convertFromArray((array)$default)
};
}

/** @psalm-suppress UndefinedClass */
if (\OC::$CLI) { // only store definition if ran from CLI
$this->definition = $definition;
Expand Down Expand Up @@ -132,9 +119,33 @@ private function convertFromArray(array $default): string {
* @experimental 31.0.0
*/
public function getDefault(): ?string {
if ($this->defaultRaw === null) {
return null;
}

if ($this->default === null) {
$this->default = $this->convertToString($this->defaultRaw);
}

return $this->default;
}

public function convertToString(string|int|float|bool|array $entry): string {

Check failure on line 133 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

InvalidDocblock

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:133:2: InvalidDocblock: PHPDoc is required for methods in NCU. (see https://psalm.dev/008)
// in case $default is array but is not expected to be an array...
if ($this->getValueType() !== ValueType::ARRAY && is_array($entry)) {
$entry = json_encode($entry) ?? '';

Check failure on line 136 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

RedundantConditionGivenDocblockType

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:136:13: RedundantConditionGivenDocblockType: Docblock-defined type false|non-empty-string for $<tmp coalesce var>3034 is never null (see https://psalm.dev/156)

Check failure on line 136 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

DocblockTypeContradiction

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:136:36: DocblockTypeContradiction: Cannot resolve types for $<tmp coalesce var>3034 - docblock-defined type false|string does not contain null (see https://psalm.dev/155)
}

return match ($this->getValueType()) {
ValueType::MIXED => (string)$entry,

Check failure on line 140 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

PossiblyInvalidCast

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:140:32: PossiblyInvalidCast: array<array-key, mixed> cannot be cast to string (see https://psalm.dev/190)
ValueType::STRING => $this->convertFromString((string)$entry),

Check failure on line 141 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

PossiblyInvalidCast

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:141:58: PossiblyInvalidCast: array<array-key, mixed> cannot be cast to string (see https://psalm.dev/190)
ValueType::INT => $this->convertFromInt((int)$entry),

Check failure on line 142 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

RiskyCast

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:142:49: RiskyCast: Casting array<array-key, mixed> to int has possibly unintended value of 0/1 (see https://psalm.dev/313)
ValueType::FLOAT => $this->convertFromFloat((float)$entry),

Check failure on line 143 in lib/unstable/Config/Lexicon/ConfigLexiconEntry.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ncu

RiskyCast

lib/unstable/Config/Lexicon/ConfigLexiconEntry.php:143:55: RiskyCast: Casting array<array-key, mixed> to float has possibly unintended value of 0.0/1.0 (see https://psalm.dev/313)
ValueType::BOOL => $this->convertFromBool((bool)$entry),
ValueType::ARRAY => $this->convertFromArray((array)$entry)
};
}

/**
* @inheritDoc
*
Expand Down

0 comments on commit 444ad75

Please sign in to comment.