fix(admin): ignore heartbeats with an incomplete instance identity (#7764) - #7776

LGTM — the guard sits at the right level and I traced all in-repo producers before approving.

Evidence

• Websocket: WebsocketCollector.onOpen (:121-124) throws when the namespace is blank, so an open session always has one. The bootstrap report at :200-211 can only produce null, and only through the "session is closed" branch of getNamespaceId — exactly the case that used to create the :8080@grpc#ns style entry.
• Long polling: HttpLongPollingDataChangedListener:145,155 forwards a namespace that defaults to SYS_DEFAULT_NAMESPACE_ID (:317-323). Note that this path can send a blank port when the X-Real-Port header is missing (:147-151 publishes StringUtils.EMPTY), so your guard also closes a real gap there.
• Register flow: AbstractShenyuClientRegisterServiceImpl:265-271 forwards uriRegisterDTO.getNamespaceId(), which the admin fills with SYS_DEFAULT_NAMESPACE_ID (see RegistrationProtocolCompatibilityTest:120), and the client reporter defaults shenyuConfig.getNamespace() to the same constant (ShenyuConfig:79).

Two asks (non-blocking, but please answer in the PR body)

1. /instance/beat (InstanceController:115-119) still returns SUCCESS for a dropped report, so a misconfigured client keeps beating forever with no signal on its side. You already validate the identity in the service; surfacing it as a 4xx from the controller (or at least documenting the silent drop) would save the next operator an afternoon.
2. Consider defaulting only the namespace — StringUtils.defaultIfBlank(ns, SYS_DEFAULT_NAMESPACE_ID) — and keeping the hard drop for ip/port/type. An operator who sets shenyu.namespace= to blank currently gets a working (if unsightly) instance today and would get nothing after this change. Dropping is defensible; dropping silently is the part I would avoid either way.

Re-approving after the follow-up. Both points from my previous review are addressed, and I checked the two claims that carry the fix.

Namespace defaulting is the correct value

Constants.SYS_DEFAULT_NAMESPACE_ID (649330b6-...) is the same value ClientRegisterConfig falls back to when shenyu.namespace is blank, so a blank beat now lands on the key that an already-registered instance has — fetchInstanceData loads stored rows under that id, which is why blank beats used to compute ip:port@type# and never match. Verified on the producer side too: HeartbeatListener:102-121 builds the DTO from shenyuConfig.getNamespace(), which is blank when the operator does not configure a namespace, so the in-code comment is accurate rather than an assumption.

The rejected-beat signal is contained

RegisterUtils.doHeartBeat:111-116 throws IOException when the body is not success, so the choice of PARAMETER_ERROR over a 4xx only matters through that path. Both callers contain it: HeartbeatListener#sendHeartbeat:122-126 catches per server and logs "will retry", and the HTTP long-polling listener does the same. That means a permanently incomplete identity yields one ERROR per beat interval on the client and cannot cancel the scheduleAtFixedRate reporter — a visible signal without a wire-contract change, which is what I asked for. A 4xx would have been contained identically; keeping 200 is the lower-risk choice, and I am fine with it.

Removing the duplicate setLastHeartBeatTime and the now-redundant namespace term in isBlankIdentity are both correct follow-ons.

Non-blocking

1. handleBeatInfo now mutates the caller's DTO (setNamespaceId). It keeps the key and the VO consistent in one place, but the method has gained an input side effect; a local defaultIfBlank(...) passed to both the key builder and the VO would be purer. getInstanceKey(dto) is used elsewhere, so leaving it as is, is reasonable — just be aware the DTO handed in comes back modified.
2. isBlankIdentity no longer checks the namespace, which is right after defaulting, but the name still reads like a general identity check. One comment ("namespace is defaulted before this guard") keeps someone from re-adding it.

CI

pr_build (2m59s) and rat_check are green on 0b8551e1. The e2e red is one genuine failure plus cascade: e2e-case (e2e-logging-rocketmq) fails at DividePluginTest.testDivide:146 Failed to consume RocketMQ access log (a 31s wait on the access-log pipeline), and the other three e2e-case jobs are cancelled with "The operation was canceled.". Nothing in this diff touches the logging plugin, the RocketMQ client or the divide plugin, so I read that as the known log-pipeline timing flake — but e2e is a required check, so please re-run it before merge.

The guard is correct, and I checked the internal path as well: onInstanceInfoReport builds the DTO from InstanceInfoReportEvent, whose only producer (AbstractShenyuClientRegisterServiceImpl:265) filters on a non-blank host and a non-null port and passes InstanceTypeConstants.CLIENT_INSTANCE_TYPE, so nothing that reaches handleBeatInfo from the event bus loses its heartbeat because of this change. Defaulting a blank namespace to SYS_DEFAULT_NAMESPACE_ID also matches what every in-repo client sends.

Two notes, neither blocking:

1. beat() now reports the rejection in the response body (PARAMETER_ERROR) while still returning HTTP 200. A client that only checks the status code cannot tell the difference. Since InstanceBeatInfoDTO has no constraints at all, adding @NotBlank to ip/port/type would let @Valid reject it with a 400 before the controller is entered, and the body would stay a plain success message.
2. handleBeatInfo mutates the caller's DTO when it defaults the namespace. Harmless for the two current callers, but a String namespaceId = defaultIfBlank(...) local would keep the method side-effect free.

Tests cover the four blank-identity shapes plus the namespace defaulting, which is the right scope.

Reviewed the incomplete-heartbeat guard and controller result. Invalid identities are rejected before cache mutation, and blank namespaces are normalized to the default namespace for compatibility with existing producers. No blocking finding in this patch.

InstanceControllerTest and InstanceCheckServiceTest passed all 50 tests, zero failures/errors/skips. Validation baseline: PR head plus a clean, uncommitted merge of master 52339b2 in the isolated worktree. No source was manually patched or pushed.

Used isolated worktrees and the Maven wrapper with JDK 21/project compiler settings. Checkstyle passed for the targeted Maven runs; RAT and JaCoCo were skipped. No full reactor/e2e pass is claimed.