Support overriding virtual indexer property in proxies

src/EFCore.Proxies/Proxies/Internal/PropertyChangeInterceptorBase.cs

@@ -0,0 +1,77 @@
+// 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;
+using Castle.DynamicProxy;
+using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.ChangeTracking;
+using Microsoft.EntityFrameworkCore.Metadata;
+using Microsoft.EntityFrameworkCore.Metadata.Internal;
+
+namespace Microsoft.EntityFrameworkCore.Proxies.Internal
+{
+    /// <summary>
+    ///     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.
+    /// </summary>
+    public abstract class PropertyChangeInterceptorBase
+    {
+        /// <summary>
+        ///     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.
+        /// </summary>
+        protected PropertyChangeInterceptorBase([NotNull] IEntityType entityType)
+        {
+            EntityType = entityType;
+        }
+
+        /// <summary>
+        ///     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.
+        /// </summary>
+        protected virtual IEntityType EntityType { get; }
+
+        /// <summary>
+        ///     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.
+        /// </summary>
+        protected virtual ValueComparer GetValueComparer([NotNull] IProperty property)
+            => property.IsKey()
+                || property.IsForeignKey()
+                    ? property.GetKeyValueComparer()
+                    : property.GetValueComparer();
+
+        /// <summary>
+        ///     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.
+        /// </summary>
+        protected virtual string FindPropertyName([NotNull] IInvocation invocation)
+        {
+            var methodName = invocation.Method.Name;
+            if (methodName.Equals("set_Item", StringComparison.Ordinal))
+            {
+#pragma warning disable EF1001 // Internal EF Core API usage.
+                var indexerPropertyInfo = EntityType.FindIndexerPropertyInfo();
+#pragma warning restore EF1001 // Internal EF Core API usage.
+
+                if (indexerPropertyInfo != null
+                    && indexerPropertyInfo.GetSetMethod(nonPublic: true) == invocation.Method)
+                {
+                    return (string)invocation.Arguments[0];
+                }
+            }
+
+            return methodName.Substring(4);
+        }
+    }
+}

src/EFCore.Proxies/Proxies/Internal/PropertyChangedInterceptor.cs

@@ -17,11 +17,10 @@ namespace Microsoft.EntityFrameworkCore.Proxies.Internal
     ///     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.
     /// </summary>
-    public class PropertyChangedInterceptor : IInterceptor
+    public class PropertyChangedInterceptor : PropertyChangeInterceptorBase, IInterceptor
     {
         private static readonly Type _notifyChangedInterface = typeof(INotifyPropertyChanged);
 
-        private readonly IEntityType _entityType;
         private readonly bool _checkEquality;
         private PropertyChangedEventHandler _handler;
 
@@ -34,8 +33,8 @@ public class PropertyChangedInterceptor : IInterceptor
         public PropertyChangedInterceptor(
             [NotNull] IEntityType entityType,
             bool checkEquality)
+            : base(entityType)
         {
-            _entityType = entityType;
             _checkEquality = checkEquality;
         }
 
@@ -64,22 +63,17 @@ public virtual void Intercept(IInvocation invocation)
             }
             else if (methodName.StartsWith("set_", StringComparison.Ordinal))
             {
-                var propertyName = methodName.Substring(4);
+                var propertyName = FindPropertyName(invocation);
 
-                var property = _entityType.FindProperty(propertyName);
+                var property = EntityType.FindProperty(propertyName);
                 if (property != null)
                 {
-                    var comparer = property.IsKey()
-                        || property.IsForeignKey()
-                            ? property.GetKeyValueComparer()
-                            : property.GetValueComparer();
-
-                    HandleChanged(invocation, property, comparer);
+                    HandleChanged(invocation, property, GetValueComparer(property));
                 }
                 else
                 {
-                    var navigation = _entityType.FindNavigation(propertyName)
-                        ?? (INavigationBase)_entityType.FindSkipNavigation(propertyName);
+                    var navigation = EntityType.FindNavigation(propertyName)
+                        ?? (INavigationBase)EntityType.FindSkipNavigation(propertyName);
 
                     if (navigation != null)
                     {

src/EFCore.Proxies/Proxies/Internal/PropertyChangingInterceptor.cs

@@ -17,11 +17,10 @@ namespace Microsoft.EntityFrameworkCore.Proxies.Internal
     ///     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.
     /// </summary>
-    public class PropertyChangingInterceptor : IInterceptor
+    public class PropertyChangingInterceptor : PropertyChangeInterceptorBase, IInterceptor
     {
         private static readonly Type _notifyChangingInterface = typeof(INotifyPropertyChanging);
 
-        private readonly IEntityType _entityType;
         private readonly bool _checkEquality;
         private PropertyChangingEventHandler _handler;
 
@@ -34,8 +33,8 @@ public class PropertyChangingInterceptor : IInterceptor
         public PropertyChangingInterceptor(
             [NotNull] IEntityType entityType,
             bool checkEquality)
+            : base(entityType)
         {
-            _entityType = entityType;
             _checkEquality = checkEquality;
         }
 
@@ -64,22 +63,17 @@ public virtual void Intercept(IInvocation invocation)
             }
             else if (methodName.StartsWith("set_", StringComparison.Ordinal))
             {
-                var propertyName = methodName.Substring(4);
+                var propertyName = FindPropertyName(invocation);
 
-                var property = _entityType.FindProperty(propertyName);
+                var property = EntityType.FindProperty(propertyName);
                 if (property != null)
                 {
-                    var comparer = property.IsKey()
-                        || property.IsForeignKey()
-                            ? property.GetKeyValueComparer()
-                            : property.GetValueComparer();
-
-                    HandleChanging(invocation, property, comparer);
+                    HandleChanging(invocation, property, GetValueComparer(property));
                 }
                 else
                 {
-                    var navigation = _entityType.FindNavigation(propertyName)
-                        ?? (INavigationBase)_entityType.FindSkipNavigation(propertyName);
+                    var navigation = EntityType.FindNavigation(propertyName)
+                        ?? (INavigationBase)EntityType.FindSkipNavigation(propertyName);
 
                     if (navigation != null)
                     {

src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs

@@ -145,7 +145,12 @@ protected virtual Expression CreateSnapshotExpression(
 
                 if (memberAccess.Type != propertyBase.ClrType)
                 {
-                    memberAccess = Expression.Convert(memberAccess, propertyBase.ClrType);
+                    var hasDefaultValueExpression = memberAccess.MakeHasDefaultValue(propertyBase);
+
+                    memberAccess = Expression.Condition(
+                        hasDefaultValueExpression,
+                        propertyBase.ClrType.GetDefaultValueConstant(),
+                        Expression.Convert(memberAccess, propertyBase.ClrType));
                 }
 
                 arguments[i] = (propertyBase as INavigation)?.IsCollection ?? false

src/EFCore/Extensions/Internal/ExpressionExtensions.cs

@@ -29,9 +29,9 @@ public static class ExpressionExtensions
         ///     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.
         /// </summary>
-        public static Expression MakeHasDefaultValue<TProperty>(
+        public static Expression MakeHasDefaultValue(
             [NotNull] this Expression currentValueExpression,
-            [NotNull] IPropertyBase propertyBase)
+            [CanBeNull] IPropertyBase propertyBase)
         {
             if (!currentValueExpression.Type.IsValueType)
             {
@@ -50,11 +50,12 @@ public static Expression MakeHasDefaultValue<TProperty>(
             }
 
             var property = propertyBase as IProperty;
+            var clrType = propertyBase?.ClrType ?? currentValueExpression.Type;
             var comparer = property?.GetValueComparer()
-                ?? ValueComparer.CreateDefault(typeof(TProperty), favorStructuralComparisons: false);
+                ?? ValueComparer.CreateDefault(clrType, favorStructuralComparisons: false);
 
             return comparer.ExtractEqualsBody(
-                comparer.Type != typeof(TProperty)
+                comparer.Type != clrType
                     ? Expression.Convert(currentValueExpression, comparer.Type)
                     : currentValueExpression,
                 Expression.Default(comparer.Type));

src/EFCore/Metadata/Internal/ClrPropertyGetterFactory.cs

@@ -62,7 +62,7 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
                     });
             }
 
-            var hasDefaultValueExpression = readExpression.MakeHasDefaultValue<TValue>(propertyBase);
+            var hasDefaultValueExpression = readExpression.MakeHasDefaultValue(propertyBase);
 
             if (readExpression.Type != typeof(TValue))
             {

src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs

@@ -77,7 +77,7 @@ private static Func<IUpdateEntry, TProperty> CreateCurrentValueGetter<TProperty>
                 if (currentValueExpression.Type != typeof(TProperty))
                 {
                     currentValueExpression = Expression.Condition(
-                        currentValueExpression.MakeHasDefaultValue<TProperty>(propertyBase),
+                        currentValueExpression.MakeHasDefaultValue(propertyBase),
                         Expression.Constant(default(TProperty), typeof(TProperty)),
                         Expression.Convert(currentValueExpression, typeof(TProperty)));
                 }

src/EFCore/Metadata/Internal/PropertyBase.cs

@@ -410,20 +410,13 @@ public static Expression CreateMemberAccess(
                 Expression expression = Expression.MakeIndex(
                     instanceExpression, (PropertyInfo)memberInfo, new List<Expression> { Expression.Constant(property.Name) });
 
-                if (expression.Type != property.ClrType)
-                {
-                    expression = Expression.Convert(expression, property.ClrType);
-                }
-
                 if (property.DeclaringType.IsPropertyBag)
                 {
-                    var defaultValueConstant = property.ClrType.GetDefaultValueConstant();
-
                     expression = Expression.Condition(
                         Expression.Call(
                             instanceExpression, _containsKeyMethod, new List<Expression> { Expression.Constant(property.Name) }),
                         expression,
-                        defaultValueConstant);
+                        expression.Type.GetDefaultValueConstant());
                 }
 
                 return expression;

test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs

@@ -3037,6 +3037,52 @@ public void Can_insert_update_delete_shared_type_entity_type()
                 });
         }
 
+        [ConditionalFact]
+        public void Can_insert_update_delete_proxyable_shared_type_entity_type()
+        {
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    var entity = context.Set<ProxyableSharedType>("PST").CreateInstance(
+                        c =>
+                        {
+                            c["Id"] = 1;
+                            c["Payload"] = "NewlyAdded";
+                        });
+
+                    context.Set<ProxyableSharedType>("PST").Add(entity);
+
+                    context.SaveChanges();
+                },
+                context =>
+                {
+                    var entity = context.Set<ProxyableSharedType>("PST").Single(e => (int)e["Id"] == 1);
+
+                    Assert.Equal("NewlyAdded", (string)entity["Payload"]);
+
+                    entity["Payload"] = "AlreadyUpdated";
+
+                    if (RequiresDetectChanges)
+                    {
+                        context.ChangeTracker.DetectChanges();
+                    }
+
+                    context.SaveChanges();
+                },
+                context =>
+                {
+                    var entity = context.Set<ProxyableSharedType>("PST").Single(e => (int)e["Id"] == 1);
+
+                    Assert.Equal("AlreadyUpdated", (string)entity["Payload"]);
+
+                    context.Set<ProxyableSharedType>("PST").Remove(entity);
+
+                    context.SaveChanges();
+
+                    Assert.False(context.Set<ProxyableSharedType>("PST").Any(e => (int)e["Id"] == 1));
+                });
+        }
+
         protected ManyToManyTrackingTestBase(TFixture fixture)
             => Fixture = fixture;
 

test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs

@@ -320,6 +320,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         e.CompositeId3,
                         e.LeafId
                     });
+
+            modelBuilder.SharedTypeEntity<ProxyableSharedType>(
+                "PST", b =>
+                {
+                    b.IndexerProperty<int>("Id").ValueGeneratedNever();
+                    b.IndexerProperty<string>("Payload");
+                });
         }
 
         protected override void Seed(ManyToManyContext context)

test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ProxyableSharedType.cs

@@ -0,0 +1,18 @@
+// 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;
+
+namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel
+{
+    public class ProxyableSharedType
+    {
+        private readonly Dictionary<string, object> _keyValueStore = new Dictionary<string, object>();
+
+        public virtual object this[string key]
+        {
+            get => _keyValueStore.TryGetValue(key, out var value) ? value : default;
+            set => _keyValueStore[key] = value;
+        }
+    }
+}