Jelajahi Sumber

fix: harden OMG attempt conflict reads

qmj 2 minggu lalu
induk
melakukan
682ca8805b

+ 3 - 3
ruoyi-admin/src/main/java/com/ruoyi/app/omgpay/OmgPaymentCreateService.java

@@ -42,7 +42,7 @@ public class OmgPaymentCreateService {
         String normalizedOrderId = normalizeOrderId(orderId);
         OmgPaymentOrderSnapshot order = attempts.selectOrderForUpdate(normalizedOrderId);
         validateOrder(userId, normalizedOrderId, order);
-        if (attempts.selectActiveCreatedByDdId(normalizedOrderId) != null) {
+        if (attempts.selectActiveCreatedByDdIdForUpdate(normalizedOrderId) != null) {
             throw business(PAYMENT_ATTEMPT_EXISTS, order.getStoreId());
         }
         PosStoreOmg credential = credentials.getEnabledCredential(order.getStoreId());
@@ -70,10 +70,10 @@ public class OmgPaymentCreateService {
                 return new OmgPaymentCreateOutcome(response, attempt.getId(), order.getDdId(), userId,
                         order.getStoreId(), order.getAmount(), maskMerchantTradeNo(merchantTradeNo));
             } catch (DuplicateKeyException error) {
-                if (attempts.selectActiveCreatedByDdId(order.getDdId()) != null) {
+                if (attempts.selectActiveCreatedByDdIdForUpdate(order.getDdId()) != null) {
                     throw business(PAYMENT_ATTEMPT_EXISTS, order.getStoreId());
                 }
-                boolean tradeNumberCollision = attempts.selectByMerchantTradeNo(merchantTradeNo) != null;
+                boolean tradeNumberCollision = attempts.selectByMerchantTradeNoForUpdate(merchantTradeNo) != null;
                 if (!tradeNumberCollision || number == MAX_TRADE_NUMBER_ATTEMPTS) {
                     throw business(PAYMENT_CREATION_FAILED, order.getStoreId());
                 }

+ 2 - 0
ruoyi-admin/src/main/resources/i18n/messages.properties

@@ -220,8 +220,10 @@ rider.operation.role.required=只有骑手可以操作配送订单
 
 omg.payment.ddid.required=OMG 支付订单号不能为空
 omg.payment.merchantTradeNo.required=OMG MerchantTradeNo 不能为空
+omg.payment.merchantTradeNo.invalid=OMG MerchantTradeNo 必须为最多 20 位大写英数字
 omg.payment.storeId.required=OMG 门店 ID 不能为空
 omg.payment.merchantId.required=OMG 商户号不能为空
+omg.payment.merchantId.invalid=OMG 商户号必须为最多 10 位英数字
 omg.payment.amount.invalid=OMG 支付金额必须大于 0
 omg.pay.auth.required=请先登录
 omg.pay.order.required=订单号不能为空

+ 2 - 0
ruoyi-admin/src/main/resources/i18n/messages_en_US.properties

@@ -223,8 +223,10 @@ line.pay.legacy.endpoint.disabled=Use the dedicated order operation endpoint for
 rider.operation.role.required=Only riders can operate delivery orders
 omg.payment.ddid.required=OMG payment order number is required
 omg.payment.merchantTradeNo.required=OMG MerchantTradeNo is required
+omg.payment.merchantTradeNo.invalid=OMG MerchantTradeNo must be at most 20 uppercase letters or digits
 omg.payment.storeId.required=OMG store id is required
 omg.payment.merchantId.required=OMG merchant id is required
+omg.payment.merchantId.invalid=OMG merchant id must be at most 10 letters or digits
 omg.payment.amount.invalid=OMG payment amount must be greater than 0
 omg.pay.auth.required=Please sign in first
 omg.pay.order.required=The order number is required

+ 2 - 0
ruoyi-admin/src/main/resources/i18n/messages_vi.properties

@@ -223,8 +223,10 @@ line.pay.legacy.endpoint.disabled=Hãy sử dụng API thao tác đơn hàng chu
 rider.operation.role.required=Chỉ tài xế giao hàng mới có thể thao tác đơn giao hàng
 omg.payment.ddid.required=Mã đơn hàng thanh toán OMG là bắt buộc
 omg.payment.merchantTradeNo.required=OMG MerchantTradeNo là bắt buộc
+omg.payment.merchantTradeNo.invalid=OMG MerchantTradeNo phải có tối đa 20 chữ cái in hoa hoặc chữ số
 omg.payment.storeId.required=ID cửa hàng OMG là bắt buộc
 omg.payment.merchantId.required=Mã thương nhân OMG là bắt buộc
+omg.payment.merchantId.invalid=Mã thương nhân OMG phải có tối đa 10 chữ cái hoặc chữ số
 omg.payment.amount.invalid=Số tiền thanh toán OMG phải lớn hơn 0
 omg.pay.auth.required=Vui lòng đăng nhập trước
 omg.pay.order.required=Vui lòng nhập mã đơn hàng

+ 2 - 0
ruoyi-admin/src/main/resources/i18n/messages_zh_CN.properties

@@ -224,8 +224,10 @@ rider.operation.role.required=只有骑手可以操作配送订单
 
 omg.payment.ddid.required=OMG 支付订单号不能为空
 omg.payment.merchantTradeNo.required=OMG MerchantTradeNo 不能为空
+omg.payment.merchantTradeNo.invalid=OMG MerchantTradeNo 必须为最多 20 位大写英数字
 omg.payment.storeId.required=OMG 门店 ID 不能为空
 omg.payment.merchantId.required=OMG 商户号不能为空
+omg.payment.merchantId.invalid=OMG 商户号必须为最多 10 位英数字
 omg.payment.amount.invalid=OMG 支付金额必须大于 0
 omg.pay.auth.required=请先登录
 omg.pay.order.required=订单号不能为空

+ 2 - 0
ruoyi-admin/src/main/resources/i18n/messages_zh_TW.properties

@@ -224,8 +224,10 @@ rider.operation.role.required=只有騎手可以操作配送訂單
 
 omg.payment.ddid.required=OMG 支付訂單號不能為空
 omg.payment.merchantTradeNo.required=OMG MerchantTradeNo 不能為空
+omg.payment.merchantTradeNo.invalid=OMG MerchantTradeNo 必須為最多 20 位大寫英數字
 omg.payment.storeId.required=OMG 門店 ID 不能為空
 omg.payment.merchantId.required=OMG 商戶號不能為空
+omg.payment.merchantId.invalid=OMG 商戶號必須為最多 10 位英數字
 omg.payment.amount.invalid=OMG 支付金額必須大於 0
 omg.pay.auth.required=請先登入
 omg.pay.order.required=訂單號不能為空

+ 6 - 5
ruoyi-admin/src/test/java/com/ruoyi/app/omgpay/OmgPaymentCreateServiceTest.java

@@ -54,7 +54,7 @@ class OmgPaymentCreateServiceTest {
     @Test
     void rejectsExistingCreatedAttemptBeforeCredentialOrFormWork() {
         when(attempts.selectOrderForUpdate("DD-1")).thenReturn(payableOrder());
-        when(attempts.selectActiveCreatedByDdId("DD-1")).thenReturn(new OmgPaymentAttempt());
+        when(attempts.selectActiveCreatedByDdIdForUpdate("DD-1")).thenReturn(new OmgPaymentAttempt());
         OmgPaymentBusinessException error = assertThrows(OmgPaymentBusinessException.class,
                 () -> service.create(5L, "DD-1"));
         assertEquals(PAYMENT_ATTEMPT_EXISTS, error.getCode());
@@ -82,7 +82,7 @@ class OmgPaymentCreateServiceTest {
         assertTrue(outcome.response().formFields().containsKey("CheckMacValue"));
         var orderOfCalls = inOrder(attempts, credentials, generator, formFactory);
         orderOfCalls.verify(attempts).selectOrderForUpdate("DD-1");
-        orderOfCalls.verify(attempts).selectActiveCreatedByDdId("DD-1");
+        orderOfCalls.verify(attempts).selectActiveCreatedByDdIdForUpdate("DD-1");
         orderOfCalls.verify(credentials).getEnabledCredential(77L);
         orderOfCalls.verify(generator).generate();
         orderOfCalls.verify(formFactory).create("DD-1", 100, "1000031", "KEY", "IV",
@@ -99,7 +99,7 @@ class OmgPaymentCreateServiceTest {
                 .thenReturn(new OmgPaymentForm("stage", Map.of()));
         when(attempts.createCreated(anyString(), anyString(), anyLong(), anyString(), anyInt()))
                 .thenThrow(new DuplicateKeyException("collision"));
-        when(attempts.selectActiveCreatedByDdId("DD-1")).thenReturn(null, new OmgPaymentAttempt());
+        when(attempts.selectActiveCreatedByDdIdForUpdate("DD-1")).thenReturn(null, new OmgPaymentAttempt());
 
         OmgPaymentBusinessException error = assertThrows(OmgPaymentBusinessException.class,
                 () -> service.create(5L, "DD-1"));
@@ -118,7 +118,8 @@ class OmgPaymentCreateServiceTest {
         inserted.setId(11L);
         when(attempts.createCreated(anyString(), anyString(), anyLong(), anyString(), anyInt()))
                 .thenThrow(new DuplicateKeyException("trade collision")).thenReturn(inserted);
-        when(attempts.selectByMerchantTradeNo("OMG11111111111111111")).thenReturn(new OmgPaymentAttempt());
+        when(attempts.selectByMerchantTradeNoForUpdate("OMG11111111111111111"))
+                .thenReturn(new OmgPaymentAttempt());
 
         assertEquals(11L, service.create(5L, "DD-1").attemptId());
         verify(generator, times(2)).generate();
@@ -135,7 +136,7 @@ class OmgPaymentCreateServiceTest {
                 .thenReturn(new OmgPaymentForm("stage", Map.of()));
         when(attempts.createCreated(anyString(), anyString(), anyLong(), anyString(), anyInt()))
                 .thenThrow(new DuplicateKeyException("trade collision"));
-        when(attempts.selectByMerchantTradeNo(anyString())).thenReturn(new OmgPaymentAttempt());
+        when(attempts.selectByMerchantTradeNoForUpdate(anyString())).thenReturn(new OmgPaymentAttempt());
 
         OmgPaymentBusinessException error = assertThrows(OmgPaymentBusinessException.class,
                 () -> service.create(5L, "DD-1"));

+ 3 - 0
ruoyi-system/src/main/java/com/ruoyi/system/omgpay/domain/OmgPaymentAttempt.java

@@ -9,6 +9,9 @@ import lombok.Data;
 
 import java.util.Date;
 
+/**
+ * Local append-only record of an OMG checkout form created for an order.
+ */
 @Data
 @TableName("pos_order_omg_attempt")
 public class OmgPaymentAttempt {

+ 3 - 0
ruoyi-system/src/main/java/com/ruoyi/system/omgpay/domain/OmgPaymentOrderSnapshot.java

@@ -2,6 +2,9 @@ package com.ruoyi.system.omgpay.domain;
 
 import lombok.Data;
 
+/**
+ * Server-owned order facts locked while an OMG payment attempt is created.
+ */
 @Data
 public class OmgPaymentOrderSnapshot {
     private Long id;

+ 10 - 2
ruoyi-system/src/main/java/com/ruoyi/system/omgpay/mapper/OmgPaymentAttemptMapper.java

@@ -4,9 +4,17 @@ import com.ruoyi.system.omgpay.domain.OmgPaymentAttempt;
 import com.ruoyi.system.omgpay.domain.OmgPaymentOrderSnapshot;
 import org.apache.ibatis.annotations.Param;
 
+/**
+ * Persistence boundary for locking order facts and appending OMG payment attempts.
+ */
 public interface OmgPaymentAttemptMapper {
     OmgPaymentOrderSnapshot selectOrderForUpdate(@Param("ddId") String ddId);
-    OmgPaymentAttempt selectActiveCreatedByDdId(@Param("ddId") String ddId);
-    OmgPaymentAttempt selectByMerchantTradeNo(@Param("merchantTradeNo") String merchantTradeNo);
+
+    /** Current read used both before creation and after an active-order unique-key conflict. */
+    OmgPaymentAttempt selectActiveCreatedByDdIdForUpdate(@Param("ddId") String ddId);
+
+    /** Current read used after an insert conflict to classify a committed trade-number collision. */
+    OmgPaymentAttempt selectByMerchantTradeNoForUpdate(@Param("merchantTradeNo") String merchantTradeNo);
+
     int insertCreated(OmgPaymentAttempt row);
 }

+ 10 - 2
ruoyi-system/src/main/java/com/ruoyi/system/omgpay/service/IOmgPaymentAttemptService.java

@@ -3,10 +3,18 @@ package com.ruoyi.system.omgpay.service;
 import com.ruoyi.system.omgpay.domain.OmgPaymentAttempt;
 import com.ruoyi.system.omgpay.domain.OmgPaymentOrderSnapshot;
 
+/**
+ * Transaction-participating operations needed to create one OMG payment attempt.
+ */
 public interface IOmgPaymentAttemptService {
     OmgPaymentOrderSnapshot selectOrderForUpdate(String ddId);
-    OmgPaymentAttempt selectActiveCreatedByDdId(String ddId);
-    OmgPaymentAttempt selectByMerchantTradeNo(String merchantTradeNo);
+
+    /** Current read that makes committed active attempts visible during duplicate classification. */
+    OmgPaymentAttempt selectActiveCreatedByDdIdForUpdate(String ddId);
+
+    /** Current read that classifies a committed MerchantTradeNo unique-key collision. */
+    OmgPaymentAttempt selectByMerchantTradeNoForUpdate(String merchantTradeNo);
+
     OmgPaymentAttempt createCreated(String ddId, String merchantTradeNo, Long storeId,
                                     String merchantId, Integer amount);
 }

+ 14 - 4
ruoyi-system/src/main/java/com/ruoyi/system/omgpay/service/impl/OmgPaymentAttemptServiceImpl.java

@@ -11,6 +11,9 @@ import org.springframework.stereotype.Service;
 
 import java.util.Date;
 
+/**
+ * Validates and persists OMG attempt facts inside the caller's creation transaction.
+ */
 @Service
 public class OmgPaymentAttemptServiceImpl implements IOmgPaymentAttemptService {
 
@@ -28,13 +31,14 @@ public class OmgPaymentAttemptServiceImpl implements IOmgPaymentAttemptService {
     }
 
     @Override
-    public OmgPaymentAttempt selectActiveCreatedByDdId(String ddId) {
-        return StrUtil.isBlank(ddId) ? null : mapper.selectActiveCreatedByDdId(ddId.trim());
+    public OmgPaymentAttempt selectActiveCreatedByDdIdForUpdate(String ddId) {
+        return StrUtil.isBlank(ddId) ? null : mapper.selectActiveCreatedByDdIdForUpdate(ddId.trim());
     }
 
     @Override
-    public OmgPaymentAttempt selectByMerchantTradeNo(String merchantTradeNo) {
-        return StrUtil.isBlank(merchantTradeNo) ? null : mapper.selectByMerchantTradeNo(merchantTradeNo.trim());
+    public OmgPaymentAttempt selectByMerchantTradeNoForUpdate(String merchantTradeNo) {
+        return StrUtil.isBlank(merchantTradeNo)
+                ? null : mapper.selectByMerchantTradeNoForUpdate(merchantTradeNo.trim());
     }
 
     @Override
@@ -63,12 +67,18 @@ public class OmgPaymentAttemptServiceImpl implements IOmgPaymentAttemptService {
         if (StrUtil.isBlank(merchantTradeNo)) {
             throw new ServiceException(MessageUtils.message("omg.payment.merchantTradeNo.required"));
         }
+        if (!merchantTradeNo.trim().matches("[A-Z0-9]{1,20}")) {
+            throw new ServiceException(MessageUtils.message("omg.payment.merchantTradeNo.invalid"));
+        }
         if (storeId == null) {
             throw new ServiceException(MessageUtils.message("omg.payment.storeId.required"));
         }
         if (StrUtil.isBlank(merchantId)) {
             throw new ServiceException(MessageUtils.message("omg.payment.merchantId.required"));
         }
+        if (!merchantId.trim().matches("[A-Za-z0-9]{1,10}")) {
+            throw new ServiceException(MessageUtils.message("omg.payment.merchantId.invalid"));
+        }
         if (amount == null || amount <= 0) {
             throw new ServiceException(MessageUtils.message("omg.payment.amount.invalid"));
         }

+ 5 - 4
ruoyi-system/src/main/resources/mapper/omgpay/OmgPaymentAttemptMapper.xml

@@ -17,20 +17,21 @@
         FROM pos_order WHERE dd_id = #{ddId} LIMIT 1 FOR UPDATE
     </select>
 
-    <select id="selectActiveCreatedByDdId"
+    <!-- Locking current reads see attempts committed by a transaction that won a unique-key race. -->
+    <select id="selectActiveCreatedByDdIdForUpdate"
             resultType="com.ruoyi.system.omgpay.domain.OmgPaymentAttempt">
         SELECT <include refid="attemptColumns"/>
         FROM pos_order_omg_attempt
         WHERE active_dd_id = #{ddId}
-        LIMIT 1
+        LIMIT 1 FOR UPDATE
     </select>
 
-    <select id="selectByMerchantTradeNo"
+    <select id="selectByMerchantTradeNoForUpdate"
             resultType="com.ruoyi.system.omgpay.domain.OmgPaymentAttempt">
         SELECT <include refid="attemptColumns"/>
         FROM pos_order_omg_attempt
         WHERE merchant_trade_no = #{merchantTradeNo}
-        LIMIT 1
+        LIMIT 1 FOR UPDATE
     </select>
 
     <!-- active_dd_id is a DB-generated key invariant, so Java inserts only the created facts. -->

+ 9 - 0
ruoyi-system/src/test/java/com/ruoyi/system/omgpay/mapper/OmgPaymentAttemptMapperContractTest.java

@@ -24,11 +24,20 @@ class OmgPaymentAttemptMapperContractTest {
         assertTrue(xml.contains("FROM pos_order"));
         assertTrue(xml.contains("FOR UPDATE"));
         assertTrue(xml.contains("FROM pos_order_omg_attempt"));
+        assertTrue(xml.contains("selectActiveCreatedByDdIdForUpdate"));
+        assertTrue(xml.contains("selectByMerchantTradeNoForUpdate"));
+        assertTrue(countOccurrences(xml, "LIMIT 1 FOR UPDATE") >= 2);
         assertFalse(xml.contains("pos_order_omg_payment"));
         assertFalse(xml.contains("pos_order_omg_refund"));
         assertTrue(sql.contains("UNIQUE KEY uk_omg_attempt_trade_no"));
         assertTrue(sql.contains("UNIQUE KEY uk_omg_attempt_active_dd"));
         assertTrue(sql.contains("IF(attempt_status = 0, dd_id, NULL)"));
+        assertTrue(sql.contains("merchant_trade_no VARCHAR(20) CHARACTER SET ascii COLLATE ascii_bin"));
+        assertTrue(sql.contains("merchant_id VARCHAR(10) CHARACTER SET ascii COLLATE ascii_bin"));
+    }
+
+    private int countOccurrences(String source, String value) {
+        return (source.length() - source.replace(value, "").length()) / value.length();
     }
 
     private String loadMapperXml() throws IOException {

+ 2 - 0
ruoyi-system/src/test/java/com/ruoyi/system/omgpay/service/OmgPaymentAttemptI18nContractTest.java

@@ -19,8 +19,10 @@ class OmgPaymentAttemptI18nContractTest {
     private static final List<String> OMG_KEYS = List.of(
             "omg.payment.ddid.required",
             "omg.payment.merchantTradeNo.required",
+            "omg.payment.merchantTradeNo.invalid",
             "omg.payment.storeId.required",
             "omg.payment.merchantId.required",
+            "omg.payment.merchantId.invalid",
             "omg.payment.amount.invalid"
     );
 

+ 21 - 0
ruoyi-system/src/test/java/com/ruoyi/system/omgpay/service/OmgPaymentAttemptServiceTest.java

@@ -40,6 +40,12 @@ class OmgPaymentAttemptServiceTest {
                 () -> service.createCreated("DD-1", "   ", 10L, "M1", 100));
     }
 
+    @Test
+    void createCreatedRejectsInvalidMerchantTradeNoUsingMessageUtils() {
+        assertLocalizedValidationFailure("omg.payment.merchantTradeNo.invalid",
+                () -> service.createCreated("DD-1", "omg-invalid", 10L, "M1", 100));
+    }
+
     @Test
     void createCreatedRejectsNullStoreIdUsingMessageUtils() {
         assertLocalizedValidationFailure("omg.payment.storeId.required",
@@ -52,6 +58,21 @@ class OmgPaymentAttemptServiceTest {
                 () -> service.createCreated("DD-1", "OMG123", 10L, "   ", 100));
     }
 
+    @Test
+    void createCreatedRejectsInvalidMerchantIdUsingMessageUtils() {
+        assertLocalizedValidationFailure("omg.payment.merchantId.invalid",
+                () -> service.createCreated("DD-1", "OMG123", 10L, "merchant-too-long", 100));
+    }
+
+    @Test
+    void currentReadQueriesUseTrimmedKeys() {
+        service.selectActiveCreatedByDdIdForUpdate(" DD-1 ");
+        service.selectByMerchantTradeNoForUpdate(" OMG123 ");
+
+        org.mockito.Mockito.verify(mapper).selectActiveCreatedByDdIdForUpdate("DD-1");
+        org.mockito.Mockito.verify(mapper).selectByMerchantTradeNoForUpdate("OMG123");
+    }
+
     @Test
     void createCreatedRejectsNonPositiveAmountUsingMessageUtils() {
         assertLocalizedValidationFailure("omg.payment.amount.invalid",

+ 2 - 2
updatesql/sql.md

@@ -684,9 +684,9 @@ GROUP BY dd_id HAVING COUNT(*)>1;
 CREATE TABLE pos_order_omg_attempt (
   id BIGINT NOT NULL AUTO_INCREMENT,
   dd_id VARCHAR(64) NOT NULL,
-  merchant_trade_no VARCHAR(64) CHARACTER SET ascii COLLATE ascii_bin NOT NULL,
+  merchant_trade_no VARCHAR(20) CHARACTER SET ascii COLLATE ascii_bin NOT NULL,
   store_id BIGINT NOT NULL,
-  merchant_id VARCHAR(32) CHARACTER SET ascii COLLATE ascii_bin NOT NULL,
+  merchant_id VARCHAR(10) CHARACTER SET ascii COLLATE ascii_bin NOT NULL,
   amount INT NOT NULL,
   attempt_status TINYINT NOT NULL COMMENT '0=CREATED',
   active_dd_id VARCHAR(64)