From 9f7ae443e44eca366bbdd21fa08b63f5afcea728 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Mon, 30 Dec 2019 18:26:45 -0800 Subject: [PATCH] Metadata: Add Getter/Setter support for indexed properties Part of #13610 --- .../ConventionEntityTypeExtensions.cs | 22 +++++++ .../Extensions/MutableEntityTypeExtensions.cs | 24 ++++++++ .../Extensions/PropertyBaseExtensions.cs | 17 ++++++ src/EFCore/Infrastructure/ModelValidator.cs | 2 +- .../Internal/ClrPropertyGetterFactory.cs | 12 +++- .../Internal/ClrPropertySetterFactory.cs | 16 ++++-- src/EFCore/Metadata/Internal/EntityType.cs | 4 +- .../Internal/InternalEntityTypeBuilder.cs | 2 +- src/EFCore/Metadata/Internal/TypeBase.cs | 25 ++++++++ .../Metadata/Internal/TypeBaseExtensions.cs | 23 ++++---- src/EFCore/Properties/CoreStrings.Designer.cs | 10 +++- src/EFCore/Properties/CoreStrings.resx | 57 ++++++++++--------- src/Shared/PropertyInfoExtensions.cs | 2 +- .../Internal/ClrPropertyGetterFactoryTest.cs | 26 +++++++++ .../Internal/ClrPropertySetterFactoryTest.cs | 32 +++++++++++ 15 files changed, 224 insertions(+), 50 deletions(-) diff --git a/src/EFCore/Extensions/ConventionEntityTypeExtensions.cs b/src/EFCore/Extensions/ConventionEntityTypeExtensions.cs index 599b52d5354..728503970ec 100644 --- a/src/EFCore/Extensions/ConventionEntityTypeExtensions.cs +++ b/src/EFCore/Extensions/ConventionEntityTypeExtensions.cs @@ -504,6 +504,28 @@ public static IConventionProperty AddProperty( bool setTypeConfigurationSource = true, bool fromDataAnnotation = false) => entityType.AddProperty(name, propertyType, null, setTypeConfigurationSource, fromDataAnnotation); + /// + /// Adds an indexed property to this entity type. + /// + /// The entity type to add the property to. + /// The name of the property to add. + /// The type of value the property will hold. + /// Indicates whether the type configuration source should be set. + /// Indicates whether the configuration was specified using a data annotation. + /// The newly created property. + public static IConventionProperty AddIndexedProperty( + [NotNull] this IConventionEntityType entityType, [NotNull] string name, [NotNull] Type propertyType, + bool setTypeConfigurationSource = true, bool fromDataAnnotation = false) + { + Check.NotNull(entityType, nameof(entityType)); + + var indexerPropertyInfo = entityType.GetIndexerPropertyInfo(); + + return indexerPropertyInfo != null + ? entityType.AddProperty(name, propertyType, indexerPropertyInfo, setTypeConfigurationSource, fromDataAnnotation) + : null; + } + /// /// Gets the index defined on the given property. Returns null if no index is defined. /// diff --git a/src/EFCore/Extensions/MutableEntityTypeExtensions.cs b/src/EFCore/Extensions/MutableEntityTypeExtensions.cs index a7a19109e54..f54186f7529 100644 --- a/src/EFCore/Extensions/MutableEntityTypeExtensions.cs +++ b/src/EFCore/Extensions/MutableEntityTypeExtensions.cs @@ -6,6 +6,8 @@ using System.Linq.Expressions; using System.Reflection; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Utilities; @@ -463,6 +465,28 @@ public static IMutableProperty AddProperty( [NotNull] this IMutableEntityType entityType, [NotNull] string name, [NotNull] Type propertyType) => entityType.AddProperty(name, propertyType, null); + /// + /// Adds an indexed property to this entity type. + /// + /// The entity type to add the property to. + /// The name of the property to add. + /// The type of value the property will hold. + /// The newly created property. + public static IMutableProperty AddIndexedProperty( + [NotNull] this IMutableEntityType entityType, [NotNull] string name, [NotNull] Type propertyType) + { + Check.NotNull(entityType, nameof(entityType)); + + var indexerPropertyInfo = entityType.GetIndexerPropertyInfo(); + if (indexerPropertyInfo == null) + { + throw new InvalidOperationException( + CoreStrings.NonIndexerEntityType(name, entityType.DisplayName(), typeof(string).ShortDisplayName())); + } + + return entityType.AddProperty(name, propertyType, indexerPropertyInfo); + } + /// /// Gets the index defined on the given property. Returns null if no index is defined. /// diff --git a/src/EFCore/Extensions/PropertyBaseExtensions.cs b/src/EFCore/Extensions/PropertyBaseExtensions.cs index f60775cc779..730f2a0ce23 100644 --- a/src/EFCore/Extensions/PropertyBaseExtensions.cs +++ b/src/EFCore/Extensions/PropertyBaseExtensions.cs @@ -82,6 +82,23 @@ public static string GetFieldName([NotNull] this IPropertyBase propertyBase) public static bool IsShadowProperty([NotNull] this IPropertyBase property) => Check.NotNull(property, nameof(property)).GetIdentifyingMemberInfo() == null; + /// + /// Gets a value indicating whether this is an indexer property. An indexer property is one that is accessed through + /// indexer on entity class. + /// + /// The property to check. + /// + /// True if the property is an indexer property, otherwise false. + /// + public static bool IsIndexerProperty([NotNull] this IPropertyBase property) + { + Check.NotNull(property, nameof(property)); + + var identifyingMemberInfo = property.GetIdentifyingMemberInfo(); + + return identifyingMemberInfo != null && identifyingMemberInfo.IsSameAs(property.DeclaringType.GetIndexerPropertyInfo()); + } + /// /// /// Gets the being used for this property. diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index ecc38f320b8..576070f54e5 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -783,7 +783,7 @@ protected virtual void ValidateFieldMapping( .GetDeclaredProperties() .Cast() .Concat(entityType.GetDeclaredNavigations()) - .Where(p => !p.IsShadowProperty())); + .Where(p => !p.IsShadowProperty() && !p.IsIndexerProperty())); var constructorBinding = (InstantiationBinding)entityType[CoreAnnotationNames.ConstructorBinding]; diff --git a/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs b/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs index 27909e58e13..8b7952ed017 100644 --- a/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs +++ b/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs @@ -40,7 +40,7 @@ protected override IClrPropertyGetter CreateGeneric( Expression.Lambda>(readExpression, entityParameter).Compile(), Expression.Lambda>(hasDefaultValueExpression, entityParameter).Compile()); + + Expression CreateMemberAccess(Expression parameter) + { + return propertyBase?.IsIndexerProperty() == true + ? Expression.MakeIndex( + entityParameter, (PropertyInfo)memberInfo, new List() { Expression.Constant(propertyBase.Name) }) + : (Expression)Expression.MakeMemberAccess(parameter, memberInfo); + } } } } diff --git a/src/EFCore/Metadata/Internal/ClrPropertySetterFactory.cs b/src/EFCore/Metadata/Internal/ClrPropertySetterFactory.cs index 0cd7413fec0..f8a917bd8e3 100644 --- a/src/EFCore/Metadata/Internal/ClrPropertySetterFactory.cs +++ b/src/EFCore/Metadata/Internal/ClrPropertySetterFactory.cs @@ -45,8 +45,7 @@ protected override IClrPropertySetter CreateGeneric(setter) : (IClrPropertySetter)new ClrPropertySetter(setter); + + Expression CreateMemberAssignment(Expression parameter) + { + return propertyBase?.IsIndexerProperty() == true + ? Expression.Assign( + Expression.MakeIndex( + entityParameter, (PropertyInfo)memberInfo, new List() { Expression.Constant(propertyBase.Name) }), + convertedParameter) + : Expression.MakeMemberAccess(parameter, memberInfo).Assign(convertedParameter); + } } } } diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 81f540f5689..85a25d3d560 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -1599,7 +1599,7 @@ private Type ValidateClrMember(string name, MemberInfo memberInfo, bool throwOnN if (name != memberInfo.GetSimpleMemberName()) { - if ((memberInfo as PropertyInfo)?.IsEFIndexerProperty() != true) + if ((memberInfo as PropertyInfo)?.IsIndexerProperty() != true) { if (throwOnNameMismatch) { @@ -2066,7 +2066,7 @@ public virtual Property AddProperty( if (memberInfo != null && propertyType != memberInfo.GetMemberType() - && (memberInfo as PropertyInfo)?.IsEFIndexerProperty() != true) + && (memberInfo as PropertyInfo)?.IsIndexerProperty() != true) { if (typeConfigurationSource != null) { diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 6aabdf9f7ba..3461a2bf8a4 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -682,7 +682,7 @@ public virtual bool CanAddProperty([NotNull] Type propertyType, [NotNull] string var memberInfo = Metadata.ClrType.GetMembersInHierarchy(name).FirstOrDefault(); if (memberInfo != null && propertyType != memberInfo.GetMemberType() - && (memberInfo as PropertyInfo)?.IsEFIndexerProperty() != true + && (memberInfo as PropertyInfo)?.IsIndexerProperty() != true && typeConfigurationSource != null) { return false; diff --git a/src/EFCore/Metadata/Internal/TypeBase.cs b/src/EFCore/Metadata/Internal/TypeBase.cs index 49f917ce675..caecbf0ab52 100644 --- a/src/EFCore/Metadata/Internal/TypeBase.cs +++ b/src/EFCore/Metadata/Internal/TypeBase.cs @@ -25,6 +25,7 @@ public abstract class TypeBase : ConventionAnnotatable, IMutableTypeBase, IConve private readonly Dictionary _ignoredMembers = new Dictionary(StringComparer.Ordinal); + private PropertyInfo _indexerPropertyInfo; private Dictionary _runtimeProperties; private Dictionary _runtimeFields; @@ -168,6 +169,29 @@ public virtual IReadOnlyDictionary GetRuntimeFields() return _runtimeFields; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual PropertyInfo GetIndexerPropertyInfo() + { + if (ClrType == null) + { + return null; + } + + if (_indexerPropertyInfo == null) + { + var indexerPropertyInfo = GetRuntimeProperties().Values.FirstOrDefault(pi => pi.IsIndexerProperty()); + + Interlocked.CompareExchange(ref _indexerPropertyInfo, indexerPropertyInfo, null); + } + + return _indexerPropertyInfo; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -198,6 +222,7 @@ public virtual void ClearCaches() { _runtimeProperties = null; _runtimeFields = null; + _indexerPropertyInfo = null; Thread.MemoryBarrier(); } diff --git a/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs b/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs index f6846cca13e..3132d156fe8 100644 --- a/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs +++ b/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs @@ -45,6 +45,15 @@ public static IReadOnlyDictionary GetRuntimeProperties([No public static IReadOnlyDictionary GetRuntimeFields([NotNull] this ITypeBase type) => (type as TypeBase).GetRuntimeFields(); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public static PropertyInfo GetIndexerPropertyInfo([NotNull] this ITypeBase type) + => (type as TypeBase).GetIndexerPropertyInfo(); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -53,17 +62,9 @@ public static IReadOnlyDictionary GetRuntimeFields([NotNull] /// public static MemberInfo FindClrMember([NotNull] this TypeBase type, [NotNull] string name) { - if (type.GetRuntimeProperties().TryGetValue(name, out var property)) - { - return property; - } - - if (type.GetRuntimeFields().TryGetValue(name, out var field)) - { - return field; - } - - return null; + return type.GetRuntimeProperties().TryGetValue(name, out var property) + ? property + : (MemberInfo)(type.GetRuntimeFields().TryGetValue(name, out var field) ? field : null); } /// diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index d4a9adfad03..25169f966c4 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -1,4 +1,4 @@ -// +// using System; using System.Reflection; @@ -2308,6 +2308,14 @@ public static string SkipNavigationNonCollection([CanBeNull] object navigation, GetString("SkipNavigationNonCollection", nameof(navigation), nameof(entityType)), navigation, entityType); + /// + /// Cannot add property '{property}' on entity type '{entity}' since there is no indexer on '{entity}' taking a single argument of type '{type}'. + /// + public static string NonIndexerEntityType([CanBeNull] object property, [CanBeNull] object entity, [CanBeNull] object type) + => string.Format( + GetString("NonIndexerEntityType", nameof(property), nameof(entity), nameof(type)), + property, entity, type); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 372b8d2ba90..54d2bba2744 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1,17 +1,17 @@  - @@ -1245,4 +1245,7 @@ The skip navigation '{navigation}' on entity type '{entityType}' is not a collection. Only collection skip navigation properties are currently supported. + + Cannot add property '{property}' on entity type '{entity}' since there is no indexer on '{entity}' taking a single argument of type '{type}'. + \ No newline at end of file diff --git a/src/Shared/PropertyInfoExtensions.cs b/src/Shared/PropertyInfoExtensions.cs index 267d4c9f6b2..69c8330c93f 100644 --- a/src/Shared/PropertyInfoExtensions.cs +++ b/src/Shared/PropertyInfoExtensions.cs @@ -22,7 +22,7 @@ public static bool IsCandidateProperty(this PropertyInfo propertyInfo, bool need && (!publicOnly || propertyInfo.GetMethod.IsPublic) && propertyInfo.GetIndexParameters().Length == 0; - public static bool IsEFIndexerProperty([NotNull] this PropertyInfo propertyInfo) + public static bool IsIndexerProperty([NotNull] this PropertyInfo propertyInfo) { if (propertyInfo.PropertyType == typeof(object)) { diff --git a/test/EFCore.Tests/Metadata/Internal/ClrPropertyGetterFactoryTest.cs b/test/EFCore.Tests/Metadata/Internal/ClrPropertyGetterFactoryTest.cs index b87650fa6eb..63469236328 100644 --- a/test/EFCore.Tests/Metadata/Internal/ClrPropertyGetterFactoryTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/ClrPropertyGetterFactoryTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.InMemory.Metadata.Conventions; @@ -82,6 +83,19 @@ public void Delegate_getter_is_returned_for_struct_property_info() new Customer { Id = 7, Fuel = new Fuel(1.0) })); } + [ConditionalFact] + public void Delegate_getter_is_returned_for_index_property() + { + var modelBuilder = new ModelBuilder(InMemoryConventionSetBuilder.Build()); + modelBuilder.Entity().Property(e => e.Id); + var propertyA = modelBuilder.Entity().Metadata.AddIndexedProperty("PropertyA", typeof(string)); + var propertyB = modelBuilder.Entity().Metadata.AddIndexedProperty("PropertyB", typeof(int)); + modelBuilder.FinalizeModel(); + + Assert.Equal("ValueA", new ClrPropertyGetterFactory().Create(propertyA).GetClrValue(new IndexedClass { Id = 7 })); + Assert.Equal(123, new ClrPropertyGetterFactory().Create(propertyB).GetClrValue(new IndexedClass { Id = 7 })); + } + private class Customer { internal int Id { get; set; } @@ -93,5 +107,17 @@ private struct Fuel public Fuel(double volume) => Volume = volume; public double Volume { get; } } + + private class IndexedClass + { + private readonly Dictionary _internalValues = new Dictionary + { + {"PropertyA", "ValueA" }, + {"PropertyB", 123 } + }; + + internal int Id { get; set; } + internal object this[string name] => _internalValues[name]; + } } } diff --git a/test/EFCore.Tests/Metadata/Internal/ClrPropertySetterFactoryTest.cs b/test/EFCore.Tests/Metadata/Internal/ClrPropertySetterFactoryTest.cs index 79d9c78bb56..a8cc7a04c6a 100644 --- a/test/EFCore.Tests/Metadata/Internal/ClrPropertySetterFactoryTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/ClrPropertySetterFactoryTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using Microsoft.EntityFrameworkCore.Infrastructure; using Xunit; @@ -229,6 +230,25 @@ public void Delegate_setter_throws_if_no_setter_found() () => new ClrPropertySetterFactory().Create(property)); } + [ConditionalFact] + public void Delegate_setter_can_set_index_properties() + { + var entityType = CreateModel().AddEntityType(typeof(IndexedClass)); + var propertyA = entityType.AddIndexedProperty("PropertyA", typeof(string)); + var propertyB = entityType.AddIndexedProperty("PropertyB", typeof(int)); + + var indexedClass = new IndexedClass { Id = 7 }; + + Assert.Equal("ValueA", indexedClass["PropertyA"]); + Assert.Equal(123, indexedClass["PropertyB"]); + + new ClrPropertySetterFactory().Create(propertyA).SetClrValue(indexedClass, "UpdatedValue"); + new ClrPropertySetterFactory().Create(propertyB).SetClrValue(indexedClass, 42); + + Assert.Equal("UpdatedValue", indexedClass["PropertyA"]); + Assert.Equal(42, indexedClass["PropertyB"]); + } + private IMutableModel CreateModel() => new Model(); @@ -275,6 +295,18 @@ private class BaseEntity public int NoSetterProperty { get; } } + private class IndexedClass + { + private readonly Dictionary _internalValues = new Dictionary + { + {"PropertyA", "ValueA" }, + {"PropertyB", 123 } + }; + + internal int Id { get; set; } + internal object this[string name] { get => _internalValues[name]; set => _internalValues[name] = value; } + } + #endregion } }