Make property bag entity types use an indexer prop rather than a shad…

…ow prop (#21813) Allow indexer properties to be used without first initializing them Fixes #21718 Fixes #21720
dotnet · Jul 28, 2020 · 9c8f116 · 9c8f116
1 parent ba8c2c9
commit 9c8f116
Show file tree

Hide file tree

Showing 22 changed files with 161 additions and 119 deletions.
diff --git a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs
@@ -159,7 +159,7 @@ private static void ProcessJObjectProperty(IConventionEntityTypeBuilder entityTy
                 var jObjectProperty = entityType.FindDeclaredProperty(JObjectPropertyName);
                 if (jObjectProperty != null)
                 {
-                    entityType.Builder.HasNoUnusedShadowProperties(new[] { jObjectProperty });
+                    entityType.Builder.RemoveUnusedImplicitProperties(new[] { jObjectProperty });
                 }
             }
         }

diff --git a/src/EFCore.Relational/Storage/Internal/TypeMappedRelationalParameter.cs b/src/EFCore.Relational/Storage/Internal/TypeMappedRelationalParameter.cs
@@ -70,9 +70,8 @@ public override void AddDbParameter(DbCommand command, object value)
             Check.NotNull(command, nameof(command));
 
             command.Parameters
-                .Add(
-                    RelationalTypeMapping
-                        .CreateParameter(command, Name, value, IsNullable));
+                .Add(RelationalTypeMapping
+                    .CreateParameter(command, Name, value, IsNullable));
         }
     }
 }
diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -697,8 +697,7 @@ private T ReadTemporaryValue<T>(int storeGeneratedIndex)
             => _temporaryValues.GetValue<T>(storeGeneratedIndex);
 
         internal static readonly MethodInfo GetCurrentValueMethod
-            =
-            typeof(InternalEntityEntry).GetTypeInfo().GetDeclaredMethods(nameof(GetCurrentValue)).Single(
+            = typeof(InternalEntityEntry).GetTypeInfo().GetDeclaredMethods(nameof(GetCurrentValue)).Single(
                 m => m.IsGenericMethod);
 
         /// <summary>

diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs
@@ -942,14 +942,6 @@ private static InternalEntityEntry FindOrCreateJoinEntry(
                     new MaterializationContext(ValueBuffer.Empty, entry.StateManager.Context));
 
                 joinEntry = entry.StateManager.GetOrCreateEntry(joinEntity, joinEntityType);
-
-                foreach (var property in joinEntityType.GetProperties()) // Remove when #21720 is implemented
-                {
-                    if (property.IsIndexerProperty())
-                    {
-                        ((PropertyBase)property).Setter.SetClrValue(joinEntity, property.ClrType.GetDefaultValue());
-                    }
-                }
             }
 
             SetForeignKeyProperties(joinEntry, entry, skipNavigation.ForeignKey, setModified, fromQuery);

diff --git a/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs b/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs
@@ -141,9 +141,7 @@ protected virtual Expression CreateSnapshotExpression(
                 }
 
                 var memberInfo = propertyBase.GetMemberInfo(forMaterialization: false, forSet: false);
-                var memberAccess = propertyBase.IsIndexerProperty()
-                    ? Expression.MakeIndex(entityVariable, (PropertyInfo)memberInfo, new[] { Expression.Constant(propertyBase.Name) })
-                    : (Expression)Expression.MakeMemberAccess(entityVariable, memberInfo);
+                var memberAccess = PropertyBase.CreateMemberAccess(propertyBase, entityVariable, memberInfo);
 
                 if (memberAccess.Type != propertyBase.ClrType)
                 {

diff --git a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs
@@ -106,22 +106,20 @@ IReadOnlyList<IConventionProperty> GetOrCreateProperties(
             [CanBeNull] IEnumerable<MemberInfo> memberInfos, bool fromDataAnnotation = false);
 
         /// <summary>
-        ///     Removes shadow properties in the given list if they are not part of any metadata object.
+        ///     Removes properties in the given list if they are not part of any metadata object.
         /// </summary>
         /// <param name="properties"> The properties to remove. </param>
-        /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
-        IConventionEntityTypeBuilder HasNoUnusedShadowProperties(
-            [NotNull] IReadOnlyList<IConventionProperty> properties, bool fromDataAnnotation = false);
+        IConventionEntityTypeBuilder RemoveUnusedImplicitProperties([NotNull] IReadOnlyList<IConventionProperty> properties);
 
         /// <summary>
         ///     Removes shadow properties in the given list if they are not part of any metadata object.
         /// </summary>
         /// <param name="properties"> The properties to remove. </param>
         /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
-        [Obsolete("Use HasNoUnusedShadowProperties")]
+        [Obsolete("Use RemoveUnusedImplicitProperties")]
         IConventionEntityTypeBuilder RemoveUnusedShadowProperties(
             [NotNull] IReadOnlyList<IConventionProperty> properties, bool fromDataAnnotation = false)
-            => HasNoUnusedShadowProperties(properties, fromDataAnnotation);
+            => RemoveUnusedImplicitProperties(properties);
 
         /// <summary>
         ///     Returns an object that can be used to configure the service property with the given member info.

diff --git a/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs b/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs
@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System.Collections.Generic;
 using System.Linq;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Infrastructure;
@@ -23,8 +22,6 @@ public class ManyToManyJoinEntityTypeConvention :
         ISkipNavigationForeignKeyChangedConvention,
         ISkipNavigationRemovedConvention
     {
-        private const string JoinEntityTypeNameTemplate = "{0}{1}";
-
         /// <summary>
         ///     Creates a new instance of <see cref="ManyToManyJoinEntityTypeConvention" />.
         /// </summary>
@@ -112,10 +109,7 @@ private void CreateJoinEntityType(IConventionSkipNavigationBuilder skipNavigatio
             var inverseEntityType = inverseSkipNavigation.DeclaringEntityType;
             var model = declaringEntityType.Model;
 
-            var joinEntityTypeName = string.Format(
-                    JoinEntityTypeNameTemplate,
-                    declaringEntityType.ShortName(),
-                    inverseEntityType.ShortName());
+            var joinEntityTypeName = declaringEntityType.ShortName() + inverseEntityType.ShortName();
             if (model.FindEntityType(joinEntityTypeName) != null)
             {
                 var otherIdentifiers = model.GetEntityTypes().ToDictionary(et => et.Name, et => 0);
@@ -152,7 +146,8 @@ private static ForeignKey CreateSkipNavigationForeignKey(
             => joinEntityTypeBuilder
                 .HasRelationship(
                     skipNavigation.DeclaringEntityType,
-                    ConfigurationSource.Convention)
+                    ConfigurationSource.Convention,
+                    required: true)
                 .IsUnique(false, ConfigurationSource.Convention)
                 .Metadata;
     }

diff --git a/src/EFCore/Metadata/ITypeBase.cs b/src/EFCore/Metadata/ITypeBase.cs
@@ -37,5 +37,10 @@ public interface ITypeBase : IAnnotatable
         ///     Gets whether this entity type can share its ClrType with other entities.
         /// </summary>
         bool HasSharedClrType { get; }
+
+        /// <summary>
+        ///     Gets whether this entity type has an indexer which is able to contain arbitrary properties.
+        /// </summary>
+        bool IsPropertyBag { get; }
     }
 }
diff --git a/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs b/src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs
@@ -40,7 +40,7 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
             Expression readExpression;
             if (memberInfo.DeclaringType.IsAssignableFrom(typeof(TEntity)))
             {
-                readExpression = CreateMemberAccess(entityParameter);
+                readExpression = PropertyBase.CreateMemberAccess(propertyBase, entityParameter, memberInfo);
             }
             else
             {
@@ -57,7 +57,7 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
                         Expression.Condition(
                             Expression.ReferenceEqual(converted, Expression.Constant(null)),
                             Expression.Default(memberInfo.GetMemberType()),
-                            CreateMemberAccess(converted))
+                            PropertyBase.CreateMemberAccess(propertyBase, converted, memberInfo))
                     });
             }
 
@@ -74,14 +74,6 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
             return new ClrPropertyGetter<TEntity, TValue>(
                 Expression.Lambda<Func<TEntity, TValue>>(readExpression, entityParameter).Compile(),
                 Expression.Lambda<Func<TEntity, bool>>(hasDefaultValueExpression, entityParameter).Compile());
-
-            Expression CreateMemberAccess(Expression parameter)
-            {
-                return propertyBase?.IsIndexerProperty() == true
-                    ? Expression.MakeIndex(
-                        parameter, (PropertyInfo)memberInfo, new List<Expression> { Expression.Constant(propertyBase.Name) })
-                    : (Expression)Expression.MakeMemberAccess(parameter, memberInfo);
-            }
         }
     }
 }
diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs
@@ -2361,6 +2361,10 @@ public virtual Property AddProperty(
                             memberInfo.Name, this.DisplayName(), memberInfo.DeclaringType?.ShortDisplayName()));
                 }
             }
+            else if (IsPropertyBag)
+            {
+                memberInfo = FindIndexerPropertyInfo();
+            }
             else
             {
                 Check.DebugAssert(ClrType?.GetMembersInHierarchy(name).FirstOrDefault() == null,