The refund that debited twice

import java.lang.reflect.*;
import java.util.*;

/**
 * Two root causes, compounding.
 *
 * CAUSE 1 — self-invocation. refundAll() called refundOne() on `this`, so the
 * call never crossed the proxy and @Transactional on refundOne was inert. The
 * only transaction opened was refundAll's, which means the whole batch was one
 * unit rather than one unit per refund. That is why it only reproduced with
 * more than one item: with a single refund the outer transaction happens to
 * give the same boundary.
 *
 * CAUSE 2 — Spring's default rollback rule. LedgerException extends Exception,
 * so it is checked, and Spring commits on checked exceptions. R2's reversal was
 * written, the ledger write threw, and the reversal committed anyway — the
 * orphaned row finance reported.
 *
 * Why adding more annotations did not help: the second @Transactional was on a
 * method still being self-invoked. The annotation was never the missing piece;
 * the proxy boundary was.
 *
 * FIX 1 — move refundOne into its own bean and call it through the injected
 * reference. The call now crosses that bean's proxy, so each refund gets its
 * own transaction and one failure cannot take the others with it.
 *
 * FIX 2 — rollbackFor = Exception.class, reproduced here as the proxy rolling
 * back on any Throwable. LedgerException now rolls its refund back, so a
 * reversal is never left without a ledger entry.
 *
 * What makes this impossible rather than fixed:
 *
 *   1. Keep the transactional method on a different bean from its caller. Once
 *      the boundary is a bean boundary, self-invocation cannot happen by
 *      accident — it would require injecting yourself.
 *   2. Do not throw checked exceptions from transactional code, or set
 *      rollbackFor once on a shared meta-annotation so nobody has to remember.
 *   3. Assert on transaction count in a test. Both defects were invisible to
 *      code review and obvious the moment you count BEGIN lines.
 *
 * The general lesson: annotation-driven behaviour fails silently by design.
 * Nothing warns you that an annotation did not apply, so the only defence is a
 * test that observes the behaviour rather than the annotation.
 */
public class Solution {

    static class LedgerException extends Exception {
        LedgerException(String m) { super(m); }
    }

    static final List<String> reversals = new ArrayList<>();
    static final List<String> ledger = new ArrayList<>();
    static final List<String> committed = new ArrayList<>();

    /** FIX 1: the transactional unit lives on its own bean. */
    interface SingleRefundService {
        void refundOne(String id) throws Exception;
    }

    static class RealSingleRefundService implements SingleRefundService {
        @Override
        public void refundOne(String id) throws Exception {
            reversals.add(id);
            System.out.println("    reversed " + id);
            if (id.equals("R2")) {
                throw new LedgerException("ledger service unreachable for " + id);
            }
            ledger.add(id);
            System.out.println("    ledgered " + id);
        }
    }

    /** The orchestrator is deliberately NOT transactional. */
    static class BatchRefundService {
        private final SingleRefundService single;   // the proxy, injected

        BatchRefundService(SingleRefundService single) {
            this.single = single;
        }

        void refundAll(List<String> ids) {
            for (String id : ids) {
                try {
                    single.refundOne(id);          // crosses the proxy
                } catch (Exception e) {
                    System.out.println("    batch continued past " + id);
                }
            }
        }
    }

    /**
     * FIX 2: rollbackFor = Exception.class — roll back on anything thrown,
     * not only unchecked.
     */
    static SingleRefundService transactional(SingleRefundService target) {
        return (SingleRefundService) Proxy.newProxyInstance(
            SingleRefundService.class.getClassLoader(),
            new Class<?>[] { SingleRefundService.class },
            (proxy, method, args) -> {
                System.out.println("  BEGIN   " + method.getName());
                int reversalMark = reversals.size();
                int ledgerMark = ledger.size();
                try {
                    Object r = method.invoke(target, args);
                    committed.add(method.getName());
                    System.out.println("  COMMIT  " + method.getName());
                    return r;
                } catch (InvocationTargetException e) {
                    Throwable cause = e.getCause();
                    System.out.println("  ROLLBACK " + method.getName()
                        + " — " + cause.getClass().getSimpleName());
                    while (reversals.size() > reversalMark) reversals.remove(reversals.size() - 1);
                    while (ledger.size() > ledgerMark) ledger.remove(ledger.size() - 1);
                    throw cause;
                }
            });
    }

    public static void main(String[] args) {
        SingleRefundService single = transactional(new RealSingleRefundService());
        BatchRefundService batchService = new BatchRefundService(single);

        List<String> batch = List.of("R1", "R2", "R3");
        System.out.println("refunding batch of " + batch.size() + ":");
        batchService.refundAll(batch);

        System.out.println();
        System.out.println("transactions opened = " + committed.size());
        System.out.println("reversals written   = " + reversals);
        System.out.println("ledger entries      = " + ledger);

        Set<String> both = new TreeSet<>(reversals);
        both.retainAll(ledger);
        Set<String> orphaned = new TreeSet<>(reversals);
        orphaned.removeAll(ledger);

        System.out.println("consistent refunds  = " + both);
        System.out.println("orphaned reversals  = " + orphaned);

        // Two committed here plus the rolled-back one means a transaction was
        // opened per refund, which is the property that was missing.
        boolean perRefundTx = committed.size() == batch.size() - 1;
        boolean noOrphans = orphaned.isEmpty();
        boolean othersSurvived = both.contains("R1") && both.contains("R3");

        System.out.println();
        System.out.println("one transaction per refund : " + perRefundTx);
        System.out.println("no orphaned reversals      : " + noOrphans);
        System.out.println("R1 and R3 still refunded   : " + othersSurvived);
        System.out.println(perRefundTx && noOrphans && othersSurvived ? "PASS" : "FAIL");
    }
}