HY-love-sleep commented on PR #7166:
URL: https://github.com/apache/shenyu/pull/7166#issuecomment-5787498903
Thanks — all three points are real, and the first one is a runtime bug
rather than a style issue. Fixed in `021431b01`.
**1. `setIfAbsent(..., 0L, ...)` throws at runtime → the value is `"0"`, and
the raw type is gone.**
Reproduced before touching the code, outside the mocks:
```java
RedisSerializer raw = new StringRedisSerializer();
raw.serialize(Long.valueOf(0L)); // ClassCastException: class
java.lang.Long cannot be cast to class java.lang.String
raw.serialize("0"); // [48]
```
The template the handler builds is `ReactiveRedisTemplate<String, String>`,
so the value goes through `StringRedisSerializer`'s bridge method, exactly as
you described. The counter is now created with the String `"0"`.
I also removed the cause of the compile-time silence: `doExecute`,
`isAllowed` and `recordTokensUsage` declare `ReactiveRedisTemplate<String,
String>` instead of the raw type, so passing a `Long` to `setIfAbsent` no
longer compiles. Typing `isAllowed` forced one adjacent change: its
`defaultIfEmpty(0L)` on a `Mono<String>` is `defaultIfEmpty("0")` now (the read
path is unaffected — `Long.parseLong` gets the same value it got before), and
the redundant `.toString()` before it is gone.
**2. Tests.** The mocks are `ReactiveRedisTemplate<String, String>` /
`ReactiveValueOperations<String, String>` now, so the compiler rejects a wrong
value type, and the three cases are covered:
- counter created: the window comes from `setIfAbsent`, and neither
`getExpire` nor `expire` is called;
- existing counter with a window: it keeps it, `expire` is never called (the
regression this PR fixes);
- existing counter without any expiration: `expire` is called once.
I also added a small test that pushes the value through the real
`stringSerializationContext()` value pair, since you suggested exercising the
serializer instead of only mocking around it — that is the assertion that would
have caught the `Long`.
**3. A counter that exists without a TTL.** Handled the way you suggested,
and deliberately not with an unconditional `expire`, which would have brought
the sliding window back: the `false` branch reads `getExpire` and only sets the
expiration when it is absent (negative or zero).
Rebased onto master (it was 4 commits behind).
```
./mvnw -pl shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-token-limiter -am
test
```
BUILD SUCCESS, checkstyle clean, `AiTokenLimiterPluginTest` 11 tests, module
15 tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
