Aias00 commented on code in PR #7264:
URL: https://github.com/apache/shenyu/pull/7264#discussion_r4110269073
##########
shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java:
##########
@@ -231,32 +227,26 @@ protected Mono<Void> handleRuleIfNull(final String
pluginName, final ServerWebEx
}
private Pair<Boolean, SelectorData> matchSelector(final ServerWebExchange
exchange, final Collection<SelectorData> selectors) {
- List<SelectorData> filterCollectors = selectors.stream()
- .filter(selector -> selector.getEnabled() &&
filterSelector(selector, exchange))
- .distinct()
- .collect(Collectors.toList());
- if (filterCollectors.size() > 1) {
- return Pair.of(Boolean.FALSE, manyMatchSelector(filterCollectors));
- } else {
- return Pair.of(Boolean.TRUE,
filterCollectors.stream().findFirst().orElse(null));
+ SelectorData best = null;
+ int bestSpecificity = -1;
+ boolean unique = true;
+ for (SelectorData selector : selectors) {
+ if (!selector.getEnabled() || !filterSelector(selector, exchange))
{
+ continue;
+ }
+ if (Objects.nonNull(best)) {
+ if (best.equals(selector)) {
+ continue;
+ }
+ unique = false;
+ }
+ int specificity = MatchModeEnum.match(selector.getMatchMode(),
MatchModeEnum.AND) ? CollectionUtils.size(selector.getConditionList()) : 0;
Review Comment:
Non-blocking, and I am raising it only because this oracle will outlive the
code it protects.
`SinglePassMatchingTest` computes the expected winner with its own
`min(Comparator.comparingInt(...).thenComparing(...))`. Since the old
`manyMatchSelector` / `manyMatchRule` are gone, nothing here compares new
behaviour against old behaviour - it proves the new loop agrees with a second
description of the same rule, over 100 shuffled orderings. That is real value
(ordering sensitivity is exactly what can break in a single-pass rewrite), but
it cannot catch a semantics change that both descriptions share.
Before approving I therefore brute-forced the two algorithms myself -
master's `distinct()` + `groupingBy` + `min(comparing(sort))` against this
single pass - on 200,000 randomised inputs covering enabled/disabled
candidates, AND/OR match modes, sort values 0..2, condition counts 0..3,
duplicates and shuffled orderings:
```
random cases compared: 200000
divergences: 0
```
So I am satisfied. The cheap way to keep that guarantee in-tree rather than
in a reviewer's scratch directory is one hand-written multi-candidate case in
the existing `AbstractShenyuPluginTest` that pins the outcome (max
AND-condition count wins, then lowest sort, first wins on ties), so a future
refactor cannot quietly move it.
Also note `CollectionUtils.size(...)` replaces `getConditionList().size()`,
so a null condition list now scores specificity 0 instead of throwing - an
improvement, but it changes behaviour under malformed data rather than being
pure cleanup.
--
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]
