lianetm commented on code in PR #19461: URL: https://github.com/apache/kafka/pull/19461#discussion_r2049165848
########## group-coordinator/src/test/java/org/apache/kafka/coordinator/group/OffsetMetadataManagerTest.java: ########## @@ -1308,6 +1308,75 @@ public void testConsumerGroupOffsetCommit() { ); } + @Test + public void testConsumerGroupOffsetCommitWithTopicIds() { + Uuid topicId = Uuid.randomUuid(); + OffsetMetadataManagerTestContext context = new OffsetMetadataManagerTestContext.Builder().build(); + + // Create an empty group. + ConsumerGroup group = context.groupMetadataManager.getOrMaybeCreatePersistedConsumerGroup( + "foo", + true + ); + + // Add member. + group.updateMember(new ConsumerGroupMember.Builder("member") + .setMemberEpoch(10) + .setPreviousMemberEpoch(10) + .build() + ); + + CoordinatorResult<OffsetCommitResponseData, CoordinatorRecord> result = context.commitOffset( + new OffsetCommitRequestData() + .setGroupId("foo") + .setMemberId("member") + .setGenerationIdOrMemberEpoch(10) + .setTopics(List.of( + new OffsetCommitRequestData.OffsetCommitRequestTopic() + .setTopicId(topicId) + .setName("bar") Review Comment: great, same page (at the protocol level is one or the other, internally normalized so having both) ########## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java: ########## @@ -45,20 +45,24 @@ public static class Builder extends AbstractRequest.Builder<OffsetCommitRequest> private final OffsetCommitRequestData data; - public Builder(OffsetCommitRequestData data, boolean enableUnstableLastVersion) { - super(ApiKeys.OFFSET_COMMIT, enableUnstableLastVersion); + private Builder(OffsetCommitRequestData data, short oldestAllowedVersion, short latestAllowedVersion) { + super(ApiKeys.OFFSET_COMMIT, oldestAllowedVersion, latestAllowedVersion); this.data = data; } - public Builder(OffsetCommitRequestData data) { - this(data, false); + public static Builder forTopicIdsAndNames(OffsetCommitRequestData data, boolean enableUnstableLastVersion) { Review Comment: agree, I was actually confused looking at this as if we already knew the version (build phase, agree). Makes sense to validate we have both in the OffsetCommitRequestData indeed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org