Repository navigation
fix(admin): carry namespaceid on appauth delete events - #7962
Merged
Merged
Conversation
Contributor
|
/retest |
Aias00
approved these changes
Oct 22, 2026
Aias00
left a comment
Aias00
left a comment
Contributor
There was a problem hiding this comment.
Review: #7962 fix(admin): carry namespaceid on appauth delete events
Verdict: approved — cleanly fixes namespace data loss when deleting AppAuth records, ensuring multi-namespace cache eviction operates correctly on downstream gateways.
What I verified
- Problem:
- In
AppAuthServiceImpl.delete(List<String> ids), the previous implementation builtAppAuthDatawithout setting.namespaceId(...), and dispatched a singlenew DataChangedEvent(ConfigGroupEnum.APP_AUTH, DataEventTypeEnum.DELETE, appAuthData)without passingnamespaceId. - In multi-namespace environments, gateways listening for namespace-scoped synchronization either ignored the deletion event or failed to evict auth credentials under non-default namespaces.
- In
- Solution:
AppAuthData.builder()now preserves.namespaceId(...), defaulting blank or null IDs toConstants.SYS_DEFAULT_NAMESPACE_IDfor legacy data compatibility.- Deleted records are partitioned via
.collect(Collectors.groupingBy(AppAuthData::getNamespaceId)). - Separate, scoped
DataChangedEventinstances with explicitnamespaceIdare published for each partition:namespaceDataList.forEach((namespaceId, dataList) -> eventPublisher.publishEvent( new DataChangedEvent(ConfigGroupEnum.APP_AUTH, DataEventTypeEnum.DELETE, dataList, namespaceId)));
- Test verification:
AppAuthServiceTestadds thorough unit tests:testDeletePreservesNamespace: verifies single-namespace event emission.testDeletePublishesOneEventPerNamespace: verifies multi-namespace batches emit distinct events per namespace with correct data payloads.testDeleteFallsBackToDefaultNamespaceWhenMissing: checks fallback to default namespace.- Negative cases: empty/null IDs, unmapped IDs, and zero-row deletions do not publish events.
- CI:
- Unit tests,
pr_build, license check, CodeQL, and e2e passed.
- Unit tests,
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the problem
AppAuthServiceImpl#deletebuilt theAppAuthDataof a DELETE event without anamespaceId, and published one event for the whole batch.Data-changed listeners treat a null namespace as
SYS_DEFAULT_NAMESPACE_ID:AbstractNodeDataChangedListener#onAppAuthChangedbuilds the key prefix fromdefault, so the keyns-A.<authDataId>.<appKey>is never deleted.WebsocketDataChangedListenergroups by each item's namespace, so the DELETE message only reaches gateways of the default namespace.Impact: after
POST /appAuth/batchDeletein a non-default namespace (e.g.ns-A), gateways ofns-Akeep the deleted appKey/secret, and signed requests with it are still accepted. If the default namespace has an app auth with the same appKey, that one is wrongly removed instead.What this PR does
namespaceIdon everyAppAuthDataof the DELETE event. A missing value on a legacy row falls back toSYS_DEFAULT_NAMESPACE_ID, the same default the listeners use, so the delete is never rolled back by an NPE.DataChangedEvent(APP_AUTH, DELETE, data, namespaceId)per namespace. A batch spanning several namespaces is no longer routed by its first item only (findFirst()in the node listeners).Delivery timing is unchanged:
DataChangedEventDispatcherstill defers every event toafterCommit. In cluster mode,ClusterDataChangedEventForwarderalready forwards the event'snamespaceIdto the master.Tests
AppAuthServiceTest:ns-Aand in the default namespace keeps the namespace on both the event and its datans-A×2,ns-B, default) publishes exactly 3 events, each with only its own app authsMake sure that:
./mvnw clean install -Dmaven.javadoc.skip=true.