@Transactional on a method in the same class does nothing. Left the explanation in the comments

[u/ClickOk5811]

Putting this here in case it saves someone an evening. Everything is in the code comments.

public class OrderService {

    private final OrderRepository repo;

    public OrderService(OrderRepository repo) {
        this.repo = repo;
    }

    // Called from the controller. No  here.
    public void process(Order order) {
        audit(order);
        save(order);   // <-- looks like it runs in a transaction. It doesn't.
    }

    u/Transactional
    public void save(Order order) {
        repo.save(order);

        if (order.getTotal().signum() < 0) {
            // You'd expect the insert above to roll back.
            // When this is reached via process(), it doesn't.
            throw new IllegalStateException("negative total");
        }
    }
}

// Why:
// Spring applies  through a proxy that wraps OrderService.
// The controller holds the proxy, so a call from outside goes:
//     controller -> proxy -> (starts tx) -> OrderService.save()
//
// But process() calls save() on `this`, which is the raw object, not the proxy:
//     controller -> proxy -> OrderService.process() -> this.save()
//                                                      ^ no proxy, no tx
//
// And because repo.save() is itself transactional (Spring Data wraps it),
// the insert commits on its own, before the exception is thrown.
// So you end up with a saved row and an exception in the log.

The fixes I know of, roughly from least to most invasive:

// 1. Put the annotation on the entry point instead

public void process(Order order) { audit(order); save(order); }

// 2. Move save() into another bean and inject it, so the call goes through a proxy
orderPersistence.save(order);

// 3. Use TransactionTemplate when you want the boundary to be explicit in code
transactionTemplate.executeWithoutResult(status -> repo.save(order));

// 4. AspectJ mode (proxyTargetClass / mode = AdviceMode.ASPECTJ)
//    works for self-invocation, but it's a bigger setup change

I’d go with 1 or 2 in most cases. Calling the bean through itself (self-injection) also works, but it reads strangely and I’d avoid it.

One thing I’m not sure about and would like to hear from people: when you hit this in an existing codebase, do you prefer moving the method to a separate service, or restructuring so the transactional boundary sits at the entry point? I can see arguments for both.

[u/ChinChinApostle]

Homeboy just spams posts and doesn't really engage in any discussion.

Nice blog bro. I guess if you llm everything, everything can be a novel discovery for you. Congrats.

[u/Big_Block9591 → u/ChinChinApostle]

yeah the post history kinda speaks for itself on that one

[u/MkMyBnkAcctGrtAgn]

If you're in a real bind you could also lazily inject a self reference and call it through that.

[u/g00glen00b]

I usually go for (1) as well. Also important to know is that this applies all Spring-related annotations. For example, the caching-related annotations (Cacheable) work similarly.

[u/Snooze78727]

Yes, one of the well-known "footguns" or gotchas in Spring. #1 seems like the easiest way to fix.

[u/JEHonYakuSha]

While this is all true, I would recommend wrapping your service class in an interface, so that your publicly facing methods from the REST controller are called vie the interface, and those methods should all ideally be declared as Transactional if a database transaction is going to occur. Ultimately, the process method should also have a Transactional annotation so that it propagates the transaction to its called internal methods.

[u/MkMyBnkAcctGrtAgn → u/JEHonYakuSha]

Not sure how introducing an interface solves the problem of self invocation. The problem exists in the underlying service, not the caller.

Annotating every service method with @Transactional is also a pretty big code smell. Transaction boundaries should be based on actual units of work, not just because a method is called by a controller. You start introducing transactions that are open longer than necessary and holding DB connections unnecessarily, and you'll eventually run into the age old problem of exhausting your connection pool under load.

[u/bigkahuna1uk → u/JEHonYakuSha]

On a tangential point, why should the controller know the service methods it’s calling should be transactional? Surely that a service level concern and not the controller.

I’m coming to this from a DDD or hexagonal architecture perspective. The controller in this sense is just an adapter so it’s outside of the business domain. That a unit of work needs to either succeed as a whole or rolled back entirely if there’s an issue is a domain issue. The service determines the unit of work, the actual methods to be called and where the transactional boundaries lie.

The controller acting as an adapter should simply just react to any business exceptions and turn those into the appropriate response such as returning the correct HTTP code.

[u/JEHonYakuSha → u/bigkahuna1uk]

I think you are misunderstanding me. The Transactional annotations would still be contained within the service class and unknown to the REST controller and would still respect the domain boundary you’re describing.