Affects: 5.3.x

Issue identified while upgrading from Spring 5.0.x to 5.3.x

public class CustomException extends RuntimeException {

   public CustomException(String message, Throwable th) {
       super(message, th);
   }
}
public interface UserDAO {
   void updateUser();
}
@Repository
public class UserDAOImpl implements UserDAO {

   // inject entity manager (hibernate provider underneath)

   @Override
   public void updateUser() {
       // update db throws some runtime exception (org.hibernate.HibernateException for example)
   }
}
public interface MyService {
   void makeUpdate();
}
@Service
public class MyServiceImpl implements MyService {

   @Autowired
   UserDAO userDAO;

   @Transactional(noRollbackFor = CustomException.class)
   public void makeUpdate() {
        try {
            userDAO.updateUser();
        } catch (Exception ex) {
            Throwable rootCause = org.apache.commons.lang3.exception.ExceptionUtils.getRootCause(ex);
            if (rootCause instanceof SQLException) {
                 // get sql error code (specific error codes used in db stored procedures)
                 if (errorCode == x) {
                     throw new CustomException(ex.getMessage(), ex);
                 }
            }
            throw ex;
        }
   }
}

The above setup works as expected in Spring 5.0.x where the transaction is not rolled back when CustomException is thrown. But on upgrading to 5.3.x, an exception thrown in UserDAO marks the transaction for rollback. This looks to be working in Spring 5.0.x because Spring does not track / parse class UserDAO and method updateUser for @Transactional. So there is NO TransactionInfo (within TransactionAspectSupport) created before invoking the updateUser method and hence no attempt to mark the transaction for rollback.

In 5.3.x, Spring is able to source TransactionAttribute for UserDAO because SpringTransactionAnnotationParser identifies it as a candidate for @Transactional and hence TransactionInfo is created before the method is invoked, and the transaction is marked for rollback when RuntimeException is thrown.

This would work if updateUser method in UserDAO is annotated with @Transactional(noRollbackFor...), but I would like to start transaction in the service layer.

Comment From: kiranshm973

created a sample project to demonstrate the issue https://github.com/ksm-973/spring-transaction-rollback-issue/tree/master/demo

Update springBootVersion in gradle.properties to 2.0.0.RELEASE (uses spring 5.0.4) and run UserServiceTest -> Test runs successfully Update springBootVersion in gradle.properties to 2.7.0 (uses spring 5.3.20) and run UserServiceTest -> Test fails

Comment From: sbrannen

Thanks for providing the demo project. We'll look into it.


As a side note, test classes and test methods have to be public with JUnit 4. Thus, anyone else trying out this project will need to add the missing public modifiers.

Comment From: sbrannen

FYI: if you set the Spring Boot version to 2.6.9 (in gradle.properties) and the Spring Framework version to 5.3.21 (in build.gradle as below), the test will still pass.

ext['spring-framework.version'] = '5.3.21'

Thus, it does not appear to be an issue with the core Spring Framework itself.

However, I have confirmed that the test fails as soon as you upgrade Spring Boot from 2.6.9 to 2.7.0.

Comment From: sbrannen

After further analysis, I have determined that the upgrade to H2 2.x is causing the issue.

Running the test with Spring Boot 2.7.0, Spring Framework 5.3.21 and H2 1.4.200 passes.

ext['spring-framework.version'] = '5.3.21'
ext['h2.version'] = '1.4.200'

Switching h2.version to 2.1.212 causes the test to fail.

Comment From: kiranshm973

Thanks for looking into this. Original issue was identified in a stand alone spring app which uses SQL Server database.

Spring: 5.3.20 JPA: 2.1 Hibernate: 5.2.17 sql jdbc driver: mssql-jdbc:8.2.0.jre8

Issue was possibly caused by the change for this https://github.com/spring-projects/spring-framework/issues/22420

Comment From: sbrannen

The issue with the demo application is not an issue with Spring Framework or Spring Boot.

Rather, the issue pertains to Hibernate's support for H2 2.0 and the fact that the test catches and swallows all exceptions and that the service catches all exceptions. The latter two issues mask the first issue.

If you inspect the actual exceptions and log, you will see that there are numerous SQL exceptions. The following is the initial cause.

Caused by: org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "create table [*]user (id integer generated by default as identity, name varchar(255), primary key (id))"; expected "identifier"; SQL statement:
create table user (id integer generated by default as identity, name varchar(255), primary key (id)) [42001-214]

To fix the problem, you need to declare the user table with enclosing backticks as follows

@Entity
@Table(name = "`user`")
public class User {
  // ...
}

After doing that, your test should pass with Spring Boot 2.7.0, Spring Framework 5.3.21, and H2 2.1.214.

In light of that, I am closing this issue as invalid.

Comment From: sbrannen

Thanks for looking into this. Original issue was identified in a stand alone spring app which uses SQL Server database.

Spring: 5.3.20 JPA: 2.1 Hibernate: 5.2.17 sql jdbc driver: mssql-jdbc:8.2.0.jre8

If you are able to provide us a sample application that reproduces the problem, feel free to share additional information in this issue and we will consider reopening it.

Thanks

Comment From: kiranshm973

Thank you!

I overlooked few details but I have now updated the demo project with the code setup where the tests work with 5.0.x but fails with 5.3.x.

https://github.com/kiranshm973/spring-transaction-rollback-issue/tree/master/demo

Issue seems to be happening when a @Transactional method is overridden. I agree the usage of Transactional annotation in the demo project is not optimal but wanted to point you to the behavior.

Comment From: sbrannen

I think the change in behavior might be a result of commit 42d6d7ec4e2ddabc9c50dab6d6ac572ef46d2a6a, due to the switch from getMergedAnnotationAttributes to findMergedAnnotationAttributes, but I have yet to verify that.

I just wanted to leave that note here in case somebody else investigates in depth before I do.

Comment From: snicoll

I've updated the sample to a supported version and I am not sure I understand it, nor how the referred commit would be related. The createUser calls invoke the DAO with a transaction that should not rollback on NoRollbackException. I can see that the transaction source is parsed and a RuleBasedTransactionAttribute is created with the correct value for the exception.

However when the exception in the DAO is thrown, the transaction source is the one from the DAO, not the one that started the transaction. At this stage, I'd need some help from @jhoeller as I am not sure whether that's legit and what to investigate next.

And just to be clear @kiranshm973, this has nothing to do with an upgrade to H2, right?

Comment From: snicoll

Closing due to the lack of requested feedback.