Skip to content

fix(client): tolerate null connection lists from client providers - #2926

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-client-null-lists
Open

fix(client): tolerate null connection lists from client providers#2926
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-client-null-lists

Conversation

@yyqdbngt

@yyqdbngt yyqdbngt commented Sep 1, 2026

Copy link
Copy Markdown

Summary

The client connection endpoints trust the provider layer to return non-null lists, but the ClientProvider contract only declares List<...> return types:

  • ProducerConnectionService.listConnections() called .stream() directly on the provider result, so a provider reporting an unavailable lookup as null surfaced as a raw NPE 500.
  • ClientService and listProducerGroups() passed a null straight through the API as data: null, breaking the list contract the frontend maps over.

The service layer now normalizes a null provider result to an empty list, matching how the provider implementations already report "nothing found" (List.of()).

Why

The provider interface is the boundary between Studio and the RocketMQ runtime; implementations can report unavailable lookups as null without violating anything, and the endpoints should degrade to "no connections" instead of a 500 or a null payload.

Testing

Extended ClientServiceTest (+2) and ProducerConnectionServiceTest (+2) with null-provider-result cases for all four endpoints.

mvn -q -Dtest="ClientServiceTest,ProducerConnectionServiceTest" test
Tests run: 12, Failures: 0, Errors: 0, Skipped: 0

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean defensive fix that prevents NPE when client providers return null for unavailable lookups.

Observations

  • Correctness ✓ — nullToEmpty() helper centralizes the null-safety pattern, applied consistently across ClientService, ProducerConnectionService, and ConsumerConnectionService.
  • Tests ✓ — Good coverage including null-returning mock scenarios.
  • Design — The Javadoc on nullToEmpty clearly explains the rationale.

Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants