Grails 2.4.4: How to reliably rollback in a complex service method

Question

Consider the following service (transactional by default). A player must always have one account. A player without at least one corresponding account is an error state.

class playerService {
   def createPlayer() {
      Player p new Player(name: "Stephen King")
      if (!p.save()) {
         return [code: -1, errors:p.errors]
      }
      Account a = new Account(type: "cash")
      if (!a.save()) {
         // rollback p !
         return [code: -2, errors:a.errors]
      }
      // commit only now!
      return [code: 0, player:p] 
   }
 }

I have seen this pattern by experienced grails developers, and when I tell them that if creation of the account of the player fails for any reason, it wont rollback the player, and will leave the DB in an invalid state, they look at me like I am mad because grails handles rolling back the player because services are transactional right?

So then, being a SQL guy, I look for a way to call rollback in grails. There isn't one. According to various posts, there are only 2 ways to force grails to rollback in a service:

1. throw an unchecked exception. You know what this is right?
2. don't use service methods or transactional annotations, use this construct:

.

DomainObject.withTransaction {status ->
     //stuff
     if (someError) {
        status.setRollbackOnly()
     }
}

1. throw an unchecked exception

---

1.1 So we must throw runtime exceptions to rollback. This is ok for me (I like exceptions), but this wont gel with the grails developers we have who view exceptions as a throwback to Java and is uncool. It also means we have to change the whole way the app currently uses its service layer.

1.2 If an exception is thrown, you lose the p.errors - you lose the validation detail.

1.3 Our new grails devs don't know the difference between an unchecked and an checked exception, and don't know how to tell the difference. This is really dangerous.

1.4. use .save(failOnError: true) I am a big fan of using this, but its not appropriate everywhere. Sometimes you need to check the reason before going further, not throw an exception. Are the exceptions it can generate always checked, always unchecked, or either? I.e. will failOnError AWLAYS rollback, no matter what the cause? No one I have asked knows the answer to this, which is disturbing, they are using blind faith to avoid corrupted/inconsistent DBs.

1.5 What happens if a controller calls service A, which calls Service B, then service C. Service A must catch any exception and return a nicely formatted return value to the controller. If Service C throws an exception, which is caught by Service A, will service Bs transactions be rolled back? This is critical to know to be able to construct a working application.

UPDATE 1: Having done some tests, it appears that any runtime exception, even if thrown and caught in some unrelated child calls, will cause everything in the parent to rollback. However, it is not easy to know in the parent session that this rollback has happened - you need to make sure that if you catch any exception, you either rethrow, or pass some notice back to the caller to show that it has failed in such a way that everything else will be rolled back.

2. withTransaction

---

2.1 This seems a bazaar construct. How do I call this, and what do I pass in for the "status" parameter? What is "setRollbackOnly" exactly. Why is it not just called "rollback". What is the "Only" part? It is tied to a domain object, when your method may want to do update several different domain objects.

2.2 Where are you supposed to put this code? In with the DomainObject class? In the source folder (i.e. not in a service or controller?)? Directly in the controller? (we don't want to duplicate business logic in the controllers)

3. Ideal situation.

---

3.1 The general case is we want every thing we do in a service method to roll back if anything in that service method cant be saved for any reason, or throws any exception for any reason (checked or unchecked).

3.2 Ideally I would like service methods to "always rollback, unless I explicitly call commit", which is the safest strategy , but this is not possible I believe.

The question is how do I achieve the ideal situation?

Will calling save(failOnError:true) ALWAYS rollback everything, no matter what the reason for failing? This is not perfect, as it is not easy for the caller to know which domain object save caused the issue.

Or do people define lots of exception classes which subclass runtimeException, then explicit catch each of them in the controller to create the appropriate response? This is the old Java way, and our groovy devs pooh pooh this approach due to the amount of boiler plate code we will have to write.

What methods do people use to achieve this?

Answer 1

I wouldn't call myself an expert, and this question is over a year old, but I can answer some of these questions, if only for future searchers. I'm just now refactoring some controllers to use services in order to take advantage of transactions.

I have seen this pattern by experienced grails developers, and when I tell them that if creation of the account of the player fails for any reason, it wont rollback the player, and will leave the DB in an invalid state, they look at me like I am mad because grails handles rolling back the player because services are transactional right?

I'm not seeing in the documentation where it explicitly states that returning from a service method does not rollback the transaction, but I can't imagine that this would be a very sane behavior. Still, testing is an easy way to prove yourself.

1.2 If an exception is thrown, you lose the p.errors - you lose the validation detail.

Since you're the one throwing the exception, you can throw the errors along with it. For instance:

// in service
    if (!email.save()) {
        throw new ValidationException("Couldn't save email ${params.id}", email.errors)
    }

When you catch the exception, you reload the instance (because throwing an exception clears the session), put the errors back into the instance, and then pass that to the view as usual:

// in controller
    } catch (ValidationException e) {
        def email = Email.read(id)
        email.errors = e.errors
        render view: "edit", model: [emailInstance: email]
    }

This is discussed under the heading "Validation Errors and Rollback", down the page from http://grails.github.io/grails-doc/2.4.4/guide/single.html#transactionsRollbackAndTheSession.

1.4. use .save(failOnError: true) I am a big fan of using this, but its not appropriate everywhere. Sometimes you need to check the reason before going further, not throw an exception. Are the exceptions it can generate always checked, always unchecked, or either? I.e. will failOnError AWLAYS rollback, no matter what the cause? No one I have asked knows the answer to this, which is disturbing, they are using blind faith to avoid corrupted/inconsistent DBs.

failOnError will cause save() to throw a ValidationException, so yes, if you're in a transaction and aren't checking that exception, the transaction will be rolled back.

Generally speaking, it seems to be un-"Grailsy" to use failOnError a lot, probably for the reasons you listed (e.g., lack of control). Instead, you check whether save() failed (if (!save()) ...), and take action based on that.

2. withTransaction

I'm not sure the point of this, because SpringSource really encourages the use of services for everything. I personally don't like it, either.

If you want to make a particular service non-transactional, and then make one method of it transactional, you can just annotate that one method with @Transactional (unless your developers also dislike annotations because they're too "Java" ;) ).

Note! As soon as you mark a single method with @Transactional, the overall service will become non-transactional.

3.1 The general case is we want every thing we do in a service method to roll back if anything in that service method cant be saved for any reason, or throws any exception for any reason (checked or unchecked).

I feel like checked exceptions are generally considered not "Groovy" (which also makes them not Grails-y). Not sure about the reason for that.

However, it looks like you can tell your service to rollback on your checked exceptions, by listing them in the rollbackFor option to @Transactional.

Or do people define lots of exception classes which subclass runtimeException, then explicit catch each of them in the controller to create the appropriate response? This is the old Java way, and our groovy devs pooh pooh this approach due to the amount of boiler plate code we will have to write.

The nice thing about Groovy is that you can write your boiler plate once and then call it repeatedly. A pattern I've seen a lot, and am currently using, is something like this:

private void validate(Long id, Closure closure) {
    try {
        closure()
    } catch (ValidationException e) {
        def email = Email.read(id)
        email.errors = e.errors
        render view: "edit", model: [emailInstance: email]
    } catch (OtherException e) {
        def email = Email.read(id)
        flash.error = "${e.message}: ${e.reasons}"
        render view: "show", model: [emailInstance: email]
    } catch (Throwable t) {
        flash.error = "Unexpected error $t: ${t.message}"
        redirect action: "list"
    }
}

And then call it in each controller action like so:

def update(Long id, Long version) {
    withInstance(id, version) { Email emailInstance ->
        validate(emailInstance.id) {
            emailService.update(emailInstance, params)
            flash.message = "Email $id updated at ${new Date()}."
            redirect action: "show", id: emailInstance.id
        }
    }
}

(withInstance is another similar method that DRYs up the check for existence and optimistic locking.)

This approach has downsides. You get the same set of redirects in every action; you probably want to write one set of methods for each controller; and it seems kind of silly to pass a closure into a method and expect the method to know what exceptions the closure will throw. But hey, programming's all about tradeoffs, right?

Anyway, hope that is at least interesting.