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

Can't bind IDictionary #442

Closed
ghost opened this Issue May 21, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@ghost

ghost commented May 21, 2016

Properties which directly expose IDictionary aren't bound, while properties which derive from IDictionary will.

For example:

public interface IMyDictionary<TKey, TValue> : IDictionary<TKey, TValue> { }

public class MyDictionary<TKey, TValue> : Dictionary<TKey, TValue>, IMyDictionary<TKey, TValue> { }

public class MySettings
{
  public IDictionary<string, string> WontBind { get; } = new Dictionary<string, string>();
  public IMyDictionary<string, string> WillBind { get; } = new MyDictionary<string, string>();
}
@adamholdenyall

This comment has been minimized.

adamholdenyall commented Jun 7, 2016

Is there any chance that a similar change could be made to support ICollection?

For instance, OpenIdConnectOptions has an ICollection for its "Scope" property. I've worked around this by adding a "Scopes" string with space seperator, getting the string value, and splitting into a list in code. But automatic binding to a list would be preferable.

EDIT: I just realized that there is no setter for that property... So probably not a good example of the issue. You would have to clear, then add all the properties from the JSON.

@andrewlock

This comment has been minimized.

Contributor

andrewlock commented Jun 10, 2016

@adamholdenyall the proposed pull request #453 would actually deal with this situation too. Previously, the binder would not find directly exposed IDictionary<> or ICollection<> properties, the fix allows it to find them.

When a property is an interface, it must already be initialised in order to be bound (the configuration binder wouldn't know which concrete class to assign to an interface property), so the lack of a setter is not a problem.

WRT OpenIdConnectOptions, the proposed fix would allow that to be bound as you hope.

@reduckted

This comment has been minimized.

reduckted commented Jul 4, 2016

the configuration binder wouldn't know which concrete class to assign to an interface property

Is there any reason why the binder couldn't create a List<> for an ICollection<> property (and an IEnumerable<> property for that matter), and a Dictionary<> for an IDictionary<> property? The fact that the property is an interface means the implementation type shouldn't matter (and if it did, the caller could just initialize it to the desired type before calling in to the binder).

@HaoK HaoK added the enhancement label Jul 9, 2016

@HaoK

This comment has been minimized.

Member

HaoK commented Jul 11, 2016

Fixed via #453

@HaoK HaoK closed this Jul 11, 2016

@HaoK

This comment has been minimized.

Member

HaoK commented Jul 11, 2016

Creation issues for generic interfaces like IList/ICollection/IEnumerable will be tracked via #466

@HaoK HaoK reopened this Jul 11, 2016

@HaoK HaoK closed this Jul 11, 2016

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