Grails pitfalls: Don't do flush:true when you actually want a transaction (and II)

Today somebody at work ask me about some statements I made in the previous entry "Grails pitfalls: Don't do flush:true when you actually want a transaction".

He thought that when doing

company.save(flush:true)

It didn't matter it was within or outside a transaction, the transaction will eventually commit the transaction event tough there were an exception on the way.

I didn't know that either until I worked in a project where I needed to return the number of persisted items just after persisting a new one but just before doing some critical calculation that could raise an exception and eventually rollback the entire transaction.

Think about it, if the domain wasn't persisted and "committed" (notice the quotes) I couldn't count the right number of persisted entities, but What if the subsequent calculation failed? I had already saved the domain class even tough it was incorrect to do so. It was a dead alley until I had a conversation with a senior engineer. More or less it was like the following:

Mario: Ciaran I can't save the instance and see it in the following line
Ciaran: Then flush:true the save statement
Mario: What? But save(flush:true) commits the statement.
Ciaran: Nop, it makes the instructions visible for the rest of the transaction
Mario: Ummm, I don't quite follow you but let's test it anyway.

That was the most difficult task, How to test that assertion?

Let's say we have the following service method:

    def saveAndFail(Company company) {
      company.user = springSecurityService.currentUser
      company.save(flush:true)
      throw new IllegalArgumentException("Ahhhh")
    }

I did the following test:

   package whatever

   import static org.junit.Assert.assertThat
   import static org.hamcrest.CoreMatchers.notNullValue
   import static org.hamcrest.CoreMatchers.is
     
   import grails.plugin.spock.IntegrationSpec
   import org.codehaus.groovy.grails.plugins.springsecurity.SpringSecurityUtils
         
  class CompanyServiceFlushIntegrationSpec extends IntegrationSpec{
       
    def companyService
         
    def "Test flush:true"(){ 
      setup: "Building a valid company instance"
        def user = validUser.save(failOnError:true,flush:true)
        def company = new Company(
          user: user,
          name: "name",
          companyCode : "companyCode"
        )
      when:"Saving a valid company instance"
          def savedCompany = SpringSecurityUtils.doWithAuth("username"){
              companyService.saveAndFail(company)
          }
      then: "The result should be the saved company"
        thrown(Exception)
        assertThat(Company.count(),is(0))
    }
        
    def getValidUser(){
      def validUser = new ApplicationUser(
        name: "name",
        username: "username",
        password: "password",
        locale: Locale.getDefault()
      ) 
    }       
        
 }

But it didn't work. I got:

Failure:  Test flush:true(whatever.CompanyServiceFlushIntegrationSpec)
|  java.lang.AssertionError: 
Expected: is <0>
     got: <1>

Let's analyze how the test worked. First, we should know that by default integration tests are transactional by default. That means that at the end of every test method if there is any transaction it will be rolled back....AT THE END. In our method we threw an exception "catched" by our thrown(Exception) statement. So there was no rollback there, so it makes sense that at line number 28 the assertion fails because the exception has not been propagated outside the test method, so that the count statement is aware of the persisted company because of the flush:true statement.

But I want to prove that the transaction actually does a rollback because of the exception we put on purpose. We can only do that establishing our own transaction scope. To do this is neccesary to declare the test as "not transactional", and then to wrap the service's invocation as it will happen at runtime.

   package whatever

   import static org.junit.Assert.assertThat
   import static org.hamcrest.CoreMatchers.notNullValue
   import static org.hamcrest.CoreMatchers.is
     
   import grails.plugin.spock.IntegrationSpec
   import org.codehaus.groovy.grails.plugins.springsecurity.SpringSecurityUtils
         
  class CompanyServiceFlushIntegrationSpec extends IntegrationSpec{
       
    static transactional = false

    def companyService
         
    def "Test flush:true"(){ 
      setup: "Building a valid company instance"
        def user = validUser.save(failOnError:true,flush:true)
        def company = new Company(
          user: user,
          name: "name",
          companyCode : "companyCode"
        )
      when:"Saving a valid company instance"
          Company.withTransaction{
            def savedCompany = SpringSecurityUtils.doWithAuth("username"){
              companyService.saveAndFail(company)
            }
          }
      then: "The result should be the saved company"
        thrown(Exception)
        assertThat(Company.count(),is(0))
    }
        
    def getValidUser(){
      def validUser = new ApplicationUser(
        name: "name",
        username: "username",
        password: "password",
        locale: Locale.getDefault()
      ) 
    }       
        
 }

And voila!

| Tests PASSED - view reports in...

