Skip to content

Make precedence source ordered - #623

Open
linglingye001 wants to merge 3 commits into
mainfrom
linglingye/precedence-source-ordered
Open

linglingye001 wants to merge 3 commits into
mainfrom
linglingye/precedence-source-ordered

Conversation

@linglingye001

@linglingye001 linglingye001 commented Sep 10, 2026

Copy link
Copy Markdown
Member

Why this PR?

Fix #621

ConfigurationFeatureDefinitionProvider always evaluates the Microsoft schema first, source order is never consulted when the same flag is split across schemas. A Microsoft-schema definition from the first provider beats a .NET-schema definition from every later provider.

Visible Changes

  • Tracks which schema last defines each feature by configuration-source order.
  • Later sources win across .NET and Microsoft schemas when custom merging is enabled.
  • Microsoft schema wins when both schemas occur in the same source.
  • Deduplicates cross-schema entries during feature enumeration.
  • Leaves default, non-custom behavior unchanged.

Example

When custom merging is enabled:

Provider` 1: Microsoft schema → MyFlag = false
Provider 2: .NET schema → MyFlag = targeting filter

Winning schema map:
MyFlag → .NET

Result: Provider 2 wins and the targeting filter is parsed.


private void LoadFeatureDefinitionSections()
{
_dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_dotnetFeatureDefinitionSections comes from the aggregated _configuration, while the winning schema is determined source by source.

I would expect that when custom merging is enabled, .NET feature flag sections should also be collected source by source. The current implementation is kind of inconsistant

IConfigurationSection dotnetFeatureManagementSection = configuration
.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName);

foreach (IConfigurationSection featureSection in dotnetFeatureManagementSection.GetChildren())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, ConfigurationFeatureDefinitionProvider kept two responsibilities separate:

  1. Loading feature definition configuration sections.
  2. Parsing those sections into feature definitions.

_dotnetFeatureDefinitionSections and _microsoftFeatureDefinitionSections acted as the ground truth, and the contents were interpreted only when feature definitions were requested.

In the current implementation, the loading phase also partially parse the feature definitions while traversing the configuration sources. It reads feature flag IDs to maintain _featureDefinitionSchemas.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use the following data structure to maintain feature definition sections grouped by configuration source:

private IEnumerable<FeatureDefinitionSectionsBySource> _featureDefinitionSources;

private class FeatureDefinitionSectionsBySource
{
    public IEnumerable<IConfigurationSection> DotnetSections { get; }

    public IEnumerable<IConfigurationSection> MicrosoftSections { get; }
}

When getting a feature definition, we can traverse the configuration sources from highest to lowest precedence. Within each source, we check the Microsoft schema first, then .NET schema.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouping sections by source would retain the information needed for precedence without a separate schema map. I think it is a cleaner design.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Microsoft-schema flag in appsettings.json silently shadows the same flag from Azure App Configuration (.NET schema)

2 participants