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.