Problem with Equals in EntityWithTypedId<IdT> and NHibernate proxies

[Kevin]

Hello all,

I've run into a bit of an issue with the implementation of Equals in
EntityWithTypedId<IdT> and the fact that NHibernate can sometimes
return dynamic proxies when using lazy loading.

I'm just getting started with NHibernate so my apologies if I am
butchering any of the details here. I ran into the problem in a test
where I was comparing for equivalence a list of entities persisted and
evicted in setup to a corresponding list loaded in the method under
test. One of the entities appeared to be coming back as the proxy
type and looks to be the result of the fact that the same entity was
loaded as a lazy proxy through an association on another entity loaded
within the target method.

I had figured that the type check in the Equals method might be the
issue and once I stepped through with the debugger it appears my
comparison was failing here:

if (compareTo == null || !GetType().Equals(compareTo.GetType()))
return false;

I don't know that I have an answer as I realize Billy has very
carefully considered ithe implications of Equals and HashCode here but
I'm wondering if it may make sense to either ditch the GetType check
or consider some alternatives within Nhibernate itself. I know there
is some way to specify an interface to be used for dynamic proxies if
necessary. However, if we are casting using as, is it sufficient to
just satisfy the equality condition if compareTo is not null?

A few interesting, related links I came across:

http://cwmaier.blogspot.com/2007/07/liskov-substitution-principle-equals.html
(Hibernate Specific)
http://blogs.chayachronicles.com/sonofnun/archive/2007/03/30/230.aspx

Thanks in advance for any help!

[Kevin]

I hate to reply to my own post here but does anyone have any thoughts
here? I would figure that someone would've run into this same problem
unless I am missing something very basic.

[Billy]

Kevin,

I believe your suggestion for using a "cast as" and check for null
would solve the comparison-to-proxy you've encountered. I will add
this as an issue and address accordingly.

Billy

[Kevin]

Billy,

This may already be obvious to you but a coworker did point out to me
that we do need to somehow check for type equality. Otherwise, we
could run into a situation where objects with the same id but
different types would pass on the call to HasSameNonDefaultIdAs

Kevin

[Billy]

Hi Kevin,

Your point is valid and already taken care of by the framework via a
type comparison. The hash code creation process also takes this into
account.

Please let me know if there is a specific bit of code that you're
concerned with. You brought up the issue concerning the possible
comparison with a proxied object. Was there another concern as well?

Billy

[Jay Oliver]

My understanding of Kevin's root problem is the following scenario:

Customer w/ ID 17 is not equal to Customer Proxy w/ ID 17

I think this would depend on the order you compared them. The proxy
can be cast back to the customer, but not vice versa.

[Jay Oliver]

I guess I hit send a little too quickly there.

To clarify, my understanding from his post is that a proxy and a non-
proxy will never be equivalent with the current code.

When I referred to the order of comparison mattering, I was referring
to the suggestion to remove the GetType equality check and replace it
with some sort of casting and/or IsAssignableFrom type check.

[Jay Oliver]

I can reproduce the issue, although for some reason trying to explain
things related to proxies turns me into an idiot today...

Start with the Northwind project, as it exists in trunk.

Add the following test to the CustomerRepositoryTests unit test class:

[Test]
public void ProxyEqualityTest()
{
var customer = CreatePersistedCustomer("GADGE", "Bob
Ross", "Gadgets Inc.", "USA");
var order = CreatePersistentOrder(customer, DateTime.Now);

var fetchedOrder = orderRepository.Get(order.Id);
var proxyCustomer = fetchedOrder.OrderedBy;

Assert.IsTrue(proxyCustomer.Equals(customer));
Assert.IsTrue(customer.Equals(proxyCustomer));
}

This test passes and everyone is happy.

Now, go into the CustomerMap class and disable eager loading by
commenting out this code: mapping.SetAttribute("lazy", "false");

Run the test again and the second assert will. Sadness commences.

Coming up with a solution is a different story - but this illustrates
the problem at least.

[Billy]

Having a breaking test is immensely helpful...thank you! I'll wrastle
with it a bit. ;)

Billy

[Jay Oliver]

I want to point out that this isn't because the comparison is to the
"original" customer. If it was limited to that situation, it'd
probably be a fairly rare edge case. However, the test below also
fails, and I think it represents a much more common scenario.

[Test]
public void ProxyEqualityTest()
{
var originalCustomer = CreatePersistedCustomer("GADGE",

"Bob Ross", "Gadgets Inc.", "USA");

var originalOrder = CreatePersistentOrder
(originalCustomer, DateTime.Now);

var fetchedOrder = orderRepository.Get(originalOrder.Id);
var associatedCustomer = fetchedOrder.OrderedBy;
var fetchedCustomer = customerRepository.Get("GADGE");

Assert.IsTrue(associatedCustomer.Equals(fetchedCustomer));
Assert.IsTrue(fetchedCustomer.Equals(associatedCustomer));

[Luis Abreu]

Can't you just cast it to T before calling the method?

Luis

[Luis Abreu]

Btw, where does it fail? On the type comparison? Does it compares the IDs?

> -----Original Message-----
> From: sharp-arc...@googlegroups.com [mailto:sharp-
> archit...@googlegroups.com] On Behalf Of Jay Oliver
> Sent: quinta-feira, 5 de Março de 2009 19:30
> To: S#arp Architecture

[Jay Oliver]

I believe (but have not yet verified) that it's this check:

!GetType().Equals(compareTo.GetType()

The T in EntityWithTypedId<T> is the type of the ID, not the Type of
the Entity.

[Luis Abreu]

> -----Original Message-----
> From: sharp-arc...@googlegroups.com [mailto:sharp-
> archit...@googlegroups.com] On Behalf Of Jay Oliver
> Sent: quinta-feira, 5 de Março de 2009 21:39
> To: S#arp Architecture
> Subject: Re: Problem with Equals in EntityWithTypedId<IdT> and
> NHibernate proxies
>
>

> I believe (but have not yet verified) that it's this check:
>
> !GetType().Equals(compareTo.GetType()
>
> The T in EntityWithTypedId<T> is the type of the ID, not the Type of
> the Entity.

I was suggesting something like:

* adding protected virtual method called CompareTypes. You'd only need to
override this if you were using proxies
* override it and do a type check based on the current type

Would something like this solve it? (that is, if it's failing on the type
comparison)

Luis

[Jay Oliver]

If you were willing to override something per entity, it could work. I
think it would be a lot of effort and kind of messy though. I'm hoping
that there's a way to resolve this at a framework level.

[Jay Oliver]

Billy, Kevin pointed out to me that all NH proxy objects implement
INHibernateProxy, which might help with this, although I'm reluctant
to suggest anything that directly ties Core to NH.

[Billy]

I found a useful means to "unproxy" an object:

public static object Unproxy(object possiblyProxiedObject) {
if (possiblyProxiedObject is INHibernateProxy) {
LazyInitializer li =
NHibernateProxyHelper.GetLazyInitializer((INHibernateProxy)
possiblyProxiedObject);
return li.GetImplementation();
}

return possiblyProxiedObject;
}

But as you noted, this would require a dependency to NHibernate.dll
added directly to SharpArch.Core. We could add an IUnproxyService to
SharpArch.Core, implement it within SharpArchData, and use a service
locator to load the concrete unproxy service. But that seems like
quite a tangle. Thoughts?

Billy

[Jay Oliver]

Well, we don't actually need to unproxy it, right? I think all the
other checks work fine if we can get past the type check.

How do you feel about something along these lines? Embedding the
string is kind of yucky, but it has the benefit of simplicity.

public override bool Equals(object obj) {
EntityWithTypedId<IdT> compareTo = obj as
EntityWithTypedId<IdT>;

if (ReferenceEquals(this, compareTo))
return true;

if (compareTo == null || (!GetType().Equals
(compareTo.GetType()) &&
!IsProxyOfSameType(GetType(), compareTo.GetType())))
return false;

if (HasSameNonDefaultIdAs(compareTo))
return true;

// Since the Ids aren't the same, both of them must be
transient to
// compare domain signatures; because if one is transient
and the
// other is a persisted entity, then they cannot be the
same object.
return IsTransient() && compareTo.IsTransient() &&
HasSameObjectSignatureAs(compareTo);
}

protected bool IsProxyOfSameType(Type type, Type compareType)
{
return (compareType.GetInterface("INHibernateProxy")) !=
null && type.Equals(compareType.BaseType);

[Jay Oliver]

One thing I can't figure out is why proxy.Equals(notProxy) ever
worked.

I wonder if they're shadowing GetType() in the proxy.

[Jay Oliver]

Yeah... quick test, and I think that is in fact what's happening.

Added this to Customer:

public virtual Type GetRealType()
{
return GetType();
}

Ran this code:

Console.WriteLine(associatedCustomer.GetType());
Console.WriteLine(associatedCustomer.GetRealType());

Got:

CustomerProxy8713892bc5274335b24bc1b75847a590
Northwind.Core.Customer

[James]

I believe I've just run into this issue too. I was trying to do
something along the lines of:

IList<Products> distinctProducts = productRepository.GetAll().Distinct
();

Duplicates are not being removed correctly because some of the
products returned by the repository are in fact NHibernate proxy
objects and the equality check with real product is erroneously
returning false.

> http://cwmaier.blogspot.com/2007/07/liskov-substitution-principle-equ...
> (Hibernate Specific)http://blogs.chayachronicles.com/sonofnun/archive/2007/03/30/230.aspx

[Jay Oliver]

In re-reading my last post, I came up with a better solution -
basically take advantage of the fact that NHibernate's proxies are
shadowing GetType().

All we need to do is add one level of indirection to the type
comparison,so that the call comes from an object that knows the
concrete type of the proxy. The easiest way I could come up with to
actually implement this is below:

Add this method to EntityWithTypedId:

protected virtual Type GetActualType()
{
return GetType();
}

Check the check in Equals to this:

if (compareTo == null || !GetType().Equals
(compareTo.GetActualType()) )

the this.GetType() call doesn't need to be modified, because if "this"
is a proxy it already hits the shadowed method.

[Berryl Hesh]

I hit this problem too and it was driving me nuts. I got around it
with the hack below, which helped me see what was actually happening.

I like Jay's latest solution better though. Nice work!

private bool IsProxy(EntityWithTypedId<IdT> other) {
var myType = GetType().Name;
var otherType = other.GetType().Name;
return otherType.StartsWith(myType + "Proxy");

[Billy]

That worked like a charm Jay...great job on this fix! I've checked in
the change.

Billy

[Jay Oliver]

No problem. Glad it helped, sounds like more people than I expected
were affected by this issue, but had come up with their own
workarounds.