apache / apache/shenyu

[Proposal] Selector Logic Refactoring

Open
#6,081 1 comment 0 reactions 0 assignees View on GitHub
status: volunteer wanted
Dominant language
Java
Stars
8.8k
Forks
3.1k
Avg merge
7d 1h
Merged PRs (30d)
85

Description

## Background

While reading the source code, I found some issues, so I proposed this proposal.

## Extract the common parts of PluginData, SelectorData, and RuleData into BaseData

```java
public class BaseData {

private String id;

private String name;

private Boolean enabled;

private Integer sort;

private String namespaceId;

// ...other code.
// define common method, such as:
String getId();

}

public Class XXXData extends BaseData {

private String pluginId;
private String pluginName;
private Integer matchMode;
// etc.

// overidle getId()

// getters and setters
}
```

Inheritance reduces code duplication, improves code maintainability, and better conforms to object-oriented inheritance, embodying an "is-a" relationship.

## Rule Selection Logic Refactoring

`org.apache.shenyu.plugin.base.AbstractShenyuPlugin#execute()`: The `execute()` method now performs the selector and rule matching logic.

1. Matches the selector;
2. Matches the rule;
3. Executes the plugin, passing the plugin's selectorData and RuleData to the specific plugin for execution.

```java
public Mono execute(final ServerWebExchange exchange, final ShenyuPluginChain chain) {

// init data cache
initCacheConfig();

// get plugin named
final String pluginName = named();

// choose plugin data
PluginData pluginData = BaseDataCache.getInstance().obtainPluginData(pluginName);
// early exit
if (Objects.isNull(pluginData) || !pluginData.getEnabled()) {
return chain.execute(exchange);
}
final String path = getRawPath(exchange);

// selector data of plugin
List selectors = BaseDataCache.getInstance().obtainSelectorData(pluginName);
if (CollectionUtils.isEmpty(selectors)) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
SelectorData selectorData = obtainSelectorDataCacheIfEnabled(path);
// handle Selector
if (Objects.nonNull(selectorData) && StringUtils.isBlank(selectorData.getId())) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
if (Objects.isNull(selectorData)) {
selectorData = trieMatchSelector(exchange, pluginName, path);
if (Objects.isNull(selectorData)) {
selectorData = defaultMatchSelector(exchange, selectors, path);
if (Objects.isNull(selectorData)) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
}
}
printLog(selectorData, pluginName);
if (!selectorData.getContinued()) {
// if continued, not match rules
return doExecute(exchange, chain, selectorData, defaultRuleData(selectorData));
}

// selector rule data
List rules = BaseDataCache.getInstance().obtainRuleData(selectorData.getId());
if (CollectionUtils.isEmpty(rules)) {
return handleRuleIfNull(pluginName, exchange, chain);
}
if (selectorData.getType() == SelectorTypeEnum.FULL_FLOW.getCode()) {
//get last
RuleData rule = rules.get(rules.size() - 1);
printLog(rule, pluginName);
return doExecute(exchange, chain, selectorData, rule);
}
// lru map as L1 cache,the cache is enabled by default.
// if the L1 cache fails to hit, using L2 cache based on trie cache.
// if the L2 cache fails to hit, execute default strategy.
RuleData ruleData = obtainRuleDataCacheIfEnabled(path);
if (Objects.nonNull(ruleData) && Objects.isNull(ruleData.getId())) {
return handleRuleIfNull(pluginName, exchange, chain);
}
if (Objects.isNull(ruleData)) {
// L1 cache not exist data, try to get data through trie cache
ruleData = trieMatchRule(exchange, selectorData, path);
// trie cache fails to hit, execute default strategy
if (Objects.isNull(ruleData)) {
ruleData = defaultMatchRule(exchange, rules, path);
if (Objects.isNull(ruleData)) {
return handleRuleIfNull(pluginName, exchange, chain);
}
}
}
printLog(ruleData, pluginName);

// exec logic
return doExecute(exchange, chain, selectorData, ruleData);
}
```

From the above code, three lines of repeated code appear during data selection:

```java
XXXData xxxData = BaseDataCache.getInstance().obtainXXXData("name/id");
```

This makes the entire code look redundant and complex. Several optimization strategies are possible. One possible approach is to use the Template Method and Strategy pattern to optimize the code, creating a clearer structure and improving readability and extensibility.

First, define a top-level data provider interface, using BaseDataCache as the data provider:

```java
public interface DataProvider {
List getData(String key);
}
```

Next, the plugin, selector, and rule implement a data provider class and implement the DataProvider interface:

```java
public class PluginDataProvider implements DataProvider {
@Override
public List getData(String pluginName) {
PluginData data = BaseDataCache.getInstance().obtainPluginData(pluginName);
return data != null ? Collections.singletonList(data) : Collections.emptyList();
}
}

// selector and rule ...
```

Then define an abstract class for data processing, extract the common code and encapsulate it:

```java
public abstract class AbstractDataHandler {
private final DataProvider dataProvider;

protected AbstractDataHandler(DataProvider dataProvider) {
this.dataProvider = dataProvider;
}

protected List getData(String key) {
return dataProvider.getData(key);
}

protected abstract Mono handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain);

protected abstract T matchData(ServerWebExchange exchange, List dataList, String path);

protected abstract boolean shouldContinue(T data);
}
```

After that, they are implemented separately. The code implementation is roughly as follows:

```java
public class PluginDataHandler extends AbstractDataHandler {
public PluginDataHandler() {
super(new PluginDataProvider());
}

@Override
protected Mono handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain) {
return chain.execute(exchange);
}

@Override
protected PluginData matchData(ServerWebExchange exchange, List dataList, String path) {
return dataList.isEmpty() ? null : dataList.get(0);
}

@Override
protected boolean shouldContinue(PluginData data) {
return data != null && data.getEnabled();
}
}
```

Finally, rewrite the execute method:

```java
public Mono execute(final ServerWebExchange exchange, final ShenyuPluginChain chain) {
initCacheConfig();
final String pluginName = named();
final String path = getRawPath(exchange);

// Process plugin data
PluginDataHandler pluginHandler = new PluginDataHandler();
List pluginDataList = pluginHandler.getData(pluginName);
if (!pluginHandler.shouldContinue(pluginDataList.get(0))) {
return pluginHandler.handleEmpty(pluginName, exchange, chain);
}

// Process selector data
SelectorDataHandler selectorHandler = new SelectorDataHandler();
List selectorDataList = selectorHandler.getData(pluginName);
if (CollectionUtils.isEmpty(selectorDataList)) {
return selectorHandler.handleEmpty(pluginName, exchange, chain);
}

SelectorData selectorData = selectorHandler.matchData(exchange, selectorDataList, path);
if (selectorData == null) {
return selectorHandler.handleEmpty(pluginName, exchange, chain);
}

printLog(selectorData, pluginName);
if (!selectorHandler.shouldContinue(selectorData)) {
return doExecute(exchange, chain, selectorData, defaultRuleData(selectorData));
}

RuleDataHandler ruleHandler = new RuleDataHandler();
List ruleDataList = ruleHandler.getData(selectorData.getId());
if (CollectionUtils.isEmpty(ruleDataList)) {
return ruleHandler.handleEmpty(pluginName, exchange, chain);
}

if (selectorData.getType() == SelectorTypeEnum.FULL_FLOW.getCode()) {
RuleData rule = ruleDataList.get(ruleDataList.size() - 1);
printLog(rule, pluginName);
return doExecute(exchange, chain, selectorData, rule);
}

// Process rule data
RuleData ruleData = ruleHandler.matchData(exchange, ruleDataList, path);
if (ruleData == null) {
return ruleHandler.handleEmpty(pluginName, exchange, chain);
}

printLog(ruleData, pluginName);
return doExecute(exchange, chain, selectorData, ruleData);
}
```

This is a viable solution to make the code more structured, deterministic, and scalable.
Of course, this is currently just an initiative, so if you have any comments or suggestions, please feel free to raise them and discuss them. I'd be happy to contribute. thanks.

# Chinese Version

## PluginData、SelectorData 和 RuleData 提取公共部分为 BaseData

