0

One @Transactional method calling to another 2 methods which are also present in @Transactional method but while one of the called method getting exception it the transaction should be rolled back , its not happening

-----The Main Transactional method-------------
@Transactional(propagation = Propagation.REQUIRES_NEW,rollbackFor = RestException.class)
    public BaseDto createFPSAndUser(FpsStoreDto fpsStoreDto){

        log.info("<--Starts FPSStoreService .createFPSAndUser-->"+fpsStoreDto);
        BaseDto baseDto = new BaseDto();
        try {
            UserDetailDto  userDetailDto = fpsStoreDto.getUserDetailDto();
            userDetailDto.setCreatedBy(fpsStoreDto.getCreatedBy());
            baseDto = createFPSStore(fpsStoreDto);
            if(baseDto.getStatusCode() != 0){
                throw new RestException(ErrorCodeDescription.getDescription(baseDto.getStatusCode()));

            }
            userDetailDto.setFpsStore(null);
            baseDto = userDetailService.createUserDetail(userDetailDto);
            if(baseDto.getStatusCode() != 0){
                throw new RestException(ErrorCodeDescription.getDescription(baseDto.getStatusCode()));
            }
            FPSStore fpsStore =  fpsStoreRepository.findByCode(fpsStoreDto.getCode());
            UserDetail userDetail = userDetailRepository.findByUserId(userDetailDto.getUserId());
            userDetail.setFpsStore(fpsStore);
            userDetailRepository.save(userDetail);
            baseDto.setStatusCode(0);
        } catch(RestException restException){
            log.info("RestException -:", restException);
            restException.printStackTrace();
            baseDto.setStatusCode(baseDto.getStatusCode());
        } catch (Exception exception) {
            log.info("Exception -:",exception);
            exception.printStackTrace();
            baseDto.setStatusCode(ErrorCodeDescription.ERROR_GENERIC.getErrorCode());
        }
        log.info("<--Ends FPSStoreService .createFPSAndUser-->"+baseDto);
        return baseDto;
    }

------------------Called method 1st-----------

@Transactional(propagation = Propagation.REQUIRED)
    public BaseDto createFPSStore(FpsStoreDto fpsStoreDto) {
    _________________________
    __________________________
    ________________________
 return baseDto;

}

------------------------2nd Transactional method-----
@Transactional(propagation = Propagation.REQUIRED)
    public BaseDto createUserDetail(UserDetailDto userDetaildto) {
_______________
_______________
_______________
return baseDto
}
Jordi Castilla
  • 26,609
  • 8
  • 70
  • 109
Debendra Parida
  • 279
  • 3
  • 9
  • This looks like you're asking us to write your code. Stackoverflow is not a code-writing community! Also please format your code properly!!! – ParkerHalo Nov 30 '15 at 13:08
  • Error prone code and complex transactional configuration. Also, remove the rollBackFor thing. – We are Borg Nov 30 '15 at 13:16
  • Hi ParkerHalo . thanks for the suggestion for the formatting the code next time on wards I will not give the chance to ask me for this . I am asking the reason not the code .please let me know why it is not working .. and as I think this stackoverflow is for that .... – Debendra Parida Nov 30 '15 at 13:16
  • Thanks We are Borg.. – Debendra Parida Nov 30 '15 at 13:17
  • 2
    There are at lest 2 things wrong with this, first spring uses proxies to apply AOP so basically the `@Transactional` on the internal method calls are ignored, next you are catching all exception and thus springs tx code never sees it and thinks everything is ok and will commit. – M. Deinum Nov 30 '15 at 13:20
  • @M.Deinum The `createUserDetail` is called on a proxy, so it will have the correct transactionality, but you're right about the `createFPSStore()`. – Kayaman Nov 30 '15 at 13:23
  • @Kayaman I am also keeping createFPSStore() method by @Transactional(propagation = Propagation.REQUIRED) . – Debendra Parida Nov 30 '15 at 13:32
  • @DebendraParida Yes, but you still don't understand how they work. – Kayaman Nov 30 '15 at 13:37

3 Answers3

2

You've set rollbackFor=RestException.class, yet your code catches that very exception and doesn't rethrow it. From Spring's point of view RestException was never thrown by the method, and there's no reason to rollback the transaction.

If you want the rollback to happen, you need to do throw restException; in the end of your catch block.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • Then what should be the rollBackFor ..? is it should be rollBackfor = Exception.class – Debendra Parida Nov 30 '15 at 13:13
  • no, @kayaman is saying that you shouldn't catch "all" the exception just to log and set a status code .. you should throw `RestException` if you want your transaction to be rollbacked – Pras Nov 30 '15 at 13:18
  • @DebendraParida The `rollbackFor` only happens if the exception is thrown out of the method. You catch the exception and return `baseDto`, so even if an exception happened **inside** the method, it won't be seen by Spring, which is looking at it from the **outside**. – Kayaman Nov 30 '15 at 13:20
  • @DebendraParida Of course it's not working, because you don't understand what you're doing. I never said to remove `rollbackFor`. – Kayaman Nov 30 '15 at 13:29
1

You are telling Spring that rollback the transaction only when

rollbackFor = RestException.class

But if you catch the excecption

catch(RestException restException){

Spring will never get notice that an exception was thrown. You need to remove your catches blockes (both of them) or you can throw the exception at the end of your catch

    catch(RestException restException){
        log.info("RestException -:", restException);
        restException.printStackTrace();
        baseDto.setStatusCode(baseDto.getStatusCode());
        throw restException;
    }
reos
  • 8,766
  • 6
  • 28
  • 34
  • I got your point but the thing is I have to return the BaseDto where the cause of failure is present so If I am rethrowing the RestException my status code will be gone ...any way thanks .. – Debendra Parida Nov 30 '15 at 14:04
  • You can also rollback the transaction manually with TransactionAspectSupport.currentTransactionStatus().setRollbackOnly(); here there is the documentation http://docs.spring.io/spring/docs/3.0.x/reference/transaction.html#transaction-declarative-rolling-back – reos Nov 30 '15 at 14:16
  • Hi Thanks .. I just used the TransactionAspectSupport.currentTransactionStatus().setRollbackOnly() in my exception blocks and my problem solved thanks all for your comment and suggestion – Debendra Parida Dec 01 '15 at 10:03
0

@Transactional tell the container (spring) to handle the transaction management for the annotated method call.

  • This is done using proxy, see understanding aop proxies which means that :
    • Annotation is considered only for call external to the annotated object
    • Only exception thrown outside of the method boundaries will be handled by the container

`

  • each annotated method has its own transaction logical context which means that :
    • Even if your main method has the parameter rollbackFor=RestException.class the inner ones won't inherit the configuration and won't trigger a rollback when throwing a rest exception
    • If an inner method trigger a rollback due to an exception thrown during it's execution, the transaction will be rolled back even if the exception is caught by the caller, every subsequent access to the database will result in an UnexpectedRollbackException
Community
  • 1
  • 1
Gab
  • 7,869
  • 4
  • 37
  • 68