test(return): cover isolation and concurrency; fix receipt and packing races
This commit is contained in:
+3
-1
@@ -18,5 +18,7 @@ public final class PdaReturnReceiptRequests {
|
||||
@Size(max = 128) String cartonMark,
|
||||
@NotEmpty @Size(max = 500) List<@Valid Item> items,
|
||||
@NotBlank @Size(max = 255) String idempotencyKey) { }
|
||||
public record Item(@NotBlank @Size(max = 128) String skuCode, @Positive long receivedQuantity) { }
|
||||
public record Item(@NotBlank @Size(max = 128) String skuCode,
|
||||
@com.fasterxml.jackson.databind.annotation.JsonDeserialize(using = ReturnQuantityDeserializer.class)
|
||||
@Positive long receivedQuantity) { }
|
||||
}
|
||||
|
||||
+3
-1
@@ -41,5 +41,7 @@ public final class ReturnInventoryRequests {
|
||||
@NotNull @Positive BigDecimal weight,
|
||||
@NotBlank @Size(max = 255) String idempotencyKey) { }
|
||||
|
||||
public record ValidItem(@NotBlank @Size(max = 128) String skuCode, @Positive long quantity) { }
|
||||
public record ValidItem(@NotBlank @Size(max = 128) String skuCode,
|
||||
@com.fasterxml.jackson.databind.annotation.JsonDeserialize(using = ReturnQuantityDeserializer.class)
|
||||
@Positive long quantity) { }
|
||||
}
|
||||
|
||||
+1
@@ -22,6 +22,7 @@ public final class ReturnOrderRequests {
|
||||
@NotEmpty @Size(max = 500) List<@Valid Item> items) { }
|
||||
public record Item(@NotBlank @Size(max = 128) String skuCode,
|
||||
@Size(max = 255) String productNameZh,
|
||||
@com.fasterxml.jackson.databind.annotation.JsonDeserialize(using = ReturnQuantityDeserializer.class)
|
||||
@Positive long expectedQuantity) { }
|
||||
public record Query(@Min(1) long page, @Min(1) @Max(200) long size, String returnNo,
|
||||
String trackingNo, String skuCode, String status) {
|
||||
|
||||
+24
@@ -0,0 +1,24 @@
|
||||
package com.baiyue.lastleg.returnreceipt.dto;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonParser;
|
||||
import com.fasterxml.jackson.core.JsonToken;
|
||||
import com.fasterxml.jackson.databind.DeserializationContext;
|
||||
import com.fasterxml.jackson.databind.JsonDeserializer;
|
||||
import com.fasterxml.jackson.databind.JsonMappingException;
|
||||
import java.io.IOException;
|
||||
|
||||
/** Return quantities are whole units; never truncate fractional JSON input. */
|
||||
public final class ReturnQuantityDeserializer extends JsonDeserializer<Long> {
|
||||
@Override
|
||||
public Long deserialize(JsonParser parser, DeserializationContext context) throws IOException {
|
||||
if (!parser.hasToken(JsonToken.VALUE_NUMBER_INT)) {
|
||||
return context.reportInputMismatch(Long.class, "退货数量必须为整数");
|
||||
}
|
||||
return parser.getLongValue();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Long getNullValue(DeserializationContext context) throws JsonMappingException {
|
||||
return context.reportInputMismatch(Long.class, "退货数量不能为空");
|
||||
}
|
||||
}
|
||||
+2
@@ -12,4 +12,6 @@ import java.util.List;
|
||||
public interface ReturnInventoryCartonItemMapper extends BaseMapper<ReturnInventoryCartonItem> {
|
||||
@Select("SELECT * FROM ll_return_inventory_carton_item WHERE carton_id=#{cartonId} ORDER BY sku_code")
|
||||
List<ReturnInventoryCartonItem> listCarton(@Param("cartonId") long cartonId);
|
||||
@Select("SELECT * FROM ll_return_inventory_carton_item WHERE carton_id=#{cartonId} ORDER BY sku_code FOR UPDATE")
|
||||
List<ReturnInventoryCartonItem> lockCarton(@Param("cartonId") long cartonId);
|
||||
}
|
||||
|
||||
+2
@@ -12,6 +12,8 @@ import java.util.List;
|
||||
public interface ReturnReceiptMapper extends BaseMapper<ReturnReceipt> {
|
||||
@Select("SELECT * FROM ll_return_receipt WHERE operator_id=#{operatorId} AND idempotency_key=#{key} LIMIT 1")
|
||||
ReturnReceipt findKey(@Param("operatorId") String operatorId, @Param("key") String key);
|
||||
@Select("SELECT * FROM ll_return_receipt WHERE operator_id=#{operatorId} AND idempotency_key=#{key} LIMIT 1 FOR UPDATE")
|
||||
ReturnReceipt lockKey(@Param("operatorId") String operatorId, @Param("key") String key);
|
||||
@Select("SELECT * FROM ll_return_receipt WHERE return_order_id=#{orderId} ORDER BY signed_at DESC,id DESC")
|
||||
List<ReturnReceipt> listOrder(@Param("orderId") Long orderId);
|
||||
}
|
||||
|
||||
+12
-3
@@ -237,11 +237,19 @@ public class ReturnInventoryService {
|
||||
if (selectedOrders.stream().anyMatch(order -> !customerId.equals(order.getCustomerId()))) throw conflict("不同客户的退货单禁止合箱");
|
||||
List<ReturnInventoryCarton> sources = new ArrayList<>();
|
||||
Map<Long, ReturnOrder> sourceOwners = new LinkedHashMap<>();
|
||||
Map<String, ReturnOrder> candidateOwners = new java.util.TreeMap<>();
|
||||
for (ReturnOrder order : selectedOrders) for (ReturnReceipt receipt : receiptMapper.listOrder(order.getId())) {
|
||||
ReturnInventoryCarton carton = cartons.findReceipt(receipt.getId());
|
||||
if (carton != null && AVAILABLE.equals(carton.getStatus())) {
|
||||
ReturnInventoryCarton locked = cartons.lockMark(carton.getCartonMark());
|
||||
sources.add(locked); sourceOwners.put(locked.getId(), order);
|
||||
candidateOwners.put(carton.getCartonMark(), order);
|
||||
}
|
||||
}
|
||||
// All requests acquire source locks in the same order. The earlier
|
||||
// candidate query is only a snapshot: availability must be checked again.
|
||||
for (Map.Entry<String, ReturnOrder> candidate : candidateOwners.entrySet()) {
|
||||
ReturnInventoryCarton locked = cartons.lockMark(candidate.getKey());
|
||||
if (locked != null && AVAILABLE.equals(locked.getStatus())) {
|
||||
sources.add(locked); sourceOwners.put(locked.getId(), candidate.getValue());
|
||||
}
|
||||
}
|
||||
if (sources.isEmpty()) throw conflict("所选退货单没有可合箱的库存箱");
|
||||
@@ -258,7 +266,8 @@ public class ReturnInventoryService {
|
||||
Map<Long, Map<String, Long>> attachedByOrder = new LinkedHashMap<>();
|
||||
long total = 0;
|
||||
for (ReturnInventoryCarton source : sources) {
|
||||
List<ReturnInventoryCartonItem> sourceItems = cartonItems.listCarton(source.getId());
|
||||
// Read committed quantities under the source lock, never a stale MVCC snapshot.
|
||||
List<ReturnInventoryCartonItem> sourceItems = cartonItems.lockCarton(source.getId());
|
||||
long sourceTotal = 0; int sourceTypes = 0;
|
||||
for (ReturnInventoryCartonItem item : sourceItems) {
|
||||
String sku = item.getSkuCode().toUpperCase(java.util.Locale.ROOT);
|
||||
|
||||
+8
-1
@@ -268,6 +268,13 @@ public class ReturnReceiptService {
|
||||
if (order == null || !String.valueOf(identity.overseasWarehouseId()).equals(order.getOverseasWarehouseId())) {
|
||||
throw notFound("当前绑定仓没有该退货单");
|
||||
}
|
||||
// Waiting for the order lock may allow the same request to commit. Use a
|
||||
// current read, not the repeatable-read snapshot established by findKey.
|
||||
ReturnReceipt concurrentReplay = receipts.lockKey(identity.operatorKey(), key);
|
||||
if (concurrentReplay != null) {
|
||||
if (!requestHash.equals(concurrentReplay.getRequestHash())) throw conflict("幂等键已用于不同的签收请求");
|
||||
return confirmed(order, concurrentReplay, COMPLETED.equals(order.getStatus()));
|
||||
}
|
||||
if (!Set.of(PENDING_RECEIPT, PARTIALLY_RECEIVED).contains(order.getStatus())) {
|
||||
throw conflict("仅待签收或签收中的退货单允许 PDA 签收");
|
||||
}
|
||||
@@ -287,7 +294,7 @@ public class ReturnReceiptService {
|
||||
receipt.setSignedAt(now); receipt.setCreatedAt(now);
|
||||
try { receipts.insert(receipt); }
|
||||
catch (DuplicateKeyException concurrent) {
|
||||
ReturnReceipt existing = receipts.findKey(identity.operatorKey(), key);
|
||||
ReturnReceipt existing = receipts.lockKey(identity.operatorKey(), key);
|
||||
if (existing != null) return replayReceipt(existing, requestHash);
|
||||
throw concurrent;
|
||||
}
|
||||
|
||||
+2
-1
@@ -64,7 +64,8 @@ class RelabelDocumentServiceTest {
|
||||
service.downloadFile("101","RL-1","TOTAL",null,99L);
|
||||
verify(files).download("101","OVERSEAS_RELABEL_TOTAL_DOCUMENT","11",99L);
|
||||
assertThatThrownBy(()->service.downloadFile("101","RL-1","TOTAL",null,98L)).hasMessageContaining("未绑定");
|
||||
assertThatThrownBy(()->service.downloadFile("101","RL-1","other",null,99L)).hasMessageContaining("资料范围");
|
||||
assertThatThrownBy(()->service.downloadFile("101","RL-1","other",null,99L))
|
||||
.isInstanceOfSatisfying(ResponseStatusException.class, error -> assertThat(error.getStatusCode().value()).isEqualTo(400));
|
||||
assertThatThrownBy(()->service.downloadFile("101","RL-1","ITEM",null,99L)).hasMessageContaining("指定明细");
|
||||
verifyNoMoreInteractions(files);
|
||||
}
|
||||
|
||||
+29
@@ -0,0 +1,29 @@
|
||||
package com.baiyue.lastleg.returnreceipt.dto;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
class ReturnQuantityDeserializerTest {
|
||||
private final ObjectMapper mapper = new ObjectMapper();
|
||||
|
||||
@Test
|
||||
void allThreeReturnQuantityInputsRejectFractionalOrCoercedValues() {
|
||||
for (String value : new String[] { "1.5", "0.5", "1.0", "1e1", "null", "\"1\"", "true", "9223372036854775808" }) {
|
||||
assertThrows(JsonProcessingException.class, () -> mapper.readValue(
|
||||
"{\"skuCode\":\"A\",\"expectedQuantity\":" + value + "}", ReturnOrderRequests.Item.class), value);
|
||||
assertThrows(JsonProcessingException.class, () -> mapper.readValue(
|
||||
"{\"skuCode\":\"A\",\"receivedQuantity\":" + value + "}", PdaReturnReceiptRequests.Item.class), value);
|
||||
assertThrows(JsonProcessingException.class, () -> mapper.readValue(
|
||||
"{\"skuCode\":\"A\",\"quantity\":" + value + "}", ReturnInventoryRequests.ValidItem.class), value);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void integerQuantitiesRemainExact() throws Exception {
|
||||
assertEquals(2L, mapper.readValue("{\"skuCode\":\"A\",\"expectedQuantity\":2}", ReturnOrderRequests.Item.class).expectedQuantity());
|
||||
assertEquals(2L, mapper.readValue("{\"skuCode\":\"A\",\"receivedQuantity\":2}", PdaReturnReceiptRequests.Item.class).receivedQuantity());
|
||||
assertEquals(Long.MAX_VALUE, mapper.readValue("{\"skuCode\":\"A\",\"quantity\":9223372036854775807}", ReturnInventoryRequests.ValidItem.class).quantity());
|
||||
}
|
||||
}
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
package com.baiyue.lastleg.returnreceipt.service;
|
||||
|
||||
import com.baiyue.lastleg.returnreceipt.mapper.ReturnReceiptMapper;
|
||||
import com.baiyue.lastleg.returnreceipt.mapper.ReturnInventoryCartonItemMapper;
|
||||
import org.apache.ibatis.annotations.Select;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
class ReturnCurrentReadContractTest {
|
||||
@Test
|
||||
void concurrentReplayAndInventoryReadBypassRepeatableReadSnapshots() throws Exception {
|
||||
String receiptSql = String.join(" ", ReturnReceiptMapper.class.getMethod("lockKey", String.class, String.class).getAnnotation(Select.class).value());
|
||||
String itemSql = String.join(" ", ReturnInventoryCartonItemMapper.class.getMethod("lockCarton", long.class).getAnnotation(Select.class).value());
|
||||
assertTrue(receiptSql.endsWith("FOR UPDATE"));
|
||||
assertTrue(itemSql.endsWith("FOR UPDATE"));
|
||||
assertTrue(itemSql.contains("ORDER BY sku_code"));
|
||||
}
|
||||
}
|
||||
+63
-1
@@ -31,6 +31,10 @@ import static org.mockito.ArgumentMatchers.anyLong;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
class ReturnInventoryServiceTest {
|
||||
@@ -132,7 +136,7 @@ class ReturnInventoryServiceTest {
|
||||
when(receipts.listOrder(11L)).thenReturn(List.of(receipt));
|
||||
when(cartons.findReceipt(21L)).thenReturn(source);
|
||||
when(cartons.lockMark("RC-SOURCE")).thenReturn(source);
|
||||
when(cartonItems.listCarton(31L)).thenReturn(List.of(skuA, skuB));
|
||||
when(cartonItems.lockCarton(31L)).thenReturn(List.of(skuA, skuB));
|
||||
when(numbers.next(anyString(), anyString(), anyLong())).thenReturn("RC-MERGED");
|
||||
when(balances.adjustCartonQuantity(anyString(), anyString(), anyString(), anyLong())).thenReturn(1);
|
||||
|
||||
@@ -152,6 +156,64 @@ class ReturnInventoryServiceTest {
|
||||
assertThat(target.getValue().getSourceReturnNo()).isEqualTo("RT-1");
|
||||
}
|
||||
|
||||
@Test
|
||||
void sourceEmptiedByAnotherMergeCannotBeConsumedAgainAfterLockWait() {
|
||||
ReturnInventoryCarton locked = prepareConcurrentSource();
|
||||
locked.setStatus(ReturnInventoryService.MERGED); locked.setTotalQuantity(0L);
|
||||
var error = assertThrows(ResponseStatusException.class, () -> service.merge(
|
||||
new PdaWarehouseIdentity("operator-2", "操作员2", 42L), mergeRequest(1)));
|
||||
assertThat(error.getStatusCode().value()).isEqualTo(409);
|
||||
verify(cartonItems, never()).lockCarton(anyLong());
|
||||
verify(cartons, never()).insert(any(ReturnInventoryCarton.class));
|
||||
verify(balances, never()).adjustCartonQuantity(anyString(), anyString(), anyString(), anyLong());
|
||||
}
|
||||
|
||||
@Test
|
||||
void partiallyConsumedSourceUsesCurrentLockedSkuQuantityNotEarlierSnapshot() {
|
||||
ReturnInventoryCarton locked = prepareConcurrentSource();
|
||||
locked.setTotalQuantity(1L);
|
||||
when(cartonItems.lockCarton(31L)).thenReturn(List.of(item(41L, "SKU-A", 1L)));
|
||||
var error = assertThrows(ResponseStatusException.class, () -> service.merge(
|
||||
new PdaWarehouseIdentity("operator-2", "操作员2", 42L), mergeRequest(2)));
|
||||
assertThat(error.getStatusCode().value()).isEqualTo(409);
|
||||
verify(cartonItems).lockCarton(31L);
|
||||
verify(cartonItems, never()).listCarton(31L);
|
||||
verify(cartons, never()).insert(any(ReturnInventoryCarton.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void fullyConsumedSourceIsMergedAndSkuCartonCountRemainsOne() {
|
||||
ReturnInventoryCarton locked = prepareConcurrentSource();
|
||||
when(cartonItems.lockCarton(31L)).thenReturn(List.of(item(41L, "SKU-A", 2L)));
|
||||
when(numbers.next(anyString(), anyString(), anyLong())).thenReturn("RC-TARGET");
|
||||
when(balances.adjustCartonQuantity(anyString(), anyString(), anyString(), anyLong())).thenReturn(1);
|
||||
service.merge(new PdaWarehouseIdentity("operator-2", "操作员2", 42L), mergeRequest(2));
|
||||
assertThat(locked.getTotalQuantity()).isZero();
|
||||
assertThat(locked.getSkuTypes()).isZero();
|
||||
assertThat(locked.getStatus()).isEqualTo(ReturnInventoryService.MERGED);
|
||||
verify(cartonItems).deleteById(41L);
|
||||
verify(balances).adjustCartonQuantity("10001", "42", "SKU-A", 0L);
|
||||
}
|
||||
|
||||
private ReturnInventoryCarton prepareConcurrentSource() {
|
||||
ReturnReceipt receipt = new ReturnReceipt(); receipt.setId(21L);
|
||||
ReturnInventoryCarton snapshot = new ReturnInventoryCarton();
|
||||
snapshot.setId(31L); snapshot.setCartonMark("RC-SOURCE"); snapshot.setStatus(ReturnInventoryService.AVAILABLE);
|
||||
ReturnInventoryCarton locked = new ReturnInventoryCarton();
|
||||
locked.setId(31L); locked.setCartonMark("RC-SOURCE"); locked.setStatus(ReturnInventoryService.AVAILABLE);
|
||||
locked.setCustomerId("10001"); locked.setOverseasWarehouseId("42"); locked.setSourceBatchNo("RB-1"); locked.setSourceReturnNo("RT-1");
|
||||
when(orders.findByNos("42", List.of("RT-1"))).thenReturn(List.of(completedOrder()));
|
||||
when(receipts.listOrder(11L)).thenReturn(List.of(receipt));
|
||||
when(cartons.findReceipt(21L)).thenReturn(snapshot);
|
||||
when(cartons.lockMark("RC-SOURCE")).thenReturn(locked);
|
||||
return locked;
|
||||
}
|
||||
|
||||
private ReturnInventoryRequests.Merge mergeRequest(long quantity) {
|
||||
return new ReturnInventoryRequests.Merge(List.of("RT-1"), List.of(new ReturnInventoryRequests.ValidItem("SKU-A", quantity)),
|
||||
BigDecimal.ONE, BigDecimal.ONE, BigDecimal.ONE, BigDecimal.ONE, "concurrent-key");
|
||||
}
|
||||
|
||||
private ReturnOrder completedOrder() {
|
||||
ReturnOrder order = new ReturnOrder();
|
||||
order.setId(11L);
|
||||
|
||||
+32
@@ -244,6 +244,38 @@ class ReturnReceiptServiceTest {
|
||||
when(inventory.createReceiptCarton(eq(order), any(ReturnReceipt.class), any(), any())).thenReturn(carton);
|
||||
}
|
||||
|
||||
@Test
|
||||
void sameKeyCommittedWhileWaitingForOrderLockReplaysCompletedReceipt() throws Exception {
|
||||
var request = confirm("RETURN_NO", null, null, "concurrent-key", List.of(new PdaReturnReceiptRequests.Item("SKU-A", 1)));
|
||||
ReturnOrder completed = order("RT-1", "10", ReturnReceiptService.COMPLETED, 1, 1);
|
||||
ReturnReceipt committed = new ReturnReceipt();
|
||||
committed.setReceiptNo("RR-COMMITTED"); committed.setReceiptBatchNo("RB-COMMITTED"); committed.setCartonMark("RC-COMMITTED");
|
||||
committed.setRequestHash(java.util.HexFormat.of().formatHex(java.security.MessageDigest.getInstance("SHA-256")
|
||||
.digest(new ObjectMapper().writeValueAsBytes(request))));
|
||||
when(orders.lockNo("RT-1")).thenReturn(completed);
|
||||
when(receipts.lockKey("operator-1", "concurrent-key")).thenReturn(committed);
|
||||
|
||||
var result = service.confirm(identity, request);
|
||||
|
||||
assertEquals("RR-COMMITTED", result.receiptNo());
|
||||
assertEquals("RB-COMMITTED", result.receiptBatchNo());
|
||||
assertEquals(1, result.receivedQuantity());
|
||||
assertTrue(result.fullyMatched());
|
||||
verify(receipts, never()).insert(any(ReturnReceipt.class));
|
||||
verifyNoInteractions(inventory, batches, orderItems);
|
||||
}
|
||||
|
||||
@Test
|
||||
void concurrentSameKeyWithDifferentPayloadStillConflicts() {
|
||||
when(orders.lockNo("RT-1")).thenReturn(order("RT-1", "10", ReturnReceiptService.COMPLETED, 1, 1));
|
||||
ReturnReceipt committed = new ReturnReceipt(); committed.setRequestHash("different-content");
|
||||
when(receipts.lockKey("operator-1", "concurrent-key")).thenReturn(committed);
|
||||
var error = assertThrows(ResponseStatusException.class, () -> service.confirm(identity,
|
||||
confirm("RETURN_NO", null, null, "concurrent-key", List.of(new PdaReturnReceiptRequests.Item("SKU-A", 1)))));
|
||||
assertEquals(409, error.getStatusCode().value());
|
||||
verifyNoInteractions(inventory, batches, orderItems);
|
||||
}
|
||||
|
||||
private static ReturnOrder order(String returnNo, String warehouseId, String status, long expected, long received) {
|
||||
ReturnOrder order = new ReturnOrder();
|
||||
order.setId(1L);
|
||||
|
||||
Reference in New Issue
Block a user