Skip to content

[ISSUE #995] Harden cluster page against null component lists for real clusters - #1036

Merged
lizhimins merged 1 commit into
apache:rocketmq-studiofrom
123123213weqw:fix/cluster-null-safety-995
Aug 6, 2026
Merged

[ISSUE #995] Harden cluster page against null component lists for real clusters#1036
lizhimins merged 1 commit into
apache:rocketmq-studiofrom
123123213weqw:fix/cluster-null-safety-995

Conversation

@123123213weqw

Copy link
Copy Markdown

What is the purpose of the change

When a real RocketMQ cluster (configured via STUDIO_ROCKETMQ_NAMESRV_ADDR) is described by RealClusterProvider and no proxy is provisioned, the produced ClusterVO leaves proxies, tpsHistory and config as null. The cluster page dereferences them directly (c.proxies.length, cluster.config, summary counts), so a null payload throws a TypeError and the page goes blank.

This PR makes the payload safe on both sides:

  • Backend: RealClusterProvider now defaults a live cluster's absent runtime collections to empty — proxies/tpsHistory as List.of(), config as an empty ClusterConfigVO — so the API never serializes JSON null for these component lists.
  • Frontend: every brokers/proxies/nameServers/config access in the cluster page is guarded with ?? [] / ?? {}, and the summary counts are extracted into a pure countClusterComponents helper with unit tests.

Brief changelog

  • server/.../cluster/broker/RealClusterProvider.java: default config and tpsHistory to safe empty values (proxies already defaulted to List.of()).
  • web/src/pages/cluster/index.tsx: null-safe access to brokers, proxies, nameServers, selectedProxy.proxies, cluster.config; use countClusterComponents.
  • web/src/pages/cluster/clusterStats.ts (new): pure summary-count helper with empty-collection fallbacks.
  • web/src/pages/cluster/clusterStats.test.ts (new): unit tests for empty, mixed and null component lists.
  • server/.../cluster/broker/RealClusterProviderTest.java: new test asserting non-null proxies/tpsHistory/config for a live cluster without a proxy.

Verifying this change

  • mvn test -Dtest=RealClusterProviderTest — 4 tests pass (3 existing + 1 new).
  • npm test (vitest run) — 391 tests pass, including the new clusterStats.test.ts.
  • eslint src/pages/cluster — no errors.

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Closes #995

@123123213weqw

Copy link
Copy Markdown
Author

Hello maintainers! This is my first pull request to this repo, so the CI workflow needs approval to run. @Aias00 @zhaohai could you please approve the workflow run for this PR? All tests pass locally (backend mvn test -Dtest=RealClusterProviderTest 4/4, frontend vitest run 391/391, eslint clean). Thanks!

@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 — defensive improvement with proper error handling and test coverage.


Automated review by github-manager-bot

@RockteMQ-AI

Copy link
Copy Markdown

⚠️ Merge conflict detected

This PR has conflicts with the base branch and cannot be merged. Please rebase or merge the base branch into your branch and resolve the conflicts:

git fetch origin
git checkout fix/cluster-null-safety-995
git rebase origin/rocketmq-studio
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved.


Automated notification by github-manager-bot

@lizhimins
lizhimins force-pushed the fix/cluster-null-safety-995 branch from dd4c635 to f2497a9 Compare August 6, 2026 05:43
@lizhimins
lizhimins merged commit 48794d5 into apache:rocketmq-studio Aug 6, 2026
3 checks passed
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