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

Reply via email to