Fix attempt of static proxies to call base method for abstract classes (#1884)

Previously it was failing PEVerify similarly to #1728 but for abstract classes (this is possible in case of a polymorphic entities). Unlike interfaces the abstract base class can go into a situation when lazy initializer is not yet available, eg. code in a constructor.

Also test the explicit interface case, check access into constructor, and check proxy on type with
interfaces.

And correctly call explicit interface methods of base type

Author: hazzik

src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs

@@ -33,13 +33,57 @@ public class InternalInterfaceTestClass : IInternal
 
 		public interface IPublic
 		{
-			int Id { get; }
+			int Id { get; set; }
+			string Name { get; set; }
 		}
 
 		[Serializable]
 		public class PublicInterfaceTestClass : IPublic
 		{
 			public virtual int Id { get; set; }
+			public virtual string Name { get; set; }
+
+			public PublicInterfaceTestClass()
+			{
+				// Check access to properties from the default constructor do not fail once proxified
+				Id = -1;
+				Assert.That(Id, Is.EqualTo(-1));
+				Name = "Unknown";
+				Assert.That(Name, Is.EqualTo("Unknown"));
+			}
+		}
+
+		[Serializable]
+		public class PublicExplicitInterfaceTestClass : IPublic
+		{
+			int IPublic.Id { get; set; }
+			string IPublic.Name { get; set; }
+
+			public PublicExplicitInterfaceTestClass()
+			{
+				// Check access to properties from the default constructor do not fail once proxified
+				IPublic pub = this;
+				pub.Id = -1;
+				Assert.That(pub.Id, Is.EqualTo(-1));
+				pub.Name = "Unknown";
+				Assert.That(pub.Name, Is.EqualTo("Unknown"));
+			}
+		}
+
+		[Serializable]
+		public abstract class AbstractTestClass : IPublic
+		{
+			protected AbstractTestClass()
+			{
+				Id = -1;
+				Assert.That(Id, Is.Zero);
+				Name = "Unknown";
+				Assert.That(Name, Is.Null);
+			}
+
+			public abstract int Id { get; set; }
+			
+			public abstract string Name { get; set; } 
 		}
 
 		[Serializable]
@@ -156,8 +200,8 @@ public void VerifyProxyForClassWithAdditionalInterface()
 				// By way of the "proxy" attribute on the "class" mapping, an interface to use for the
 				// lazy entity load proxy instead of the persistentClass can be specified. This is "translated" into
 				// having an additional interface in the interface list, instead of just having INHibernateProxy.
-				// (Quite a loosy semantic...)
-				new HashSet<System.Type> {typeof(INHibernateProxy), typeof(IPublic)},
+				// (Quite a loose semantic...)
+				new HashSet<System.Type> { typeof(INHibernateProxy), typeof(IPublic) },
 				null, null, null);
 
 #if NETFX
@@ -174,6 +218,85 @@ public void VerifyProxyForClassWithAdditionalInterface()
 #endif
 		}
 
+		[Test]
+		public void VerifyProxyForClassWithInterface()
+		{
+			var factory = new StaticProxyFactory();
+			factory.PostInstantiate(
+				typeof(PublicInterfaceTestClass).FullName,
+				typeof(PublicInterfaceTestClass),
+				new HashSet<System.Type> {typeof(INHibernateProxy)},
+				null, null, null);
+
+#if NETFX
+			VerifyGeneratedAssembly(
+				() =>
+				{
+#endif
+					var proxy = factory.GetProxy(1, null);
+					Assert.That(proxy, Is.Not.Null);
+					Assert.That(proxy, Is.InstanceOf<IPublic>());
+					Assert.That(proxy, Is.InstanceOf<PublicInterfaceTestClass>());
+
+					// Check interface and implicit implementations do both call the delegated state
+					var state = new PublicInterfaceTestClass { Id = 5, Name = "State" };
+					proxy.HibernateLazyInitializer.SetImplementation(state);
+					var pub = (IPublic) proxy;
+					var ent = (PublicInterfaceTestClass) proxy;
+					Assert.That(pub.Id, Is.EqualTo(5), "IPublic.Id");
+					Assert.That(ent.Id, Is.EqualTo(5), "entity.Id");
+					Assert.That(pub.Name, Is.EqualTo("State"), "IPublic.Name");
+					Assert.That(ent.Name, Is.EqualTo("State"), "entity.Name");
+					ent.Id = 10;
+					pub.Name = "Test";
+					Assert.That(pub.Id, Is.EqualTo(10), "IPublic.Id");
+					Assert.That(state.Id, Is.EqualTo(10), "state.Id");
+					Assert.That(ent.Name, Is.EqualTo("Test"), "entity.Name");
+					Assert.That(state.Name, Is.EqualTo("Test"), "state.Name");
+#if NETFX
+				});
+#endif
+		}
+
+		[Test]
+		public void VerifyProxyForClassWithExplicitInterface()
+		{
+			var factory = new StaticProxyFactory();
+			factory.PostInstantiate(
+				typeof(PublicExplicitInterfaceTestClass).FullName,
+				typeof(PublicExplicitInterfaceTestClass),
+				new HashSet<System.Type> {typeof(INHibernateProxy)},
+				null, null, null);
+#if NETFX
+			VerifyGeneratedAssembly(
+				() =>
+				{
+#endif
+					var proxy = factory.GetProxy(1, null);
+					Assert.That(proxy, Is.Not.Null);
+					Assert.That(proxy, Is.InstanceOf<IPublic>());
+					Assert.That(proxy, Is.InstanceOf<PublicExplicitInterfaceTestClass>());
+					
+					// Check interface and implicit implementations do both call the delegated state
+					IPublic state = new PublicExplicitInterfaceTestClass();
+					state.Id = 5;
+					state.Name = "State";
+					proxy.HibernateLazyInitializer.SetImplementation(state);
+					var entity = (IPublic) proxy;
+					Assert.That(entity.Id, Is.EqualTo(5), "Id");
+					Assert.That(entity.Name, Is.EqualTo("State"), "Name");
+					
+					entity.Id = 10;
+					entity.Name = "Test";
+					Assert.That(entity.Id, Is.EqualTo(10), "entity.Id");
+					Assert.That(state.Id, Is.EqualTo(10), "state.Id");
+					Assert.That(entity.Name, Is.EqualTo("Test"), "entity.Name");
+					Assert.That(state.Name, Is.EqualTo("Test"), "state.Name");
+#if NETFX
+				});
+#endif
+		}
+
 		[Test]
 		public void VerifyProxyForRefOutClass()
 		{
@@ -219,6 +342,30 @@ public void VerifyProxyForRefOutClass()
 #endif
 		}
 
+		[Test]
+		public void VerifyProxyForAbstractClass()
+		{
+			var factory = new StaticProxyFactory();
+			factory.PostInstantiate(
+				typeof(AbstractTestClass).FullName,
+				typeof(AbstractTestClass),
+				new HashSet<System.Type> { typeof(INHibernateProxy) },
+				null, null, null);
+
+#if NETFX
+			VerifyGeneratedAssembly(
+				() =>
+				{
+#endif
+					var proxy = factory.GetProxy(1, null);
+					Assert.That(proxy, Is.Not.Null);
+					Assert.That(proxy, Is.InstanceOf<IPublic>());
+					Assert.That(proxy, Is.InstanceOf<AbstractTestClass>());
+#if NETFX
+				});
+#endif
+		}
+
 		[Test]
 		public void InitializedProxyStaysInitializedAfterDeserialization()
 		{
@@ -369,10 +516,10 @@ public void VerifyFieldInterceptorProxyWithAdditionalInterface()
 				// By way of the "proxy" attribute on the "class" mapping, an interface to use for the
 				// lazy entity load proxy instead of the persistentClass can be specified. This is "translated" into
 				// having an additional interface in the interface list, instead of just having INHibernateProxy.
-				// (Quite a loosy semantic...)
+				// (Quite a loose semantic...)
 				// The field interceptor proxy ignores this setting, as it does not delegate its implementation
 				// to an instance of the persistentClass, and so cannot implement interface methods if it does not
-				// inherit the persitentClass.
+				// inherit the persistentClass.
 				new HashSet<System.Type> {typeof(INHibernateProxy), typeof(IPublic)},
 				null, null, null);
 #if NETFX

src/NHibernate/Proxy/NHibernateProxyBuilder.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Linq.Expressions;
 using System.Reflection;
 using System.Reflection.Emit;
 using System.Runtime.Serialization;
@@ -366,19 +367,20 @@ private static void ImplementCallMethodOnImplementation(
 		}
 
 		private static void EmitCallBaseIfLazyInitializerIsNull(
-			ILGenerator IL, MethodInfo method, FieldInfo lazyInitializerField, System.Type parentType)
+			ILGenerator IL,
+			MethodInfo method,
+			FieldInfo lazyInitializerField,
+			System.Type parentType)
 		{
 			/*
-				<if (method.DeclaringType.IsAssignableFrom(parentType))
-				{>
 				if (this.__lazyInitializer == null)
-					return base.<method>(args..)
+				<if (method.IsAbstract)
+				{>
+				return default;
+				<} else {>
+				return base.<method>(args..);
 				<}>
 			 */
-			if (!method.DeclaringType.IsAssignableFrom(parentType))
-				// The proxy does not derive from a type implementing the method, do not attempt
-				// calling its base. In such case, the lazy initializer is never null.
-				return;
 
 			// When deriving from the entity class, the entity class constructor may trigger
 			// virtual calls accessing the proxy state before its own constructor has a chance
@@ -393,9 +395,76 @@ private static void EmitCallBaseIfLazyInitializerIsNull(
 			IL.Emit(OpCodes.Ldnull);
 			IL.Emit(OpCodes.Bne_Un, skipBaseCall);
 
-			IL.Emit(OpCodes.Ldarg_0);
-			EmitCallMethod(IL, OpCodes.Call, method);
-			IL.Emit(OpCodes.Ret);
+			if (method.DeclaringType.IsInterface &&
+			    method.DeclaringType.IsAssignableFrom(parentType))
+			{
+				var interfaceMap = parentType.GetInterfaceMap(method.DeclaringType);
+				var methodIndex = Array.IndexOf(interfaceMap.InterfaceMethods, method);
+				method = interfaceMap.TargetMethods[methodIndex];
+			}
+
+			if (method.IsAbstract)
+			{
+				/*
+				 * return default(<ReturnType>);
+				 */
+				
+				if (!method.ReturnType.IsValueType)
+				{
+					IL.Emit(OpCodes.Ldnull);
+				}
+				else if (method.ReturnType != typeof(void))
+				{
+					var local = IL.DeclareLocal(method.ReturnType);
+					IL.Emit(OpCodes.Ldloca, local);
+					IL.Emit(OpCodes.Initobj, method.ReturnType);
+					IL.Emit(OpCodes.Ldloc, local);
+				}
+
+				IL.Emit(OpCodes.Ret);
+			}
+			else if (method.IsPrivate)
+			{
+				/*
+				 * var mi = (MethodInfo)MethodBase.GetMethodFromHandle(<method>, <parentType>);
+				 * var delegate = (<delegateType>)mi.CreateDelegate(typeof(<delegateType>), this);
+				 * delegate.Invoke(args...);
+				 */
+				
+				var parameters = method.GetParameters();
+
+				var delegateType = Expression.GetDelegateType(
+					parameters.Select(p => p.ParameterType).Concat(new[] {method.ReturnType}).ToArray());
+
+				var invokeDelegate = delegateType.GetMethod("Invoke");
+
+				IL.Emit(OpCodes.Ldtoken, method);
+				IL.Emit(OpCodes.Ldtoken, parentType);
+				IL.Emit(OpCodes.Call, ReflectionCache.MethodBaseMethods.GetMethodFromHandleWithDeclaringType);
+				IL.Emit(OpCodes.Castclass, typeof(MethodInfo));
+				IL.Emit(OpCodes.Ldtoken, delegateType);
+				IL.Emit(OpCodes.Call, ReflectionCache.TypeMethods.GetTypeFromHandle);
+				IL.Emit(OpCodes.Ldarg_0);
+				IL.Emit(
+					OpCodes.Callvirt,
+					typeof(MethodInfo).GetMethod(
+						nameof(MethodInfo.CreateDelegate),
+						new[] {typeof(System.Type), typeof(object)}));
+				IL.Emit(OpCodes.Castclass, delegateType);
+
+				EmitCallMethod(IL, OpCodes.Callvirt, invokeDelegate);
+				IL.Emit(OpCodes.Ret);
+			}
+			else
+			{
+				/*
+				 * base.<method>(args...);
+				 */
+				
+				IL.Emit(OpCodes.Ldarg_0);
+				EmitCallMethod(IL, OpCodes.Call, method);
+				IL.Emit(OpCodes.Ret);
+			}
 
 			IL.MarkLabel(skipBaseCall);
 		}