This is an automated email from the ASF dual-hosted git repository.
xiaoyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new be2c3ff1c [type:fix] fix trie match error (#4515)
be2c3ff1c is described below
commit be2c3ff1ce7cc721c08e54a0c3cdb1b9b89f6381
Author: moremind <[email protected]>
AuthorDate: Fri Mar 31 11:06:37 2023 +0800
[type:fix] fix trie match error (#4515)
* [type:fix] fix trie match error
* [type:fix] fix trie match error
* [type:fix] fix trie match error
* [type:fix] fix trie match error
* [type:fix] fix trie remove error
* [type:fix] fix trie remove error
* [type:fix] fix trie remove error
---------
Co-authored-by: xiaoyu <[email protected]>
---
.../shenyu/admin/service/impl/RuleServiceImpl.java | 23 +++++++++++++--
.../src/main/resources/application.yml | 1 +
.../apache/shenyu/common/config/ShenyuConfig.java | 23 ++++++++++++++-
.../shenyu/plugin/base/AbstractShenyuPlugin.java | 8 +++++
.../base/cache/CommonPluginDataSubscriber.java | 12 +++++---
.../apache/shenyu/plugin/base/trie/ShenyuTrie.java | 34 +++++++++-------------
.../plugin/base/trie/ShenyuTrieRuleListener.java | 6 +++-
.../base/cache/CommonPluginDataSubscriberTest.java | 2 +-
.../web/controller/LocalPluginControllerTest.java | 1 +
9 files changed, 81 insertions(+), 29 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
index 6a01c9a46..27e1c048d 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
@@ -54,6 +54,7 @@ import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -154,8 +155,26 @@ public class RuleServiceImpl implements RuleService {
final int ruleCount = ruleMapper.updateSelective(ruleDO);
// need old data for cleaning
- final List<RuleConditionDO> beforeRuleCondition =
ruleConditionMapper.selectByQuery(new RuleConditionQuery(ruleDO.getId()));
-
+ List<RuleConditionDO> beforeRuleCondition =
ruleConditionMapper.selectByQuery(new RuleConditionQuery(ruleDO.getId()));
+ List<RuleConditionDTO> beforRuleCondition =
beforeRuleCondition.stream().map(ruleConditionDO ->
+ RuleConditionDTO.builder()
+ .ruleId(ruleConditionDO.getRuleId())
+ .operator(ruleConditionDO.getOperator())
+ .paramName(ruleConditionDO.getParamName())
+ .paramType(ruleConditionDO.getParamType())
+ .paramValue(ruleConditionDO.getParamValue())
+ .build()).collect(Collectors.toList());
+ List<RuleConditionDTO> currentRuleCondition =
ruleDTO.getRuleConditions().stream().map(ruleConditionDTO ->
+ RuleConditionDTO.builder()
+ .ruleId(ruleConditionDTO.getRuleId())
+ .operator(ruleConditionDTO.getOperator())
+ .paramName(ruleConditionDTO.getParamName())
+ .paramType(ruleConditionDTO.getParamType())
+ .paramValue(ruleConditionDTO.getParamValue())
+ .build()).collect(Collectors.toList());
+ if (CollectionUtils.isEqualCollection(beforRuleCondition,
currentRuleCondition)) {
+ beforeRuleCondition = Collections.emptyList();
+ }
//delete rule condition then add
ruleConditionMapper.deleteByQuery(new
RuleConditionQuery(ruleDO.getId()));
diff --git a/shenyu-bootstrap/src/main/resources/application.yml
b/shenyu-bootstrap/src/main/resources/application.yml
index b120d8430..d667e4c10 100644
--- a/shenyu-bootstrap/src/main/resources/application.yml
+++ b/shenyu-bootstrap/src/main/resources/application.yml
@@ -75,6 +75,7 @@ shenyu:
initialCapacity: 10000 # initial capacity in cache
maximumSize: 10000 # max size in cache
trie:
+ enabled: true
childrenSize: 10000
pathVariableSize: 1000
pathRuleCacheSize: 1000
diff --git
a/shenyu-common/src/main/java/org/apache/shenyu/common/config/ShenyuConfig.java
b/shenyu-common/src/main/java/org/apache/shenyu/common/config/ShenyuConfig.java
index 42401f124..1bce57657 100644
---
a/shenyu-common/src/main/java/org/apache/shenyu/common/config/ShenyuConfig.java
+++
b/shenyu-common/src/main/java/org/apache/shenyu/common/config/ShenyuConfig.java
@@ -1827,6 +1827,9 @@ public class ShenyuConfig {
* shenyu trie config.
*/
public static class ShenyuTrieConfig {
+
+ private Boolean enabled = Boolean.TRUE;
+
private Long childrenSize = 10000L;
private Long pathRuleCacheSize = 1000L;
@@ -1838,7 +1841,25 @@ public class ShenyuConfig {
* @see TrieMatchModeEvent
*/
private String matchMode =
TrieMatchModeEvent.ANT_PATH_MATCH.getMatchMode();
-
+
+ /**
+ * get match enabled.
+ *
+ * @return Boolean
+ */
+ public Boolean getEnabled() {
+ return enabled;
+ }
+
+ /**
+ * set match enabled.
+ *
+ * @param enabled enabled
+ */
+ public void setEnabled(final Boolean enabled) {
+ this.enabled = enabled;
+ }
+
/**
* get trie children size.
*
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java
index 29ff186dc..7c0d31348 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/AbstractShenyuPlugin.java
@@ -60,6 +60,8 @@ public abstract class AbstractShenyuPlugin implements
ShenyuPlugin {
private ShenyuConfig.MatchCache matchCacheConfig;
private ShenyuTrie trie;
+
+ private ShenyuConfig.ShenyuTrieConfig trieConfig;
/**
* this is Template Method child has Implement your own logic.
@@ -167,6 +169,9 @@ public abstract class AbstractShenyuPlugin implements
ShenyuPlugin {
if (Objects.isNull(trie)) {
trie = SpringBeanUtils.getInstance().getBean(ShenyuTrie.class);
}
+ if (Objects.isNull(trieConfig)) {
+ trieConfig =
SpringBeanUtils.getInstance().getBean(ShenyuConfig.class).getTrie();
+ }
}
private void cacheSelectorData(final String path, final SelectorData
selectorData) {
@@ -310,6 +315,9 @@ public abstract class AbstractShenyuPlugin implements
ShenyuPlugin {
}
private RuleData trieMatchRule(final ServerWebExchange exchange, final
SelectorData selectorData, final String path) {
+ if (!trieConfig.getEnabled()) {
+ return null;
+ }
RuleData ruleData = null;
ShenyuTrieNode shenyuTrieNode = trie.match(path, selectorData.getId());
if (Objects.nonNull(shenyuTrieNode)) {
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriber.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriber.java
index 5174271bb..49d49060f 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriber.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriber.java
@@ -207,11 +207,15 @@ public class CommonPluginDataSubscriber implements
PluginDataSubscriber {
Optional.ofNullable(handlerMap.get(ruleData.getPluginName()))
.ifPresent(handler -> handler.handlerRule(ruleData));
MatchDataCache.getInstance().removeRuleData(ruleData.getPluginName());
- if
(CollectionUtils.isEmpty(ruleData.getBeforeConditionDataList())) {
- eventPublisher.publishEvent(new
RuleTrieEvent(RuleTrieEventEnum.INSERT, ruleData));
+ if (ruleData.getEnabled()) {
+ if
(CollectionUtils.isEmpty(ruleData.getBeforeConditionDataList())) {
+ eventPublisher.publishEvent(new
RuleTrieEvent(RuleTrieEventEnum.INSERT, ruleData));
+ } else {
+ // if rule data has before condition, update trie
+ eventPublisher.publishEvent(new
RuleTrieEvent(RuleTrieEventEnum.UPDATE, ruleData));
+ }
} else {
- // if rule data has before condition, update trie
- eventPublisher.publishEvent(new
RuleTrieEvent(RuleTrieEventEnum.UPDATE, ruleData));
+ eventPublisher.publishEvent(new
RuleTrieEvent(RuleTrieEventEnum.REMOVE, ruleData));
}
}
}
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrie.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrie.java
index 13e47002a..1d8727449 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrie.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrie.java
@@ -26,10 +26,12 @@ import org.apache.shenyu.common.dto.RuleData;
import org.apache.shenyu.common.enums.TrieMatchModeEvent;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.stream.Collectors;
public class ShenyuTrie {
@@ -293,13 +295,9 @@ public class ShenyuTrie {
/**
* remove trie node.
- * remove rules: query node of the current path, if the node exists,
- * checks whether the current node is mapped to multiple plug-in rules.
- * if the plug-in rules have only on mapping, remove the node from parent.
- * if current node exists multi mappings, remove the mapping.
*
- * @param paths paths
- * @param ruleData ruleData
+ * @param paths path list
+ * @param ruleData rule data
*/
public void remove(final List<String> paths, final RuleData ruleData) {
if (CollectionUtils.isNotEmpty(paths)) {
@@ -309,15 +307,15 @@ public class ShenyuTrie {
/**
* remove trie node.
- * remove rules: query node of the current path, if the node exists,
- * checks whether the current node is mapped to multiple plug-in rules.
- * if the plug-in rules have only on mapping, remove the node from parent.
- * if current node exists multi mappings, remove the mapping.
+ * <p> query node of the current path, if the node exists and the node
exist the value of pathRuleCache,
+ * delete a rule with the same ruleId from pathRuleCache.</p>
+ * <p> if current rule data list is empty, children and pathVariablesSet
is null,remove concurrent node from parent node.</p>
*
* @param path path
* @param ruleData ruleData
*/
public void remove(final String path, final RuleData ruleData) {
+ Objects.requireNonNull(ruleData.getId(), "rule id cannot be empty");
if (StringUtils.isNotBlank(path)) {
String strippedPath = StringUtils.strip(path, "/");
String[] pathParts = StringUtils.split(strippedPath, "/");
@@ -327,21 +325,17 @@ public class ShenyuTrie {
if (Objects.nonNull(currentNode) &&
Objects.nonNull(currentNode.getPathRuleCache())) {
// check current mapping
List<RuleData> ruleDataList =
getVal(currentNode.getPathRuleCache(), ruleData.getSelectorId());
- if (CollectionUtils.isNotEmpty(ruleDataList) &&
ruleDataList.size() == 1 && Objects.isNull(currentNode.getChildren())) {
+ ruleDataList =
Optional.ofNullable(ruleDataList).orElse(Collections.emptyList());
+ synchronized (ruleData.getSelectorId()) {
+ ruleDataList.removeIf(rule ->
ruleData.getId().equals(rule.getId()));
+ }
+ if (CollectionUtils.isEmpty(ruleDataList) &&
Objects.isNull(currentNode.getChildren())
+ && Objects.isNull(currentNode.getPathVariablesSet())) {
// remove current node from parent node
String[] parentPathArray = Arrays.copyOfRange(pathParts,
0, pathParts.length - 1);
String parentPath = String.join("/", parentPathArray);
ShenyuTrieNode parentNode = this.getNode(parentPath);
parentNode.getChildren().remove(key);
- } else {
- // remove plugin mapping
- List<RuleData> delRuleData =
getVal(currentNode.getPathRuleCache(), ruleData.getSelectorId());
- if (CollectionUtils.isNotEmpty(delRuleData)) {
- synchronized (ruleData.getSelectorId()) {
- delRuleData.removeIf(rule ->
rule.getId().equals(ruleData.getSelectorId()));
-
currentNode.getPathRuleCache().put(ruleData.getSelectorId(), delRuleData);
- }
- }
}
}
}
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrieRuleListener.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrieRuleListener.java
index 1925ba216..c78adc1e3 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrieRuleListener.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/trie/ShenyuTrieRuleListener.java
@@ -51,7 +51,11 @@ public class ShenyuTrieRuleListener implements
ApplicationListener<RuleTrieEvent
final ShenyuTrie shenyuTrie =
SpringBeanUtils.getInstance().getBean(ShenyuTrie.class);
switch (eventEnum) {
case INSERT:
- shenyuTrie.putNode(uriPaths, ruleData, ruleData.getId());
+ synchronized (ruleData.getId()) {
+ // insert rule data must remove original rule, and the
operation must atomic
+ shenyuTrie.remove(uriPaths, ruleData);
+ shenyuTrie.putNode(uriPaths, ruleData,
ruleData.getId());
+ }
break;
case UPDATE:
final List<ConditionData> beforeConditionDataList =
ruleData.getBeforeConditionDataList();
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
index 62b6163c0..986966358 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
@@ -184,7 +184,7 @@ public final class CommonPluginDataSubscriberTest {
public void testOnRuleSubscribe() {
baseDataCache.cleanRuleData();
- RuleData ruleData =
RuleData.builder().id("1").selectorId(mockSelectorId1).pluginName(mockPluginName1).sort(1).build();
+ RuleData ruleData =
RuleData.builder().id("1").selectorId(mockSelectorId1).enabled(true).pluginName(mockPluginName1).sort(1).build();
commonPluginDataSubscriber.onRuleSubscribe(ruleData);
assertNotNull(baseDataCache.obtainRuleData(ruleData.getSelectorId()));
assertEquals(Lists.newArrayList(ruleData),
baseDataCache.obtainRuleData(ruleData.getSelectorId()));
diff --git
a/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
b/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
index fbddc5c14..3c5cbc01a 100644
---
a/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
+++
b/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
@@ -426,6 +426,7 @@ public final class LocalPluginControllerTest {
.selectorId(testSelectorId)
.pluginName("testPluginName")
.id(testId)
+ .enabled(true)
.build();
}
