diff --git a/README.md b/README.md index d7f1d47..75cf11e 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ production：`labops.reservation-expiry.mode=rabbit`。 | `uk_payment_request_idempotency` 唯一索引 | 重复发起在**入队时**就被挡掉，还没碰到渠道 | | 渠道认商户幂等键 | 万一还是重复发了，渠道返回原交易而不是新建一笔 | | `PaymentRequestRetryJob` 定时补偿 + 指数退避 | 发不出去的请求会一直重试，而不是无声丢失 | -| 重试耗尽 → 差账工单 | 真钱卡住时交给人，而不是又一行没人看的日志 | +| 重试耗尽 → 差账工单（`PaymentRequestRetryJob` 补扫漏建的） | 真钱卡住时交给人，而不是又一行没人看的日志；建单失败也不会让请求停在 `ABANDONED` 却没人管 | > 键必须同时覆盖退款。只给支付加键、然后给退款加重试，等于把「重复扣款」换成「重复退款」。 diff --git a/src/main/java/com/arthur/labops/payment/PaymentDispatchService.java b/src/main/java/com/arthur/labops/payment/PaymentDispatchService.java index 7eb443b..81f9ce6 100644 --- a/src/main/java/com/arthur/labops/payment/PaymentDispatchService.java +++ b/src/main/java/com/arthur/labops/payment/PaymentDispatchService.java @@ -113,10 +113,10 @@ public boolean enqueue(String orderNo, PaymentTransactionType type, long amountC * row, so holding it here would make the call wait on itself. The completion * write takes the lock afterwards, in its own short transaction. */ - public PaymentRequest attempt(String idempotencyKey) { + public void attempt(String idempotencyKey) { PaymentRequest request = requestRepository.findByIdempotencyKey(idempotencyKey).orElse(null); if (request == null || request.isSettled()) { - return null; + return; } Applicability applicability = applicability(request); if (applicability != Applicability.OWED) { @@ -127,7 +127,7 @@ public PaymentRequest attempt(String idempotencyKey) { ticketService.raiseOutboundHaltedTicket(request, "金额已变化，请求所记金额 " + request.getAmountCents() + " 分不再与订单当前状态相符"); } - return null; + return; } String channelKey = request.channelKey(); @@ -138,11 +138,13 @@ public PaymentRequest attempt(String idempotencyKey) { channel.charge(request.getOrderNo(), request.getAmountCents(), channelKey); } stateService.markSent(idempotencyKey, channelKey); - return null; } catch (RuntimeException exception) { log.warn("Outbound payment request failed key={} channelKey={} type={} orderNo={}", idempotencyKey, channelKey, request.getType(), request.getOrderNo(), exception); - return stateService.markFailed(idempotencyKey, channelKey, exception.toString()); + PaymentRequest abandoned = stateService.markFailed(idempotencyKey, channelKey, exception.toString()); + if (abandoned != null) { + ticketService.raiseOutboundFailureTicket(abandoned); + } } } } diff --git a/src/main/java/com/arthur/labops/payment/PaymentRequestRepository.java b/src/main/java/com/arthur/labops/payment/PaymentRequestRepository.java index e4f590c..f68deaf 100644 --- a/src/main/java/com/arthur/labops/payment/PaymentRequestRepository.java +++ b/src/main/java/com/arthur/labops/payment/PaymentRequestRepository.java @@ -71,4 +71,31 @@ where request.status in ( order by request.id """) List findDueKeys(@Param("now") Instant now); + + /** + * ABANDONED requests that never got their outbound ticket. + * + *

The dispatch path commits ABANDONED and only then raises the ticket in a + * transaction of its own, so a failure in between leaves a request that the + * retry scan no longer selects and nobody is looking at. Matching on the + * discrepancy key rather than on a flag keeps this in step with the ticket + * service's own dedupe: a request already halted with a ticket for another + * reason shares the key and is not picked up again. + * + *

Requests whose order or equipment cannot be resolved are left out — the + * ticket service can only audit those, and would do so on every scan. + */ + @Query(""" + select request from PaymentRequest request + where request.status = com.arthur.labops.payment.PaymentRequestStatus.ABANDONED + and exists ( + select 1 from PaymentOrder paymentOrder, Equipment equipment + where paymentOrder.orderNo = request.orderNo + and equipment.id = paymentOrder.equipmentId) + and not exists ( + select 1 from FaultWorkOrder ticket + where ticket.discrepancyKey = concat('outbound|', request.idempotencyKey)) + order by request.id + """) + List findAbandonedWithoutTicket(); } diff --git a/src/main/java/com/arthur/labops/payment/PaymentRequestRetryJob.java b/src/main/java/com/arthur/labops/payment/PaymentRequestRetryJob.java index 71633de..2c788a8 100644 --- a/src/main/java/com/arthur/labops/payment/PaymentRequestRetryJob.java +++ b/src/main/java/com/arthur/labops/payment/PaymentRequestRetryJob.java @@ -44,15 +44,31 @@ public void retryDueRequests() { List due = new ArrayList<>(requestRepository.findDueKeys(Instant.now())); for (String key : due) { try { - PaymentRequest abandoned = dispatchService.attempt(key); - if (abandoned != null) { - // Out of retries. A human has to look at real money now, so this - // becomes a ticket rather than another line in a log nobody reads. - ticketService.raiseOutboundFailureTicket(abandoned); - } + dispatchService.attempt(key); } catch (RuntimeException exception) { log.warn("Retrying outbound payment request {} failed", key, exception); } } } + + /** + * Backstop for the ticket an exhausted request should already have. + * + *

{@link PaymentDispatchService#attempt} raises it right after ABANDONED + * commits, but in a separate transaction: if that write fails, the request is + * out of {@link #retryDueRequests()} for good and the stuck money would have + * no human on it. Raising is idempotent on the request's discrepancy key, so + * racing the dispatch path at worst loses one insert to the unique index. + */ + @Scheduled(fixedDelayString = "${labops.payment.outbound.retry-interval:30s}") + public void raiseMissingAbandonedTickets() { + for (PaymentRequest abandoned : requestRepository.findAbandonedWithoutTicket()) { + try { + ticketService.raiseOutboundFailureTicket(abandoned); + } catch (RuntimeException exception) { + log.warn("Raising missed ticket for abandoned payment request {} failed", + abandoned.getIdempotencyKey(), exception); + } + } + } } diff --git a/src/test/java/com/arthur/labops/AbandonedPaymentTicketBackstopIntegrationTest.java b/src/test/java/com/arthur/labops/AbandonedPaymentTicketBackstopIntegrationTest.java new file mode 100644 index 0000000..ec71d3c --- /dev/null +++ b/src/test/java/com/arthur/labops/AbandonedPaymentTicketBackstopIntegrationTest.java @@ -0,0 +1,111 @@ +package com.arthur.labops; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.web.servlet.MockMvc; + +import com.arthur.labops.payment.PaymentIdempotency; +import com.arthur.labops.payment.PaymentRequest; +import com.arthur.labops.payment.PaymentRequestRepository; +import com.arthur.labops.payment.PaymentRequestRetryJob; +import com.arthur.labops.payment.PaymentRequestStatus; +import com.arthur.labops.payment.PaymentService; +import com.arthur.labops.payment.PaymentTransactionType; +import com.arthur.labops.payment.channel.SimulatedPaymentChannel; +import com.arthur.labops.workorder.FaultWorkOrderRepository; +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * An ABANDONED request whose ticket write was lost must still end up with exactly + * one ticket: the status commits before the ticket's own transaction, and nothing + * else ever looks at an ABANDONED request again. + */ +@SpringBootTest(properties = { + "labops.payment.window=10m", + "labops.payment.channel.callback-mode=MANUAL", + "labops.payment.outbound.retry-interval=1h", + "spring.datasource.url=jdbc:h2:mem:abandoned-ticket-backstop;MODE=MySQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE", + "labops.payment.channel.bill-directory=target/test-channel-bills/abandoned-ticket-backstop" +}) +@AutoConfigureMockMvc +class AbandonedPaymentTicketBackstopIntegrationTest { + + private static final long PRICE_CENTS = 6_000L; + + @Autowired private MockMvc mockMvc; + @Autowired private ObjectMapper objectMapper; + @Autowired private SimulatedPaymentChannel channel; + @Autowired private PaymentRequestRepository requests; + @Autowired private FaultWorkOrderRepository tickets; + @Autowired private PaymentRequestRetryJob retryJob; + + private String orderNo; + + @BeforeEach + void setUp() throws Exception { + TestAuth.clearCache(); + channel.reset(); + PaymentScenario scenario = new PaymentScenario(mockMvc, objectMapper); + Long equipmentId = scenario.createPricedEquipment("ABANDON-BACKSTOP", PRICE_CENTS); + Long reservationId = scenario.createReservation(equipmentId, 1); + scenario.approve(reservationId); + orderNo = PaymentService.orderNoFor(reservationId); + } + + @Test + void backstopRaisesTheLostTicketOnceAndOnlyOnce() { + // The state the dispatch path leaves behind when markFailed committed + // ABANDONED and the follow-up ticket transaction then threw. + String refundKey = PaymentIdempotency.cancellationRefund(orderNo); + PaymentRequest request = abandoned(orderNo, refundKey); + String ticketKey = "outbound|" + refundKey; + assertThat(tickets.existsByDiscrepancyKey(ticketKey)).isFalse(); + assertThat(requests.findAbandonedWithoutTicket()) + .extracting(PaymentRequest::getIdempotencyKey).contains(refundKey); + + retryJob.raiseMissingAbandonedTickets(); + + assertOneTicket(ticketKey, request); + assertThat(requests.findAbandonedWithoutTicket()) + .extracting(PaymentRequest::getIdempotencyKey).doesNotContain(refundKey); + + retryJob.raiseMissingAbandonedTickets(); + retryJob.retryDueRequests(); + + assertOneTicket(ticketKey, request); + assertThat(requests.findByIdempotencyKey(refundKey).orElseThrow().getStatus()) + .isEqualTo(PaymentRequestStatus.ABANDONED); + } + + @Test + void requestsWithoutAResolvableOrderAreNotRescannedForever() { + // The ticket service can only audit these; selecting them would write the + // same audit line on every scan. + String unknownOrder = "LF-UNKNOWN-" + System.nanoTime(); + abandoned(unknownOrder, PaymentIdempotency.cancellationRefund(unknownOrder)); + + assertThat(requests.findAbandonedWithoutTicket()) + .extracting(PaymentRequest::getOrderNo).doesNotContain(unknownOrder); + } + + private PaymentRequest abandoned(String order, String key) { + PaymentRequest request = new PaymentRequest(order, key, PaymentTransactionType.REFUND, PRICE_CENTS); + boolean exhausted = false; + while (!exhausted) { + exhausted = request.markFailed("java.lang.IllegalStateException: transport failure"); + } + return requests.saveAndFlush(request); + } + + private void assertOneTicket(String ticketKey, PaymentRequest request) { + assertThat(tickets.findAll()) + .filteredOn(ticket -> ticketKey.equals(ticket.getDiscrepancyKey())) + .singleElement().satisfies(ticket -> assertThat(ticket.getDescription()) + .contains("REFUND", String.valueOf(request.getAttempts()), request.getLastError())); + } +} \ No newline at end of file diff --git a/src/test/java/com/arthur/labops/PaymentRetryExhaustionIntegrationTest.java b/src/test/java/com/arthur/labops/PaymentRetryExhaustionIntegrationTest.java new file mode 100644 index 0000000..9b7cd36 --- /dev/null +++ b/src/test/java/com/arthur/labops/PaymentRetryExhaustionIntegrationTest.java @@ -0,0 +1,145 @@ +package com.arthur.labops; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +import java.time.Duration; +import java.time.Instant; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.web.servlet.MockMvc; + +import com.arthur.labops.payment.PaymentDispatchService; +import com.arthur.labops.payment.PaymentIdempotency; +import com.arthur.labops.payment.PaymentOrderRepository; +import com.arthur.labops.payment.PaymentOrderStatus; +import com.arthur.labops.payment.PaymentRequest; +import com.arthur.labops.payment.PaymentRequestRepository; +import com.arthur.labops.payment.PaymentRequestRetryJob; +import com.arthur.labops.payment.PaymentRequestStatus; +import com.arthur.labops.payment.PaymentService; +import com.arthur.labops.payment.PaymentTransactionType; +import com.arthur.labops.payment.channel.ChannelEntryType; +import com.arthur.labops.payment.channel.SimulatedPaymentChannel; +import com.arthur.labops.workorder.FaultWorkOrderRepository; +import com.fasterxml.jackson.databind.ObjectMapper; + +@SpringBootTest(properties = { + "labops.payment.window=10m", + "labops.payment.channel.callback-mode=MANUAL", + "labops.payment.outbound.retry-interval=1h", + "spring.datasource.url=jdbc:h2:mem:retry-exhaustion;MODE=MySQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE", + "labops.payment.channel.bill-directory=target/test-channel-bills/retry-exhaustion" +}) +@AutoConfigureMockMvc +class PaymentRetryExhaustionIntegrationTest { + + private static final long PRICE_CENTS = 6_000L; + + @Autowired private MockMvc mockMvc; + @Autowired private ObjectMapper objectMapper; + @Autowired private SimulatedPaymentChannel channel; + @Autowired private PaymentOrderRepository orders; + @Autowired private PaymentRequestRepository requests; + @Autowired private FaultWorkOrderRepository tickets; + @Autowired private PaymentRequestRetryJob retryJob; + @Autowired private PaymentDispatchService dispatch; + @Autowired private JdbcTemplate jdbc; + + private PaymentScenario scenario; + private Long reservationId; + private String orderNo; + private String refundKey; + + @BeforeEach + void setUp() throws Exception { + TestAuth.clearCache(); + channel.reset(); + scenario = new PaymentScenario(mockMvc, objectMapper); + Long equipmentId = scenario.createPricedEquipment("RETRY-EXHAUST", PRICE_CENTS); + reservationId = scenario.createReservation(equipmentId, 1); + scenario.approve(reservationId); + orderNo = PaymentService.orderNoFor(reservationId); + refundKey = PaymentIdempotency.cancellationRefund(orderNo); + assertThat(scenario.payViaApi(orderNo, "student", "student123")).isEqualTo(200); + Await.until("the payment dispatch to complete", () -> requests + .findByIdempotencyKey(PaymentIdempotency.payment(orderNo)) + .filter(request -> request.getStatus() == PaymentRequestStatus.SENT).isPresent()); + channel.deliverPending(); + assertThat(orders.findByOrderNo(orderNo).orElseThrow().getStatus()).isEqualTo(PaymentOrderStatus.PAID); + } + + @Test + void eventRetryCreatesATicketWhenTransportFailsAfterSevenChannelRejections() throws Exception { + channel.rejectNextOutboundFinal(7); + assertThat(scenario.cancelAsStudent(reservationId).get("status")).isEqualTo("REFUNDING"); + for (int attempt = 0; attempt < 7; attempt++) { + int expectedAttempt = attempt; + Await.until("refund attempt " + attempt + " to await its rejection callback", () -> { + PaymentRequest request = refund(); + return request.getStatus() == PaymentRequestStatus.SENT + && request.getChannelAttempt() == expectedAttempt; + }); + assertThat(refund().getAttempts()).isEqualTo(attempt + 1); + assertThat(tickets.existsByDiscrepancyKey("outbound|" + refundKey)).isFalse(); + if (attempt == 6) { + channel.failNextOutbound(1); + } + channel.deliverPending(); + } + + await().atMost(Duration.ofSeconds(5)).untilAsserted(this::assertExhaustedWithOneTicket); + assertThat(refund().getChannelAttempt()).isEqualTo(7); + channel.redeliverAll(); + dispatch.attempt(refundKey); + retryJob.retryDueRequests(); + assertExhaustedWithOneTicket(); + } + + @Test + void scheduledRetryAlsoCreatesOneTicketWhenTransportFailuresExhaustTheBudget() { + // Persist the first seven failed sends without queuing an immediate event; + // the scanner must perform the final send itself. + PaymentRequest request = new PaymentRequest(orderNo, refundKey, PaymentTransactionType.REFUND, PRICE_CENTS); + for (int attempt = 0; attempt < 7; attempt++) { + assertThat(request.markFailed("previous transport failure")).isFalse(); + } + requests.saveAndFlush(request); + jdbc.update("update payment_requests set next_attempt_at=? where idempotency_key=?", + java.sql.Timestamp.from(Instant.now().minusSeconds(1)), refundKey); + assertThat(tickets.existsByDiscrepancyKey("outbound|" + refundKey)).isFalse(); + channel.failNextOutbound(1); + + retryJob.retryDueRequests(); + + assertExhaustedWithOneTicket(); + assertThat(refund().getChannelAttempt()).isZero(); + retryJob.retryDueRequests(); + dispatch.attempt(refundKey); + assertExhaustedWithOneTicket(); + } + + private PaymentRequest refund() { + return requests.findByIdempotencyKey(refundKey).orElseThrow(); + } + + private void assertExhaustedWithOneTicket() { + PaymentRequest request = refund(); + assertThat(request.getStatus()).isEqualTo(PaymentRequestStatus.ABANDONED); + assertThat(request.getAttempts()).isEqualTo(8); + assertThat(request.getLastError()).contains("IllegalStateException"); + assertThat(requests.findDueKeys(Instant.now())).doesNotContain(refundKey); + assertThat(tickets.findAll()) + .filteredOn(ticket -> ("outbound|" + refundKey).equals(ticket.getDiscrepancyKey())) + .singleElement().satisfies(ticket -> assertThat(ticket.getDescription()) + .contains("REFUND", "8", request.getLastError())); + assertThat(channel.ledger()) + .filteredOn(entry -> entry.orderNo().equals(orderNo) && entry.type() == ChannelEntryType.REFUND) + .isEmpty(); + } +}