Skip to content

Commit

Permalink
[release/7.0] Fix configuration binding with types implementing IDict…
Browse files Browse the repository at this point in the history
…ionary<,> (#79019)

* Fix configuration binding with types implementing IDictionary<,>

* OOB package authoring changes

Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>
  • Loading branch information
tarekgh and carlossanlop committed Nov 30, 2022
1 parent e721ae9 commit 30426a7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig

if (dictionaryInterface != null)
{
BindConcreteDictionary(bindingPoint.Value!, dictionaryInterface, config, options);
BindDictionary(bindingPoint.Value!, dictionaryInterface, config, options);
}
else
{
Expand Down Expand Up @@ -549,25 +549,28 @@ private static bool CanBindToTheseConstructorParameters(ParameterInfo[] construc
}
}

BindConcreteDictionary(dictionary, dictionaryType, config, options);
BindDictionary(dictionary, genericType, config, options);

return dictionary;
}

// Binds and potentially overwrites a concrete dictionary.
// Binds and potentially overwrites a dictionary object.
// This differs from BindDictionaryInterface because this method doesn't clone
// the dictionary; it sets and/or overwrites values directly.
// When a user specifies a concrete dictionary or a concrete class implementing IDictionary<,>
// in their config class, then that value is used as-is. When a user specifies an interface (instantiated)
// in their config class, then it is cloned to a new dictionary, the same way as other collections.
[RequiresDynamicCode(DynamicCodeWarningMessage)]
[RequiresUnreferencedCode("Cannot statically analyze what the element type is of the value objects in the dictionary so its members may be trimmed.")]
private static void BindConcreteDictionary(
object? dictionary,
private static void BindDictionary(
object dictionary,
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties)]
Type dictionaryType,
IConfiguration config, BinderOptions options)
{
Debug.Assert(dictionaryType.IsGenericType &&
(dictionaryType.GetGenericTypeDefinition() == typeof(IDictionary<,>) || dictionaryType.GetGenericTypeDefinition() == typeof(Dictionary<,>)));

Type keyType = dictionaryType.GenericTypeArguments[0];
Type valueType = dictionaryType.GenericTypeArguments[1];
bool keyTypeIsEnum = keyType.IsEnum;
Expand All @@ -589,13 +592,10 @@ private static bool CanBindToTheseConstructorParameters(ParameterInfo[] construc

Debug.Assert(dictionary is not null);

Type dictionaryObjectType = dictionary.GetType();

MethodInfo tryGetValue = dictionaryObjectType.GetMethod("TryGetValue", BindingFlags.Public | BindingFlags.Instance)!;
MethodInfo tryGetValue = dictionaryType.GetMethod("TryGetValue", DeclaredOnlyLookup)!;
PropertyInfo? indexerProperty = dictionaryType.GetProperty("Item", DeclaredOnlyLookup);

// dictionary should be of type Dictionary<,> or of type implementing IDictionary<,>
PropertyInfo? setter = dictionaryObjectType.GetProperty("Item", BindingFlags.Public | BindingFlags.Instance);
if (setter is null || !setter.CanWrite)
if (indexerProperty is null || !indexerProperty.CanWrite)
{
// Cannot set any item on the dictionary object.
return;
Expand Down Expand Up @@ -623,7 +623,7 @@ private static bool CanBindToTheseConstructorParameters(ParameterInfo[] construc
options: options);
if (valueBindingPoint.HasNewValue)
{
setter.SetValue(dictionary, valueBindingPoint.Value, new object[] { key });
indexerProperty.SetValue(dictionary, valueBindingPoint.Value, new object[] { key });
}
}
catch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<EnableDefaultItems>true</EnableDefaultItems>
<IsPackable>true</IsPackable>
<EnableAOTAnalyzer>true</EnableAOTAnalyzer>
<ServicingVersion>1</ServicingVersion>
<ServicingVersion>2</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<PackageDescription>Functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration.</PackageDescription>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,7 @@ public void CanBindInitializedCustomIndirectlyDerivedIEnumerableList()
}

[Fact]
public void CanBindInitializedIReadOnlyDictionaryAndDoesNotMofifyTheOriginal()
public void CanBindInitializedIReadOnlyDictionaryAndDoesNotModifyTheOriginal()
{
// A field declared as IEnumerable<T> that is instantiated with a class
// that indirectly implements IEnumerable<T> is still bound, but with
Expand Down Expand Up @@ -1672,6 +1672,13 @@ public class ImplementerOfIDictionaryClass<TKey, TValue> : IDictionary<TKey, TVa
public bool TryGetValue(TKey key, out TValue value) => _dict.TryGetValue(key, out value);

System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => _dict.GetEnumerator();

// The following are members which have the same names as the IDictionary<,> members.
// The following members test that there's no System.Reflection.AmbiguousMatchException when binding to the dictionary.
private string? v;
public string? this[string key] { get => v; set => v = value; }
public bool TryGetValue() { return true; }

}

public class ExtendedDictionary<TKey, TValue> : Dictionary<TKey, TValue>
Expand Down

0 comments on commit 30426a7

Please sign in to comment.