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.

Sign up for GitHub

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

Jump to bottom

Binding collection is not working for options in .net 7.0.1 - it is working in version 7.0.0 and below #79904

Closed
msmolka opened this issue Dec 22, 2022 · 13 comments
Closed

Binding collection is not working for options in .net 7.0.1 - it is working in version 7.0.0 and below #79904

msmolka opened this issue Dec 22, 2022 · 13 comments
Assignees
@tarekgh
Labels
area-Extensions-Options needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone
7.0.x

Comments

@msmolka
Copy link

msmolka commented Dec 22, 2022

Description

I have following code for options:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/c4f6d097351e2d40126cd4be67d506e4fae6b1b3/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Above code works properly in version 7.0.0 and below

However after applying 7.0.1 patch it stopped working.
We had to change code to following:

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/4434801b441d3d8ba93c00649be148df52b43ed5/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Reproduction Steps

Grab repo at point:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/tree/f60a40de740681ed58cbd9906dcf46466f7513e6

2 test are failing for .net 7.0 (only after applying patch). They are passing before applying 7.0.1 patch

Expected behavior

There are no braking changes in patch version

Actual behavior

Breaking changes in patch version

Regression?

it is working on version 7.0.0. and below

Known Workarounds

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/master/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Configuration

No response

Other information

No response
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 22, 2022
@jeffschwMSFT
Copy link
Member

@msmolka thanks for providing the code with tests. Can you provide a small repro that highlights what is happening and what you would expect to happen?

@jeffschwMSFT jeffschwMSFT added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 1, 2023
@ghost
Copy link

ghost commented Jan 1, 2023

This issue has been marked needs-author-action and may be missing some important information.

@ghost
Copy link

ghost commented Jan 3, 2023

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

Issue Details

Description

I have following code for options:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/c4f6d097351e2d40126cd4be67d506e4fae6b1b3/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Above code works properly in version 7.0.0 and below

However after applying 7.0.1 patch it stopped working.
We had to change code to following:

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/4434801b441d3d8ba93c00649be148df52b43ed5/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Reproduction Steps

Grab repo at point:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/tree/f60a40de740681ed58cbd9906dcf46466f7513e6

2 test are failing for .net 7.0 (only after applying patch). They are passing before applying 7.0.1 patch

Expected behavior

There are no braking changes in patch version

Actual behavior

Breaking changes in patch version

Regression?

it is working on version 7.0.0. and below

Known Workarounds

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/master/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Configuration

No response

Other information

No response

Author: msmolka
Assignees: -
Labels:

untriaged, area-Extensions-HttpClientFactory, needs-author-action
Milestone: -

@ghost
Copy link

ghost commented Jan 3, 2023

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

Issue Details

Description

I have following code for options:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/c4f6d097351e2d40126cd4be67d506e4fae6b1b3/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Above code works properly in version 7.0.0 and below

However after applying 7.0.1 patch it stopped working.
We had to change code to following:

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/4434801b441d3d8ba93c00649be148df52b43ed5/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Reproduction Steps

Grab repo at point:
https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/tree/f60a40de740681ed58cbd9906dcf46466f7513e6

2 test are failing for .net 7.0 (only after applying patch). They are passing before applying 7.0.1 patch

Expected behavior

There are no braking changes in patch version

Actual behavior

Breaking changes in patch version

Regression?

it is working on version 7.0.0. and below

Known Workarounds

https://github.com/msmolka/ZNetCS.AspNetCore.IPFiltering/blob/master/src/ZNetCS.AspNetCore.IPFiltering/OptionBase.cs

Configuration

No response

Other information

No response

Author: msmolka
Assignees: -
Labels:

untriaged, area-Extensions-Options, needs-author-action
Milestone: -

@tarekgh tarekgh added this to the 7.0.x milestone Jan 3, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 3, 2023
@tarekgh tarekgh self-assigned this Jan 3, 2023
@gao-artur
Copy link

I am hitting the same issue. The setter will be executed only when the corresponding property exists in the configuration source.
For example:

public class MyOptions
{
    private int _numOfRetries;
    public int NumOfRetries
    {
        get => _numOfRetries;
        set => _numOfRetries = value != 0 ? value : 5;
    }
}

var myOptions = configuration.GetSection("MyOptions").Get<MyOptions>();

In .NET 6, when the MyOptions:NumOfRetries is not defined in any configuration source, the setter is executed and NumOfRetries = 5. In .NET 7.0.1 the setter is not executed and NumOfRetries = 0

