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

[release/7.0] Fix Configuration Binding with Instantiated Objects #81250

Merged
merged 2 commits into from
Feb 9, 2023

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Jan 27, 2023

Fixes #79766

Backport of #81050 to release/7.0

/cc @tarekgh

Customer Impact

When apps/services bind a configuration to a collection which is created with specific behavior (like storing elements with ignore casing to avoid duplicate items differ only in casing), the configuration binder replaces this created collection with a newly created one which not necessarily have same behavior (ignore casing). This causes the apps ends up with a collection containing a list of unexpected items which causes failures to the apps as the app getting unexpected collection elements.

This is a regression from .NET 6.0.

Testing

Done manual tests with all different types of collections. The changes passed all regression tests. Also, more tests are added for the cases we are fixing.

Risk

Medium, I scoped the changes as much as I could while ensuring I addressed the issues we are trying to fix.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@ghost
Copy link

ghost commented Jan 27, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #81050 to release/7.0

/cc @tarekgh

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: tarekgh
Assignees: tarekgh
Labels:

area-Extensions-Configuration

Milestone: -

@tarekgh tarekgh added this to the 7.0.x milestone Jan 27, 2023
@tarekgh tarekgh requested a review from halter73 January 27, 2023 01:31
@tarekgh tarekgh added the Servicing-consider Issue for next servicing release review label Jan 27, 2023
@tarekgh tarekgh added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 27, 2023
@tarekgh
Copy link
Member Author

tarekgh commented Jan 27, 2023

This is approved offline by e-mail.

Comment on lines +8 to 9
<ServicingVersion>4</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
Copy link
Member

@carlossanlop carlossanlop Feb 7, 2023

Choose a reason for hiding this comment

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

Nice, thanks for adding the change. I'll keep in mind when doing branding that GeneratePackageOnBuild was already set to true from the previous servicing release, so I don't reset it.

Edit: Here's the branding PR. I did not reset that package: #81777

@carlossanlop
Copy link
Member

Approved by Tactics for 7.0.4.
Signed-off by area owner.
Required OOB package changes look good. GeneratePackageOnBuild was kept true in the base branch.
CI failure unrelated: #81544
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit ce46be3 into dotnet:release/7.0 Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2023
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.

3 participants