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

ConfigurationBinder polymorphism on bind - does not support binding to derived members #93130

Open
ericstj opened this issue Oct 6, 2023 · 2 comments
Labels
area-Extensions-Configuration needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration source-generator Indicates an issue with a source generator feature
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Oct 6, 2023

Description

Related: #83599, #92137, #91324

The runtime binder will use the runtime type when deciding what members to bind, however the source generator will only use the referenced type - which may be a base type.

We could improve this by including cast tests to derived types when handling a base type's member.
EG:

public static void BindCore(IConfiguration configuration, ref Base instance)
{
    if (instance is Derived1 derived1)
        BindCore(configuration, ref derived1);
    else if (instance is Derived2 derived2)
        BindCore(configuration, ref derived2);
    else if (instance is Derived3 derived3)
        BindCore(configuration, ref derived3);
    else 
    { 
      // abstract binding logic/
    } 
}

We might be able to use some common type multiplexing method to do this, like a bind for object that checks for casting to various types, but it would need to be careful of the order in which to examine types.

This approach would create code overlap in handling of members on base types -- we have this overlap today. If we wanted to try and remove that we could make base bind methods responsible for binding base members - at least for cases where the initialization strategy is parameterless construct.

In addition to how we code-gen this we'd need to think about how we discover derived types. Do we always consider derived types that might be in the compilation? Do we add some API for developers to express which derived types we should look for?

Reproduction Steps

Note: requires fix for #92137 project: configAbstract.zip

using Microsoft.Extensions.Configuration;

var c = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string,string?>()
    {
        ["Value"] = "Hello",
        ["Member:Value"] = "Hello world",
        ["Member:Value2"] = "Hello world2"
    })
    .Build();

MyBase x = new Derived()
{
    Member = new Derived()
};

c.Bind(x);
Console.WriteLine(x.Value);
Console.WriteLine(x.Member?.Value);
Console.WriteLine(((Derived?)x.Member)?.Value2);

Derived d = new();
c.GetSection("Member").Bind(d);
Console.WriteLine(d.Value);
Console.WriteLine(d.Value2);

public abstract class MyBase 
{   
    public MyBase() {} 
    public virtual string? Value {get; set;}

    public MyBase? Member { get; set;}
}

public class Derived : MyBase
{
    public Derived() : base()
    { }

    public string? Value2 { get; set; }
}

Expected behavior

Ideally, the derived type will bind the same way, regardless of bind call:

Hello
Hello world
Hello world2

Hello world
Hello world2

Actual behavior

Only the members in the reference type are bound.

Hello
Hello world

Hello world
Hello world2

Regression?

No, but it's a gap between runtime binder and source-gen binder.

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 6, 2023
@ghost
Copy link

ghost commented Oct 6, 2023

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

Issue Details

Description

Related: #83599, #92137, #91324

The runtime binder will use the runtime type when deciding what members to bind, however the source generator will only use the referenced type - which may be a base type.

We could improve this by including cast tests to derived types when handling a base type's member.
EG:

public static void BindCore(IConfiguration configuration, ref Base instance)
{
    if (instance is Derived1 derived1)
        BindCore(configuration, ref derived1);
    else if (instance is Derived2 derived2)
        BindCore(configuration, ref derived2);
    else if (instance is Derived2 derived3)
        BindCore(configuration, ref derived3);
    else 
    { 
      // abstract binding logic/
    } 
}

We might be able to use some common type multiplexing method to do this, like a bind for object that checks for casting to various types, but it would need to be careful of the order in which to examine types.

This approach would create code overlap in handling of members on base types -- we have this overlap today. If we wanted to try and remove that we could make base bind methods responsible for binding base members - at least for cases where the initialization strategy is parameterless construct.

In addition to how we code-gen this we'd need to think about how we discover derived types. Do we always consider derived types that might be in the compilation? Do we add some API for developers to express which derived types we should look for?

Reproduction Steps

Note: requires fix for #92137 project: configAbstract.zip

using Microsoft.Extensions.Configuration;

var c = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string,string?>()
    {
        ["Value"] = "Hello",
        ["Member:Value"] = "Hello world",
        ["Member:Value2"] = "Hello world2"
    })
    .Build();

MyBase x = new Derived()
{
    Member = new Derived()
};

c.Bind(x);
Console.WriteLine(x.Value);
Console.WriteLine(x.Member?.Value);
Console.WriteLine(((Derived?)x.Member)?.Value2);

Derived d = new();
c.GetSection("Member").Bind(d);
Console.WriteLine(d.Value);
Console.WriteLine(d.Value2);

public abstract class MyBase 
{   
    public MyBase() {} 
    public virtual string? Value {get; set;}

    public MyBase? Member { get; set;}
}

public class Derived : MyBase
{
    public Derived() : base()
    { }

    public string? Value2 { get; set; }
}

Expected behavior

Ideally, the derived type will bind the same way, regardless of bind call:

Hello
Hello world
Hello world2

Hello world
Hello world2

Actual behavior

Only the members in the reference type are bound.

Hello
Hello world

Hello world
Hello world2

Regression?

No, but it's a gap between runtime binder and source-gen binder.

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: ericstj
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@tarekgh tarekgh added this to the 9.0.0 milestone Oct 6, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 6, 2023
@tarekgh tarekgh added untriaged New issue has not been triaged by the area owner source-generator Indicates an issue with a source generator feature labels Oct 6, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 6, 2023
@ericstj ericstj modified the milestones: 9.0.0, Future Aug 6, 2024
@ericstj ericstj added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 6, 2024
@ericstj
Copy link
Member Author

ericstj commented Aug 6, 2024

Moving out -- haven't seen requests for this behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Configuration needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

2 participants