Fork me on GitHub

15 Apr 2009

When is a Pirate not a Pirate? When it's a HibernateProxy

At work we're in the middle of upgrading the Sky Entertainment sites from Grails 1.0.3 to Grails 1.1 - a long blog post will follow with a tale of our woes! One of the changes in Grails 1.1 is that one-to-one domain associations are now lazy by default. This raised an interesting problem for us as it meant we were now sometimes dealing with HibernateProxy instances where before we weren't.

For example, let's imagine we have a Pet domain class that is has an owner that is a Person and there exists another domain class, Pirate that is a specialisation of Person:

Pet.groovy

class Pet {
String type
String name
Person owner
}

Person.groovy

class Person {

String name

boolean equals(Object o) {
if (this.is(o)) return true
if (o == null) return false
if (!(o instanceof Person)) return false
return name == o.name
}


// hashCode and toString ommitted
}

Pirate.groovy

class Pirate extends Person {

String nickname

boolean equals(Object o) {
if (!super.equals(o)) return false
if (!(o instanceof Pirate)) return false
return nickname == o.nickname
}

// hashCode and toString ommitted
}

We want to test that either a regular Person or a Pirate can own a Pet. To do so we write an integration test like this:

void testAssignPersonAsPetOwner() {
Person terry = new Person(name: 'Terry Elbow')
Pet rex = new Pet(type: 'Dog', name: 'Rex', owner: terry)
Pet.withSession {session ->
assert terry.save(flush: true)
assert rex.save(flush: true)
session.clear()
}
assertEquals(terry, Pet.findByName('Rex').owner)
}

void testAssignPirateAsPetOwner() {
Person longJohn = new Pirate(name: 'John Silver', nickname: 'Long John')
Pet capnFlint = new Pet(type: 'Parrot', name: "Cap'n Flint", owner: longJohn)
Pet.withSession {session ->
assert longJohn.save(flush: true)
assert capnFlint.save(flush: true)
session.clear()
}
assertEquals(longJohn, Pet.findByName("Cap'n Flint").owner)
}

Simple enough. In both cases we just create a Pet and its owner, ensure they save properly, then assert that the owner is correct when we read the Pet back from the database. However the second test, where we assign a Pirate as the owner, fails. The final assertion fails with the message:

expected:<Pirate[Long John]> but was:<Pirate[Long John]>

Great... what?

When we read the Pet instance back from the database the owner association is set to lazy load so Pet.findByName("Cap'n Flint").owner is a HibernateProxy. The failure is caused by the fact that as owner is declared as being of type Person the proxy generated extends Person and therefore the instanceof check in Pirate's equals method fails...

if (!(o instanceof Pirate)) return false

In our case o is an instance of Person and an instance of HibernateProxy which if loaded would yield an instance of Pirate but is not itself an instance of Pirate.

Our options at this point seem to be...

  1. Set owner to eager fetch.
  2. Remove the instanceof check from the equals implementation in Pirate
  3. Initialise the HibernateProxy so equals is working with the real object

The first option is just admitting defeat! Loading objects from the database when it's not necessary is wasteful and we don't want to go creating that kind of tech debt just to get our test working. The second option is not acceptable as it would mean the equals implementation on Pirate would violate the general contract of equals as aPerson == aPirate would return false but aPirate == aPerson would throw MissingPropertyException when it tried to execute the line:

return nickname == o.nickname

This leaves us with figuring out a way to initialise the proxy so we're always dealing with real objects. You may think it's an undesirable side-effect of the equals implementation to potentially force a database read but consider that it will do so anyway to perform the comparison of the nickname property.

There is a convenience initialize method in org.hibernate.Hibernate that can force a proxy to load, but it is void so our equals method would still have a reference to the HibernateProxy that is not actually an instance of Pirate. What we need to do is:

if (o instanceof org.hibernate.proxy.HibernateProxy) {
o = o.hibernateLazyInitializer.implementation
}
if (!(o instanceof Pirate)) return false

As it turns out we can't do this directly in the equals implementation. It appears that getAt(String) is overridden in HibernateProxy's meta class so we get the error:

No such property: hibernateLazyInitializer for class: Person_$$_javassist_0

As luck would have it Grails has a utility method for doing exactly what we want and as it's written in Java no amount of meta class trickery will hide the hibernateLazyInitializer property from it. Our final, working equals implementation for Pirate looks like:

boolean equals(Object o) {
if (!super.equals(o)) return false
if (o instanceof HibernateProxy) {
o = GrailsHibernateUtil.unwrapProxy(o)
}
if (!(o instanceof Pirate)) return false
return nickname == o.nickname
}

Note: this is a simple example so we'll gloss over the fact that Pirate's equals isn't symmetric as

new Person(name: 'X') == new Pirate(name: 'X', nickname: 'Y')

returns true while flipping the operands causes it to return false. The problem and the solution apply any time inheritance and lazy-loading run up against class checking whether via instanceof, Class.isAssignableFrom, switch statements using a Class as a case, etc.

5 comments:

Peter Delahunty said...

Great post. I however have some thoughts on your chosen solution.

Your transparent persistence ORM is not so transparent anymore with having to know about HibernateProxy in your equals method.

For me some of the most powerful agile princepals are YAGNI (You ain't gonna need it) and "Do the simplest thing that works"

I would have just gone with option 1 (the simplest) and simply just added eager fetching. Your assumption about database over head might turn out to be insignificant. So you therefore assume performance issues without testing performance issues and therefore violating YAGNI.

Pete

Rob said...

I can certainly see your point regarding the purity of the domain class, and sure the example is trivial. In a real world use case however, especially when one-to-many relationships are involved, eager fetching may well be sub-optimal. We've done performance testing and made some significant improvements by enabling lazy loading, in some cases in places where we'd turned on eager fetch for convenience.

Ideally, I think this solution could be rolled into the Grails Hibernate plugin to restore the transparency and make the behaviour automatic.

Interestingly if you look at line 605 (in Grails 1.1) of org.codehaus.groovy.grails.plugins.orm.hibernate.HibernatePluginSupport it looks like there is something going with instanceof being overridden on domain object metaclasses but either I'm missing the point of what it's doing or it's not really working.

Graeme Rocher said...

GORM provides an instanceOf method to deal with this scenario. You should always use this over the regular instanceof:

boolean equals(Object o) {
if (!super.equals(o)) return false
if (!(o.instanceOf(Pirate))) return false
return nickname == o.nickname
}

It will transparently deal with proxies for you.

Sascha said...

Thanks a lot too you all, writing my own equals method with o.instanceOf(...) solved some issues in our project.

Rokki Winchester said...

Thanks for posting this. I wish it was able to be translated, but for some reason Google toolbar isn't working. I copy pasted it into advanced writers review another application and read the post.