I had no companies saved even though I used flush:true.

Clojure & Groovy: Exposing classes and methods

I'm trying to learn a new language: Clojure. It's really different than anything I've faced before...and I like it. But eventually I'd like to use it in some other apps coded in some other JVM languages such as Groovy ;)

In order to do that I'm using a Gradle project having different modules written in different JVM languages (Java, Scala, Clojure), and a Groovy module with Spock specs to test them all.

So I came up with the following Clojure file:

 (ns polyglot.clojure.sample
     (:gen-class              
      :name polyglot.clojure.sample.ClojureUtils
      :methods [
         [lowerit [String] String]
         #^{:static true} [upperit [String] String] 
       ]                      
     )
     (:require [clojure.string :as st])
  )   
        
  (defn -lowerit [this message]
    (st/lower-case message))                                                                                                                                
  
  (defn -upperit [message]   
    (st/upper-case message))

And then I was able to test it with the following Spock specification:

package polyglot
       
   import polyglot.clojure.sample.ClojureUtils
   import spock.lang.Specification
   
   class UseClojureGromGroovySpec extends Specification{
   
     def "Getting the message using cars"(){
       setup: "Creating a car"
        def car = new Car(brand:"Seat",model:"Leon")
        def util = new ClojureUtils()   
      when: "Initializing Clojure"
        def instanceMessage = util.lowerit(car.brand)
        def staticMessage = ClojureUtils.upperit(car.brand) 
      then: "The message should be like the following"
        instanceMessage == "seat"                                                                                                                           
        staticMessage == "SEAT"
    }
  }

What I have learnt so far is:

•  File location:

(ns polyglot.clojure.sample

Points out to the clojure file. In this sample the file was /polyglot/clojure/sample.clj

• Class name:

:name polyglot.clojure.sample.ClojureUtils

To be able to tell Clojure which Class to create, you have to specify the whole path, the "qualified name" so to speak. It's a little bit annoying to repeat the package when it could have been guessed from the namespace (ns attribute). But maybe I'm wrong and it's just that I don't know how to do it yet. Following the docs:

"The package-qualified name of the class to be generated"

• Specifying instance methods:

:methods [
         [lowerit [String] String]
       ] 

This line exposes the method lowerit, method having an String as parameter. It should return an String as well. The method is implemented as:

(defn -lowerit [this message]
    (st/lower-case message))      

Notice that instance and static methods, both are implemented with an "-" symbol. Following the Clojure documentation :

"...Given a generated class org.mydomain.MyClass with a method named mymethod, gen-class will generate an implementation that looks for a function named by (str prefix mymethod) (default prefix: "-")..."

• Specifying static methods:

:methods [         
         #^{:static true} [upperit [String] String] 
       ] 

The way of exposing static methods has a different syntax in the methods: block, but same way of implementing the method.

  (defn -upperit [message]   
    (st/upper-case message))

• Importing third party libraries:

Because I first started using Clojure with the REPL I forgot some assumptions, such as REPL imports some namespaces by default. So I first try to implement lowerit as:

(defn -lowerit [this message]
    (clojure.string/lower-case message))  

That failed because the compiler could find the String type and even less the lower-case method. So after some search I found out how to do it. Before trying to use the String class I had to import it in the required: block (You can give the library an alias in order to use it through your implementation).

(:require [clojure.string :as st])

References:

• Polyglot Gradle Project (Github): I built it to know how to mix different JVM languages.
• Clojuredocs: A good Clojure reference site

Grails pitfalls: Controllers and how to reuse code

There're a lot of common repetitive tasks that arises when coding controllers, services...etc. I've seen many times repeating code that has to do with pagination.

def list() {
    params.max = params.max ?: grailsApplication.config.list.default.maxElements
  
    def result = GeographicArea.findAll(params)        
    render view: 'list', model: [list:result]
}

It's really painful to repeat the code to establish default maximum or minimum number of results every time we have to deal with some pagination. It's easier to create a method maybe in some parent class, to be able to call it from those classes inheriting that class. We can use inheritance, mixins, AST...etc.

The pros of using inheritance is you have the behavior because of compilation (AST would be the same here) while doing Mixins would have a performance penalty.

Many times my favorite choice is to inherit a BaseController where I include most of the boilerplate code.

class BaseController{
  
 def configurationService

    def getPaginationParams(){
      if (!params.max){
        params.max = configurationService.defaulMaximumListElements()

        //params.min...etc
      }
      params
    }
  
  }

And then I make the child class to extend the BaseController class

class GeographicAreaController extends BaseController{  
    def list() {    
       def result = GeographicArea.findAll(paginationParams)        
       render view: 'list', model: [list:result]
      }   
}

Grails pitfalls: Inheriting a parent class doesn't mean a new table

Sometimes we need to audit our domain classes. And sometimes you open a couple of classes and realize auditing fields have been copied in every class:

  class Product{
     Date dateCreated
     Date lastUpdated
  }

  class Category{
     Date dateCreated
     Date lastUpdated
  }

As you may imagine, this could improve a lot. But most of newbies don't want to create a new class and make the previous classes inherit from it because they think they will be creating a new table.

But... Did you know you can inherit from a class without creating a new table?

The solution is really simple, the parent class should be in src/groovy and it should be an abstract class. Then you can extend that class. The child class will have all the parent's fields but without the penalty of having a new table.

  // src/main/groovy/myapp/BaseEntity.groovy
  abstract class BaseEntity{
     Date dateCreated
     Date lastUpdated
  }

  // grails-app/domain/myapp/Product.groovy
  class Product extends BaseEntity { }

  // grails-app/domain/myapp/Category.groovy
  class Category extends BaseEntity{ }

Grails pitfalls: Don't do flush:true when you actually want a transaction

Putting persistence code in controllers leads to poor maintenance, reduces the chance of reusing code, and makes controllers extremely hard to test.

One of the first problems a junior programmer experience is to realize the domain class hasn't been saved when he/she expected.

After explaining to them the best way of approaching this type of situations is to put the persistence code in the service layer, they ignore you because they find they can force the commit doing:

  product.save(flush:true)

For what else would be this parameter for? (Sarcasm ;)) If you were only saving a given instance without any relationship...still I wouldn't do that. At least forget about the use of flush:true in that context and use withTransaction:

  Product.withTransaction{
    product.save()
  }

