Skip to content

Commit

Permalink
Fix duplicated methods generated in proxies (nhibernate#2264)
Browse files Browse the repository at this point in the history 
Fix nhibernate#2085

Co-authored-by: Alexander Zaytsev <hazzik@gmail.com>
  • Loading branch information
@fredericDelaporte @hazzik
fredericDelaporte and hazzik committed Nov 11, 2019
1 parent 6ab71d0 commit d8931ac
Show file tree
Hide file tree
Showing 2 changed files with 280 additions and 17 deletions.
163 changes: 159 additions & 4 deletions src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ internal interface IInternal
{
int Id { get; }
}

public interface IWithEqualsAndGetHashCode
{
bool Equals(object that);
int GetHashCode();
}

[Serializable]
public class InternalInterfaceTestClass : IInternal
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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<System.Type> {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<IWithEqualsAndGetHashCode>());
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()
{
Expand All @@ -237,12 +300,30 @@ public void VerifyProxyForClassWithInterface()
Assert.That(proxy, Is.Not.Null);
Assert.That(proxy, Is.InstanceOf<IPublic>());
Assert.That(proxy, Is.InstanceOf<PublicInterfaceTestClass>());
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");
Expand Down Expand Up @@ -276,16 +357,21 @@ public void VerifyProxyForClassWithExplicitInterface()
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
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";
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");
Expand All @@ -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<System.Type> {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<IPublic>());
Assert.That(proxy, Is.InstanceOf<PublicExplicitInterfaceWithSameMembersTestClass>());
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()
{
Expand Down
134 changes: 121 additions & 13 deletions src/NHibernate/Proxy/ProxyBuilderHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -101,11 +102,19 @@ internal static IEnumerable<MethodInfo> GetProxiableMethods(System.Type type)

internal static IEnumerable<MethodInfo> GetProxiableMethods(System.Type type, IEnumerable<System.Type> 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<MethodInfo>(GetProxiableMethods(type), new MethodInfoComparer(type));
foreach (var interfaceMethod in interfaces.SelectMany(i => i.GetMethods()))
{
proxiableMethods.Add(interfaceMethod);
}

return proxiableMethods;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -253,5 +294,72 @@ private static string[] GenerateTypeNames(int count)

return result;
}

/// <summary>
/// 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.
/// </summary>
private class MethodInfoComparer : IEqualityComparer<MethodInfo>
{
private readonly Dictionary<System.Type, Dictionary<MethodInfo, MethodInfo>> _interfacesMap;

public MethodInfoComparer(System.Type baseType)
{
_interfacesMap = BuildInterfacesMap(baseType);
}

private static Dictionary<System.Type, Dictionary<MethodInfo, MethodInfo>> BuildInterfacesMap(System.Type type)
{
return type.GetInterfaces()
.Distinct()
.ToDictionary(i => i, i => ToDictionary(type.GetInterfaceMap(i)));
}

private static Dictionary<MethodInfo, MethodInfo> ToDictionary(InterfaceMapping interfaceMap)
{
var map = new Dictionary<MethodInfo, MethodInfo>(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();
}
}
}
}

0 comments on commit d8931ac

Please sign in to comment.