Don’t change the parameter value in a Hibernate setter

A while back a co-worker asked for some help with a Hibernate problem – he thought he’d found a serious bug. He was an experienced database architect but new to Hibernate, so he’d written some basic test code. It was trivial – just reading objects from various HQL queries and displaying them to the console. The weird thing was, after reading an object the ‘name’ property was always null (even though the column wasn’t – the sample data had values for every column) and weirder still after reading a row, the column became null.

I looked at his mapping files and his environment but everything seemed fine. Then I noticed that his setter method looked something like this:

public void setName(String naem) {
  this.name = name;
}

So it was a classic typo – the parameter was misspelled, so the setter was just setting the field to itself (Eclipse has a check for this and I find it’s best to flag it as an error). Since the state at flush time was different from the original values that Hibernate caches for dirty detection, Hibernate dutifully pushed the “change” to the database.

I was reminded of this today because of a similar bug that was introduced last week. One of the developers was seeing an unexpected optimistic locking exception in an entity that wasn’t being updated concurrently – in fact there are no public setters in the class.

Since the class is effectively immutable, I added the mutable='false' attribute to the class element in the mapping file, removed the non-public setters only called by Hibernate during instantiation and switched to field access. I also removed the optimistic locking <version> element since there’s no need to check for concurrent updates, and the bug went away.

I felt guilty checking that in though since it felt like fixing the symptom and not the real problem – just setting up someone else the next time something similar triggered the root bug. Luckily I realized the real issue a little while later.

We have a custom type for storing dates independent of timezone (it converts to GMT on insert and we adjust for the current time zone on read) and it was implemented using java.sql.Timestamp. Apparently this caused problems when comparing equality with java.util.Date, so the custom type’s nullSafeGet was changed to read the database timestamp and convert to a java.util.Date. As with the earlier example – an apparent change of the value (Timestamp‘s equals method isn’t symmetric with Date‘s) caused Hibernate to detect the change and push the update to the database. Two users were concurrently reading the object and Hibernate was pushing both changes to the database, hence the optimistic locking exception.

So I’ll keep the entity class immutable – it matches the actual usage – but will fix the custom type to do the right thing.

The first time I burned myself like this was with mapped collections. I had a mapped List on an entity and when I wrote the unit tests for the DAO tier, I mistakenly created test SQL scripts with index column values that started at 1 instead of 0. So I was seeing that the lists always had N+1 elements, and the first was always null. I spent way too long banging on this and finally put in a hack where I created a copy of the List in the setter (a new ArrayList), keeping only non-null elements.

This was a side project I was working on from home, and I ended up getting too busy and leaving the project. I recently started working on it again after being away for a year, and was able to fix a lot of dumb mistakes I’d made (this was my first time using Spring and Hibernate a real project).

The folks on the team mentioned that the SQL logs showed lots of deletes and inserts every time the entity was loaded from the database, resulting in significant slowness, so they wanted me to take a look. It didn’t take long to realize that it was due to replacing the managed PersistentList with a new list. Since it’s a new instance, Hibernate doesn’t detect specific changes so it deletes all of the old rows and inserts all the new, even in a case like this where there was no net change. In the production code, there were no nulls and in fact no actual changes, so no actions were necessary at all. This realization lead to the discovery of the unit test bug, and I was able to remove the hack and just keep the List that Hibernate passed in the setter.

A while back a co-worker asked for some help with a Hibernate problem – he thought he’d found a serious bug. He was an experienced database architect but new to...

This entry was posted on Tuesday, December 04th, 2007 at 12:36am and is filed under hibernate, java. You can follow any responses to this entry through the RSS 2.0 feed. Both comments and pings are currently closed.