Skip to content

Commit bec0beb

Browse files
yyyCodeyyyCode
authored
Merge pull request #57 from yyyCode/fix/idempotent-key-resolve-failopen
fix(idempotent): Key 解析改为全有或全无，杜绝键粒度静默变粗与解析异常穿透
2 parents fed3ca7 + 3e27c6e commit bec0beb

5 files changed

Lines changed: 176 additions & 62 deletions

File tree

‎OpenBlog-framework/framework-idempotent/src/main/java/com/yqz/openblog/idempotent/aspect/RepeatExecuteLimitAspect.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,18 @@ public RepeatExecuteLimitAspect(RepeatExecuteKeyResolver keyResolver,
5656
@Around("@annotation(repeatLimit)")
5757
public Object around(ProceedingJoinPoint pjp, RepeatExecuteLimit repeatLimit) throws Throwable {
5858
String name = repeatLimit.name();
59-
List<String> keys = keyResolver.resolve(pjp, repeatLimit.keys());
60-
// 防御：任一幂等 Key 为空则跳过幂等保护（记录警告），避免错误配置锁死业务
61-
if (name.isBlank() || keys.isEmpty() || keys.stream().anyMatch(String::isBlank)) {
62-
log.warn("[idempotent] 幂等 Key 为空，跳过幂等保护。name={}, keys={}", name, keys);
59+
if (name.isBlank()) {
60+
log.warn("[idempotent] 注解 name 为空，无法生成幂等 Key，跳过幂等保护。method={}",
61+
pjp.getSignature().toShortString());
6362
return pjp.proceed();
6463
}
64+
// 防御：Key 缺失或任一表达式解析不出值 → **整组作废**、跳过幂等保护（fail-open）。
65+
// 只丢掉解析失败的那一项会让 Key 粒度静默变粗、误伤正常请求，具体原因由 resolver 记 warn。
66+
Optional<List<String>> resolvedKeys = keyResolver.resolve(pjp, repeatLimit.keys());
67+
if (resolvedKeys.isEmpty()) {
68+
return pjp.proceed();
69+
}
70+
List<String> keys = resolvedKeys.get();
6571

6672
String lockKey = keyBuilder.lockKey(name, keys);
6773
String flagKey = keyBuilder.flagKey(name, keys);

‎OpenBlog-framework/framework-idempotent/src/main/java/com/yqz/openblog/idempotent/core/RepeatExecuteKeyResolver.java‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,31 @@
88
import org.springframework.core.DefaultParameterNameDiscoverer;
99
import org.springframework.core.ParameterNameDiscoverer;
1010
import org.springframework.expression.EvaluationContext;
11-
import org.springframework.expression.EvaluationException;
1211
import org.springframework.expression.ExpressionParser;
1312
import org.springframework.expression.spel.standard.SpelExpressionParser;
1413

1514
import java.lang.reflect.Method;
1615
import java.util.ArrayList;
1716
import java.util.List;
17+
import java.util.Optional;
18+
import java.util.Set;
19+
import java.util.concurrent.ConcurrentHashMap;
1820

1921
/**
2022
* 解析 @RepeatExecuteLimit#keys 中的 SpEL 表达式为实际字符串值。
2123
* 复用 Spring 标准 MethodBasedEvaluationContext + DefaultParameterNameDiscoverer 模式，
2224
* 依赖根 pom 的 &lt;parameters&gt;true&lt;/parameters&gt; 取到方法参数名。
2325
*
24-
* 防御语义：表达式解析失败（如中间对象为 null）只跳过该 Key 并记 warn，
25-
* 与切面"任一 Key 为空即跳过幂等保护"的策略一致——幂等组件绝不让业务请求 500。
26+
* <p>防御语义（fail-open，且**全有或全无**）：要么整组 Key 全部解析成功并返回，要么整组作废返回
27+
* {@link Optional#empty()}，由切面跳过幂等保护。两条铁律：
28+
* <ul>
29+
* <li><b>不丢单项</b>：绝不能"只丢掉解析不出来的那一个 Key、其余照用"。那样 Key 粒度会静默变粗
30+
* （如 articleId+uid+requestId 退化成 articleId+uid），把同一用户窗口内的正常第二次请求误判成重复——
31+
* 评论场景下表现为"新内容被吞掉、返回上一条的缓存结果"。</li>
32+
* <li><b>不抛异常</b>：语法错误抛 ParseException、求值错误抛 EvaluationException（二者没有共同父类），
33+
* 一并吞掉并告警，否则异常穿透切面导致业务 500，违反"幂等组件绝不让业务请求失败"的契约。</li>
34+
* </ul>
35+
* 求值结果为空（含 SpEL 求值为 null，如客户端未传 requestId）同样视为整组作废。
2636
*/
2737
public class RepeatExecuteKeyResolver {
2838

@@ -31,26 +41,55 @@ public class RepeatExecuteKeyResolver {
3141
private final ParameterNameDiscoverer nameDiscoverer = new DefaultParameterNameDiscoverer();
3242
private final ExpressionParser parser = new SpelExpressionParser();
3343

34-
public List<String> resolve(ProceedingJoinPoint pjp, String[] keys) {
35-
List<String> result = new ArrayList<>();
36-
if (keys == null || keys.length == 0) {
37-
return result;
38-
}
44+
/** 已告警过的位置，避免配置错误在高频请求下反复刷日志（内容上限 = 注解方法数 × keys 数，不会无限增长） */
45+
private final Set<String> warnedLocations = ConcurrentHashMap.newKeySet();
46+
47+
/**
48+
* @return 解析成功的 Key 列表；Key 缺失、表达式空白/非法/求值为空时返回 {@link Optional#empty()}（整组作废）
49+
*/
50+
public Optional<List<String>> resolve(ProceedingJoinPoint pjp, String[] keys) {
3951
MethodSignature signature = (MethodSignature) pjp.getSignature();
4052
Method method = signature.getMethod();
53+
String where = method.getDeclaringClass().getSimpleName() + "#" + method.getName();
54+
55+
if (keys == null || keys.length == 0) {
56+
warnOnce(where + " | <未声明 keys>", "未声明 keys", null);
57+
return Optional.empty();
58+
}
59+
4160
Object[] args = pjp.getArgs();
4261
EvaluationContext context = new MethodBasedEvaluationContext(null, method, args, nameDiscoverer);
62+
List<String> values = new ArrayList<>(keys.length);
4363
for (String key : keys) {
4464
if (key == null || key.isBlank()) {
45-
continue;
65+
warnOnce(where + " | <空白表达式>", "存在空白表达式", null);
66+
return Optional.empty();
4667
}
68+
Object value;
4769
try {
48-
Object value = parser.parseExpression(key).getValue(context);
49-
result.add(value == null ? "" : String.valueOf(value));
50-
} catch (EvaluationException e) {
51-
log.warn("[idempotent] SpEL 解析失败，跳过该幂等 Key。expr={}", key, e);
70+
value = parser.parseExpression(key).getValue(context);
71+
} catch (Exception e) { // ParseException 与 EvaluationException 都归这里，绝不上抛
72+
warnOnce(where + " | " + key, "表达式无法解析", e);
73+
return Optional.empty();
74+
}
75+
if (value == null || String.valueOf(value).isBlank()) {
76+
warnOnce(where + " | " + key, "表达式求值为空", null);
77+
return Optional.empty();
5278
}
79+
values.add(String.valueOf(value));
80+
}
81+
return Optional.of(values);
82+
}
83+
84+
/** 同一位置只告警一次：配置错误不应随请求量刷屏，但也必须看得见 */
85+
private void warnOnce(String location, String reason, Exception cause) {
86+
if (!warnedLocations.add(location)) {
87+
return;
88+
}
89+
if (cause == null) {
90+
log.warn("[idempotent] {}，本次跳过幂等保护（fail-open）。location={}", reason, location);
91+
} else {
92+
log.warn("[idempotent] {}，本次跳过幂等保护（fail-open）。location={}", reason, location, cause);
5393
}
54-
return result;
5594
}
5695
}

‎OpenBlog-framework/framework-idempotent/src/test/java/com/yqz/openblog/idempotent/aspect/RepeatExecuteLimitAspectTest.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,20 @@ public Result reply(Long commentId, Long uid, TestReq req) {
9191
public Result createNoWindow(Long articleId, Long uid, TestReq req) {
9292
return new Result("ok");
9393
}
94+
95+
/** keys 里故意写错一个属性名：模拟配置错误 */
96+
@RepeatExecuteLimit(name = "comment_create", keys = {"#req.artcleId", "#uid", "#req.requestId"},
97+
durationTime = 30, strategy = IdempotentStrategy.RETURN_SAME_RESULT)
98+
public Result createWithMisspelledKey(Long articleId, Long uid, TestReq req) {
99+
return new Result("ok");
100+
}
101+
102+
/** keys 里故意写成非法 SpEL：模拟语法错误 */
103+
@RepeatExecuteLimit(name = "comment_create", keys = {"#articleId +", "#uid"},
104+
durationTime = 30, strategy = IdempotentStrategy.RETURN_SAME_RESULT)
105+
public Result createWithMalformedKey(Long articleId, Long uid, TestReq req) {
106+
return new Result("ok");
107+
}
94108
}
95109

96110
private static final String FLAG_CREATE = "openblog:test:repeat:flag:comment_create:42:7:r1";
@@ -352,6 +366,36 @@ void blankKey_skipsIdempotency_andExecutes() throws Throwable {
352366
assertFalse(flagStore.isSuccess(FLAG_REPLY));
353367
}
354368

369+
@Test
370+
void misspelledKeyExpression_skipsIdempotency_andExecutes() throws Throwable {
371+
// 一个 Key 解析不出来 → 整组作废、跳过防重。
372+
// 若只是"丢掉解析失败的那一项"，Key 会退化成 (articleId, uid) 并照常防重，
373+
// 同一用户 30s 内的第二条正常评论会被判为重复（返回第一条的缓存结果）。
374+
Class<?>[] params = {Long.class, Long.class, TestReq.class};
375+
stubInvocation("createWithMisspelledKey", params,
376+
new Object[]{42L, 7L, reqWith("r1")}, Result.class, new Result("ok"));
377+
378+
Object out = aspect.around(pjp, annotationOf("createWithMisspelledKey", params));
379+
380+
assertEquals("ok", ((Result) out).text);
381+
verify(lockSupport, never()).tryLock(anyString());
382+
assertFalse(flagStore.isSuccess("openblog:test:repeat:flag:comment_create:42:7"));
383+
}
384+
385+
@Test
386+
void malformedKeyExpression_doesNotThrow_skipsIdempotency() throws Throwable {
387+
// 非法 SpEL 抛 ParseException（非 EvaluationException）：不能让它穿透切面变成业务 500
388+
Class<?>[] params = {Long.class, Long.class, TestReq.class};
389+
stubInvocation("createWithMalformedKey", params,
390+
new Object[]{42L, 7L, reqWith("r1")}, Result.class, new Result("ok"));
391+
392+
Object out = aspect.around(pjp, annotationOf("createWithMalformedKey", params));
393+
394+
assertEquals("ok", ((Result) out).text);
395+
verify(lockSupport, never()).tryLock(anyString());
396+
assertFalse(flagStore.isSuccess(FLAG_CREATE));
397+
}
398+
355399
@Test
356400
void concurrentSameKey_exactlyOneExecution_bothReturnSameResult() throws Throwable {
357401
Class<?>[] params = {Long.class, Long.class, TestReq.class};

‎OpenBlog-framework/framework-idempotent/src/test/java/com/yqz/openblog/idempotent/core/RepeatExecuteKeyResolverTest.java‎

Lines changed: 68 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import java.lang.reflect.Method;
88
import java.util.List;
9+
import java.util.Optional;
910

1011
import static org.junit.jupiter.api.Assertions.assertEquals;
12+
import static org.junit.jupiter.api.Assertions.assertTrue;
1113
import static org.mockito.Mockito.mock;
1214
import static org.mockito.Mockito.when;
1315

@@ -30,85 +32,107 @@ public void create(Long articleId, Long uid, TestReq req) {
3032
}
3133
}
3234

33-
@Test
34-
void resolvesSpelKeysFromMethodArgs() throws Exception {
35+
private ProceedingJoinPoint pjpWith(Object[] args) throws Exception {
3536
Method method = Target.class.getMethod("create", Long.class, Long.class, TestReq.class);
3637
MethodSignature sig = mock(MethodSignature.class);
3738
when(sig.getMethod()).thenReturn(method);
3839

3940
ProceedingJoinPoint pjp = mock(ProceedingJoinPoint.class);
4041
when(pjp.getSignature()).thenReturn(sig);
42+
when(pjp.getArgs()).thenReturn(args);
43+
return pjp;
44+
}
4145

46+
private TestReq reqWith(String requestId) {
4247
TestReq req = new TestReq();
43-
req.setRequestId("req-123");
44-
when(pjp.getArgs()).thenReturn(new Object[]{42L, 7L, req});
48+
req.setRequestId(requestId);
49+
return req;
50+
}
51+
52+
@Test
53+
void allKeysResolve_returnsValuesInOrder() throws Exception {
54+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
4555

4656
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
47-
List<String> keys = resolver.resolve(pjp, new String[]{"#articleId", "#uid", "#req.requestId"});
57+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#articleId", "#uid", "#req.requestId"});
4858

49-
assertEquals(List.of("42", "7", "req-123"), keys);
59+
assertEquals(Optional.of(List.of("42", "7", "req-123")), keys);
5060
}
5161

5262
@Test
53-
void blankExpressionsAreSkipped() throws Exception {
54-
Method method = Target.class.getMethod("create", Long.class, Long.class, TestReq.class);
55-
MethodSignature sig = mock(MethodSignature.class);
56-
when(sig.getMethod()).thenReturn(method);
57-
58-
ProceedingJoinPoint pjp = mock(ProceedingJoinPoint.class);
59-
when(pjp.getSignature()).thenReturn(sig);
60-
when(pjp.getArgs()).thenReturn(new Object[]{42L, 7L, new TestReq()});
63+
void nullIntermediateProperty_makesWholeKeySetUnusable() throws Exception {
64+
// req 为 null → #req.requestId 抛 EvaluationException。
65+
// 绝不能"丢掉这一项、只留 #uid"：那样 Key 会退化成 (articleId, uid)，
66+
// 把同一用户 30s 内的第二条正常评论误判成重复。
67+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, null});
6168

6269
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
63-
List<String> keys = resolver.resolve(pjp, new String[]{"", "#uid"});
70+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#req.requestId", "#uid"});
6471

65-
assertEquals(List.of("7"), keys);
72+
assertTrue(keys.isEmpty());
6673
}
6774

6875
@Test
69-
void nullIntermediateProperty_skipsThatKey() throws Exception {
70-
Method method = Target.class.getMethod("create", Long.class, Long.class, TestReq.class);
71-
MethodSignature sig = mock(MethodSignature.class);
72-
when(sig.getMethod()).thenReturn(method);
73-
74-
ProceedingJoinPoint pjp = mock(ProceedingJoinPoint.class);
75-
when(pjp.getSignature()).thenReturn(sig);
76-
when(pjp.getArgs()).thenReturn(new Object[]{42L, 7L, null});
76+
void misspelledProperty_makesWholeKeySetUnusable() throws Exception {
77+
// 对象上的属性名写错（#req.artcleId）→ EvaluationException。
78+
// 绝不能"丢掉这一项、只留 #uid"：那样 Key 会静默退化成更粗的粒度，误伤正常请求。
79+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
7780

7881
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
79-
List<String> keys = resolver.resolve(pjp, new String[]{"#req.requestId", "#uid"});
82+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#req.artcleId", "#uid"});
8083

81-
assertEquals(List.of("7"), keys);
84+
assertTrue(keys.isEmpty());
8285
}
8386

8487
@Test
85-
void nullOrEmptyKeysArray_returnsEmptyList() throws Exception {
86-
Method method = Target.class.getMethod("create", Long.class, Long.class, TestReq.class);
87-
MethodSignature sig = mock(MethodSignature.class);
88-
when(sig.getMethod()).thenReturn(method);
88+
void unknownVariable_makesWholeKeySetUnusable() throws Exception {
89+
// 变量名写错（#artcleId）：SpEL 解析为 null，同样必须整组作废
90+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
8991

90-
ProceedingJoinPoint pjp = mock(ProceedingJoinPoint.class);
91-
when(pjp.getSignature()).thenReturn(sig);
92-
when(pjp.getArgs()).thenReturn(new Object[]{42L, 7L, new TestReq()});
92+
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
93+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#artcleId", "#uid"});
94+
95+
assertTrue(keys.isEmpty());
96+
}
97+
98+
@Test
99+
void malformedExpression_doesNotThrow_makesKeySetUnusable() throws Exception {
100+
// 语法错误抛的是 ParseException（不是 EvaluationException）：
101+
// 必须兜住，否则异常穿透切面 → 业务请求 500，违反幂等组件 fail-open 契约
102+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
93103

94104
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
95-
assertEquals(List.of(), resolver.resolve(pjp, null));
96-
assertEquals(List.of(), resolver.resolve(pjp, new String[]{}));
105+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#articleId +", "#uid"});
106+
107+
assertTrue(keys.isEmpty());
97108
}
98109

99110
@Test
100-
void nullPropertyValue_resolvesToEmptyString() throws Exception {
101-
Method method = Target.class.getMethod("create", Long.class, Long.class, TestReq.class);
102-
MethodSignature sig = mock(MethodSignature.class);
103-
when(sig.getMethod()).thenReturn(method);
111+
void blankExpression_makesKeySetUnusable() throws Exception {
112+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
104113

105-
ProceedingJoinPoint pjp = mock(ProceedingJoinPoint.class);
106-
when(pjp.getSignature()).thenReturn(sig);
107-
when(pjp.getArgs()).thenReturn(new Object[]{42L, 7L, new TestReq()});
114+
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
115+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"", "#uid"});
116+
117+
assertTrue(keys.isEmpty());
118+
}
119+
120+
@Test
121+
void nullPropertyValue_makesKeySetUnusable() throws Exception {
122+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith(null)});
108123

109124
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
110-
List<String> keys = resolver.resolve(pjp, new String[]{"#req.requestId"});
125+
Optional<List<String>> keys = resolver.resolve(pjp, new String[]{"#req.requestId"});
111126

112-
assertEquals(List.of(""), keys);
127+
assertTrue(keys.isEmpty());
128+
}
129+
130+
@Test
131+
void nullOrEmptyKeysArray_isUnusable() throws Exception {
132+
ProceedingJoinPoint pjp = pjpWith(new Object[]{42L, 7L, reqWith("req-123")});
133+
134+
RepeatExecuteKeyResolver resolver = new RepeatExecuteKeyResolver();
135+
assertTrue(resolver.resolve(pjp, null).isEmpty());
136+
assertTrue(resolver.resolve(pjp, new String[]{}).isEmpty());
113137
}
114138
}

‎docs/superpowers/specs/2026-08-29-idempotent-framework-design.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ else: # CACHE_REJECT
117117

118118
**关键机制**：
119119
- **双重检测**：请求 A 执行完写 flag 并释放锁后，B 才获锁，若不做 ⑤，B 会再执行一次 → 重复。有 ⑤ 则 B 查 flag 已是 `success` → 拦截。
120+
- **Key 解析全有或全无**：`keys` 中任一表达式解析失败（拼写错误 / 中间对象为 null / 语法错误）或求值为空（如客户端未传 `requestId`），**整组 Key 作废**、本次跳过防重并告警，而不是"丢掉那一项、其余照用"——后者会让 Key 粒度静默变粗（退化成 `articleId+uid`），把同一用户窗口内的正常第二次请求误判成重复。`ParseException` 与 `EvaluationException` 一并吞掉，绝不让解析异常穿透切成业务 500。
120121
- **切面顺序**：幂等 `-11` 最外层 → `@Transactional` 最内层。`proceed()` 返回时事务已提交，⑦ 在事务提交后写标识，正常路径不会出现"标识写了但事务回滚"的不一致。
121122
- **写失败降级**：⑦ 写失败仅记日志，业务可继续但失去后续防重，可接受。
122123
- **业务异常**：`proceed()` 抛出的异常原样上抛，不写 flag，不吞异常。

0 commit comments

Comments
 (0)