[f1amingo]

Which Issue(s) This PR Fixes

Fixes #11466

Brief Description

validateGroup currently compares against the compile-time constant GROUP_MAX_LENGTH directly. This PR extracts the group max length into a field, keeping the default limit of 120 and all validation behavior unchanged. A guarded setter (values above 255 are capped, non-positive values fall back to 120) preserves the retry/DLQ topic length invariant. All three group creation paths (single create, batch create, auto-create) require no per-call-site changes.

How Did You Test This Change?

Ran TopicValidatorTest with JDK 8; 10 tests passed, including the new cases covering the 255 boundary, the cap, and the fallback to the default.

[RockteMQ-AI]

LGTM. Clean implementation that adds broker-side configurability for subscription group max length while preserving backward compatibility.

Summary

• Added subscriptionGroupMaxLength to BrokerConfig (default 120, capped at 255)
• Introduced overloaded TopicValidator.validateGroup(String, int) with defensive bounds checking
• Updated 3 broker-side call sites (single create, batch create, auto-create) to use configured value
• Comprehensive test coverage (121 tests total across 3 test classes)

Design Notes

The implementation correctly preserves backward compatibility:

• Default value (120) matches the existing GROUP_MAX_LENGTH constant
• Client-side and gRPC validation remain unchanged (still use hardcoded 120)
• Only broker-side operations are affected

The cap at 255 (RETRY_OR_DLQ_TOPIC_MAX_LENGTH) is appropriate because retry/DLQ topics are constructed as RETRY<group> and %DLQ%<group> (5 chars + group length). As noted in the PR description, groups exceeding 122 characters cannot use DLQ topics due to the 127-byte broker limit on topic names.

Minor Observation

The PR description mentions that callers must ensure retry topics stay within 255 characters when raising the limit, but the code doesn't enforce this automatically. This is a reasonable design choice (caller responsibility), but worth documenting in BrokerConfig Javadoc if not already present.

Test Coverage

• ✅ Boundary tests (0, negative, 255, >255 values)
• ✅ Integration tests (AdminBrokerProcessor, SubscriptionGroupManager)
• ✅ Validation logic tests (TopicValidator)

All tests pass on JDK 8 as reported. Ready to merge.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.78%. Comparing base (57c2dc0) to head (c659141).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #11467      +/-   ##
=============================================
- Coverage      49.79%   49.78%   -0.01%     
- Complexity     14387    14389       +2     
=============================================
  Files           1390     1391       +1     
  Lines         103264   103356      +92     
  Branches       13504    13518      +14     
=============================================
+ Hits           51419    51460      +41     
- Misses         45657    45676      +19     
- Partials        6188     6220      +32     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds subscriptionGroupMaxLength to BrokerConfig (default 120, capped at 255), making the broker-side subscription group name length configurable. All broker-side validation points (single create, batch create, auto-create) use the configured value, while client and gRPC validation remain unchanged for backward compatibility.

Findings

• [Info] The cap at 255 (RETRY_OR_DLQ_TOPIC_MAX_LENGTH) is correct — it prevents users from configuring a value that would break retry/DLQ topic generation. The Javadoc clearly warns that groups over 122 characters cannot use %DLQ%<group> due to the 127-byte broker write limit.
• [Info] The fallback logic in validateGroup(group, maxLength) — non-positive values use the default, values above 255 are capped — is defensive and prevents misconfiguration.
• [Info] Batch creation correctly rejects the entire batch if any group exceeds the limit (testUpdateAndCreateSubscriptionGroupListRejectsWholeBatchWhenOneTooLong). This is the right behavior — partial creation would be confusing.
• [Info] Test coverage is comprehensive: configurable length, cap at 255, fallback to default, blank/illegal validation, auto-create, single/batch creation. The tests verify both the validation result and the remark message content.

Suggestions

• Consider logging a warning at broker startup if subscriptionGroupMaxLength is set above 122, reminding operators that DLQ topics for groups in the range [123, 255] will fail. This makes the constraint visible in logs rather than only in documentation.
• The validateGroup(group, maxLength) overload shadows the local variable groupMaxLength with the parameter. Consider renaming the parameter to configuredMaxLength for clarity, or use this.groupMaxLength to make the shadowing explicit.

Overall this is a solid enhancement with good defensive design and comprehensive testing.

---

Automated review by github-manager-bot