Updates to shared-type entity type handling in proxies

Fixes #22337

* Check for virtual indexer properties and throw unless type is Dictionary<string, _> and has only primary keys
* Specific exception for the Dictionary case
* Verified many-to-many with payload requires this pattern and works

See also #22336

src/EFCore.Proxies/Properties/ProxiesStrings.resx

@@ -117,14 +117,20 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="DictionaryCannotBeProxied" xml:space="preserve">
+    <value>'{dictionaryType}' is not suitable for use as a change-tracking proxy because its indexer property is not virtual. Consider using an implementation of '{interfaceType}' that allows overriding of the indexer.</value>
+  </data>
   <data name="FieldProperty" xml:space="preserve">
     <value>Property '{property}' on entity type '{entityType}' is mapped without a CLR property. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
   </data>
   <data name="ItsASeal" xml:space="preserve">
     <value>Entity type '{entityType}' is sealed. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
   </data>
+  <data name="NonVirtualIndexerProperty" xml:space="preserve">
+    <value>The mapped indexer property on entity type '{entityType}' is not virtual. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
+  </data>
   <data name="NonVirtualProperty" xml:space="preserve">
-    <value>Property '{property}' on entity type '{entityType}' is not virtual. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
+    <value>Property '{1_entityType}.{0_property}' is not virtual. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
   </data>
   <data name="ProxiesNotEnabled" xml:space="preserve">
     <value>Unable to create proxy for '{entityType}' because proxies are not enabled. Call 'DbContextOptionsBuilder.UseChangeTrackingProxies' or 'DbContextOptionsBuilder.UseLazyLoadingProxies' to enable proxies.</value>

src/EFCore.Proxies/Proxies/Internal/ProxyBindingRewriter.cs

@@ -65,9 +65,10 @@ public virtual void ProcessModelFinalizing(
             {
                 foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
                 {
-                    if (entityType.ClrType?.IsAbstract == false)
+                    var clrType = entityType.ClrType;
+                    if (clrType?.IsAbstract == false)
                     {
-                        if (entityType.ClrType.IsSealed)
+                        if (clrType.IsSealed)
                         {
                             throw new InvalidOperationException(ProxiesStrings.ItsASeal(entityType.DisplayName()));
                         }
@@ -102,15 +103,15 @@ public virtual void ProcessModelFinalizing(
                             }
 
                             if (_options.UseChangeTrackingProxies
-                                && navigationBase.PropertyInfo.SetMethod?.IsVirtual == false)
+                                && navigationBase.PropertyInfo.SetMethod?.IsReallyVirtual() == false)
                             {
                                 throw new InvalidOperationException(
                                     ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
                             }
 
                             if (_options.UseLazyLoadingProxies)
                             {
-                                if (!navigationBase.PropertyInfo.GetMethod.IsVirtual
+                                if (!navigationBase.PropertyInfo.GetMethod.IsReallyVirtual()
                                     && (!(navigationBase is INavigation navigation
                                         && navigation.ForeignKey.IsOwnership)))
                                 {
@@ -122,6 +123,59 @@ public virtual void ProcessModelFinalizing(
                             }
                         }
 
+                        if (_options.UseChangeTrackingProxies)
+                        {
+                            var indexerChecked = false;
+                            foreach (var property in entityType.GetDeclaredProperties()
+                                .Where(p => !p.IsShadowProperty()))
+                            {
+                                if (property.IsIndexerProperty())
+                                {
+                                    if (!indexerChecked)
+                                    {
+                                        indexerChecked = true;
+
+                                        if (!property.PropertyInfo.SetMethod.IsReallyVirtual())
+                                        {
+                                            if (clrType.IsGenericType
+                                                && clrType.GetGenericTypeDefinition() == typeof(Dictionary<,>)
+                                                && clrType.GenericTypeArguments[0] == typeof(string))
+                                            {
+                                                if (entityType.GetProperties().Any(p => !p.IsPrimaryKey()))
+                                                {
+                                                    throw new InvalidOperationException(
+                                                        ProxiesStrings.DictionaryCannotBeProxied(
+                                                            clrType.ShortDisplayName(),
+                                                            typeof(IDictionary<,>).MakeGenericType(clrType.GenericTypeArguments)
+                                                                .ShortDisplayName()));
+                                                }
+                                            }
+                                            else
+                                            {
+                                                throw new InvalidOperationException(
+                                                    ProxiesStrings.NonVirtualIndexerProperty(entityType.DisplayName()));
+                                            }
+                                        }
+                                    }
+                                }
+                                else
+                                {
+                                    if (property.PropertyInfo == null)
+                                    {
+                                        throw new InvalidOperationException(
+                                            ProxiesStrings.FieldProperty(property.Name, entityType.DisplayName()));
+                                    }
+
+                                    if (property.PropertyInfo.SetMethod?.IsReallyVirtual() == false)
+                                    {
+                                        throw new InvalidOperationException(
+                                            ProxiesStrings.NonVirtualProperty(property.Name, entityType.DisplayName()));
+                                    }
+                                }
+
+                            }
+                        }
+
                         void UpdateConstructorBindings(string bindingAnnotationName, InstantiationBinding binding)
                         {
                             if (_options.UseLazyLoadingProxies)
@@ -176,21 +230,6 @@ void UpdateConstructorBindings(string bindingAnnotationName, InstantiationBindin
                                             new ObjectArrayParameterBinding(binding.ParameterBindings)
                                         },
                                         proxyType));
-
-                                foreach (var prop in entityType.GetDeclaredProperties().Where(p => !p.IsShadowProperty()))
-                                {
-                                    if (prop.PropertyInfo == null)
-                                    {
-                                        throw new InvalidOperationException(
-                                            ProxiesStrings.FieldProperty(prop.Name, entityType.DisplayName()));
-                                    }
-
-                                    if (prop.PropertyInfo.SetMethod?.IsVirtual == false)
-                                    {
-                                        throw new InvalidOperationException(
-                                            ProxiesStrings.NonVirtualProperty(prop.Name, entityType.DisplayName()));
-                                    }
-                                }
                             }
                         }
                     }

src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs

@@ -41,6 +41,11 @@ public virtual object Create(
             var entityType = context.Model.FindRuntimeEntityType(type);
             if (entityType == null)
             {
+                if (context.Model.IsShared(type))
+                {
+                    throw new InvalidOperationException(CoreStrings.EntityTypeNotFoundSharedProxy(type.ShortDisplayName()));
+                }
+
                 throw new InvalidOperationException(CoreStrings.EntityTypeNotFound(type.ShortDisplayName()));
             }
 

src/EFCore/Metadata/Internal/TypeBase.cs

@@ -60,8 +60,10 @@ protected TypeBase([NotNull] Type type, [NotNull] Model model, ConfigurationSour
 
             Name = model.GetDisplayName(type);
             ClrType = type;
-            HasSharedClrType = false;
-            IsPropertyBag = type.IsPropertyBagType();
+
+            var isPropertyBag = type.IsPropertyBagType();
+            IsPropertyBag = isPropertyBag;
+            HasSharedClrType = isPropertyBag;
         }
 
         /// <summary>

src/EFCore/Properties/CoreStrings.resx

@@ -402,6 +402,9 @@
   <data name="EntityTypeNotFound" xml:space="preserve">
     <value>The entity type '{entityType}' was not found. Ensure that the entity type has been added to the model.</value>
   </data>
+  <data name="EntityTypeNotFoundSharedProxy" xml:space="preserve">
+    <value>The type '{clrType}' is configured as a shared-type entity type, but the entity type name is not known. Ensure that CreateProxy is called on a DbSet created specifically for the shared-type entity type through use of a `DbContext.Set` overload that accepts an entity type name.</value>
+  </data>
   <data name="EntityTypeNotInRelationship" xml:space="preserve">
     <value>The specified entity type '{entityType}' is invalid. It should be either the dependent entity type '{dependentType}' or the principal entity type '{principalType}' or an entity type derived from one of them.</value>
   </data>

src/Shared/MethodInfoExtensions.cs

@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
+using JetBrains.Annotations;
 
 namespace System.Reflection
 {
@@ -14,5 +15,10 @@ public static bool IsContainsMethod(this MethodInfo method)
                 && method.DeclaringType.GetInterfaces().Append(method.DeclaringType).Any(
                     t => t == typeof(IList)
                         || (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(ICollection<>)));
+
+
+        public static bool IsReallyVirtual([NotNull] this MethodInfo method)
+            => method.IsVirtual && !method.IsFinal;
+
     }
 }

test/EFCore.Proxies.Tests/ChangeDetectionProxyTests.cs

@@ -2,7 +2,10 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.ComponentModel;
+using System.Linq;
+using System.Reflection.Metadata;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
@@ -33,6 +36,24 @@ public void Throws_if_non_virtual_property()
                     () => context.Model).Message);
         }
 
