Spring: Uncaught Exception in Transactional Method

Question

I'm building a RESTful API and have the following update method in my ProductController:

@Slf4j
@RestController
@RequiredArgsConstructor
public class ProductController implements ProductAPI {

    private final ProductService productService;

    @Override
    public Product updateProduct(Integer id, @Valid UpdateProductDto productDto) throws ProductNotFoundException,
            ProductAlreadyExistsException {

        log.info("Updating product {}", id);
        log.debug("Update Product DTO: {}", productDto);

        Product product = productService.updateProduct(id, productDto);

        log.info("Updated product {}", id);
        log.debug("Updated Product: {}", product);

        return product;
    }

}

The throwable exceptions come from the ProductService which has the following implementation:

package com.example.ordersapi.product.service.impl;

import com.example.ordersapi.product.api.dto.CreateProductDto;
import com.example.ordersapi.product.api.dto.UpdateProductDto;
import com.example.ordersapi.product.entity.Product;
import com.example.ordersapi.product.exception.ProductAlreadyExistsException;
import com.example.ordersapi.product.exception.ProductNotFoundException;
import com.example.ordersapi.product.mapper.ProductMapper;
import com.example.ordersapi.product.repository.ProductRepository;
import com.example.ordersapi.product.service.ProductService;
import lombok.RequiredArgsConstructor;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

@Service
@RequiredArgsConstructor
public class ProductServiceImpl implements ProductService {

    private final ProductRepository productRepository;
    private final ProductMapper productMapper;

    @Override
    public Set<Product> getAllProducts() {
        return StreamSupport.stream(productRepository.findAll().spliterator(), false)
                .collect(Collectors.toSet());
    }

    @Override
    public Product getOneProduct(Integer id) throws ProductNotFoundException {
        return productRepository.findById(id)
                .orElseThrow(() -> new ProductNotFoundException(id));
    }

    @Override
    public Product createProduct(CreateProductDto productDto) throws ProductAlreadyExistsException {
        Product product = productMapper.createProductDtoToProduct(productDto);
        Product savedProduct = saveProduct(product);

        return savedProduct;
    }

    private Product saveProduct(Product product) throws ProductAlreadyExistsException {
        try {
            return productRepository.save(product);
        } catch (DataIntegrityViolationException ex) {
            throw new ProductAlreadyExistsException(product.getName());
        }
    }

    /**
     * Method needs to be wrapped in a transaction because we are making two database queries:
     *  1. Finding the Product by id (read)
     *  2. Updating found product (write)
     *
     *  Other database clients might perform a write operation over the same entity between our read and write,
     *  which would cause inconsistencies in the system. Thus, we have to operate over a snapshot of the database and
     *  commit or rollback (and probably re-attempt the operation?) depending if its state has changed meanwhile.
     */
    @Override
    @Transactional
    public Product updateProduct(Integer id, UpdateProductDto productDto) throws ProductNotFoundException,
            ProductAlreadyExistsException {

        Product foundProduct = getOneProduct(id);
        boolean productWasUpdated = false;

        if (productDto.getName() != null && !productDto.getName().equals(foundProduct.getName())) {
            foundProduct.setName(productDto.getName());
            productWasUpdated = true;
        }

        if (productDto.getDescription() != null && !productDto.getDescription().equals(foundProduct.getDescription())) {
            foundProduct.setDescription(productDto.getDescription());
            productWasUpdated = true;
        }

        if (productDto.getImageUrl() != null && !productDto.getImageUrl().equals(foundProduct.getImageUrl())) {
            foundProduct.setImageUrl(productDto.getImageUrl());
            productWasUpdated = true;
        }

        if (productDto.getPrice() != null && !productDto.getPrice().equals(foundProduct.getPrice())) {
            foundProduct.setPrice(productDto.getPrice());
            productWasUpdated = true;
        }

        Product updateProduct = productWasUpdated ? saveProduct(foundProduct) : foundProduct;

        return updateProduct;
    }

}

Because I've set the NAME column as UNIQUE in my database, when an update is issued with an already existing name, the repository save method will throw a DataIntegrityViolationException. In the createProduct method it works fine, but in the updateProduct, the call to the private method saveProduct doesn't catch the Exception no matter what and thus the DataIntegrityViolationException bubbles to the Controller.

I know it is because I'm wrapping the code in a transaction because removing the @Transactional "solves" the problem. I think it has something to do with the fact that Spring uses a Proxy to wrap the method in a transaction and thus the service call in the controller isn't actually calling the service method (directly). Even though, I don't understand why does it ignore the catch branch to throw the ProductAlreadyExistsException when it works fine for the ProductNotFoundException.

I know I could also make another trip to the database, try to find a product by name and in case of absence would I try to save my entity back. But that would make everything even more inefficient.

I can also catch the DataIntegrityViolationException in the controller layer and throw the ProductAlreadyExistsException there but I would be exposing details from the persistence layer there, which doesn't seem appropriate.

Is there a way to handle it all in the service layer, as I'm trying to do now?

P.S.: Outsourcing the logic to a new method and use it internally seems to work but only because calls to the this won't actually be intercepted by the Transaction Manager proxy and thus no transaction is actually performed

Answer 1

To make it work you would need to change saveProduct method as follows:

        try {
            return productRepository.saveAndFlush(product);
        } catch (DataIntegrityViolationException ex) {
            throw new ProductAlreadyExistsException(product.getName());
        }

When you use the save() method, the data associated with the save operation will not be flushed to your database unless and until an explicit call to flush() or commit() method is made. That causes DataIntegrityViolationException being thrown later that you would expect and therefore it isn't beeing catched in the snipper above.

saveAndFlush() method on the other hand, flushes the data immediately. That should trigger expected exception being catched immidietely and re-throwed as ProductAlreadyExistsException.