Copilot commented on code in PR #6336:
URL: https://github.com/apache/shenyu/pull/6336#discussion_r3195505452
##########
shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java:
##########
@@ -154,43 +173,52 @@ private RuleData obtainRuleDataCacheIfEnabled(final
String path) {
}
private void cacheSelectorData(final String path, final SelectorData
selectorData) {
- if (Boolean.FALSE.equals(selectorMatchConfig.getCache().getEnabled())
|| Objects.isNull(selectorData)
- || Boolean.TRUE.equals(selectorData.getMatchRestful())) {
- return;
- }
- int initialCapacity =
selectorMatchConfig.getCache().getInitialCapacity();
- long maximumSize = selectorMatchConfig.getCache().getMaximumSize();
- if (StringUtils.isBlank(selectorData.getId())) {
- MatchDataCache.getInstance().cacheSelectorData(path, selectorData,
initialCapacity, maximumSize);
- return;
- }
- List<ConditionData> conditionList = selectorData.getConditionList();
- if (CollectionUtils.isNotEmpty(conditionList)) {
- boolean isUriCondition = conditionList.stream().allMatch(v ->
URI_CONDITION_TYPE.equals(v.getParamType()));
- if (isUriCondition) {
- MatchDataCache.getInstance().cacheSelectorData(path,
selectorData, initialCapacity, maximumSize);
- }
- }
+ cacheMatchData(
+ path,
+ selectorData,
+ selectorMatchConfig.getCache(),
+ SelectorData::getId,
+ SelectorData::getMatchRestful,
+ SelectorData::getConditionList,
+ MatchDataCache.getInstance()::cacheSelectorData);
}
-
+
private void cacheRuleData(final String path, final RuleData ruleData) {
- // if the ruleCache is disabled or rule data is null, not cache rule
data.
- if (Boolean.FALSE.equals(ruleMatchConfig.getCache().getEnabled()) ||
Objects.isNull(ruleData)
- || Boolean.TRUE.equals(ruleData.getMatchRestful())) {
+ cacheMatchData(
+ path,
+ ruleData,
+ ruleMatchConfig.getCache(),
+ RuleData::getId,
+ RuleData::getMatchRestful,
+ RuleData::getConditionDataList,
+ MatchDataCache.getInstance()::cacheRuleData);
+ }
+
+ private <T> void cacheMatchData(final String path,
+ final T data,
+ final ShenyuConfig.MatchCacheConfig
cacheConfig,
+ final Function<T, String> idGetter,
+ final Function<T, Boolean> restfulGetter,
+ final Function<T, List<ConditionData>>
conditionGetter,
+ final CacheWriter<T> cacheWriter) {
+ // if the cache is disabled or data is null or matchRestful, do not
cache.
+ if (Boolean.FALSE.equals(cacheConfig.getEnabled())
+ || Objects.isNull(data)
+ || Boolean.TRUE.equals(restfulGetter.apply(data))) {
return;
}
- int initialCapacity = ruleMatchConfig.getCache().getInitialCapacity();
- long maximumSize = ruleMatchConfig.getCache().getMaximumSize();
- if (StringUtils.isBlank(ruleData.getId())) {
- MatchDataCache.getInstance().cacheRuleData(path, ruleData,
initialCapacity, maximumSize);
+ final int initialCapacity = cacheConfig.getInitialCapacity();
+ final long maximumSize = cacheConfig.getMaximumSize();
+ // empty-id sentinel: always cached to short-circuit the next miss.
+ if (StringUtils.isBlank(idGetter.apply(data))) {
Review Comment:
`cacheMatchData` treats a blank id (null/""/whitespace) as the
negative-cache sentinel (`StringUtils.isBlank(...)`). However,
`MatchDataCache.removeEmptySelectorData/removeEmptyRuleData` only remove
entries where `getId()` is `null`. This mismatch can leave stale negative-cache
entries behind if an empty-string id is ever cached, and (with the new
`StringUtils.isBlank(ruleData.getId())` check) could incorrectly short-circuit
matching until eviction. Consider aligning the sentinel definition across
caching, lookup, and eviction (e.g., use `StringUtils.isBlank(getId())` in the
removal helpers, or restrict sentinel caching to `null` ids only).
--
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]
