Thursday, September 4, 2014

I don't like Hibernate/Grails part 6, how to save objects using refresh()


This one took some time to figure out, but the closer I got to unraveling the problem, the more my jaw dropped.  I have seen mysterious errors in Geb/Spock tests, the errors where not hard to fix but the behavior was very scary.  Tests have been trying  to save a domain object (and failing to save it) - but that object was not modified by the test nor there was any explicit logic to save it!

Eventually, I was able to reproduce it 'from scratch'  (using current Grails 2.4.3 and 2.3.3 project with all new project defaults).  I use this simple domain class:
   class Simple {
      String name
   }


and I assume I have a record for it with name 'simple1' saved in the database. Consider this code:

   def simple1 = Simple.findByName('simple1') //(1)

   Simple.withNewSession {
     simple1.refresh() //(2)
   }

   Simple.findAll() //(3)


If simple1 is concurrently modified, GORM will actually attempt to save simple1 on the line marked with (3).  If this happens, the above code will fail with HibernateOptimisticLockingFailureException because Simple class has implicit version property.

Side Note:  I think, developers who configure 'version false' on their domain objects are indeed very brave!

Please note that the above code has NOT modified simple1 in any way. What happens here: line marked as (2) sets dirty flag on simple1 object!  This should be somewhat embarrassing bug because developers may use refresh() to reset the object content and to prevent any saving from happening. In reality, the very use of refresh() can actually trigger the save!

Here is a test method which demonstrates the issue (this test passes in current latest version 2.4.3 and in 2.3.3): 

void testNewSessionWithRefresh() {
  def simple = Simple.findByName('simple1')

  //start concurrent mod
  def session = sessionFactory.currentSession
  Sql sql = new Sql(session.connection())
  sql.execute("""

     update simple
     set version=version + 1, name='new_' || name
  """.toString()
  )

  def rec = sql.firstRow(

     "select name from simple where id=${simple.id}".toString()
  )
  assert rec[0] == 'new_simple1'
  //end concurrent mod

  Simple.withNewSession {
      simple.refresh()
  }

  assert simple.dirty

}

Maybe it is the nature of ugly side-effects that if you combine them (auto-save + session) new ugly (and uglier) side-effects are born?

Fail Fast?:  You can argue that refresh() call should not be made in a context of different hibernate session.  Yes, but if this is the case, should I not expect an exception thrown from that refresh() call?   This is yet one of the cases, so typical to Grails/Hibernate technology stack, where bad code works fine most of the time. A lot of code in Grails is like a ticking bomb.

I have uncovered this particular problem by analyzing weird behavior in my Geb functional tests. Geb code triggers server activity,  which is concurrent to Geb test itself.  How can I be sure that all my code is safe from this problem?  Should I avoid using more then one hibernate session per, say, one HTTP request?  Yes, that would go well with Grails/Spring design in general but there are situations where having more than one session is needed (as discussed in my post part 3).

Exercise for the reader:  Change last line in the above test method to:
   assert !simple.dirty

The test now will fail.  Fix the test so it passes by inserting this line of code (and only this line) somewhere in the test: 
   Simple.findAll()

Yeah, seriously, this is one more of 'these' problems as well.

Note on Flush Mode:  As discussed in the previous post, and in http://jira.grails.org/browse/GRAILS-11687: Grails 2.4.3 Flush Mode behavior is very intermittent. Line (3) in the above code snapshot fails with Grails 2.4.3 so, in this case, GORM decided to use FlushMode auto-like behavior.  Examples from the previous post had GORM behave in manual-like fashion with Grails 2.4.3.  The flush mode configuration in both examples is the same and if you decided to print the value on the current session it would print 'AUTO'.

Testing for such problems:  Concurrency problems are hard to test for.  I expect these to be handled for me by the framework as much as possible.  Complex concurrency issues maybe a consequence of business requirements that my code is trying to implement,  I can handle these.  But, concurrency issues should not be something the framework throws at me 'just because'.  As we have seen already (in post part 2) concurrency is not something Grails does well.  

This is simply one more example of a not testable problem.
Unit tests are a non-starter here, I may have some luck uncovering these with integration or functional tests if I know where to look.  But this is not very likely.

References:
http://jira.grails.org/browse/GRAILS-11701

Next Post:  My plan is to write one post about what it is like to work on a more complex project that uses Grails.   Something that goes beyond a simple CRUD application.  Instead of providing concrete code examples, I will simply describe some difficulties my team has encountered.   After that I want to come back to the discussion started in part 1: testing in Grails.

  

No comments:

Post a Comment