dajac commented on code in PR #14635:
URL: https://github.com/apache/kafka/pull/14635#discussion_r1383049498
##########
clients/src/main/java/org/apache/kafka/common/requests/LeaveGroupResponse.java:
##########
@@ -62,7 +62,7 @@ public LeaveGroupResponse(LeaveGroupResponseData data, short
version) {
if (version >= 3) {
this.data = data;
} else {
- if (data.members().size() != 1) {
Review Comment:
I was looking a bit more into this issue. I think that we could harden a bit
the logic because we could effectively get no members only when there is a top
level error. How about this?
```
if (data.errorCode() != Errors.NONE.code()) {
this.data = new
LeaveGroupResponseData().setErrorCode(data.errorCode());
} else {
if (data.members().size() != 1) {
throw new UnsupportedVersionException("LeaveGroup
response version " + version +
" can only contain one member, got " +
data.members().size() + " members.");
}
this.data = new
LeaveGroupResponseData().setErrorCode(data.members().get(0).errorCode());
}
```
@dongnuo123 What do you think?
##########
clients/src/test/java/org/apache/kafka/common/requests/LeaveGroupResponseTest.java:
##########
@@ -165,4 +167,33 @@ public void testEqualityWithMemberResponses() {
assertEquals(primaryResponse.hashCode(),
reversedResponse.hashCode());
}
}
+
+ @Test
Review Comment:
nit: You could use the following to parameterized the test.
```
@ParameterizedTest
@ApiKeyVersionsSource(apiKey = ApiKeys.LEAVE_GROUP)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