```java
public class BaseData {

private String id;

private String name;

private Boolean enabled;

private Integer sort;

private String namespaceId;

// ...other code.
// define common method, such as:
String getId();

}

public Class XXXData extends BaseData {

private String pluginId;
private String pluginName;
private Integer matchMode;
private Integer type;
private Boolean logged;
private Boolean continued = Boolean.TRUE;
private String handle;
private List conditionList;
private List beforeConditionList;
private Boolean matchRestful;

// overidle getId()

// getters and setters
}
}
```

上面这种方法通过继承减少了代码重复,提高了代码的可维护性而且更符合面向对象的继承特性,体现了 "is-a" 的关系。

## 规则选择逻辑重构

`org.apache.shenyu.plugin.base.AbstractShenyuPlugin#execute()`,在 `execute()` 方法中执行选择器和规则的匹配逻辑。

1. 匹配选择器;
2. 匹配规则;
3. 执行插件,将插件的 selectorData 和 RuleData 传递到具体插件以执行。

```java
public Mono execute(final ServerWebExchange exchange, final ShenyuPluginChain chain) {

// 初始化缓存配置
initCacheConfig();

// 获取插件名称
final String pluginName = named();

// 插件数据选择
PluginData pluginData = BaseDataCache.getInstance().obtainPluginData(pluginName);
// early exit
if (Objects.isNull(pluginData) || !pluginData.getEnabled()) {
return chain.execute(exchange);
}
final String path = getRawPath(exchange);

// 插件选择器数据
List selectors = BaseDataCache.getInstance().obtainSelectorData(pluginName);
if (CollectionUtils.isEmpty(selectors)) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
SelectorData selectorData = obtainSelectorDataCacheIfEnabled(path);
// handle Selector
if (Objects.nonNull(selectorData) && StringUtils.isBlank(selectorData.getId())) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
if (Objects.isNull(selectorData)) {
selectorData = trieMatchSelector(exchange, pluginName, path);
if (Objects.isNull(selectorData)) {
selectorData = defaultMatchSelector(exchange, selectors, path);
if (Objects.isNull(selectorData)) {
return handleSelectorIfNull(pluginName, exchange, chain);
}
}
}
printLog(selectorData, pluginName);
if (!selectorData.getContinued()) {
// if continued, not match rules
return doExecute(exchange, chain, selectorData, defaultRuleData(selectorData));
}

// 选择器规则数据
List rules = BaseDataCache.getInstance().obtainRuleData(selectorData.getId());
if (CollectionUtils.isEmpty(rules)) {
return handleRuleIfNull(pluginName, exchange, chain);
}
if (selectorData.getType() == SelectorTypeEnum.FULL_FLOW.getCode()) {
//get last
RuleData rule = rules.get(rules.size() - 1);
printLog(rule, pluginName);
return doExecute(exchange, chain, selectorData, rule);
}
// lru map as L1 cache,the cache is enabled by default.
// if the L1 cache fails to hit, using L2 cache based on trie cache.
// if the L2 cache fails to hit, execute default strategy.
RuleData ruleData = obtainRuleDataCacheIfEnabled(path);
if (Objects.nonNull(ruleData) && Objects.isNull(ruleData.getId())) {
return handleRuleIfNull(pluginName, exchange, chain);
}
if (Objects.isNull(ruleData)) {
// L1 cache not exist data, try to get data through trie cache
ruleData = trieMatchRule(exchange, selectorData, path);
// trie cache fails to hit, execute default strategy
if (Objects.isNull(ruleData)) {
ruleData = defaultMatchRule(exchange, rules, path);
if (Objects.isNull(ruleData)) {
return handleRuleIfNull(pluginName, exchange, chain);
}
}
}
printLog(ruleData, pluginName);

// 执行后续逻辑
return doExecute(exchange, chain, selectorData, ruleData);
}
```

从上述代码中,在数据选择时出现了三行重复的代码:

```java
XXXData xxxData = BaseDataCache.getInstance().obtainXXXData("name/id");
```

使得整个代码看上去冗余且复杂。对此可以采取一些优化策略。一种可能是方案是使用模板方法和策略模式来优化代码,使其具有更加清晰的代码结构并且提升可读性和扩展性。

