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

Allow configuration binder to bind single elements to array #58060

Merged
merged 2 commits into from
Aug 25, 2021

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Aug 24, 2021

Backport approved by tactics offline for 6.0.


PR Content

1st PR: #57204
2nd PR: #57872 (applies leftover PR feedback from original)

Customer Impact

When trying to bind from an XML configuration provider, we now can allow to bind single elements to array rather than leaving them as null:

<TvShow>
  <Metadata>
    <Name>Prison Break</Name>
  </Metadata>
</TvShow>

This however would be breaking for other providers (like JSON configuration provider) where this is not expected.
So for example, when binding to:

public class TvShow
{
  public Metadata[] Metadata { get; set; }
}

public class Metadata
{
  public string Name { get; set; }
}

Even if we change the below JSON:

{
   "tvshow": {
        "metadata": [
            {
               "name": "PrisonBreak"
            }
         ]
    }
}

so that tvshows is an object with a single metadata:

{
   "tvshow": {
        "metadata": {
            "name": "PrisonBreak"
          }
    }
}

would still bind properly even though json is not specifying an array.

Testing

Using the test that is part of the linked PRs.

Risk

Low – Might incur a little bit of perf cost when app context switch is set to true.

If a customer is not interested in this breaking change they can set the introduced app context switch Microsoft.Extensions.Configuration.BindSingleElementsToArray to false and get the old behavior back.

Regression?

No

More links

Visit dotnet/docs#25779 for more information.

@ghost
Copy link

ghost commented Aug 24, 2021

Tagging subscribers to this area: @maryamariyan, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

Approved by tactics offline.

Author: maryamariyan
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@ericstj ericstj added the Servicing-approved Approved for servicing release label Aug 25, 2021
@ericstj
Copy link
Member

ericstj commented Aug 25, 2021

Approved by @SteveMCarroll in mail.

@ericstj ericstj merged commit c8159b3 into dotnet:release/6.0 Aug 25, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants