Aias00 commented on code in PR #7143:
URL: https://github.com/apache/shenyu/pull/7143#discussion_r4060323176
##########
shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/main/java/org/apache/shenyu/register/client/http/HttpClientRegisterRepository.java:
##########
@@ -169,14 +171,28 @@ protected void doPersistMcpTools(final
McpToolsRegisterDTO registerDTO) {
@Override
public void closeRepository() {
- if (Objects.nonNull(uriRegisterDTO)) {
- uriRegisterDTO.setEventType(EventType.DELETED);
- doRegister(uriRegisterDTO, Constants.URI_PATH, Constants.URI);
- }
- if (Objects.nonNull(apiDocRegisterDTO)) {
- apiDocRegisterDTO.setEventType(EventType.OFFLINE);
- doRegister(apiDocRegisterDTO, Constants.API_DOC_PATH,
Constants.API_DOC_TYPE);
- }
+ uriRegisterDTOs.values().forEach(registerDTO -> {
Review Comment:
The move from `static` fields to per-instance maps is correct and fixes real
cross-instance leakage (two `HttpClientRegisterRepository` instances in one JVM
previously overwrote each other's pending offline payload, and the
`resetStatics()` reflection helper in the test was a symptom of that).
Two follow-ups, non-blocking:
1. `closeRepository()` iterates the maps but never clears them. A second
call (or a close followed by any later re-registration + close) re-sends
`DELETED`/`OFFLINE` for entries that were already offlined, and the repository
keeps holding every DTO until it is GC'd. `uriRegisterDTOs.clear()` /
`apiDocRegisterDTOs.clear()` after the loops would make close idempotent.
2. `uriIdentity` / `apiDocIdentity` here duplicate the identity logic that
PR #7142 adds to `FailbackRegistryRepository` (`metaDataIdentity`). The two key
spaces can drift apart silently - failback retries and shutdown offline events
would then disagree about what "the same registration" means. Consider a small
shared helper so both call sites derive identity from one place.
Also note `apiDocIdentity` has no namespace dimension (`ApiDocRegisterDTO`
carries no `namespaceId`), so API docs with the same path in different
namespaces still collapse to one entry - same limitation as #7142. Fine for
now, but worth a comment in the code so it is a known gap rather than an
oversight.
--
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]
