RockteMQ-AI commented on code in PR #2197:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2197#discussion_r3784110847
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/MetricsService.java:
##########
@@ -188,13 +197,20 @@ private BigDecimal parseStepMillis(String step) {
Matcher matcher = DURATION_PART_PATTERN.matcher(value);
BigDecimal millis = BigDecimal.ZERO;
int position = 0;
+ int previousUnitOrder = -1;
while (matcher.find()) {
if (matcher.start() != position) {
throw badRequest("Metric query step is invalid");
}
Review Comment:
Good improvement — enforcing canonical unit ordering (e.g. `1h30m` is valid,
`30m1h` is rejected) prevents ambiguous duration expressions. The `UNIT_ORDER`
map is clean.
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/MetricsService.java:
##########
@@ -40,7 +40,7 @@ public class MetricsService {
private static final long MAX_RANGE_SECONDS = 31L * 24 * 60 * 60;
private static final long MAX_SAMPLE_POINTS = 11_000L;
private static final Pattern NUMBER_PATTERN =
Pattern.compile("\\d+(?:\\.\\d+)?");
- private static final Pattern DURATION_PART_PATTERN =
Pattern.compile("(\\d+(?:\\.\\d+)?)(ms|s|m|h|d|w|y)");
+ private static final Pattern DURATION_PART_PATTERN =
Pattern.compile("(\\d+)(ms|s|m|h|d|w|y)");
Review Comment:
The regex change from `(\d+(?:\.\d+)?)` to `(\d+)` means fractional
durations like `1.5h` or `2.5s` are now rejected. This is a **breaking change**
for any existing callers that relied on fractional values. The tests confirm
this is intentional — just flagging it for awareness.
--
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]
