This is an automated email from the ASF dual-hosted git repository.
Aias00 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 f39dfdddff fix(admin): null-safe plugin lookup when publishing
selector deletions (#7364)
f39dfdddff is described below
commit f39dfdddffdc6247bd9b9335035fcaedfcdfb06f
Author: Sean-Walker0 <[email protected]>
AuthorDate: Thu Oct 1 12:01:05 2026 +0800
fix(admin): null-safe plugin lookup when publishing selector deletions
(#7364)
---
.../service/publish/SelectorEventPublisher.java | 2 +-
.../publish/SelectorEventPublisherTest.java | 11 ++++++
.../shenyu/plugin/base/cache/BaseDataCache.java | 22 ++++++++---
.../base/cache/CommonPluginDataSubscriber.java | 14 +++++--
.../shenyu/plugin/base/cache/MatchDataCache.java | 31 +++++++++++++--
.../plugin/base/cache/BaseDataCacheTest.java | 14 +++++++
.../base/cache/CommonPluginDataSubscriberTest.java | 46 ++++++++++++++++++++++
.../plugin/base/cache/MatchDataCacheTest.java | 34 ++++++++++++++++
8 files changed, 160 insertions(+), 14 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisher.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisher.java
index 93ca189d85..7121234551 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisher.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisher.java
@@ -96,7 +96,7 @@ public class SelectorEventPublisher implements
AdminDataModelChangedEventPublish
List<SelectorData> selectorDataList = selectors.stream()
.map(selectorDO -> {
String pluginName =
pluginMap.get(selectorDO.getPluginId());
- if (pluginName.equals(PluginEnum.DIVIDE.getName())) {
+ if (PluginEnum.DIVIDE.getName().equals(pluginName)) {
UpstreamCheckService.removeByKey(selectorDO.getId());
}
return SelectorDO.transFrom(selectorDO, pluginName, null);
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisherTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisherTest.java
index fa68c12a2a..63aef69ec4 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisherTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/publish/SelectorEventPublisherTest.java
@@ -157,6 +157,17 @@ class SelectorEventPublisherTest {
upstreamCheckServiceMockedStatic.verify(() ->
UpstreamCheckService.removeByKey("2"), times(1));
}
+ @Test
+ void testOnDeletedSelectorReferencingMissingPlugin() {
+ // a dangling selector whose plugin row was already deleted resolves
to a null plugin name
+ SelectorDO selector = buildSelectorDO("1", "deleted-plugin",
"selector1");
+ List<PluginDO> plugins = Collections.emptyList();
+
+ selectorEventPublisher.onDeleted(Collections.singletonList(selector),
plugins);
+
+ verify(applicationEventPublisher, times(2)).publishEvent(any());
+ }
+
@Test
void testOnDeletedEmptyCollection() {
List<SelectorDO> emptySelectors = Collections.emptyList();
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
index 745a9bdec0..fed4420289 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
@@ -18,6 +18,7 @@
package org.apache.shenyu.plugin.base.cache;
import com.google.common.collect.Maps;
+import org.apache.commons.lang3.StringUtils;
import org.apache.shenyu.common.dto.PluginData;
import org.apache.shenyu.common.dto.RuleData;
import org.apache.shenyu.common.dto.SelectorData;
@@ -136,12 +137,21 @@ public final class BaseDataCache {
*/
public void removeSelectData(final SelectorData selectorData) {
Optional.ofNullable(selectorData).ifPresent(data -> {
- selectorMap.computeIfPresent(data.getPluginName(), (key, value) ->
{
- final List<SelectorData> result = value.stream()
- .filter(selector -> !Objects.equals(selector.getId(),
data.getId()))
- .collect(Collectors.toList());
- return result.isEmpty() ? null : List.copyOf(result);
- });
+ if (StringUtils.isBlank(data.getPluginName())) {
+ // a dangling selector carries no plugin name, so its entry
may live under any plugin bucket
+ selectorMap.keySet().forEach(pluginName ->
removeSelectData(pluginName, data.getId()));
+ } else {
+ removeSelectData(data.getPluginName(), data.getId());
+ }
+ });
+ }
+
+ private void removeSelectData(final String pluginName, final String
selectorId) {
+ selectorMap.computeIfPresent(pluginName, (key, value) -> {
+ final List<SelectorData> result = value.stream()
+ .filter(selector -> !Objects.equals(selector.getId(),
selectorId))
+ .collect(Collectors.toList());
+ return result.isEmpty() ? null : List.copyOf(result);
});
}
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 a8526ed151..181b95dad4 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
@@ -18,6 +18,7 @@
package org.apache.shenyu.plugin.base.cache;
import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.shenyu.common.config.ShenyuConfig.RuleMatchCache;
import org.apache.shenyu.common.config.ShenyuConfig.SelectorMatchCache;
import org.apache.shenyu.common.dto.PluginData;
@@ -288,8 +289,11 @@ public class CommonPluginDataSubscriber implements
PluginDataSubscriber {
} else if (data instanceof SelectorData) {
SelectorData selectorData = (SelectorData) data;
BaseDataCache.getInstance().removeSelectData(selectorData);
- Optional.ofNullable(handlerMap.get(selectorData.getPluginName()))
- .ifPresent(handler ->
handler.removeSelector(selectorData));
+ // the concurrent handler map rejects null keys, and a dangling
selector carries no plugin name
+ if (StringUtils.isNotBlank(selectorData.getPluginName())) {
+
Optional.ofNullable(handlerMap.get(selectorData.getPluginName()))
+ .ifPresent(handler ->
handler.removeSelector(selectorData));
+ }
// remove match cache
if (selectorMatchConfig.getCache().getEnabled()) {
MatchDataCache.getInstance().removeSelectorData(selectorData.getPluginName(),
selectorData.getId());
@@ -302,8 +306,10 @@ public class CommonPluginDataSubscriber implements
PluginDataSubscriber {
} else if (data instanceof RuleData) {
RuleData ruleData = (RuleData) data;
BaseDataCache.getInstance().removeRuleData(ruleData);
- Optional.ofNullable(handlerMap.get(ruleData.getPluginName()))
- .ifPresent(handler -> handler.removeRule(ruleData));
+ if (StringUtils.isNotBlank(ruleData.getPluginName())) {
+ Optional.ofNullable(handlerMap.get(ruleData.getPluginName()))
+ .ifPresent(handler -> handler.removeRule(ruleData));
+ }
if (ruleMatchCacheConfig.getCache().getEnabled()) {
MatchDataCache.getInstance().removeRuleData(ruleData.getPluginName(),
ruleData.getId());
MatchDataCache.getInstance().removeEmptyRuleData(ruleData.getPluginName());
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/MatchDataCache.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/MatchDataCache.java
index 2993a5cc80..f5474911ea 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/MatchDataCache.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/MatchDataCache.java
@@ -18,6 +18,7 @@
package org.apache.shenyu.plugin.base.cache;
import com.google.common.collect.Maps;
+import org.apache.commons.lang3.StringUtils;
import org.apache.shenyu.common.cache.WindowTinyLFUMap;
import org.apache.shenyu.common.dto.RuleData;
import org.apache.shenyu.common.dto.SelectorData;
@@ -75,19 +76,28 @@ public final class MatchDataCache {
* @param selectorId selector id
*/
public void removeSelectorData(final String pluginName, final String
selectorId) {
+ if (StringUtils.isBlank(pluginName)) {
+ // a dangling selector carries no plugin name, so its entries may
live under any plugin bucket
+ SELECTOR_DATA_MAP.values().forEach(pathSelectorCache ->
+ pathSelectorCache.entrySet().removeIf(entry ->
selectorId.equals(entry.getValue().getId())));
+ return;
+ }
Map<String, SelectorData> pathSelectorCache =
SELECTOR_DATA_MAP.get(pluginName);
if (Objects.isNull(pathSelectorCache) || pathSelectorCache.isEmpty()) {
return;
}
pathSelectorCache.entrySet().removeIf(entry ->
selectorId.equals(entry.getValue().getId()));
}
-
+
/**
* remove empty selector data.
*
* @param pluginName plugin name
*/
public void removeEmptySelectorData(final String pluginName) {
+ if (StringUtils.isBlank(pluginName)) {
+ return;
+ }
Map<String, SelectorData> pathSelectorCache =
SELECTOR_DATA_MAP.get(pluginName);
if (Objects.isNull(pathSelectorCache) || pathSelectorCache.isEmpty()) {
return;
@@ -156,13 +166,19 @@ public final class MatchDataCache {
* @param ruleId ruleId
*/
public void removeRuleData(final String pluginName, final String ruleId) {
+ if (StringUtils.isBlank(pluginName)) {
+ // a rule of a deleted plugin carries no plugin name, so its
entries may live under any plugin bucket
+ RULE_DATA_MAP.values().forEach(pathRuleDataCache ->
+ pathRuleDataCache.entrySet().removeIf(entry ->
ruleId.equals(entry.getValue().getId())));
+ return;
+ }
Map<String, RuleData> pathRuleDataCache =
RULE_DATA_MAP.get(pluginName);
if (Objects.isNull(pathRuleDataCache) || pathRuleDataCache.isEmpty()) {
return;
}
pathRuleDataCache.entrySet().removeIf(entry ->
ruleId.equals(entry.getValue().getId()));
}
-
+
/**
* remove rule data by selector.
*
@@ -170,19 +186,28 @@ public final class MatchDataCache {
* @param selectorId selectorId
*/
public void removeRuleDataBySelector(final String pluginName, final String
selectorId) {
+ if (StringUtils.isBlank(pluginName)) {
+ // a dangling selector carries no plugin name, so its rule entries
may live under any plugin bucket
+ RULE_DATA_MAP.values().forEach(pathRuleDataCache ->
+ pathRuleDataCache.entrySet().removeIf(entry ->
selectorId.equals(entry.getValue().getSelectorId())));
+ return;
+ }
Map<String, RuleData> pathRuleDataCache =
RULE_DATA_MAP.get(pluginName);
if (Objects.isNull(pathRuleDataCache) || pathRuleDataCache.isEmpty()) {
return;
}
pathRuleDataCache.entrySet().removeIf(entry ->
selectorId.equals(entry.getValue().getSelectorId()));
}
-
+
/**
* remove empty rule data.
*
* @param pluginName plugin name
*/
public void removeEmptyRuleData(final String pluginName) {
+ if (StringUtils.isBlank(pluginName)) {
+ return;
+ }
Map<String, RuleData> pathRuleDataCache =
RULE_DATA_MAP.get(pluginName);
if (Objects.isNull(pathRuleDataCache) || pathRuleDataCache.isEmpty()) {
return;
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
index 918a4d44b2..2e75c29636 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
@@ -166,6 +166,20 @@ public final class BaseDataCacheTest {
assertNull(selectorMap.get(mockPluginName1));
}
+ @Test
+ public void testRemoveSelectDataSweepsAllBucketsWhenPluginNameIsMissing() {
+ SelectorData firstCachedSelectorData =
SelectorData.builder().id("1").pluginName(mockPluginName1).sort(1).build();
+ SelectorData secondCachedSelectorData =
SelectorData.builder().id("2").pluginName(mockPluginName2).sort(1).build();
+ cache.cacheSelectData(firstCachedSelectorData);
+ cache.cacheSelectData(secondCachedSelectorData);
+
+ // the deletion event of a dangling selector carries no plugin name
+ cache.removeSelectData(SelectorData.builder().id("1").build());
+
+ assertNull(cache.obtainSelectorData(mockPluginName1));
+ assertEquals(Lists.newArrayList(secondCachedSelectorData),
cache.obtainSelectorData(mockPluginName2));
+ }
+
@Test
public void testCleanSelectorData() throws NoSuchFieldException,
IllegalAccessException {
SelectorData firstCachedSelectorData =
SelectorData.builder().id("1").pluginName(mockPluginName1).build();
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 a2182f319c..cedf095d37 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
@@ -233,6 +233,52 @@ public final class CommonPluginDataSubscriberTest {
}
}
+ @Test
+ public void testUnSelectorSubscribeWithMissingPluginName() {
+ final String path = "/dangling";
+ final MatchDataCache matchDataCache = MatchDataCache.getInstance();
+ baseDataCache.cleanSelectorData();
+ matchDataCache.cleanSelectorData();
+ matchDataCache.cleanRuleDataData();
+
+ // the gateway cached the selector under its real plugin name before
the plugin row vanished
+ final SelectorData cachedSelector =
SelectorData.builder().id(mockSelectorId1).enabled(true).pluginName(mockPluginName1).build();
+ final SelectorData otherPluginSelector =
SelectorData.builder().id(mockSelectorId2).enabled(true).pluginName(mockPluginName2).build();
+ final RuleData cachedRule =
RuleData.builder().id("1").selectorId(mockSelectorId1).pluginName(mockPluginName1).build();
+ baseDataCache.cacheSelectData(cachedSelector);
+ baseDataCache.cacheSelectData(otherPluginSelector);
+ matchDataCache.cacheSelectorData(path, cachedSelector, 100, 100);
+ matchDataCache.cacheRuleData(path, cachedRule, 100, 100);
+
+ // the admin cannot resolve a plugin name for the dangling selector,
so the delete event carries none
+ final SelectorData deletion =
SelectorData.builder().id(mockSelectorId1).enabled(true).build();
+ commonPluginDataSubscriber.unSelectorSubscribe(deletion);
+
+ assertNull(baseDataCache.obtainSelectorData(mockPluginName1));
+ assertNull(matchDataCache.obtainSelectorData(mockPluginName1, path));
+ assertNull(matchDataCache.obtainRuleData(mockPluginName1, path));
+ // an unrelated plugin keeps its selector
+ assertEquals(Lists.newArrayList(otherPluginSelector),
baseDataCache.obtainSelectorData(mockPluginName2));
+ }
+
+ @Test
+ public void testUnRuleSubscribeWithMissingPluginName() {
+ final MatchDataCache matchDataCache = MatchDataCache.getInstance();
+ baseDataCache.cleanRuleData();
+ matchDataCache.cleanRuleDataData();
+
+ final RuleData cachedRule =
RuleData.builder().id("1").selectorId(mockSelectorId1).pluginName(mockPluginName1).build();
+ baseDataCache.cacheRuleData(cachedRule);
+ matchDataCache.cacheRuleData("/rule", cachedRule, 100, 100);
+
+ // the admin cannot resolve a plugin name for a rule of a deleted
plugin, so the delete event carries none
+ final RuleData deletion =
RuleData.builder().id("1").selectorId(mockSelectorId1).build();
+ commonPluginDataSubscriber.unRuleSubscribe(deletion);
+
+ assertNull(baseDataCache.obtainRuleData(mockSelectorId1));
+ assertNull(matchDataCache.obtainRuleData(mockPluginName1, "/rule"));
+ }
+
@Test
public void testRefreshSelectorDataAll() {
baseDataCache.cleanSelectorData();
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/MatchDataCacheTest.java
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/MatchDataCacheTest.java
index 5b55cee1c7..5ec68a2660 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/MatchDataCacheTest.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/MatchDataCacheTest.java
@@ -37,8 +37,12 @@ public final class MatchDataCacheTest {
private final String mockPluginName1 = "MOCK_PLUGIN_NAME_1";
+ private final String mockPluginName2 = "MOCK_PLUGIN_NAME_2";
+
private final String path1 = "/http/abc";
+ private final String path2 = "/http/def";
+
@Test
public void testCacheSelectorData() throws NoSuchFieldException,
IllegalAccessException {
SelectorData firstCachedSelectorData =
SelectorData.builder().id("1").pluginName(mockPluginName1).sort(1).build();
@@ -69,6 +73,36 @@ public final class MatchDataCacheTest {
selectorMap.clear();
}
+ @Test
+ public void
testRemoveSelectorDataSweepsAllPluginBucketsWhenPluginNameIsMissing() {
+ SelectorData firstCachedSelectorData =
SelectorData.builder().id("1").pluginName(mockPluginName1).sort(1).build();
+ SelectorData secondCachedSelectorData =
SelectorData.builder().id("2").pluginName(mockPluginName2).sort(1).build();
+ MatchDataCache.getInstance().cacheSelectorData(path1,
firstCachedSelectorData, 100, 100);
+ MatchDataCache.getInstance().cacheSelectorData(path1,
secondCachedSelectorData, 100, 100);
+
+ // the deletion event of a dangling selector carries no plugin name
+ MatchDataCache.getInstance().removeSelectorData(null, "1");
+
+
assertNull(MatchDataCache.getInstance().obtainSelectorData(mockPluginName1,
path1));
+ assertEquals(secondCachedSelectorData,
MatchDataCache.getInstance().obtainSelectorData(mockPluginName2, path1));
+ MatchDataCache.getInstance().cleanSelectorData();
+ }
+
+ @Test
+ public void
testRemoveRuleDataBySelectorSweepsAllPluginBucketsWhenPluginNameIsMissing() {
+ RuleData cacheRuleData =
RuleData.builder().id("1").selectorId("100").pluginName(mockPluginName1).sort(1).build();
+ RuleData unrelatedRuleData =
RuleData.builder().id("2").selectorId("200").pluginName(mockPluginName1).sort(1).build();
+ MatchDataCache.getInstance().cacheRuleData(path1, cacheRuleData, 100,
100);
+ MatchDataCache.getInstance().cacheRuleData(path2, unrelatedRuleData,
100, 100);
+
+ // the deletion event of a dangling selector carries no plugin name
+ MatchDataCache.getInstance().removeRuleDataBySelector(null, "100");
+
+
assertNull(MatchDataCache.getInstance().obtainRuleData(mockPluginName1, path1));
+ assertEquals(unrelatedRuleData,
MatchDataCache.getInstance().obtainRuleData(mockPluginName1, path2));
+ MatchDataCache.getInstance().cleanRuleDataData();
+ }
+
@SuppressWarnings("rawtypes")
private ConcurrentHashMap getFieldByName(final String name) throws
NoSuchFieldException, IllegalAccessException {
MatchDataCache matchDataCache = MatchDataCache.getInstance();
