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

Settings will be cleared unexpectedly if multiple providers with the same name #18809

Closed
1 task done
PMExtra opened this issue Jan 19, 2024 · 2 comments
Closed
1 task done
Assignees
Milestone

Comments

@PMExtra
Copy link
Contributor

PMExtra commented Jan 19, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Description

It seems that SettingManager supports multiple providers with the same name, but there is an issue with this implementation.

According to that:

SettingManager.GetOrNullInternalAsync(string name, string providerName, string providerKey, bool fallback = true):

if (providerName != null)
{
providers = providers.SkipWhile(c => c.Name != providerName);
}
if (!fallback || !setting.IsInherited)
{
providers = providers.TakeWhile(c => c.Name == providerName);
}

If we expect the providerName to be unique, we should constrain it within SettingManagementOptions and take it by the FirstOrDefault method. But there are SkipWhile and TakeWhile.

So, I think ABP allows multiple providers with the same name. But then, if we set a value with this provider name, it will lose the value:

SettingManager.SetAsync(string name, string value, string providerName, string providerKey, bool forceToSet = false):

if (providers.Count > 1 && !forceToSet && setting.IsInherited && value != null)
{
var fallbackValue = await GetOrNullInternalAsync(name, providers[1].Name, null);
if (fallbackValue == value)
{
//Clear the value if it's same as it's fallback value
value = null;
}
}

If the providers list contains providers named [ "U", "U", "T", ... ], the fallbackValue will still be retrieved from the providers named U.

And then, if the value equals the fallbackValue, the value will be set to null. SettingManager will clear the value for each provider U:

if (value == null)
{
foreach (var provider in providers)
{
await provider.ClearAsync(setting, providerKey);
}
}

Reproduction Steps

  1. Insert the second ISettingManagementProvider named U near the origin UserSettingManagementProvider.
  2. Call await SettingManager.SetAsync(key, value, "U", CurrentUser.Id);.
  3. Repeat the step 2 again, with the same arguments.
  4. Call await SettingManager.GetAsync(key, "U", CurrentUser.Id);

Expected behavior

Retrieved the previously set value.

Actual behavior

Retrieved null.

Regression?

No response

Known Workarounds

Retrieve fallbackValue from the second unique name of the providers, rather than the second provider's name.

var providerName = providers.Select(provider => provider.Name).Distinct().Skip(1).FirstOrDefault();
if (providerName is not null)
{
    var fallbackValue = await GetOrNullInternalAsync(name, providerName, null); 
}

Version

7.4.5

User Interface

Common (Default)

Database Provider

EF Core (Default)

Tiered or separate authentication server

None (Default)

Operation System

Windows (Default)

Other information

No response

@PMExtra PMExtra added the bug label Jan 19, 2024
@realLiangshiwei
Copy link
Member

This seems to be a problem. The framework should prevent adding SettingValueProvider with duplicate names.
I will check it

@hikalkan
Copy link
Member

Multiple providers with the same name is not allowed. The code seems trying to somehow tolerate it, but we don't expect multiple providers with the same name. It is stated in the documentation: https://docs.abp.io/en/abp/latest/Settings#custom-setting-value-providers

image

If that's the only problem, we can close this issue @realLiangshiwei

@hikalkan hikalkan modified the milestones: 8.1-preview, 8.2-preview Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants