This is an automated email from the ASF dual-hosted git repository.
wy471x 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 b56b5e009d fix(admin): skip discovery rels whose import remapping
dropped the reference (#7515)
b56b5e009d is described below
commit b56b5e009db2569fc48a34980bfc0c993b4c5bbe
Author: Sean-Walker0 <[email protected]>
AuthorDate: Sat Oct 10 21:55:37 2026 +0800
fix(admin): skip discovery rels whose import remapping dropped the
reference (#7515)
* fix(admin): skip discovery rels whose import remapping dropped the
reference
configsImport remapped discovery_rel selector and proxy-selector ids
through the import context mappings and inserted the row even when the
mapping was missing, persisting relations whose reference columns were
silently nulled (insertSelective omits null columns). The imported
namespace then contained discovery_rel rows pointing at nothing.
Resolve both references first and skip the rel when the exported id was
set but its target was not imported into this namespace (or when the
discovery carries no handler to attach the rel to), logging the skip.
Relations whose exported references were already null keep that shape.
Regression tests cover the selector-mapping miss, the proxy-selector
mapping miss (both red on the previous code, which inserted the row),
and the successful remap path.
Fixes #7469
* fix(admin): keep blank discovery rel references intact on import
Review follow-up: rels created for a TCP proxy selector store an empty
selector_id ('' in the database, not NULL), and the previous
nonNull-based check treated that blank as a remappable reference,
mapping.get("") returned null, and the rel was skipped even though its
proxy-selector reference remapped fine. The import of a namespace
containing a TCP proxy selector plus its discovery silently lost the
binding.
Only a non-blank reference is a remappable target now; blank
references keep their exported shape. Regression test replays the
rel shape produced by ProxySelectorServiceImpl#create (selectorId
empty, proxySelectorId remapped) and asserts the row is inserted with
both fields correct.
---------
Co-authored-by: Sean-Walker0
<[email protected]>
---
.../admin/service/impl/DiscoveryServiceImpl.java | 25 ++++--
.../admin/service/DiscoveryServiceImplTest.java | 91 ++++++++++++++++++++++
2 files changed, 110 insertions(+), 6 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java
index cbbdf9dbef..3d1a2e9b62 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java
@@ -532,12 +532,25 @@ public class DiscoveryServiceImpl implements
DiscoveryService {
.INSTANCE
.mapToDO(discoveryDTO.getDiscoveryRel());
discoveryRelDO.setDiscoveryHandlerId(discoveryHandlerId);
- Optional.ofNullable(discoveryRelDO.getSelectorId())
- .ifPresent(selectorId ->
discoveryRelDO.setSelectorId(context.getSelectorIdMapping().get(selectorId)));
- Optional.ofNullable(discoveryRelDO.getProxySelectorId())
- .ifPresent(proxySelectorId ->
discoveryRelDO.setProxySelectorId(context.getProxySelectorIdMapping().get(proxySelectorId)));
-
discoveryRelDO.setId(UUIDUtils.getInstance().generateShortUuid());
- discoveryRelMapper.insertSelective(discoveryRelDO);
+ // rels created for a TCP proxy selector store an empty
selector_id ('' in the
+ // database, not NULL), so only a non-blank reference is a
remappable target;
+ // blank references keep their exported shape
+ String remappedSelectorId =
StringUtils.isBlank(discoveryRelDO.getSelectorId())
+ ? discoveryRelDO.getSelectorId() :
context.getSelectorIdMapping().get(discoveryRelDO.getSelectorId());
+ String remappedProxySelectorId =
StringUtils.isBlank(discoveryRelDO.getProxySelectorId())
+ ? discoveryRelDO.getProxySelectorId() :
context.getProxySelectorIdMapping().get(discoveryRelDO.getProxySelectorId());
+ boolean selectorReferenceLost =
StringUtils.isNotBlank(discoveryRelDO.getSelectorId()) &&
Objects.isNull(remappedSelectorId);
+ boolean proxySelectorReferenceLost =
StringUtils.isNotBlank(discoveryRelDO.getProxySelectorId()) &&
Objects.isNull(remappedProxySelectorId);
+ if (Objects.isNull(discoveryHandlerId) ||
selectorReferenceLost || proxySelectorReferenceLost) {
+ // the relation's target was not imported into this
namespace; skip it instead
+ // of persisting a row whose selector/proxy-selector
reference was nulled by remapping
+ LOG.warn("skip discovery rel of discovery [{}]: referenced
selector/proxy selector was not imported", discoveryName);
+ } else {
+ discoveryRelDO.setSelectorId(remappedSelectorId);
+ discoveryRelDO.setProxySelectorId(remappedProxySelectorId);
+
discoveryRelDO.setId(UUIDUtils.getInstance().generateShortUuid());
+ discoveryRelMapper.insertSelective(discoveryRelDO);
+ }
}
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryServiceImplTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryServiceImplTest.java
index c84d994b1e..b640bcf47a 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryServiceImplTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryServiceImplTest.java
@@ -24,7 +24,12 @@ import org.apache.shenyu.admin.mapper.DiscoveryMapper;
import org.apache.shenyu.admin.mapper.DiscoveryRelMapper;
import org.apache.shenyu.admin.mapper.ProxySelectorMapper;
import org.apache.shenyu.admin.mapper.SelectorMapper;
+import org.apache.shenyu.admin.model.dto.DiscoveryDTO;
+import org.apache.shenyu.admin.model.dto.DiscoveryHandlerDTO;
+import org.apache.shenyu.admin.model.dto.DiscoveryRelDTO;
import org.apache.shenyu.admin.model.entity.DiscoveryDO;
+import org.apache.shenyu.admin.model.entity.DiscoveryRelDO;
+import org.apache.shenyu.admin.service.configs.ConfigsImportContext;
import org.apache.shenyu.admin.model.entity.SelectorDO;
import org.apache.shenyu.register.common.dto.DiscoveryConfigRegisterDTO;
import org.springframework.transaction.PlatformTransactionManager;
@@ -50,6 +55,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.given;
+import org.mockito.ArgumentCaptor;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
@@ -187,4 +193,89 @@ public final class DiscoveryServiceImplTest {
verify(discoveryMapper, never()).delete(any(), any());
verify(discoveryProcessor, never()).removeDiscovery(any());
}
+
+ @Test
+ public void testImportSkipsRelWhenSelectorMappingMissing() {
+ final ConfigsImportContext context = new ConfigsImportContext();
+ DiscoveryDTO discoveryDTO = buildImportDiscovery();
+ DiscoveryRelDTO rel = new DiscoveryRelDTO();
+ rel.setSelectorId("selector-exported");
+ discoveryDTO.setDiscoveryRel(rel);
+
+
given(discoveryMapper.selectAllByNamespaceId("ns-import")).willReturn(Collections.emptyList());
+ given(discoveryMapper.insert(any(DiscoveryDO.class))).willReturn(1);
+
+ discoveryService.importData("ns-import",
Collections.singletonList(discoveryDTO), context);
+
+ verify(discoveryRelMapper,
never()).insertSelective(any(DiscoveryRelDO.class));
+ }
+
+ @Test
+ public void testImportSkipsRelWhenProxySelectorMappingMissing() {
+ final ConfigsImportContext context = new ConfigsImportContext();
+ DiscoveryDTO discoveryDTO = buildImportDiscovery();
+ DiscoveryRelDTO rel = new DiscoveryRelDTO();
+ rel.setProxySelectorId("proxy-selector-exported");
+ discoveryDTO.setDiscoveryRel(rel);
+
+
given(discoveryMapper.selectAllByNamespaceId("ns-import")).willReturn(Collections.emptyList());
+ given(discoveryMapper.insert(any(DiscoveryDO.class))).willReturn(1);
+
+ discoveryService.importData("ns-import",
Collections.singletonList(discoveryDTO), context);
+
+ verify(discoveryRelMapper,
never()).insertSelective(any(DiscoveryRelDO.class));
+ }
+
+ @Test
+ public void testImportRemapsRelReferencesWhenMappingsExist() {
+ final ConfigsImportContext context = new ConfigsImportContext();
+ context.getSelectorIdMapping().put("selector-exported",
"selector-new");
+ DiscoveryDTO discoveryDTO = buildImportDiscovery();
+ DiscoveryRelDTO rel = new DiscoveryRelDTO();
+ rel.setSelectorId("selector-exported");
+ discoveryDTO.setDiscoveryRel(rel);
+
+
given(discoveryMapper.selectAllByNamespaceId("ns-import")).willReturn(Collections.emptyList());
+ given(discoveryMapper.insert(any(DiscoveryDO.class))).willReturn(1);
+
+ discoveryService.importData("ns-import",
Collections.singletonList(discoveryDTO), context);
+
+ ArgumentCaptor<DiscoveryRelDO> relCaptor =
ArgumentCaptor.forClass(DiscoveryRelDO.class);
+ verify(discoveryRelMapper).insertSelective(relCaptor.capture());
+ assertEquals("selector-new", relCaptor.getValue().getSelectorId());
+ }
+
+ private DiscoveryDTO buildImportDiscovery() {
+ DiscoveryDTO discoveryDTO = new DiscoveryDTO();
+ discoveryDTO.setName("discovery-import");
+ discoveryDTO.setPluginName("tcp");
+ discoveryDTO.setType("local");
+ DiscoveryHandlerDTO handlerDTO = new DiscoveryHandlerDTO();
+ handlerDTO.setId("handler-exported");
+ handlerDTO.setListenerNode("/shenyu/discovery");
+ handlerDTO.setHandler("");
+ discoveryDTO.setDiscoveryHandler(handlerDTO);
+ return discoveryDTO;
+ }
+
+ @Test
+ public void testImportKeepsBlankSelectorIdShapeAndRemapsProxySelector() {
+ ConfigsImportContext context = new ConfigsImportContext();
+ context.getProxySelectorIdMapping().put("proxy-selector-exported",
"proxy-selector-new");
+ DiscoveryDTO discoveryDTO = buildImportDiscovery();
+ DiscoveryRelDTO rel = new DiscoveryRelDTO();
+ rel.setSelectorId("");
+ rel.setProxySelectorId("proxy-selector-exported");
+ discoveryDTO.setDiscoveryRel(rel);
+
+
given(discoveryMapper.selectAllByNamespaceId("ns-import")).willReturn(Collections.emptyList());
+ given(discoveryMapper.insert(any(DiscoveryDO.class))).willReturn(1);
+
+ discoveryService.importData("ns-import",
Collections.singletonList(discoveryDTO), context);
+
+ ArgumentCaptor<DiscoveryRelDO> relCaptor =
ArgumentCaptor.forClass(DiscoveryRelDO.class);
+ verify(discoveryRelMapper).insertSelective(relCaptor.capture());
+ assertEquals("", relCaptor.getValue().getSelectorId());
+ assertEquals("proxy-selector-new",
relCaptor.getValue().getProxySelectorId());
+ }
}
