Sean-Walker0 commented on PR #7365:
URL: https://github.com/apache/shenyu/pull/7365#issuecomment-5885447947
All three suggestions adopted — thank you, with one safety note on the
second:
1. `Optional.ofNullable(shenyuDiscoveryService).ifPresent(service ->
service.unWatchInstances(key))` — done, matches the outer idiom.
2. Empty-set cleanup — the leak is real, but removing the entry alone would
break **re-registration**: `getCacheKey` returned the raw map entry, and both
`createProxySelector` variants (`Default`/`AP`) reach `cacheKey.add(key)` after
the contains check, which NPEs on a missing entry. So the cleanup lands
together with `getCacheKey` now using `computeIfAbsent(discoveryId, k -> new
HashSet<>())` — unregistered discoveries are still rejected earlier by the
service null check. The test now asserts the tombstone entry is gone and that
`getCacheKey` hands re-registration a usable set.
3. Test casts parameterized with `@SuppressWarnings("unchecked")`.
Full `shenyu-admin` module suite + checkstyle green.
--
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]
