diff --git a/src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs b/src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs index 1977b3f4049..da3c5cde84f 100644 --- a/src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs +++ b/src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs @@ -24,6 +24,12 @@ internal interface IInternal { int Id { get; } } + + public interface IWithEqualsAndGetHashCode + { + bool Equals(object that); + int GetHashCode(); + } [Serializable] public class InternalInterfaceTestClass : IInternal @@ -70,6 +76,30 @@ public PublicExplicitInterfaceTestClass() } } + [Serializable] + public class PublicExplicitInterfaceWithSameMembersTestClass : IPublic + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + + int IPublic.Id { get; set; } + string IPublic.Name { get; set; } + + public PublicExplicitInterfaceWithSameMembersTestClass() + { + // 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")); + IPublic pub = this; + pub.Id = -2; + Assert.That(pub.Id, Is.EqualTo(-2)); + pub.Name = "Unknown2"; + Assert.That(pub.Name, Is.EqualTo("Unknown2")); + } + } + [Serializable] public abstract class AbstractTestClass : IPublic { @@ -218,6 +248,39 @@ public void VerifyProxyForClassWithAdditionalInterface() #endif } + [Test] + public void VerifyProxyForInterfaceWithEqualsAndGetHashCode() + { + var factory = new StaticProxyFactory(); + factory.PostInstantiate( + typeof(IWithEqualsAndGetHashCode).FullName, + typeof(object), + new HashSet {typeof(IWithEqualsAndGetHashCode), typeof(INHibernateProxy)}, + null, null, null, false); + +#if NETFX + VerifyGeneratedAssembly( + () => + { +#endif + var proxy = factory.GetProxy(1, null); + Assert.That(proxy, Is.Not.Null); + Assert.That(proxy, Is.InstanceOf()); + var proxyType = proxy.GetType(); + var proxyMap = proxyType.GetInterfaceMap(typeof(IWithEqualsAndGetHashCode)); + Assert.That( + proxyMap.TargetMethods, + Has.One.Property("Name").EqualTo("Equals").And.Property("IsPublic").EqualTo(true), + "Equals is not implicitly implemented"); + Assert.That( + proxyMap.TargetMethods, + Has.One.Property("Name").EqualTo("GetHashCode").And.Property("IsPublic").EqualTo(true), + "GetHashCode is not implicitly implemented"); +#if NETFX + }); +#endif + } + [Test] public void VerifyProxyForClassWithInterface() { @@ -237,12 +300,30 @@ public void VerifyProxyForClassWithInterface() Assert.That(proxy, Is.Not.Null); Assert.That(proxy, Is.InstanceOf()); Assert.That(proxy, Is.InstanceOf()); + var proxyType = proxy.GetType(); + var proxyMap = proxyType.GetInterfaceMap(typeof(IPublic)); + Assert.That( + proxyMap.TargetMethods, + Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Name)).GetMethod), + "Name getter does not implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Name)).SetMethod), + "Name setter does not implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Id)).GetMethod), + "Id setter does not implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Id)).SetMethod), + "Id setter does not implement IPublic"); // 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; + IPublic pub = ent; 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"); @@ -276,8 +357,13 @@ public void VerifyProxyForClassWithExplicitInterface() Assert.That(proxy, Is.Not.Null); Assert.That(proxy, Is.InstanceOf()); Assert.That(proxy, Is.InstanceOf()); - - // Check interface and implicit implementations do both call the delegated state + var proxyType = proxy.GetType(); + Assert.That(proxyType.GetMethod($"get_{nameof(IPublic.Name)}"), Is.Null, "get Name is implicitly implemented"); + Assert.That(proxyType.GetMethod($"set_{nameof(IPublic.Name)}"), Is.Null, "set Name is implicitly implemented"); + Assert.That(proxyType.GetMethod($"get_{nameof(IPublic.Id)}"), Is.Null, "get Id is implicitly implemented"); + Assert.That(proxyType.GetMethod($"set_{nameof(IPublic.Id)}"), Is.Null, "set Id is implicitly implemented"); + + // Check explicit implementation IPublic state = new PublicExplicitInterfaceTestClass(); state.Id = 5; state.Name = "State"; @@ -285,7 +371,7 @@ public void VerifyProxyForClassWithExplicitInterface() 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"); @@ -297,6 +383,75 @@ public void VerifyProxyForClassWithExplicitInterface() #endif } + [Test] + public void VerifyProxyForClassWithExplicitInterfaceWithSameMembers() + { + var factory = new StaticProxyFactory(); + factory.PostInstantiate( + typeof(PublicExplicitInterfaceWithSameMembersTestClass).FullName, + typeof(PublicExplicitInterfaceWithSameMembersTestClass), + new HashSet {typeof(INHibernateProxy)}, + null, null, null, true); +#if NETFX + VerifyGeneratedAssembly( + () => + { +#endif + var proxy = factory.GetProxy(1, null); + Assert.That(proxy, Is.Not.Null); + Assert.That(proxy, Is.InstanceOf()); + Assert.That(proxy, Is.InstanceOf()); + var proxyType = proxy.GetType(); + var proxyMap = proxyType.GetInterfaceMap(typeof(IPublic)); + Assert.That( + proxyMap.TargetMethods, + Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Name)).GetMethod), + "class Name getter does implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Name)).SetMethod), + "class Name setter does implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Id)).GetMethod), + "class Id setter does implement IPublic"); + Assert.That( + proxyMap.TargetMethods, + Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Id)).SetMethod), + "class Id setter does implement IPublic"); + + // Check interface and implicit implementations do both call the delegated state + var state = new PublicExplicitInterfaceWithSameMembersTestClass(); + IPublic pubState = state; + state.Id = 5; + state.Name = "State"; + pubState.Id = 10; + pubState.Name = "State2"; + proxy.HibernateLazyInitializer.SetImplementation(state); + var entity = (PublicExplicitInterfaceWithSameMembersTestClass) proxy; + IPublic pubEntity = entity; + Assert.That(entity.Id, Is.EqualTo(5), "Id member"); + Assert.That(entity.Name, Is.EqualTo("State"), "Name member"); + Assert.That(pubEntity.Id, Is.EqualTo(10), "Id from interface"); + Assert.That(pubEntity.Name, Is.EqualTo("State2"), "Name from interface"); + + entity.Id = 15; + entity.Name = "Test"; + pubEntity.Id = 20; + pubEntity.Name = "Test2"; + Assert.That(entity.Id, Is.EqualTo(15), "entity.Id"); + Assert.That(state.Id, Is.EqualTo(15), "state.Id"); + Assert.That(entity.Name, Is.EqualTo("Test"), "entity.Name"); + Assert.That(state.Name, Is.EqualTo("Test"), "state.Name"); + Assert.That(pubEntity.Id, Is.EqualTo(20), "pubEntity.Id"); + Assert.That(pubState.Id, Is.EqualTo(20), "pubState.Id"); + Assert.That(pubEntity.Name, Is.EqualTo("Test2"), "pubEntity.Name"); + Assert.That(pubState.Name, Is.EqualTo("Test2"), "pubState.Name"); +#if NETFX + }); +#endif + } + [Test] public void VerifyProxyForRefOutClass() { diff --git a/src/NHibernate/Proxy/ProxyBuilderHelper.cs b/src/NHibernate/Proxy/ProxyBuilderHelper.cs index 9fdb85266df..a9868383878 100644 --- a/src/NHibernate/Proxy/ProxyBuilderHelper.cs +++ b/src/NHibernate/Proxy/ProxyBuilderHelper.cs @@ -14,6 +14,7 @@ using System.Runtime.CompilerServices; using System.Runtime.Serialization; using System.Security; +using NHibernate.Proxy.DynamicProxy; using NHibernate.Util; namespace NHibernate.Proxy @@ -101,11 +102,19 @@ internal static IEnumerable GetProxiableMethods(System.Type type) internal static IEnumerable GetProxiableMethods(System.Type type, IEnumerable interfaces) { - var proxiableMethods = - GetProxiableMethods(type) + if (type.IsInterface || type == typeof(object) || type.GetInterfaces().Length == 0) + { + return GetProxiableMethods(type) .Concat(interfaces.SelectMany(i => i.GetMethods())) .Distinct(); - + } + + var proxiableMethods = new HashSet(GetProxiableMethods(type), new MethodInfoComparer(type)); + foreach (var interfaceMethod in interfaces.SelectMany(i => i.GetMethods())) + { + proxiableMethods.Add(interfaceMethod); + } + return proxiableMethods; } @@ -136,23 +145,43 @@ internal static MethodBuilder GetObjectDataMethodBuilder(TypeBuilder typeBuilder internal static MethodBuilder GenerateMethodSignature(string name, MethodInfo method, TypeBuilder typeBuilder) { - //TODO: Should we use attributes of base method? - var methodAttributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Virtual; + var explicitImplementation = method.DeclaringType.IsInterface; + if (explicitImplementation && + (typeBuilder.BaseType == typeof(object) || +#pragma warning disable 618 + typeBuilder.BaseType == typeof(ProxyDummy)) && +#pragma warning restore 618 + (IsEquals(method) || IsGetHashCode(method))) + { + // If we are building a proxy for an interface, and it defines an Equals or GetHashCode, they must + // be implicitly implemented for overriding object methods. + // (Ideally we should check the method actually comes from the interface declared for the proxy.) + explicitImplementation = false; + } + + var methodAttributes = explicitImplementation + ? MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.HideBySig | + MethodAttributes.SpecialName | MethodAttributes.NewSlot | MethodAttributes.Virtual + : MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Virtual; if (method.IsSpecialName) methodAttributes |= MethodAttributes.SpecialName; var parameters = method.GetParameters(); - - var methodBuilder = typeBuilder.DefineMethod( - name, - methodAttributes, - CallingConventions.HasThis, - method.ReturnType, - parameters.ToArray(param => param.ParameterType)); + var implementationName = explicitImplementation + ? $"{method.DeclaringType.FullName}.{name}" + : name; + var methodBuilder = + typeBuilder.DefineMethod( + implementationName, + methodAttributes, + CallingConventions.HasThis, + method.ReturnType, + parameters.ToArray(param => param.ParameterType)); + if (explicitImplementation) + methodBuilder.SetImplementationFlags(MethodImplAttributes.Managed | MethodImplAttributes.IL); var typeArgs = method.GetGenericArguments(); - if (typeArgs.Length > 0) { var typeNames = GenerateTypeNames(typeArgs.Length); @@ -182,6 +211,18 @@ internal static MethodBuilder GenerateMethodSignature(string name, MethodInfo me return methodBuilder; } + private static bool IsGetHashCode(MethodBase method) + { + return method.Name == "GetHashCode" && method.GetParameters().Length == 0; + } + + private static bool IsEquals(MethodBase method) + { + if (method.Name != "Equals") return false; + var parameters = method.GetParameters(); + return parameters.Length == 1 && parameters[0].ParameterType == typeof(object); + } + internal static void GenerateInstanceOfIgnoresAccessChecksToAttribute( AssemblyBuilder assemblyBuilder, string assemblyName) @@ -253,5 +294,72 @@ private static string[] GenerateTypeNames(int count) return result; } + + /// + /// Method equality for the proxy building purpose: we want to equate an interface method to a base type + /// method which implements it. This implies the base type method has the same signature and there is no + /// explicit implementation of the interface method in the base type. + /// + private class MethodInfoComparer : IEqualityComparer + { + private readonly Dictionary> _interfacesMap; + + public MethodInfoComparer(System.Type baseType) + { + _interfacesMap = BuildInterfacesMap(baseType); + } + + private static Dictionary> BuildInterfacesMap(System.Type type) + { + return type.GetInterfaces() + .Distinct() + .ToDictionary(i => i, i => ToDictionary(type.GetInterfaceMap(i))); + } + + private static Dictionary ToDictionary(InterfaceMapping interfaceMap) + { + var map = new Dictionary(interfaceMap.InterfaceMethods.Length); + for (var i = 0; i < interfaceMap.InterfaceMethods.Length; i++) + { + map.Add(interfaceMap.InterfaceMethods[i], interfaceMap.TargetMethods[i]); + } + + return map; + } + + public bool Equals(MethodInfo x, MethodInfo y) + { + if (x == y) + return true; + if (x == null || y == null) + return false; + if (x.Name != y.Name) + return false; + + // If they have the same declaring type, one cannot be the implementation of the other. + if (x.DeclaringType == y.DeclaringType) + return false; + // If they belong to two different interfaces or to two different concrete types, one cannot be the + // implementation of the other. + if (x.DeclaringType.IsInterface == y.DeclaringType.IsInterface) + return false; + + var interfaceMethod = x.DeclaringType.IsInterface ? x : y; + // If the interface is not implemented by the base type, the method cannot be implemented by the + // base type method. + if (!_interfacesMap.TryGetValue(interfaceMethod.DeclaringType, out var map)) + return false; + + var baseMethod = x.DeclaringType.IsInterface ? y : x; + return map[interfaceMethod] == baseMethod; + } + + public int GetHashCode(MethodInfo obj) + { + // Hashing by name only, putting methods with the same name in the same bucket, in order to keep + // this method fast. + return obj.Name.GetHashCode(); + } + } } }