@tarekgh
Copy link
Member

tarekgh commented Jan 6, 2023 •
edited
Loading

@msmolka @gao-artur could you please provide a complete code sample reproducing the issue? Thanks!

Also, could you try the same repo with the following version of the library and let me know?

    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-alpha.1.22578.1" />

from the nuget feed:

    <add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" />

@gao-artur
Copy link

Hey @tarekgh, here is the test to reproduce the breaking change/regression:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>net6.0;net7.0</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' ">
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="6.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="6.0.0" />
  </ItemGroup>

  <ItemGroup Condition=" '$(TargetFramework)' == 'net7.0' ">
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="7.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="7.0.0" />
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.1" />
    <PackageReference Include="MSTest.TestAdapter" Version="3.0.2" />
    <PackageReference Include="MSTest.TestFramework" Version="3.0.2" />
  </ItemGroup>

</Project>
using System.Collections.Generic;
using Microsoft.Extensions.Configuration;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestProject2;

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void TestMethod1()
    {
        var configuration = new ConfigurationBuilder()
            .AddInMemoryCollection(new Dictionary<string, string>
            {
                ["MyOptions:Option1"] = "3"
            })
            .Build();

        var myOptions = configuration.GetSection("MyOptions").Get<MyOptions>();

        Assert.IsNotNull(myOptions);

        // exists in configuration and properly sets the property
        Assert.AreEqual(3, myOptions.Option1);

        // doesn't exist in configuration. In net6.0 the setter sets default value '2' but not in net7.0
        Assert.AreEqual(2, myOptions.Option2);
    }
}

public class MyOptions
{
    private int _option1;
    private int _option2;

    public int Option1
    {
        get => _option1;
        set => _option1 = value == 0 ? 1 : value;
    }

    public int Option2
    {
        get => _option2;
        set => _option2 = value == 0 ? 2 : value;
    }
}

dotnet test result

Test run for C:\github\junk\TestProject2\TestProject2\bin\Debug\net6.0\TestProject2.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.4.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: 28 ms - TestProject2.dll (net6.0)
Test run for C:\github\junk\TestProject2\TestProject2\bin\Debug\net7.0\TestProject2.dll (.NETCoreApp,Version=v7.0)
Microsoft (R) Test Execution Command Line Tool Version 17.4.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Failed TestMethod1 [20 ms]
  Error Message:
   Assert.AreEqual failed. Expected:<2>. Actual:<0>.
  Stack Trace:
     at TestProject2.UnitTest1.TestMethod1() in C:\github\junk\TestProject2\TestProject2\UnitTest1.cs:line 28
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)


Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: 31 ms - TestProject2.dll (net7.0)

dotnet --list-sdks result

6.0.404 [C:\Program Files\dotnet\sdk]
7.0.101 [C:\Program Files\dotnet\sdk]

@gao-artur
Copy link

The issue also persists in the 8.0.0-alpha.1.22578.1.

@tarekgh
Copy link
Member

tarekgh commented Jan 9, 2023

@gao-artur thanks for providing the repro. I see your repro code is failing on .NET 7.0.0 and 7.0.1 too. That is different than what @msmolka claimed that the problem was not happening on 7.0.0. @msmolka could you please provide a complete sample code to demonstrate your issue just in case it is different than what @gao-artur reported here?

@msmolka
Copy link
Author

msmolka commented Jan 9, 2023

Hi, here is working repo, but during creation I found that bug is in fact in Microsoft.Extensions.Configuration.Binder package.

When package is 7.0.0 and below all is working. 7.0.1 and above not.

Repo will throw exception in v 7.0.1 and will work below.

ConfigBinding.zip

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 9, 2023
@tarekgh
Copy link
Member

tarekgh commented Jan 9, 2023

Thanks @msmolka I'll try your repo too. I already created a local fix for @gao-artur issue.

This was referenced Jan 10, 2023
Merged
Closed
Closed
@tarekgh
Copy link
Member

tarekgh commented Jan 13, 2023

Fixed by #80562

@tarekgh tarekgh closed this as completed Jan 13, 2023
@msmolka msmolka mentioned this issue Jan 26, 2023
Merged
@tarekgh tarekgh mentioned this issue Feb 6, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tarekgh tarekgh

Labels
area-Extensions-Options needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Milestone
7.0.x
Development

No branches or pull requests
5 participants
@CarnaViire @gao-artur @msmolka @jeffschwMSFT @tarekgh