I can see a transaction going on here. Let's say we have two given transactions trying to do something at once.

  Product.withTransaction{
    product.status = ACTIVE
    product.save(flush:true) // without this line the total number will be all of them but this one
    Product.countByByStatus(ACTIVE)
  }

In the previous code without forcing flush:true we would have been omitting the product we were saving in our transaction. First time you see this code you could be thinking, OMG they are committing before the end of the transaction.

Sort answer would be: we ARE NOT committing anything yet, we are sending to the database all instructions until this point (flushing) allowing other transactions in their way to see the our transaction status. If something goes wrong before the transaction ends then changes won't persist.

The problem is that when there's no transaction boundaries the default behavior of flush:true is to commit changes. But if you do establish the boundaries, we would expect the commit/rollback to happen once we exit the transaction's scope.

UPDATE: Somebody at work asked me how to prove this statement. I've created an entry to explain this a little bit more.

Using flush:true the way people normally use it is not right, but is even funnier when they realize they can't do

product.delete(flush:true)

All these could have been avoided if people were used to put their persistence code in services. There are a couple of benefits when using services:

• You no longer have to worry about opening and closing a transaction. Convention over configuration.

A simplistic view of transactions in Grail's services would be that by default the code within a service's method is executed within a transaction. If the method throws an exception then that code will be rolled back. If everything went ok then all changes will be commited. Although you could but now you don't have to declare explicitly the boundaries of the transaction.

class ProductService{

   def addProductToCart(Product p, Cart c){ // transaction starts here

      //... code within a transaction

   } // transaction ends here (if everything went ok ;) )

}

EXPLANATORY UPDATE:

To be accurate the transaction begins not at the beginning of the method but at the service's invocation time. What does it means? It means that when you're invoking a service's method let's say from the controller, a Spring interceptor, intercepts the call and wraps that call inside a given transaction scope.

One more thing to keep in mind is that the default propagation of transactions is REQUIRED, that means that if we're calling a transactional service's method from another transactional method the latter uses the former transaction scope, or in cause there were none it creates a new one. That explains why we can call some other service methods from our service method without committing previous statements, because there were a pre-existent transaction.

You can always change the default propagation policy using the @Transactional annotation from Spring in your service methods, or using Spring AOP advices to configure transactional scopes.

Anyway if you want to dive into this topic please read "Spring Declarative Transaction Management".

• It keeps separated your view logic (controllers) from your persistence code (services)

It's clear that less code is always easier to review. Hence if you could keep separated view logic from business logic everything starts looking clearer.  Let's see an example:

If you have the following code:

class ContractController{

  def saveContract(Contract contract,Address address){

    if(contract.validate()){       

       contract.deliveryAddress = address.save(flush:true)   
       contract.save(flush:true,failOnError:true)  

       render(view:'ok')    

    } else {

       render(view:'ko')

    }

  }

}

If you had to unit test this code you should have to mock every domain class behavior (Contract and Address), and sometimes that's really a pain in the ass. Besides the fact that the address could have been saved without succeeding on doing the same with the contract instance. That type of inconsistencies are really dangerous.

Wouldn't be better to split the code like this:

class ContractController{

  def contractService 

  def saveContract(Contract contract,Address address){

    if(c.validate()){   

       contractService.saveContract(contract,address)
       render(view:'ok')    

    } else {

       render(view:'ko')

    }

  }

}

class ContractService{

   def saveContract(Contract c,Address address){

       c.deliveryAddress = address.save()
       c.save(failOnError:true) 

   }

}

Now is easier to mock the service's method, and you only have to focus on testing the view logic.

Grails Tip: Different URL depending on user's role

"We need to redirect users depending on their roles". OMG how I was supposed to do this? Well after doing some search on the usual site, I found this:

http://omarello.com/2011/09/grails-custom-target-urls-after-login/

My solution is based on the previous blog entry. The only thing I've added is using Config.groovy to register different ROLE/URL cases. I thought it worthed sharing it.

Let's say we want to redirect users with role ROLE_MANAGER  to /users/search and users with role ROLE_PROVIDER to /invoices/search. So I came up with the following lines in my Config.groovy

authenticationurl{
 mappings{
  ROLE_MANAGER.controller='users'
  ROLE_MANAGER.action='search'
  ROLE_PROVIDER.controller='invoices'
  ROLE_PROVIDER.action='search'
 }
}

The nice thing about configuration files in Grails is that you can build maps just sharing the same root, in other words, I can ask which controller and action should be used for a manager accessing to the key ROLE_MANAGER:

def map = authenticationurl.mappings.ROLE_MANAGER
def controller = map.controller // 'users'
def action = map.action // 'search'

Taking into account that premise, I built my own AuthenticationHandler. This handler takes the authenticated user's role and use it as key to look for its correspondent controller/action:

import org.codehaus.groovy.grails.plugins.springsecurity.AjaxAwareAuthenticationSuccessHandler
import org.springframework.security.core.Authentication

import org.apache.commons.logging.LogFactory

import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse

import static org.codehaus.groovy.grails.plugins.springsecurity.SpringSecurityUtils.ifAllGranted


class AuthenticationHandler extends AjaxAwareAuthenticationSuccessHandler {

 static log = LogFactory.getLog(AuthenticationHandler)

 def grailsLinkGenerator 
 def configurationService
 def springSecurityService

 @Override
 protected String determineTargetUrl(HttpServletRequest request,HttpServletResponse response) {
  /* Taking configuration mappings */ 
  def authMappings = configurationService.authenticationHandlerUrlMappings
  def defaultUrl = super.determineTargetUrl(request, response)
  if(log.isDebugEnabled()){
   log.debug("Default URL: $defaultUrl")
  }
  /* Taking the first match or the targetUrl  */
  return authMappings.findResult{k,v->
   if(ifAllGranted(k)){
    /* Here's better to get it absolute */
    grailsLinkGenerator.link(v << [absolute:true])
   }
  } ?: defaultUrl 
 }

}

The line with the authMapping variable assignment is doing grailsApplication.config. authenticationurl.mappings underneath. The value is itself a map of maps.

The method determineTargetUrl loops through the map looking for the first result matching the user's role. If there's an entry the user will be redirected to the configured URL otherwise the default URL will be used (normally it is the same URL where the user was when he tried to authenticate himself).

Notice that I'm using the service grailsLinkGenerator to generate the proper link passing the controller and action arguments to it. This works the same as if we were using the tag in any gsp. This service is available by default in Grails.

Ah of course don't forget to register your authentication handler in the resources.groovy file

authenticationSuccessHandler(AuthenticationHandler) {
        /* Reusing the security configuration */
        def conf = SpringSecurityUtils.securityConfig
        /* Configuring the bean */
        requestCache = ref('requestCache')
        redirectStrategy = ref('redirectStrategy')
        springSecurityService = ref('springSecurityService')
  grailsLinkGenerator = ref('grailsLinkGenerator')
  configurationService = ref('configurationService')
        defaultTargetUrl = conf.successHandler.defaultTargetUrl
        alwaysUseDefaultTargetUrl = conf.successHandler.alwaysUseDefault
        targetUrlParameter = conf.successHandler.targetUrlParameter
        ajaxSuccessUrl = conf.successHandler.ajaxSuccessUrl
        useReferer = conf.successHandler.useReferer
    }

Some ideas about processing a text file with Groovy

I really hated Java when reading a file. At least when using BufferedReader you could always abstract reading every line with readLine(), and now with JDK7 you can even forget about taking care of closing the stream (related article at DZone), great!! But...Can Groovy do better? I hope so :P

A couple of days ago I was looking into Groovy's java.util.File JDK documentation trying to find out the easiest way to iterate through a text file collecting information on the way.

I started doing my research with the following data:

JANUARY     100.23
FEBRUARY    23.34
MARCH       45.56
APRIL       67
MAY         78.2
JUNE        23.3
JULY        92.2
AUGUST      802.2
SEPTEMBER   87.3
OCTOBER     2.2
NOVEMBER    3.2
DECEMBER    150.4

The research

Firstly I started looping through every line in the file using the Java approach:

def file2Process = new File("/pathtofile/file.txt")
def acc = 0
def javaReader = file2Process.newReader()
while((next = javaReader.readLine()) != null){     
    acc += next.split(/\s{1,}/)?.getAt(1)?.toDouble()?:0
}

javaReader.close()
println "Java (1)->$acc"

Not bad, BufferedReader is a nice abstraction but the while statement has too many things, a declaration, assignation, and a condition altogether.

Afterwards, I don't know where it came from, I tried to iterate the reader returned from newReader() method (added by Groovy to the File class). As you may guess It looked nicer:

def reader = file2Process.newReader()
def perLine = {line-> line.split(/\s{1,}/)?.getAt(1)?.toDouble()?:0 }

def totalAmount = reader.collect(perLine).sum()

reader.close()

println "Groovy (1)->$totalAmount"

So I guess all readers are iterable, Aren't they? I don't remember they were in Java, or maybe I just forgot it. But still I had to close the reader explicitly. Common! JDK7 already do that, you can do it better! Don't you?

Yes, and it was my fault, I didn't notice about the withXXX methods added to the java.util.File class in Groovy. These methods receive a closure as parameter. Inside that closure you can use the underlying reader or stream and at the end of the closure the method takes care about closing the reader/stream.

def perLine = {line-> line.split(/\s{1,}/)?.getAt(1)?.toDouble()?:0 }
def result = file2Process.withReader{r->
    r.collect(perLine).sum()
}

println "Groovy (2)->$result"

The good thing about this is that you can still use it with JDK6 (However I encourage you to move to JDK7).

The "real" mission

The task that I needed to do was to process all lines of a text file gathering different chunks of information.

Also motivated by the rant around "functional vs OOP" I wanted to process the file in a "functional" way, which means not to use temporary variables outside the scope of the closure (aka no side effects).

That's why I found really impressive the idea that readers could be iterable. If readers were iterable I would be able to use collection methods like "inject" to populate a map with different types of information collected along the file.

Of course the file I'm showing here has little to do with the real one, with thousands of lines (and dozens of fields) coming in from a legacy Cobol system, but it works for the shake of the explanation.

So lets say I wanted to return a map with different values

• Total number of lines
• Total amount
• Total amount by quarter

I came up with this solution:

def firstQ = ['JAN','FEB','MAR','APRIL']
def secondQ = ['MAY','JUN','JUL','AUG']
def thirdQ = ['SEP','OCT','NOV','DEC']

def data = file2Process.withReader{r->
    r.inject([q1:0,q2:0,q3:0,lines:0,total:0]){map,val->
        def lineInfo = val.split(/\s{1,}/)
        def month = lineInfo?.getAt(0)?.take(3)
        def amount = lineInfo?.getAt(1)?.toBigDecimal()
        switch(month){
            case firstQ:
               map.q1 += amount
            break
            case secondQ:
               map.q2 += amount
            break
            case thirdQ:
               map.q3 += amount
            break
        }
        map.total += amount
        map.lines++
     /* Don't forget to return the map */
        map
    }
}

Which returns the map:

[q1:169.13, q2:995.9, q3:243.1, lines:12, total:1475.13]

One complaint

• Because in the real file I had to do some validations that involved the use of financial algorithms, I missed Gpars for that. But I wasn't able to make Gpars' foldParallel(..) method to work the same way as the inject(...) method does. I've sent the question to the mailing list and I'll update the entry as soon as I get an answer.

Resources

• Groovy Input Output: http://groovy.codehaus.org/Input+Output
• Groovy Streams: http://groovy.codehaus.org/JN2025-Streams

Take a look at how to transform a given file's lines with transformLine(...) method, it sprang to mind that maybe if I had to pre-process a given file I'd require that type of behavior sometime.