Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to save config in ShellSettings with more then one section #14481

Closed
wAsnk opened this issue Oct 11, 2023 · 2 comments · Fixed by #14490
Closed

Make it possible to save config in ShellSettings with more then one section #14481

wAsnk opened this issue Oct 11, 2023 · 2 comments · Fixed by #14490
Milestone

Comments

@wAsnk
Copy link
Contributor

wAsnk commented Oct 11, 2023

Currently, it is not possible to save settings to the ShellSettings with a key that has more sections. E.g. saving TestSection:TestSubSection:TestKey: "TestValue" is not possible. Because of this in ShellSettingsManager:

var sections = settings.ShellConfiguration.GetChildren()
  .Where(s => !s.GetChildren().Any())
  .ToArray();

So basically when calling this example controller:

    public async Task<IActionResult> Test()
    {
        if (!_shellHost.TryGetSettings("Default", out var shellSettings))
        {
            return NotFound();
        }

        shellSettings["SimpleKey"] = "SimpleValue";
        shellSettings["Not:So:Simple:Key"] = "NotSoSimpleValue";
        shellSettings["Default:Not:So:Simple:Key"] = "TenantSpecificNotSoSimpleValue";

        await _shellHost.UpdateShellSettingsAsync(shellSettings);

        if (!_shellHost.TryGetSettings("Default", out var shellSettingsNew))
        {
            return NotFound();
        }

        var result = $"{shellSettingsNew["SimpleKey"]}, {shellSettingsNew["Not:So:Simple:Key"]}, {shellSettingsNew["Default:Not:So:Simple:Key"]}";
        return Json(result);
    }

You will get this as a result:
image

Is there any specific reason why only keys without children are possible to save currently?

@Piedone
Copy link
Member

Piedone commented Oct 11, 2023

More context here.

This necessitates going directly to IShellConfigurationSources in Lombiq.Hosting.Tenants.Management.

@jtkech
Copy link
Member

jtkech commented Oct 12, 2023

@Piedone @wAsnk

Okay, locally I did a very simple first draft that works, see #14490 where in ShellSettingsManager I used AsEnumerable() on the ShellConfiguration because it keeps for each key the full section path. This may give you an idea to make your implementation simpler before this PR may be merged.

Here the test

shellSettings["SimpleKey"] = "SimpleValue2";
shellSettings["Not:So:Key"] = "NotSoValue2";
shellSettings["Not:So:Simple:Key"] = "NotSoSimpleValue2";

await shellHost.UpdateShellSettingsAsync(shellSettings);

I didn't use "Default:Not:So:Simple:Key", a tenant specific config doesn't have any tenant section.

In the main appsettings to check existing values and so on.

"SimpleKey": "SimpleValue",
"Not": {
  "So": {
    "Key": "NotSoValue",
    "Simple": {
      "Key": "NotSoSimpleValue"
    }
  }
}

Here the result

{
  "DatabaseProvider": "Sqlite",
  ...
  "SimpleKey": "SimpleValue2",
  "Not:So:Simple:Key": "NotSoSimpleValue2",
  "Not:So:Key": "NotSoValue2"
}

For now configs are saved in a flat way, would need a json converter, but I think that's okay, this file is not intended to be edited directly, it can still be but it will be reformatted in a flat way on the next save.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants