Skip to content

Commit d8931ac

Browse files
fredericDelaportehazzik
fredericDelaporte
and
hazzik
authored
Fix duplicated methods generated in proxies (#2264)
Fix #2085 Co-authored-by: Alexander Zaytsev <[email protected]>
1 parent 6ab71d0 commit d8931ac

File tree

2 files changed

+280
-17
lines changed

2 files changed

+280
-17
lines changed

‎src/NHibernate.Test/StaticProxyTest/StaticProxyFactoryFixture.cs

Lines changed: 159 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ internal interface IInternal
2424
{
2525
int Id { get; }
2626
}
27+
28+
public interface IWithEqualsAndGetHashCode
29+
{
30+
bool Equals(object that);
31+
int GetHashCode();
32+
}
2733

2834
[Serializable]
2935
public class InternalInterfaceTestClass : IInternal
@@ -70,6 +76,30 @@ public PublicExplicitInterfaceTestClass()
7076
}
7177
}
7278

79+
[Serializable]
80+
public class PublicExplicitInterfaceWithSameMembersTestClass : IPublic
81+
{
82+
public virtual int Id { get; set; }
83+
public virtual string Name { get; set; }
84+
85+
int IPublic.Id { get; set; }
86+
string IPublic.Name { get; set; }
87+
88+
public PublicExplicitInterfaceWithSameMembersTestClass()
89+
{
90+
// Check access to properties from the default constructor do not fail once proxified
91+
Id = -1;
92+
Assert.That(Id, Is.EqualTo(-1));
93+
Name = "Unknown";
94+
Assert.That(Name, Is.EqualTo("Unknown"));
95+
IPublic pub = this;
96+
pub.Id = -2;
97+
Assert.That(pub.Id, Is.EqualTo(-2));
98+
pub.Name = "Unknown2";
99+
Assert.That(pub.Name, Is.EqualTo("Unknown2"));
100+
}
101+
}
102+
73103
[Serializable]
74104
public abstract class AbstractTestClass : IPublic
75105
{
@@ -218,6 +248,39 @@ public void VerifyProxyForClassWithAdditionalInterface()
218248
#endif
219249
}
220250

251+
[Test]
252+
public void VerifyProxyForInterfaceWithEqualsAndGetHashCode()
253+
{
254+
var factory = new StaticProxyFactory();
255+
factory.PostInstantiate(
256+
typeof(IWithEqualsAndGetHashCode).FullName,
257+
typeof(object),
258+
new HashSet<System.Type> {typeof(IWithEqualsAndGetHashCode), typeof(INHibernateProxy)},
259+
null, null, null, false);
260+
261+
#if NETFX
262+
VerifyGeneratedAssembly(
263+
() =>
264+
{
265+
#endif
266+
var proxy = factory.GetProxy(1, null);
267+
Assert.That(proxy, Is.Not.Null);
268+
Assert.That(proxy, Is.InstanceOf<IWithEqualsAndGetHashCode>());
269+
var proxyType = proxy.GetType();
270+
var proxyMap = proxyType.GetInterfaceMap(typeof(IWithEqualsAndGetHashCode));
271+
Assert.That(
272+
proxyMap.TargetMethods,
273+
Has.One.Property("Name").EqualTo("Equals").And.Property("IsPublic").EqualTo(true),
274+
"Equals is not implicitly implemented");
275+
Assert.That(
276+
proxyMap.TargetMethods,
277+
Has.One.Property("Name").EqualTo("GetHashCode").And.Property("IsPublic").EqualTo(true),
278+
"GetHashCode is not implicitly implemented");
279+
#if NETFX
280+
});
281+
#endif
282+
}
283+
221284
[Test]
222285
public void VerifyProxyForClassWithInterface()
223286
{
@@ -237,12 +300,30 @@ public void VerifyProxyForClassWithInterface()
237300
Assert.That(proxy, Is.Not.Null);
238301
Assert.That(proxy, Is.InstanceOf<IPublic>());
239302
Assert.That(proxy, Is.InstanceOf<PublicInterfaceTestClass>());
303+
var proxyType = proxy.GetType();
304+
var proxyMap = proxyType.GetInterfaceMap(typeof(IPublic));
305+
Assert.That(
306+
proxyMap.TargetMethods,
307+
Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Name)).GetMethod),
308+
"Name getter does not implement IPublic");
309+
Assert.That(
310+
proxyMap.TargetMethods,
311+
Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Name)).SetMethod),
312+
"Name setter does not implement IPublic");
313+
Assert.That(
314+
proxyMap.TargetMethods,
315+
Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Id)).GetMethod),
316+
"Id setter does not implement IPublic");
317+
Assert.That(
318+
proxyMap.TargetMethods,
319+
Has.One.EqualTo(proxyType.GetProperty(nameof(PublicInterfaceTestClass.Id)).SetMethod),
320+
"Id setter does not implement IPublic");
240321

241322
// Check interface and implicit implementations do both call the delegated state
242323
var state = new PublicInterfaceTestClass { Id = 5, Name = "State" };
243324
proxy.HibernateLazyInitializer.SetImplementation(state);
244-
var pub = (IPublic) proxy;
245325
var ent = (PublicInterfaceTestClass) proxy;
326+
IPublic pub = ent;
246327
Assert.That(pub.Id, Is.EqualTo(5), "IPublic.Id");
247328
Assert.That(ent.Id, Is.EqualTo(5), "entity.Id");
248329
Assert.That(pub.Name, Is.EqualTo("State"), "IPublic.Name");
@@ -276,16 +357,21 @@ public void VerifyProxyForClassWithExplicitInterface()
276357
Assert.That(proxy, Is.Not.Null);
277358
Assert.That(proxy, Is.InstanceOf<IPublic>());
278359
Assert.That(proxy, Is.InstanceOf<PublicExplicitInterfaceTestClass>());
279-
280-
// Check interface and implicit implementations do both call the delegated state
360+
var proxyType = proxy.GetType();
361+
Assert.That(proxyType.GetMethod($"get_{nameof(IPublic.Name)}"), Is.Null, "get Name is implicitly implemented");
362+
Assert.That(proxyType.GetMethod($"set_{nameof(IPublic.Name)}"), Is.Null, "set Name is implicitly implemented");
363+
Assert.That(proxyType.GetMethod($"get_{nameof(IPublic.Id)}"), Is.Null, "get Id is implicitly implemented");
364+
Assert.That(proxyType.GetMethod($"set_{nameof(IPublic.Id)}"), Is.Null, "set Id is implicitly implemented");
365+
366+
// Check explicit implementation
281367
IPublic state = new PublicExplicitInterfaceTestClass();
282368
state.Id = 5;
283369
state.Name = "State";
284370
proxy.HibernateLazyInitializer.SetImplementation(state);
285371
var entity = (IPublic) proxy;
286372
Assert.That(entity.Id, Is.EqualTo(5), "Id");
287373
Assert.That(entity.Name, Is.EqualTo("State"), "Name");
288-
374+
289375
entity.Id = 10;
290376
entity.Name = "Test";
291377
Assert.That(entity.Id, Is.EqualTo(10), "entity.Id");
@@ -297,6 +383,75 @@ public void VerifyProxyForClassWithExplicitInterface()
297383
#endif
298384
}
299385

386+
[Test]
387+
public void VerifyProxyForClassWithExplicitInterfaceWithSameMembers()
388+
{
389+
var factory = new StaticProxyFactory();
390+
factory.PostInstantiate(
391+
typeof(PublicExplicitInterfaceWithSameMembersTestClass).FullName,
392+
typeof(PublicExplicitInterfaceWithSameMembersTestClass),
393+
new HashSet<System.Type> {typeof(INHibernateProxy)},
394+
null, null, null, true);
395+
#if NETFX
396+
VerifyGeneratedAssembly(
397+
() =>
398+
{
399+
#endif
400+
var proxy = factory.GetProxy(1, null);
401+
Assert.That(proxy, Is.Not.Null);
402+
Assert.That(proxy, Is.InstanceOf<IPublic>());
403+
Assert.That(proxy, Is.InstanceOf<PublicExplicitInterfaceWithSameMembersTestClass>());
404+
var proxyType = proxy.GetType();
405+
var proxyMap = proxyType.GetInterfaceMap(typeof(IPublic));
406+
Assert.That(
407+
proxyMap.TargetMethods,
408+
Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Name)).GetMethod),
409+
"class Name getter does implement IPublic");
410+
Assert.That(
411+
proxyMap.TargetMethods,
412+
Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Name)).SetMethod),
413+
"class Name setter does implement IPublic");
414+
Assert.That(
415+
proxyMap.TargetMethods,
416+
Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Id)).GetMethod),
417+
"class Id setter does implement IPublic");
418+
Assert.That(
419+
proxyMap.TargetMethods,
420+
Has.None.EqualTo(proxyType.GetProperty(nameof(PublicExplicitInterfaceWithSameMembersTestClass.Id)).SetMethod),
421+
"class Id setter does implement IPublic");
422+
423+
// Check interface and implicit implementations do both call the delegated state
424+
var state = new PublicExplicitInterfaceWithSameMembersTestClass();
425+
IPublic pubState = state;
426+
state.Id = 5;
427+
state.Name = "State";
428+
pubState.Id = 10;
429+
pubState.Name = "State2";
430+
proxy.HibernateLazyInitializer.SetImplementation(state);
431+
var entity = (PublicExplicitInterfaceWithSameMembersTestClass) proxy;
432+
IPublic pubEntity = entity;
433+
Assert.That(entity.Id, Is.EqualTo(5), "Id member");
434+
Assert.That(entity.Name, Is.EqualTo("State"), "Name member");
435+
Assert.That(pubEntity.Id, Is.EqualTo(10), "Id from interface");
436+
Assert.That(pubEntity.Name, Is.EqualTo("State2"), "Name from interface");
437+
438+
entity.Id = 15;
439+
entity.Name = "Test";
440+
pubEntity.Id = 20;
441+
pubEntity.Name = "Test2";
442+
Assert.That(entity.Id, Is.EqualTo(15), "entity.Id");
443+
Assert.That(state.Id, Is.EqualTo(15), "state.Id");
444+
Assert.That(entity.Name, Is.EqualTo("Test"), "entity.Name");
445+
Assert.That(state.Name, Is.EqualTo("Test"), "state.Name");
446+
Assert.That(pubEntity.Id, Is.EqualTo(20), "pubEntity.Id");
447+
Assert.That(pubState.Id, Is.EqualTo(20), "pubState.Id");
448+
Assert.That(pubEntity.Name, Is.EqualTo("Test2"), "pubEntity.Name");
449+
Assert.That(pubState.Name, Is.EqualTo("Test2"), "pubState.Name");
450+
#if NETFX
451+
});
452+
#endif
453+
}
454+
300455
[Test]
301456
public void VerifyProxyForRefOutClass()
302457
{

‎src/NHibernate/Proxy/ProxyBuilderHelper.cs

Lines changed: 121 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using System.Runtime.CompilerServices;
1515
using System.Runtime.Serialization;
1616
using System.Security;
17+
using NHibernate.Proxy.DynamicProxy;
1718
using NHibernate.Util;
1819

1920
namespace NHibernate.Proxy
@@ -101,11 +102,19 @@ internal static IEnumerable<MethodInfo> GetProxiableMethods(System.Type type)
101102

102103
internal static IEnumerable<MethodInfo> GetProxiableMethods(System.Type type, IEnumerable<System.Type> interfaces)
103104
{
104-
var proxiableMethods =
105-
GetProxiableMethods(type)
105+
if (type.IsInterface || type == typeof(object) || type.GetInterfaces().Length == 0)
106+
{
107+
return GetProxiableMethods(type)
106108
.Concat(interfaces.SelectMany(i => i.GetMethods()))
107109
.Distinct();
108-
110+
}
111+
112+
var proxiableMethods = new HashSet<MethodInfo>(GetProxiableMethods(type), new MethodInfoComparer(type));
113+
foreach (var interfaceMethod in interfaces.SelectMany(i => i.GetMethods()))
114+
{
115+
proxiableMethods.Add(interfaceMethod);
116+
}
117+
109118
return proxiableMethods;
110119
}
111120

@@ -136,23 +145,43 @@ internal static MethodBuilder GetObjectDataMethodBuilder(TypeBuilder typeBuilder
136145

137146
internal static MethodBuilder GenerateMethodSignature(string name, MethodInfo method, TypeBuilder typeBuilder)
138147
{
139-
//TODO: Should we use attributes of base method?
140-
var methodAttributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Virtual;
148+
var explicitImplementation = method.DeclaringType.IsInterface;
149+
if (explicitImplementation &&
150+
(typeBuilder.BaseType == typeof(object) ||
151+
#pragma warning disable 618
152+
typeBuilder.BaseType == typeof(ProxyDummy)) &&
153+
#pragma warning restore 618
154+
(IsEquals(method) || IsGetHashCode(method)))
155+
{
156+
// If we are building a proxy for an interface, and it defines an Equals or GetHashCode, they must
157+
// be implicitly implemented for overriding object methods.
158+
// (Ideally we should check the method actually comes from the interface declared for the proxy.)
159+
explicitImplementation = false;
160+
}
161+
162+
var methodAttributes = explicitImplementation
163+
? MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.HideBySig |
164+
MethodAttributes.SpecialName | MethodAttributes.NewSlot | MethodAttributes.Virtual
165+
: MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Virtual;
141166

142167
if (method.IsSpecialName)
143168
methodAttributes |= MethodAttributes.SpecialName;
144169

145170
var parameters = method.GetParameters();
146-
147-
var methodBuilder = typeBuilder.DefineMethod(
148-
name,
149-
methodAttributes,
150-
CallingConventions.HasThis,
151-
method.ReturnType,
152-
parameters.ToArray(param => param.ParameterType));
171+
var implementationName = explicitImplementation
172+
? $"{method.DeclaringType.FullName}.{name}"
173+
: name;
174+
var methodBuilder =
175+
typeBuilder.DefineMethod(
176+
implementationName,
177+
methodAttributes,
178+
CallingConventions.HasThis,
179+
method.ReturnType,
180+
parameters.ToArray(param => param.ParameterType));
181+
if (explicitImplementation)
182+
methodBuilder.SetImplementationFlags(MethodImplAttributes.Managed | MethodImplAttributes.IL);
153183

154184
var typeArgs = method.GetGenericArguments();
155-
156185
if (typeArgs.Length > 0)
157186
{
158187
var typeNames = GenerateTypeNames(typeArgs.Length);
@@ -182,6 +211,18 @@ internal static MethodBuilder GenerateMethodSignature(string name, MethodInfo me
182211
return methodBuilder;
183212
}
184213

214+
private static bool IsGetHashCode(MethodBase method)
215+
{
216+
return method.Name == "GetHashCode" && method.GetParameters().Length == 0;
217+
}
218+
219+
private static bool IsEquals(MethodBase method)
220+
{
221+
if (method.Name != "Equals") return false;
222+
var parameters = method.GetParameters();
223+
return parameters.Length == 1 && parameters[0].ParameterType == typeof(object);
224+
}
225+
185226
internal static void GenerateInstanceOfIgnoresAccessChecksToAttribute(
186227
AssemblyBuilder assemblyBuilder,
187228
string assemblyName)
@@ -253,5 +294,72 @@ private static string[] GenerateTypeNames(int count)
253294

254295
return result;
255296
}
297+
298+
/// <summary>
299+
/// Method equality for the proxy building purpose: we want to equate an interface method to a base type
300+
/// method which implements it. This implies the base type method has the same signature and there is no
301+
/// explicit implementation of the interface method in the base type.
302+
/// </summary>
303+
private class MethodInfoComparer : IEqualityComparer<MethodInfo>
304+
{
305+
private readonly Dictionary<System.Type, Dictionary<MethodInfo, MethodInfo>> _interfacesMap;
306+
307+
public MethodInfoComparer(System.Type baseType)
308+
{
309+
_interfacesMap = BuildInterfacesMap(baseType);
310+
}
311+
312+
private static Dictionary<System.Type, Dictionary<MethodInfo, MethodInfo>> BuildInterfacesMap(System.Type type)
313+
{
314+
return type.GetInterfaces()
315+
.Distinct()
316+
.ToDictionary(i => i, i => ToDictionary(type.GetInterfaceMap(i)));
317+
}
318+
319+
private static Dictionary<MethodInfo, MethodInfo> ToDictionary(InterfaceMapping interfaceMap)
320+
{
321+
var map = new Dictionary<MethodInfo, MethodInfo>(interfaceMap.InterfaceMethods.Length);
322+
for (var i = 0; i < interfaceMap.InterfaceMethods.Length; i++)
323+
{
324+
map.Add(interfaceMap.InterfaceMethods[i], interfaceMap.TargetMethods[i]);
325+
}
326+
327+
return map;
328+
}
329+
330+
public bool Equals(MethodInfo x, MethodInfo y)
331+
{
332+
if (x == y)
333+
return true;
334+
if (x == null || y == null)
335+
return false;
336+
if (x.Name != y.Name)
337+
return false;
338+
339+
// If they have the same declaring type, one cannot be the implementation of the other.
340+
if (x.DeclaringType == y.DeclaringType)
341+
return false;
342+
// If they belong to two different interfaces or to two different concrete types, one cannot be the
343+
// implementation of the other.
344+
if (x.DeclaringType.IsInterface == y.DeclaringType.IsInterface)
345+
return false;
346+
347+
var interfaceMethod = x.DeclaringType.IsInterface ? x : y;
348+
// If the interface is not implemented by the base type, the method cannot be implemented by the
349+
// base type method.
350+
if (!_interfacesMap.TryGetValue(interfaceMethod.DeclaringType, out var map))
351+
return false;
352+
353+
var baseMethod = x.DeclaringType.IsInterface ? y : x;
354+
return map[interfaceMethod] == baseMethod;
355+
}
356+
357+
public int GetHashCode(MethodInfo obj)
358+
{
359+
// Hashing by name only, putting methods with the same name in the same bucket, in order to keep
360+
// this method fast.
361+
return obj.Name.GetHashCode();
362+
}
363+
}
256364
}
257365
}

0 commit comments

Comments
 (0)