Skip to content

[Proposal] Selector Logic Refactoring #6081

@yuluo-yx

Description

@yuluo-yx

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

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.
public Mono<Void> 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<SelectorData> 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<RuleData> 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:

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:

public interface DataProvider<T> {
    List<T> getData(String key);
}

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

public class PluginDataProvider implements DataProvider<PluginData> {
    @Override
    public List<PluginData> 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:

public abstract class AbstractDataHandler<T extends BaseData> {
    private final DataProvider<T> dataProvider;

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

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

    protected abstract Mono<Void> handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain);
    
    protected abstract T matchData(ServerWebExchange exchange, List<T> dataList, String path);
    
    protected abstract boolean shouldContinue(T data);
}

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

public class PluginDataHandler extends AbstractDataHandler<PluginData> {
    public PluginDataHandler() {
        super(new PluginDataProvider());
    }

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

    @Override
    protected PluginData matchData(ServerWebExchange exchange, List<PluginData> 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:

public Mono<Void> 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<PluginData> pluginDataList = pluginHandler.getData(pluginName);
    if (!pluginHandler.shouldContinue(pluginDataList.get(0))) {
        return pluginHandler.handleEmpty(pluginName, exchange, chain);
    }

    // Process selector data
    SelectorDataHandler selectorHandler = new SelectorDataHandler();
    List<SelectorData> 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<RuleData> 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

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<ConditionData> conditionList;
    private List<ConditionData> beforeConditionList;
    private Boolean matchRestful;

    // overidle getId()

    // getters and setters
}
}

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

规则选择逻辑重构

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

  1. 匹配选择器;
  2. 匹配规则;
  3. 执行插件,将插件的 selectorData 和 RuleData 传递到具体插件以执行。
public Mono<Void> 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<SelectorData> 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<RuleData> 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);
}

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

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

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

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

public interface DataProvider<T> {
    List<T> getData(String key);
}

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

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

// selector and rule 类似

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

public abstract class AbstractDataHandler<T extends BaseData> {
    private final DataProvider<T> dataProvider;

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

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

    protected abstract Mono<Void> handleEmpty(String pluginName, ServerWebExchange exchange, ShenyuPluginChain chain);
    
    protected abstract T matchData(ServerWebExchange exchange, List<T> dataList, String path);
    
    protected abstract boolean shouldContinue(T data);
}

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

public class PluginDataHandler extends AbstractDataHandler<PluginData> {
    public PluginDataHandler() {
        super(new PluginDataProvider());
    }

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

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

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

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

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

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

    // 处理选择器数据
    SelectorDataHandler selectorHandler = new SelectorDataHandler();
    List<SelectorData> 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<RuleData> 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);
}

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions