fix(ai): populate subscriptionMode and consumeType for cloud consumer groups - #4134
fix(ai): populate subscriptionMode and consumeType for cloud consumer groups#4134zjncs wants to merge 1 commit into
Conversation
… groups
The Aliyun and Tencent converters never set subscriptionMode on
ConsumerGroupVO, and AliyunConverters.toConsumeType returns null when the
API omits messageModel. ConsumerGroupListToolHandler (the rmq.group.list
AI tool) treats both fields as required and throws IllegalStateException
on the first null, so listing consumer groups is deterministically broken
for every cloud instance - the exact invariant the Apache provider
already documents ('surface it so read paths (web detail, AI
rmq.group.list) never see a null subscriptionMode').
Cloud TCP consumer groups are push consumers, so both converters now emit
SubscriptionMode.Push, and the Aliyun consume-type mapping falls back to
CLUSTERING for a missing/unrecognized messageModel, mirroring
RocketMQMetadataProvider.parseConsumeType.
Signed-off-by: zjncs <18910855655@163.com>
RockteMQ-AI
left a comment
RockteMQ-AI
left a comment
There was a problem hiding this comment.
LGTM. Trivial change, looks good.
Automated review by github-manager-bot
…data (#4115) The AI resource tool chain was broken end to end: every one of `rmq.topic.list` and `rmq.group.list` failed, and for cloud instances the failure happened even once routing was fixed. Four separate defects, each fixed here, plus one stale test that hid a contract drift. **Routing.** Both handlers passed the tool's `cluster` input into the `instanceId` slot — `listTopicsPage(clusterId, null, ...)` — so the request never reached the `!hasText(instanceId) && hasText(clusterId)` cluster-scoped branch. Because `cluster` is `required` with `minLength: 1` in the catalog, the legacy branch was unreachable and both tools threw `404 Instance not found: DefaultCluster` on every call. The arguments are now `(null, clusterId, ...)`. Regression introduced by #3052. **Output schema.** `rmq-tools.yaml` declared `cluster.proxies`, `stats.totalProxies`, `stats.totalNameServers` and `items[].totalLag` as `type: integer` while keeping them `required`, but the handlers deliberately emit `null` to mean "unknown" — `totalLag` does so for `ConsumerLagResolver.UNKNOWN` since #3988. `ToolGatewayService.validateOutput` preserves null keys through `valueToTree`, so listing any group with an unknown lag raised `IllegalStateException` and returned 500. Those four are now `type: [integer, 'null']`; they are the complete reachable set of integer output fields whose producers can emit null. **Cloud topic type and perm.** The Aliyun converter never set `perm` at all, so every Aliyun topic carried a null perm and `TopicListToolHandler.safeProjection` threw on its first `requiredEnumName` call. Both cloud `toTopicType` implementations also returned null for a blank or unrecognised type. They now guarantee non-null: `TopicPerm.RW` for Aliyun (its ListTopics API returns no permission field, and console-created cloud topics are read-write, matching the Tencent provider) and `TopicType.NORMAL` as the fallback, mirroring the Apache provider's `parseTopicType`. The fields that no provider ever populates — `messageCount`, `tps`, `consumerGroupCount` — are deliberately untouched. **Cloud consumer group subscription mode.** Neither cloud provider set `subscriptionMode`, and Aliyun's `toConsumeType` returned null unless `messageModel` was exactly "Clustering", so `requiredEnumName` threw for every cloud consumer group. Both now set `SubscriptionMode.Push` — the pinned `alibabacloud-rocketmq20220801` response body has no subscription-mode field at all, and cloud TCP groups are push consumers — and `toConsumeType` returns `CLUSTERING`. **Test contract drift.** `AliyunInstanceProviderTest.getGroupProgressShouldMapLagRowsTest` still expected a per-topic row *and* a `broker="total"` aggregate row, but `AliyunConverters.toQueueProgressRows` has only emitted the aggregate as a fallback when there are no topic rows since #2907. The expectation was stale, not the production code: emitting both would double-count lag, because `CloudRocketMqBusinessMetricsCollector` sums `getDiffTotal()` across every row it is given. The test now asserts the fallback-only contract and a second case covers the no-topic-breakdown path. This was one of the three standing backend red lights. Folded in from #4117, #4134, #4135 and #4137, all by the same author and all part of this one chain; those PRs are closed as superseded. Merging any single one of them would not have made the tools usable.
|
Closed as folded into #4115, merged into Your change shipped as-is: both cloud providers now set This one had to be integrated rather than merged in sequence: it and #4137 both add an import to Nothing further needed from you — thank you for the fix. For context on why these were grouped: the AI resource tool chain had four independent defects (argument-slot routing, output-schema nullability, cloud topic type/perm, cloud consumer-group subscription mode) and merging any single one of them would not have made |
Motivation
The
rmq.group.listAI tool is deterministically broken for Aliyun and Tencent instances:ConsumerGroupListToolHandler.safeProjectionrequires bothsubscriptionModeandconsumeType(requiredEnumNamethrowsIllegalStateExceptionon the first null).AliyunConverters.toConsumerGroupVOnever setssubscriptionModeand itstoConsumeTypereturns null when the OpenAPI response omitsmessageModel.TencentInstanceProvider.toConsumerGroupnever setssubscriptionModeeither (its consume type is always CLUSTERING).So the first group of any cloud instance trips
IllegalStateException: Consumer group subscriptionMode is unavailable: <name>and the whole tool call fails.This contradicts the invariant the Apache provider already established in
RocketMQMetadataProvider.toConsumerGroupVO:The invariant was fixed for Apache only; both cloud converters were missed. Existing tests never catch it because they always pre-set the enum (e.g.
ToolGatewayServiceTestbuilds groups withSubscriptionMode.Push).Modification
SubscriptionMode.Push: cloud TCP consumer groups are push consumers (Aliyun'smessageModelcarries the consume model, not the subscription mode; Tencent has no POP groups).AliyunConverters.toConsumeTypefalls back toCLUSTERINGfor a missing/unrecognizedmessageModel, mirroringRocketMQMetadataProvider.parseConsumeType("falling back to CLUSTERING").Verification
mvn -f server/pom.xml test -Dtest='AliyunInstanceProviderTest,TencentInstanceProviderTest'fail-before (fix reverted, tests kept):
pass-after:
Follow-up to the Apache-side invariant; no associated issue — found by code inspection.