8

I created a service method that creates user accounts. If creation fails because the given e-mail-address is already in our database, I want to send the user an e-mail saying they are already registered:

@Transactional(noRollbackFor=DuplicateEmailException.class)
void registerUser(User user) {
   try {
      userRepository.create(user);
   catch(DuplicateEmailException e) {
      User registeredUser = userRepository.findByEmail(user.getEmail());
      mailService.sendAlreadyRegisteredEmail(registeredUser);
   }
}

This does not work. Although I marked the DuplicateEmailExcepetion as "no rollback", the second SQL query (findByEmail) still fails because the transaction was aborted.

What am I doing wrong?

There is no @Transactional annotation on the repository.

Bastian Voigt
  • 5,311
  • 6
  • 47
  • 65
  • 1
    If you try the sql part which is in your catch block in your try itself, like checking for the existence of a user already before creating, that might help! – robot_alien May 09 '17 at 08:18
  • 1
    When a runtime exception happens (which is what the data integrity violation is) the transaction will be rolled back (actually it will be marked for rollback). I suspect that your repository `UserRepository` is marked with `@Transactional` as well as your service (which would lead to this behavior due to another transactional interceptor being in the call chain). – M. Deinum May 09 '17 at 08:29
  • No, there's no `@Transactional` annotation in the repository. I tried but it did not change the behaviour – Bastian Voigt May 09 '17 at 08:31
  • I just checked the stacktrace. There is no `TransactionInterceptor` between the service and the repository. Only between the controller and the service there is one – Bastian Voigt May 09 '17 at 08:34
  • Can you give the detail about `userRepository.create(user);` method? And did you check that the `DuplicateEmailException ` is thrown by `userRepository.create(user)` method? – Zico May 15 '17 at 09:31
  • `userRepository.create()` fires an `INSERT` statement via `JdbcTemplate`, catches the `DuplicateKeyException` and re-throws it as `DuplicateEmailException` if the duplicate key is the e-mail. – Bastian Voigt May 15 '17 at 10:46
  • Are you using Postgres ? – Babl May 15 '17 at 11:09
  • Yes, Postgres with `PGPoolingDataSource`. How did you know? ;) – Bastian Voigt May 15 '17 at 11:11

4 Answers4

8

That's not a problem with Spring / JDBC or your code, the problem is with the underlying database. For example, when you are using the Postgres if any statement fails in a transaction all the subsequent statements will fail with current transaction is aborted.

For example executing the following statements on your Postgres:

> start a transaction
> DROP SEQUENCE BLA_BLA_BLA;
> Error while executing the query; ERROR: sequence "BLA_BLA_BLA" does not exist"
> SELECT * FROM USERS;
> ERROR: current transaction is aborted, commands ignored until end of transaction block

Still the SELECT and subsequent statements are expected to succeed against MySQL, Oracle and SQL Server

Babl
  • 7,446
  • 26
  • 37
  • Thanks, you made my day. +100 – Bastian Voigt May 15 '17 at 11:42
  • So for solving this problem in postgres just execute rollback command . For more details visit https://laurenthinoul.com/how-to-fix-postgres-error-current-transaction-is-aborted-commands-ignored-until-end-of-transaction-block/ – Ali mohammadi Oct 27 '17 at 19:19
  • OP wants to know how to fix this problem yet you are telling him why this error is happening which is not very helpful. – raven Jul 06 '19 at 17:07
  • @raven when you know where is the problem fixing it is just matter of implementation, there are 100 ways to fix the issue OP had, he got the answer why so how to fix it is just implementation details which I'm not going to share. – Babl Jul 12 '19 at 12:43
2

Why don't you change the logic as following :

void registerUser(User user) {
   User existingUser = userRepository.findByEmail(user.getEmail())
   if(existingUser == null){
         userRepository.create(user);
   }else{
       mailService.sendAlreadyRegisteredEmail(existingUser)
   }
}

This would ensure that only non-existing users to be inserted into the database.

Thina Garan
  • 106
  • 5
  • Yeah that's possible of course. I was just interested in finding out why spring-boot cancels the transaction although I told it not to – Bastian Voigt May 09 '17 at 08:25
  • 1
    The NnoRollbackFor defined (https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/transaction/annotation/Transactional.html ) Defines zero (0) or more exception Classes, which must be subclasses of Throwable, indicating which exception types must not cause a transaction rollback.. does your DuplicateEmailException class follow these rules? – Thina Garan May 09 '17 at 09:03
  • DuplicateEmailException extends RuntimeException, which is a subclass of Exception, which is a subclass of Throwable. So yes I think I follow these rules – Bastian Voigt May 09 '17 at 09:41
  • I think it is well explained in the following link : http://stackoverflow.com/questions/27849968/transactional-norollbackfor-runtimeexception-class-does-not-prevent-rollback – Thina Garan May 09 '17 at 09:49
  • Well, it's similar but not quite the same. He has two nested `@Transactional` annotations where I have only one. Also I'm not using Hibernate but JdbcTemplate – Bastian Voigt May 09 '17 at 10:05
  • The problem with this approach is that you are executing two commands instead of one which might impact performance; yet there seems to be no better solutions so far. – raven Jul 06 '19 at 17:10
0

@Transactional annotation is placed at incorrectly. Spring creates a AOP advisor around the method where @Transactional annotation is defined. So, in this case, pointcut will be created around registedUser method. But, registerUser method doesn't throw DuplicateEmailException. Hence, no rollback rules are evaluated.

You need to define the @Transactional rule around UserRepository.createUser method. This will ensure that Transaction pointcut created by spring doesn't rollback because of DuplicateEmailException.

public class UserRepository {

    @Transactional(noRollbackFor=DuplicateEmailException.class)
    public User createUser(){
        //if user exist, throw DuplicateEmailException
    }
}

void registerUser(User user) {
    try {
       userRepository.create(user);
    catch(DuplicateEmailException e) {
       User registeredUser = userRepository.findByEmail(user.getEmail());
       mailService.sendAlreadyRegisteredEmail(registeredUser);
    }
}
Mohit
  • 1,740
  • 1
  • 15
  • 19
  • No, it's placed perfectly. I want the registration of a new user to happen as one transaction. Otherwise, when the code gets more complex, I might end up with parts of the registration process being persisted and other parts not. – Bastian Voigt May 15 '17 at 11:29
  • Spring transaction ensure that if a transaction is already in place (with propogation as REQUIRED), use the existing one. This allows developers to put @Transactional annotation on multiple methods in same execution stack without any side effect. – Mohit May 15 '17 at 11:34
  • Yes, but when the transaction is marked for rollback this applies to both the "inner" and the "outer" transaction, so this wouldn't help – Bastian Voigt May 15 '17 at 11:41
  • If you put a rollback rule on inner transaction, then it will work fine because spring won't rollback your transaction for userRepository.create(user). Transaction won't be marked for rollback. Hence, you should see everything going as a single commit. – Mohit May 15 '17 at 11:46
-2

You could wrap the call to the userRepository in a try catch block. Or you could look first if the user exists, and abourt the creation of a new one.

Stimpson Cat
  • 1,444
  • 19
  • 44