Prevent transaction rollback when catching AccessDeniedException in inner service

Question

I have 2 services: RecentRecordService and BookService.

@Service
public class RecentRecordService {
    @Transactional
    public List<Book> getRecentReadBooks() {
        List<Long> recentReadBookIds = getRecentReadBookIds();
        List<Book> recentReadBooks = new ArrayList<>();

        for (Long bookId : recentReadBookIds) {
            try {
                Book book = bookService.getBook(bookId);
                recentReadBooks.add(book);
            } catch (AccessDeniedException e) {
                // skip
            }
        }

        return recentReadBooks;
    }
}

@Service
public class BookService {
    @Transactional
    public Book getBook(Long bookId) {
        Book book = bookDao.get(bookId);
        if (!hasReadPermission(book)) {
            throw new AccessDeniedException(); // spring-security exception
        }
        return book;
    }
}

Assume that getRecentReadBookIds() returns [1, 2, 3].

When the session user has permission for all the book IDs that returned from getRecentReadBookIds(), everything works fine. getRecentReadBooks() will return a list of 3 Books.

Suddenly, the owner of book #2 changed the permission setting of it from "Public" to "Private". Therefore, the session user can no longer read the book #2.

I was expecting that getRecentReadBooks() will return a list of 2 Books, which contains the info of book #1 and book #3. However, the method failed with the following exception:

org.springframework.transaction.UnexpectedRollbackException: Transaction rolled back because it has been marked as rollback-only

After some research, I found that it has something to do with the transaction propagation. The transation will be marked as "rollback-only" even if I use try-catch to handle the AccessDeniedException in getRecentReadBooks().

This problem seems to be solved by changing the @Transactional annotation for getBook() into:

    @Transactional(propagation = Propagation.NESTED)
    public Book getBook(Long bookId) {
        // ...
    }

or

    @Transactional(noRollbackFor = AccessDeniedException.class)
    public Book getBook(Long bookId) {
        // ...
    }

However, I was wondering if I can solve the problem by only modifying RecentRecordService. After all, RecentRecordService is the one who wants to handle AccessDeniedException on its own.

Any suggestion will help. Thanks.

R.G · Answer 1 · 2019-12-21T09:00:45.617

0

Quote from the documentation

By default, a transaction will be rolling back on RuntimeException and Error but not on checked exceptions (business exceptions)

AccessDeniedException is a subclass of RuntimeException

If in your case the AccessDeniedException used is the spring frameworks , and as per the requirement if the exception on access denied can be made a custom business exception (not a subclass of Runtime Exception ) , it should work without annotating getBook(Long bookId) method.

Update

OP decided to fix the root cause . This is to provide the rationale behind my answer for any future references

AccessDeniedException : Thrown if an Authentication object does not hold a required authority.

As per my understanding , here the access to a book is based on business logic and not user role specific. A custom business exception would be apt in such a scenario.

edited Dec 21 '19 at 09:00

answered Dec 19 '19 at 16:36

R.G

6,436
3
19
28

Thank you, but I knew that AccessDeniedException is a subclass of RuntimeException. No, I can't make the exception to a checked exception. What I want to know is a way to solve the problem with only modifying RecentRecordService. – johnlinp Dec 19 '19 at 23:57
You may need to identify the root cause and fix it probably : https://stackoverflow.com/a/2007165/4214241 – R.G Dec 20 '19 at 00:05
I think I know the root cause. I just need a proper fix for this. – johnlinp Dec 20 '19 at 02:55

score 0 · Answer 2 · answered Dec 20 '19 at 10:46

0

I don't think it is possible to solve it by only modifying the RecentRecordService because the transaction is already marked as rollback in the BookService due to a RuntimeException happens in it. And once the TransactionStatus is marked as rollback , there is no way to revert it to not rollback currently.

So , you have to make BookService#getBook() does not mark the transaction as rollback by :

@Transactional(noRollbackFor = Throwable.class)
public Book getBook(Long bookId) {

}

which means the transaction will not marked as rollback no matter what exception happen. It makes sense to do it as this method is supposed to be read-only , so it is no point to rollback when a method is not supposed to modify anythings.

answered Dec 20 '19 at 10:46

Ken Chan

84,777
26
143
172

If I want to make `BookService#getBook()` unmodified, does it make sense if I write a wrapper class for it, say `BookServiceWrapper#getBook()` and make the wrapper method as `@Transactional(noRollbackFor = Throwable.class)`? – johnlinp Dec 21 '19 at 13:29
it should not work if you just use a wrapper to wrap the `BookService#getBook()` but keep `BookService#getBook()` unchanged because it will still mark the transaction as rollback if exception happen in it. – Ken Chan Dec 21 '19 at 17:09
How about making the wrapper method as `@Transactional(Propagation.NESTED)`? – johnlinp Dec 22 '19 at 01:30

Prevent transaction rollback when catching AccessDeniedException in inner service

2 Answers2