apache / apache/shenyu

[BUG] Discovery upstream import checks duplicates before remapping handler ids

Closed
#6,465 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
8.8k
Forks
3.1k
Avg merge
7d 1h
Merged PRs (30d)
85

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current Behavior

Namespace import remaps exported discovery handler ids to target-namespace handler ids in `DiscoveryServiceImpl#importData(...)`:

```java
discoveryHandlerIdMapping.put(discoveryHandlerDO.getId(), discoveryHandlerId);
discoveryHandlerDO.setId(discoveryHandlerId);
```

But `DiscoveryUpstreamServiceImpl#importData(...)` checks duplicate upstream URLs before applying that mapping:

```java
String discoveryHandlerId = discoveryUpstreamDTO.getDiscoveryHandlerId();
Set existsUpstreamUrlSet = discoveryHandlerUpstreamMap
.getOrDefault(discoveryHandlerId, Lists.newArrayList())
.stream()
.map(DiscoveryUpstreamDO::getUpstreamUrl)
.collect(Collectors.toSet());

if (existsUpstreamUrlSet.contains(url)) {
...
continue;
}

discoveryUpstreamDTO.setDiscoveryHandlerId(discoveryHandlerIdMapping.get(discoveryUpstreamDTO.getDiscoveryHandlerId()));
discoveryUpstreamMapper.insert(discoveryUpstreamDO);
```

`discoveryHandlerUpstreamMap` is grouped by target database handler ids, but the lookup uses the exported/source handler id. So an existing upstream under the remapped target handler is not detected. The insert can then hit the database unique key `(discovery_handler_id, upstream_url)`.

Because this import method is not transactional, a duplicate-key error can also leave earlier imported upstream rows committed while later rows fail.

### Expected Behavior

The import flow should remap `discoveryHandlerId` before duplicate lookup and insert, then check duplicates against the target handler id.

Imports should either skip/report duplicate upstreams consistently or run in a transaction so partial imports are not left behind after an insert failure.

### Steps To Reproduce

1. Export/import discovery data into a namespace where the corresponding target discovery handler already has one of the upstream URLs.
2. During import, `DiscoveryServiceImpl` maps the exported handler id to a new target handler id.
3. `DiscoveryUpstreamServiceImpl#importData(...)` checks duplicates using the old exported handler id, so it does not see the existing target row.
4. The insert uses the remapped target handler id and can violate `UNIQUE(discovery_handler_id, upstream_url)`.

### Environment

```markdown
ShenYu version(s): current master
```

### Debug logs

_No response_

### Anything else?

Relevant current-master files:

- `shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java`
- `shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java`
- `db/init/mysql/schema.sql`

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with importData(...) in shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryServiceImpl.java and DiscoveryUpstreamServiceImpl.java, then inspect the unique key in db/init/mysql/schema.sql. Trace when discoveryHandlerIdMapping is applied and how discoveryHandlerUpstreamMap is built. Done means duplicate upstreams are checked under the target handler id and imports consistently skip/report duplicates or avoid partial committed data.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.