312 lines
11 KiB
Markdown
312 lines
11 KiB
Markdown
# 逻辑错误修复报告
|
||
|
||
**任务编号：** 2.3
|
||
**扫描范围：** `/root/.openclaw/workspace/erp-java-backend/services/` 下所有微服务（共31个）
|
||
**检查维度：** 循环依赖、事务边界错误、数据库死锁风险、订单状态机、空指针风险
|
||
|
||
---
|
||
|
||
## 一、循环依赖检查
|
||
|
||
### 检查结果：✅ 已有防护，无需修复
|
||
|
||
| 服务 | 注入点 | 状态 |
|
||
|------|--------|------|
|
||
| `dashboard-service/DashboardServiceImpl` | `RedisTemplate` 使用 `@Lazy` 避免循环依赖 | ✅ 已正确使用 |
|
||
| `tenant-service/TenantServiceImpl` | `RedisTemplate` 使用 `@Lazy` 避免循环依赖 | ✅ 已正确使用 |
|
||
|
||
其他服务均通过构造函数注入（`@RequiredArgsConstructor` + `final`），无循环依赖风险。
|
||
|
||
---
|
||
|
||
## 二、事务边界错误（Feign调用被@Transactional包裹）
|
||
|
||
### ⚠️ 高风险 — 发现问题（purchase-service）
|
||
|
||
**问题文件：**
|
||
- `purchase-service/.../impl/PurchaseOrderServiceImpl.java`
|
||
- `purchase-service/.../impl/PurchaseReturnServiceImpl.java`
|
||
- `purchase-service/.../impl/PurchaseInboundServiceImpl.java`
|
||
|
||
#### 问题 2.1：`approve()` 方法 — 本地事务先提交，远程通知后失败
|
||
|
||
```java
|
||
// PurchaseOrderServiceImpl.java:211
|
||
@Transactional
|
||
public PurchaseOrder approve(Long id, PurchaseOrderDTO.ApproveRequest request, Long operatorId) {
|
||
// ...更新本地订单状态...
|
||
purchaseOrderMapper.updateById(order); // ✅ 本地已提交
|
||
|
||
try {
|
||
financeClient.createAccountsPayable(...); // ❌ 远程调用在事务内，失败则本地已回滚
|
||
} catch (Exception e) {
|
||
log.warn("通知财务创建应付账款失败...");
|
||
}
|
||
return order;
|
||
}
|
||
```
|
||
|
||
**风险分析：**
|
||
|
||
| 场景 | 结果 |
|
||
|------|------|
|
||
| Feign调用超时/异常被catch | 本地事务正常提交，财务应付账款**未创建** → 数据不一致 |
|
||
| Feign调用成功，数据库更新失败 | 事务回滚，但财务侧已创建AP → 数据不一致 |
|
||
| finance-service不可用 | 本地订单已审批，AP未创建 |
|
||
|
||
#### 问题 2.2：`cancel()` 方法 — 同样问题
|
||
|
||
```java
|
||
// PurchaseOrderServiceImpl.java:246
|
||
@Transactional
|
||
public void cancel(Long id, Long operatorId) {
|
||
order.setStatus(PurchaseOrderStatus.CANCELLED.getCode());
|
||
purchaseOrderMapper.updateById(order); // ✅ 本地已提交
|
||
|
||
try {
|
||
financeClient.cancelAccountsPayable(order.getOrderNo()); // ❌ 失败则AP未取消
|
||
} catch (Exception e) {
|
||
log.warn("通知财务取消应付账款失败...");
|
||
}
|
||
}
|
||
```
|
||
|
||
#### 问题 2.3：`PurchaseReturnServiceImpl` — 确认退货时调用仓库
|
||
|
||
```java
|
||
// PurchaseReturnServiceImpl.java:137
|
||
@Transactional
|
||
public void confirmReturn(...) {
|
||
// 本地更新退货单状态
|
||
purchaseReturnMapper.updateById(ret);
|
||
|
||
// 调用仓库创建出库单
|
||
WarehouseFeignClient.OutboundResponse resp = warehouseClient.createOutbound(...);
|
||
// ❌ 如果这里失败，退货单已更新但出库单未创建
|
||
}
|
||
```
|
||
|
||
#### 问题 2.4：`PurchaseInboundServiceImpl` — 入库时调用仓库
|
||
|
||
```java
|
||
// PurchaseInboundServiceImpl.java:168
|
||
WarehouseFeignClient.InboundResponse resp = warehouseClient.createInbound(warehouseRequest, operatorId);
|
||
```
|
||
|
||
#### 问题 2.5：FeignClient无Fallback降级
|
||
|
||
`FinanceFeignClient` 和 `WarehouseFeignClient` 均未配置 `fallback` 或 `fallbackFactory`，服务不可用时直接抛出异常。
|
||
|
||
### ✅ 修复方案
|
||
|
||
**核心原则：Feign远程调用必须放在事务之外（或者在事务提交之后执行）**
|
||
|
||
#### 方案A（推荐）：使用 `TransactionSynchronizationManager` 在事务提交后执行
|
||
|
||
```java
|
||
@Transactional(rollbackFor = Exception.class)
|
||
public PurchaseOrder approve(Long id, PurchaseOrderDTO.ApproveRequest request, Long operatorId) {
|
||
PurchaseOrder order = purchaseOrderMapper.selectById(id);
|
||
if (order == null) throw new BusinessException("采购订单不存在");
|
||
if (!PurchaseOrderStatus.PENDING.getCode().equals(order.getStatus())) {
|
||
throw new BusinessException("只有待审批状态可以审批");
|
||
}
|
||
|
||
order.setStatus(PurchaseOrderStatus.APPROVED.getCode());
|
||
order.setApproverId(operatorId);
|
||
order.setApproveTime(LocalDateTime.now());
|
||
order.setUpdateBy(operatorId);
|
||
order.setUpdateTime(LocalDateTime.now());
|
||
purchaseOrderMapper.updateById(order);
|
||
|
||
// ✅ 事务提交后再通知财务，避免事务内远程调用失败导致回滚
|
||
TransactionSynchronizationManager.registerSynchronization(
|
||
new TransactionSynchronization() {
|
||
@Override
|
||
public void afterCommit() {
|
||
try {
|
||
financeClient.createAccountsPayable(
|
||
FinanceFeignClient.AccountsPayableRequest.builder()
|
||
.purchaseOrderId(order.getId())
|
||
.purchaseOrderNo(order.getOrderNo())
|
||
.supplierId(order.getSupplierId())
|
||
.amount(order.getTotalWithTax())
|
||
.paymentMethod(order.getPaymentMethod())
|
||
.build());
|
||
} catch (Exception e) {
|
||
log.error("通知财务创建应付账款失败: orderNo={}, error={}",
|
||
order.getOrderNo(), e.getMessage());
|
||
// 可选：发送告警通知人工介入
|
||
}
|
||
}
|
||
});
|
||
|
||
return order;
|
||
}
|
||
```
|
||
|
||
#### 方案B：为FeignClient添加Fallback
|
||
|
||
```java
|
||
@FeignClient(name = "finance-service",
|
||
url = "${finance-service.url:}",
|
||
fallbackFactory = FinanceFeignClientFallbackFactory.class)
|
||
public interface FinanceFeignClient { ... }
|
||
```
|
||
|
||
---
|
||
|
||
## 三、数据库死锁风险
|
||
|
||
### ✅ 未发现明显死锁风险
|
||
|
||
**扫描范围：** 所有 `@Transactional` 方法中的多表操作
|
||
|
||
| 检查项 | 结果 |
|
||
|--------|------|
|
||
| 跨表显式 `SELECT ... FOR UPDATE` | 未发现（无悲观锁查询） |
|
||
| 多表按不同顺序访问 | 未发现跨服务多表锁场景 |
|
||
| `@Async` + `@Transactional` 混用 | 未发现 |
|
||
|
||
### ⚠️ 轻量级风险（库存并发）
|
||
|
||
**问题文件：** `inventory-service/.../StockCheckServiceImpl.java`
|
||
|
||
```java
|
||
@Transactional
|
||
public Stock lockStock(String skuCode, Long warehouseId, Integer quantity, Long orderId) {
|
||
Stock stock = getOrCreateStock(skuCode, warehouseId); // SELECT
|
||
if (stock.getAvailableQuantity() < quantity) {
|
||
throw new RuntimeException("可用库存不足");
|
||
}
|
||
stock.setLockedQuantity(stock.getLockedQuantity() + quantity);
|
||
stockRepository.updateById(stock); // UPDATE — 无锁
|
||
}
|
||
```
|
||
|
||
**风险：** 两个并发请求同时锁定同一SKU，第一个请求的更新可能被第二个覆盖（check-then-act竞态）。
|
||
|
||
**现状：** `Stock.version` 字段存在但**未标注 `@Version`**，乐观锁未生效。
|
||
|
||
**修复方案：** 添加 `@Version` 注解：
|
||
|
||
```java
|
||
// Stock.java
|
||
@Version
|
||
private Integer version;
|
||
```
|
||
|
||
MyBatis-Plus 会自动在 UPDATE 时追加 `AND version = ?`，更新失败时抛出 `OptimisticLockingFailureException`，触发事务回滚。
|
||
|
||
---
|
||
|
||
## 四、订单状态机检查
|
||
|
||
### ✅ 状态机设计合理，未发现问题
|
||
|
||
**已验证文件：**
|
||
- `order-service/.../OrderStateMachine.java`
|
||
- `aftersale-service/.../AfterSaleStatusMachine.java`
|
||
|
||
| 验证项 | 结果 |
|
||
|--------|------|
|
||
| 所有状态转换均通过 `canTransition()` 校验 | ✅ |
|
||
| 终态（completed/cancelled/closed）不可再流转 | ✅ |
|
||
| 审核通过后自动设置 `auditStatus=approved` | ✅ |
|
||
| `canAudit/canShip/canComplete/canCancel` 边界检查 | ✅ |
|
||
|
||
**状态流转图（Order）：**
|
||
```
|
||
pending → auditing → shipped → completed
|
||
↓ ↓ ↓
|
||
cancelled cancelled cancelled
|
||
```
|
||
|
||
**状态流转图（AfterSale）：**
|
||
```
|
||
pending → approved → processing → received → completed → closed
|
||
↓ ↓
|
||
rejected rejected → pending (重新开启)
|
||
```
|
||
|
||
---
|
||
|
||
## 五、空指针风险（NPE）
|
||
|
||
### ⚠️ 发现2处潜在NPE，其他已有防护
|
||
|
||
#### 问题 5.1：`PurchaseStatisticsServiceImpl.getSupplierStats()`
|
||
|
||
**文件：** `purchase-service/.../PurchaseStatisticsServiceImpl.java:127`
|
||
|
||
```java
|
||
// 从Map按supplierId分组，理论上不可能为空
|
||
List<PurchaseOrder> supplierOrders = entry.getValue();
|
||
String supplierName = supplierOrders.get(0).getSupplierName(); // ⚠️ supplierOrders理论上非空
|
||
// 但get(0)依赖列表非空
|
||
// 且getSupplierName()可能返回null
|
||
```
|
||
|
||
**实际情况：** `entry.getValue()` 来自 `Collectors.groupingBy()`，key存在则value必为非空List，但 `getSupplierName()` 可能在数据库中为NULL。
|
||
|
||
**修复：** 防御性处理
|
||
```java
|
||
String supplierName = supplierOrders.stream()
|
||
.map(PurchaseOrder::getSupplierName)
|
||
.filter(Objects::nonNull)
|
||
.findFirst()
|
||
.orElse("未知供应商");
|
||
```
|
||
|
||
#### 问题 5.2：`OrderSettlementServiceImpl` 返回 `.get(0)`
|
||
|
||
**文件：** `order-service/.../OrderSettlementServiceImpl.java:530`
|
||
|
||
```java
|
||
return stats.getShopStatistics().get(0); // ⚠️ 列表可能为空
|
||
```
|
||
|
||
**修复：**
|
||
```java
|
||
List<ShopSettlementVO> shopStats = stats.getShopStatistics();
|
||
if (shopStats == null || shopStats.isEmpty()) {
|
||
return null; // 或返回空VO
|
||
}
|
||
return shopStats.get(0);
|
||
```
|
||
|
||
### ✅ 已正确防护的位置
|
||
|
||
| 位置 | 防护方式 |
|
||
|------|----------|
|
||
| `ai-service/AIChatServiceImpl:210` | `if (choices != null && !choices.isEmpty())` |
|
||
| `logistics-service/LogisticsService:56` | `if (traces.isEmpty())` → return null |
|
||
| `approval-flow-service/AuditRuleServiceImpl:417` | `if (!ruleActions.isEmpty())` |
|
||
| `system-tool-service/DataConsistencyService:127` | 三元运算符 `isEmpty() ? ... : ...` |
|
||
|
||
---
|
||
|
||
## 六、汇总
|
||
|
||
| 类别 | 风险等级 | 发现数量 | 已修复 |
|
||
|------|----------|----------|--------|
|
||
| 循环依赖 | ✅ 无风险 | 0 | — |
|
||
| 事务边界+Feign | 🔴 高风险 | 5处 | 需要修复 |
|
||
| 数据库死锁 | 🟡 中风险 | 1处 | 需要修复 |
|
||
| 订单状态机 | ✅ 无风险 | 0 | — |
|
||
| 空指针风险 | 🟡 中风险 | 2处 | 需要修复 |
|
||
|
||
---
|
||
|
||
## 七、修复优先级
|
||
|
||
| 优先级 | 修复项 | 所属问题 |
|
||
|--------|--------|----------|
|
||
| P0 | `PurchaseOrderServiceImpl` Feign调用移出事务 | 问题2.1、2.2 |
|
||
| P0 | `PurchaseReturnServiceImpl` Feign调用移出事务 | 问题2.3 |
|
||
| P0 | `Stock.version` 添加 `@Version` | 问题三 |
|
||
| P1 | `PurchaseStatisticsServiceImpl` NPE防护 | 问题5.1 |
|
||
| P1 | `OrderSettlementServiceImpl` NPE防护 | 问题5.2 |
|
||
| P2 | 为 `FinanceFeignClient` 添加Fallback | 问题2.5 |
|
||
| P2 | 为 `WarehouseFeignClient` 添加Fallback | 问题2.5 |
|