[release/6.0] Avoid proxying Clone methods on record types (#27125)

dotnet · Jan 7, 2022 · bbbd2c8 · bbbd2c8
1 parent 13bb9b0
commit bbbd2c8
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 3 deletions.
diff --git a/src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs b/src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Reflection;
 using Castle.DynamicProxy;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
@@ -25,6 +26,11 @@ public class ProxyFactory : IProxyFactory
         private static readonly Type _notifyPropertyChangedInterface = typeof(INotifyPropertyChanged);
         private static readonly Type _notifyPropertyChangingInterface = typeof(INotifyPropertyChanging);
 
+        private static readonly ProxyGenerationOptions _generationOptions
+            = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26602", out var enabled) && enabled
+                ? ProxyGenerationOptions.Default
+                : new(new ClonelessProxyGenerationHook());
+
         private readonly ProxyGenerator _generator = new();
 
         /// <summary>
@@ -64,7 +70,7 @@ public virtual Type CreateProxyType(
             => _generator.ProxyBuilder.CreateClassProxyType(
                 entityType.ClrType,
                 GetInterfacesToProxy(options, entityType.ClrType),
-                ProxyGenerationOptions.Default);
+                _generationOptions);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -99,7 +105,7 @@ private object CreateLazyLoadingProxy(
             => _generator.CreateClassProxy(
                 entityType.ClrType,
                 GetInterfacesToProxy(options, entityType.ClrType),
-                ProxyGenerationOptions.Default,
+                _generationOptions,
                 constructorArguments,
                 GetNotifyChangeInterceptors(options, entityType, new LazyLoadingInterceptor(entityType, loader)));
 
@@ -142,7 +148,7 @@ private object CreateProxy(
             => _generator.CreateClassProxy(
                 entityType.ClrType,
                 GetInterfacesToProxy(options, entityType.ClrType),
-                ProxyGenerationOptions.Default,
+                _generationOptions,
                 constructorArguments,
                 GetNotifyChangeInterceptors(options, entityType));
 
@@ -200,5 +206,12 @@ private IInterceptor[] GetNotifyChangeInterceptors(
 
             return interceptors.ToArray();
         }
+
+        private sealed class ClonelessProxyGenerationHook : AllMethodsHook
+        {
+            public override bool ShouldInterceptMethod(Type type, MethodInfo methodInfo)
+                => methodInfo.Name != "<Clone>$"
+                    && base.ShouldInterceptMethod(type, methodInfo);
+        }
     }
 }
diff --git a/test/EFCore.Proxies.Tests/ProxyTests.cs b/test/EFCore.Proxies.Tests/ProxyTests.cs
@@ -81,6 +81,15 @@ public void CreateProxy_works_for_shared_type_entity_types()
             Assert.Same(typeof(SharedTypeEntityType), context.Set<SharedTypeEntityType>("STET1").CreateProxy(_ => { }).GetType().BaseType);
         }
 
+        [ConditionalFact]
+        public void CreateProxy_works_for_record_with_base_type_entity_types()
+        {
+            using var context = new NeweyContext();
+
+            Assert.Same(typeof(March86C), context.Set<March86C>().CreateProxy().GetType().BaseType);
+            Assert.Same(typeof(March86C), context.Set<March86C>().CreateProxy(_ => { }).GetType().BaseType);
+        }
+
         [ConditionalFact]
         public void CreateProxy_throws_for_shared_type_entity_types_when_entity_type_name_not_known()
         {
@@ -345,6 +354,15 @@ public class IsOwnedButNotWeak
         {
         }
 
+        public record March86C : IndyCar
+        {
+            public virtual int Id { get; init; }
+        }
+
+        public record IndyCar
+        {
+        }
+
         private class NeweyContext : DbContext
         {
             private readonly IServiceProvider _internalServiceProvider;
@@ -418,6 +436,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                 modelBuilder.SharedTypeEntity<SharedTypeEntityType>("STET2");
 
                 modelBuilder.Entity<WithWeak>();
+
+                modelBuilder.Entity<March86C>();
             }
         }
 

diff --git a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs
@@ -388,6 +388,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         .HasForeignKey<Car>("fk_PersonId")
                         .IsRequired();
                 });
+
+                modelBuilder.Entity<RecordCar>();
             }
 
             protected virtual object CreateFullGraph(DbContext context)
@@ -1906,6 +1908,23 @@ public class Person
             public virtual Car Vehicle { get; set; }
         }
 
+        public record RecordBase
+        {
+            public virtual int Id { get; set; }
+        }
+
+        public record RecordCar : RecordBase
+        {
+            public virtual RecordPerson Owner { get; set; }
+            public virtual int? OwnerId { get; set; }
+        }
+
+        public record RecordPerson : RecordBase
+        {
+            public virtual ICollection<RecordCar> Vehicles { get; }
+                = new ObservableHashSet<RecordCar>(LegacyReferenceEqualityComparer.Instance);
+        }
+
         protected DbContext CreateContext()
             => Fixture.CreateContext();
 

diff --git a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs
@@ -51,6 +51,62 @@ public virtual void Attempting_to_save_two_entity_cycle_with_lazy_loading_throws
                 });
         }
 
+        [ConditionalFact]
+        public virtual void Can_use_record_proxies_with_base_types_to_load_reference()
+        {
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    context.AddRange(
+                        context.CreateProxy<RecordCar>(
+                            car =>
+                            {
+                                car.Owner = context.CreateProxy<RecordPerson>();
+                            }));
+
+                    context.SaveChanges();
+                },
+                context =>
+                {
+                    var car = context.Set<RecordCar>().Single();
+                    if (!DoesLazyLoading)
+                    {
+                        context.Entry(car).Reference(e => e.Owner).Load();
+                    }
+
+                    Assert.Equal(car.Owner.Id, car.OwnerId);
+                    Assert.Same(car, car.Owner.Vehicles.Single());
+                });
+        }
+
+        [ConditionalFact]
+        public virtual void Can_use_record_proxies_with_base_types_to_load_collection()
+        {
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    context.AddRange(
+                        context.CreateProxy<RecordCar>(
+                            car =>
+                            {
+                                car.Owner = context.CreateProxy<RecordPerson>();
+                            }));
+
+                    context.SaveChanges();
+                },
+                context =>
+                {
+                    var owner = context.Set<RecordPerson>().Single();
+                    if (!DoesLazyLoading)
+                    {
+                        context.Entry(owner).Collection(e => e.Vehicles).Load();
+                    }
+
+                    Assert.Equal(owner.Id, owner.Vehicles.Single().Id);
+                    Assert.Same(owner, owner.Vehicles.Single().Owner);
+                });
+        }
+
         [ConditionalFact]
         public virtual void Avoid_nulling_shared_FK_property_when_deleting()
         {