lymerin commented on code in PR #7289:
URL: https://github.com/apache/shenyu/pull/7289#discussion_r4114666458
##########
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 review. I chose the fail-fast option: `unbindDiscovery` now
throws `ShenyuAdminException` before deleting any rows or publishing removal
events when the plugin name cannot be resolved. The error identifies the
selector and states that no selectors in the batch were deleted. I also updated
the PR description to make this deletion failure behavior explicit and updated
the related tests.
--
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]