首先,定义一个顶层的数据提供接口,以 BaseDataCache 作为数据提供者:

```java
public interface DataProvider {
List getData(String key);
}
```

接下来,plugin 和 selector 和 rule 实现一个数据提供者类,并且实现 DataProvider 接口。

```java
public class PluginDataProvider implements DataProvider {
@Override
public List getData(String pluginName) {
PluginData data = BaseDataCache.getInstance().obtainPluginData(pluginName);
return data != null ? Collections.singletonList(data) : Collections.emptyList();
}
}

// selector and rule 类似
```

然后定义一个数据处理的抽象类,提取公共部分的代码并做封装:

```java
public abstract class AbstractDataHandler {
private final DataProvider dataProvider;

protected AbstractDataHandler(DataProvider dataProvider) {
this.dataProvider = dataProvider;
}

protected List getData(String key) {
return dataProvider.getData(key);
}

protected abstract Mono handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain);

protected abstract T matchData(ServerWebExchange exchange, List dataList, String path);

protected abstract boolean shouldContinue(T data);
}
```

之后分别实现,可能看起来是这样:

```java
public class PluginDataHandler extends AbstractDataHandler {
public PluginDataHandler() {
super(new PluginDataProvider());
}

@Override
protected Mono handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain) {
return chain.execute(exchange);
}

@Override
protected PluginData matchData(ServerWebExchange exchange, List dataList, String path) {
return dataList.isEmpty() ? null : dataList.get(0);
}

@Override
protected boolean shouldContinue(PluginData data) {
return data != null && data.getEnabled();
}
}
```

最后我们可以重写 execute 方法来完成重构:

```java
public Mono execute(final ServerWebExchange exchange, final ShenyuPluginChain chain) {
initCacheConfig();
final String pluginName = named();
final String path = getRawPath(exchange);

// 处理插件数据
PluginDataHandler pluginHandler = new PluginDataHandler();
List pluginDataList = pluginHandler.getData(pluginName);
if (!pluginHandler.shouldContinue(pluginDataList.get(0))) {
return pluginHandler.handleEmpty(pluginName, exchange, chain);
}

// 处理选择器数据
SelectorDataHandler selectorHandler = new SelectorDataHandler();
List selectorDataList = selectorHandler.getData(pluginName);
if (CollectionUtils.isEmpty(selectorDataList)) {
return selectorHandler.handleEmpty(pluginName, exchange, chain);
}

SelectorData selectorData = selectorHandler.matchData(exchange, selectorDataList, path);
if (selectorData == null) {
return selectorHandler.handleEmpty(pluginName, exchange, chain);
}

printLog(selectorData, pluginName);
if (!selectorHandler.shouldContinue(selectorData)) {
return doExecute(exchange, chain, selectorData, defaultRuleData(selectorData));
}

// 处理规则数据
RuleDataHandler ruleHandler = new RuleDataHandler();
List ruleDataList = ruleHandler.getData(selectorData.getId());
if (CollectionUtils.isEmpty(ruleDataList)) {
return ruleHandler.handleEmpty(pluginName, exchange, chain);
}

if (selectorData.getType() == SelectorTypeEnum.FULL_FLOW.getCode()) {
RuleData rule = ruleDataList.get(ruleDataList.size() - 1);
printLog(rule, pluginName);
return doExecute(exchange, chain, selectorData, rule);
}

RuleData ruleData = ruleHandler.matchData(exchange, ruleDataList, path);
if (ruleData == null) {
return ruleHandler.handleEmpty(pluginName, exchange, chain);
}

printLog(ruleData, pluginName);
return doExecute(exchange, chain, selectorData, ruleData);
}
```

这是一个可能的方案,使代码结构更加清晰并且具有可读性和扩展性。
当然,这目前只是一个提案,如果有任何评论或意见,欢迎提出并讨论, 非常感谢!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at org.apache.shenyu.plugin.base.AbstractShenyuPlugin#execute() and inspect PluginData, SelectorData, RuleData, and BaseDataCache to understand the current selector and rule matching flow. Before changing code, establish the intended refactoring design with maintainers; done means a scoped, agreed structure that reduces duplication without changing plugin, selector, or rule execution behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.