lymerin commented on code in PR #7289:
URL: https://github.com/apache/shenyu/pull/7289#discussion_r4114667753
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -308,22 +309,54 @@ public int deleteByNamespaceId(final List<String> ids,
final String namespaceId)
* @param selectors selectors
*/
private void unbindDiscovery(final List<SelectorDO> selectors, final
List<PluginDO> pluginDOS) {
- Map<String, String> pluginMap = ListUtil.toMap(pluginDOS,
PluginDO::getId, PluginDO::getName);
+ Map<String, String> pluginMap = new
HashMap<>(ListUtil.toMap(pluginDOS, PluginDO::getId, PluginDO::getName));
+ List<ResolvedDiscovery> resolvedDiscoveries = new ArrayList<>();
+ // Validate the whole batch before deleting any discovery rows or
publishing removal events.
for (SelectorDO selector : selectors) {
DiscoveryHandlerDO discoveryHandlerDO =
discoveryHandlerMapper.selectBySelectorId(selector.getId());
if (Objects.isNull(discoveryHandlerDO)) {
continue;
}
+ DiscoveryDO discoveryDO =
discoveryMapper.selectById(discoveryHandlerDO.getDiscoveryId());
+ String pluginName = null;
+ if (Objects.nonNull(discoveryDO)) {
+ pluginName = pluginMap.get(selector.getPluginId());
+ if (StringUtils.isBlank(pluginName)) {
+ PluginDO pluginDO =
pluginMapper.selectById(selector.getPluginId());
+ pluginName = Objects.isNull(pluginDO) ? null :
pluginDO.getName();
+ }
+ if (StringUtils.isBlank(pluginName)) {
+ pluginName = discoveryDO.getPluginName();
+ }
+ if (StringUtils.isBlank(pluginName)) {
+ throw new IllegalStateException("Cannot delete selector
batch: selector " + selector.getId()
Review Comment:
Thanks for the suggestion. I extended the guard to reject blank `selectorId`
values as well as blank plugin names, and added `namespaceId` to the warning.
Tests now cover a missing selector ID on DELETE and a blank selector ID on
UPDATE. The focused listener tests pass (5/5). This is included in `7f49de8`.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -308,22 +309,54 @@ public int deleteByNamespaceId(final List<String> ids,
final String namespaceId)
* @param selectors selectors
*/
private void unbindDiscovery(final List<SelectorDO> selectors, final
List<PluginDO> pluginDOS) {
- Map<String, String> pluginMap = ListUtil.toMap(pluginDOS,
PluginDO::getId, PluginDO::getName);
+ Map<String, String> pluginMap = new
HashMap<>(ListUtil.toMap(pluginDOS, PluginDO::getId, PluginDO::getName));
+ List<ResolvedDiscovery> resolvedDiscoveries = new ArrayList<>();
+ // Validate the whole batch before deleting any discovery rows or
publishing removal events.
for (SelectorDO selector : selectors) {
DiscoveryHandlerDO discoveryHandlerDO =
discoveryHandlerMapper.selectBySelectorId(selector.getId());
if (Objects.isNull(discoveryHandlerDO)) {
continue;
}
+ DiscoveryDO discoveryDO =
discoveryMapper.selectById(discoveryHandlerDO.getDiscoveryId());
+ String pluginName = null;
+ if (Objects.nonNull(discoveryDO)) {
+ pluginName = pluginMap.get(selector.getPluginId());
+ if (StringUtils.isBlank(pluginName)) {
+ PluginDO pluginDO =
pluginMapper.selectById(selector.getPluginId());
+ pluginName = Objects.isNull(pluginDO) ? null :
pluginDO.getName();
+ }
+ if (StringUtils.isBlank(pluginName)) {
+ pluginName = discoveryDO.getPluginName();
+ }
+ if (StringUtils.isBlank(pluginName)) {
+ throw new IllegalStateException("Cannot delete selector
batch: selector " + selector.getId()
Review Comment:
Thanks for the suggestion. I extended the guard to reject blank `selectorId`
values as well as blank plugin names, and added `namespaceId` to the warning.
Tests now cover a missing selector ID on DELETE and a blank selector ID on
UPDATE. The focused listener tests pass (5/5). This is included in `7f49de8`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