+        [ConditionalFact]
+        public void Throws_if_non_virtual_indexer_property()
+        {
+            using var context = new ChangeContext<ChangeNonVirtualIndexer>(entityBuilderAction: b => b.IndexerProperty<int>("Snoopy"));
+            Assert.Equal(
+                ProxiesStrings.NonVirtualIndexerProperty(nameof(ChangeNonVirtualIndexer)),
+                Assert.Throws<InvalidOperationException>(() => context.Model).Message);
+        }
+
+        [ConditionalFact]
+        public void Does_not_throw_when_non_virtual_indexer_not_mapped()
+        {
+            using var context = new ChangeContext<ChangeNonVirtualIndexerNotUsed>();
+
+            Assert.False(
+                context.Model.FindEntityType(typeof(ChangeNonVirtualIndexerNotUsed)).GetProperties().Any(e => e.IsIndexerProperty()));
+        }
+
         [ConditionalFact]
         public void Throws_if_non_virtual_navigation()
         {
@@ -318,6 +339,32 @@ public class ChangeNonVirtualNavEntity
             public ChangeNonVirtualNavEntity SelfRef { get; set; }
         }
 
+        public class ChangeNonVirtualIndexer
+        {
+            private readonly Dictionary<string, object> _keyValuePairs = new Dictionary<string, object>();
+
+            public virtual int Id { get; set; }
+
+            public object this[string key]
+            {
+                get => _keyValuePairs[key];
+                set => _keyValuePairs[key] = value;
+            }
+        }
+
+        public class ChangeNonVirtualIndexerNotUsed
+        {
+            private readonly Dictionary<string, object> _keyValuePairs = new Dictionary<string, object>();
+
+            public virtual int Id { get; set; }
+
+            public object this[string key]
+            {
+                get => _keyValuePairs[key];
+                set => _keyValuePairs[key] = value;
+            }
+        }
+
         public class ChangeValueEntity
         {
             public virtual int Id { get; set; }

test/EFCore.Proxies.Tests/ProxyTests.cs

@@ -71,6 +71,33 @@ public void Materialization_uses_parameterized_constructor_taking_context()
             }
         }
 
+        [ConditionalFact]
+        public void CreateProxy_works_for_shared_type_entity_types()
+        {
+            using var context = new NeweyContext();
+
+            Assert.Same(typeof(SharedTypeEntityType), context.Set<SharedTypeEntityType>("STET1").CreateProxy().GetType().BaseType);
+            Assert.Same(typeof(SharedTypeEntityType), context.Set<SharedTypeEntityType>("STET1").CreateProxy(_ => { }).GetType().BaseType);
+        }
+
+        [ConditionalFact]
+        public void CreateProxy_throws_for_shared_type_entity_types_when_entity_type_name_not_known()
+        {
+            using var context = new NeweyContext();
+
+            Assert.Equal(
+                CoreStrings.EntityTypeNotFoundSharedProxy(nameof(SharedTypeEntityType)),
+                Assert.Throws<InvalidOperationException>(() => context.CreateProxy<SharedTypeEntityType>()).Message);
+
+            Assert.Equal(
+                CoreStrings.EntityTypeNotFoundSharedProxy(nameof(SharedTypeEntityType)),
+                Assert.Throws<InvalidOperationException>(() => context.CreateProxy<SharedTypeEntityType>(_ => { })).Message);
+
+            Assert.Equal(
+                CoreStrings.EntityTypeNotFoundSharedProxy(nameof(SharedTypeEntityType)),
+                Assert.Throws<InvalidOperationException>(() => context.CreateProxy(typeof(SharedTypeEntityType))).Message);
+        }
+
         [ConditionalFact]
         public void CreateProxy_uses_parameterless_constructor()
         {
@@ -248,6 +275,11 @@ public void Throws_if_attempt_to_add_proxy_type_to_model_builder()
                     }).Message);
         }
 
+        public class SharedTypeEntityType
+        {
+            public virtual int Id { get; set; }
+        }
+
         public class March82GGtp
         {
             public virtual int Id { get; set; }
@@ -350,6 +382,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                         b.Property(e => e.Id);
                         b.Property(e => e.Sponsor);
                     });
+
+                modelBuilder.SharedTypeEntity<SharedTypeEntityType>("STET1");
+                modelBuilder.SharedTypeEntity<SharedTypeEntityType>("STET2");
             }
         }