MINOR: Enforce non-null groupState in GroupListing constructor - #23324
Open
regarmukesh3g wants to merge 1 commit into
Open
MINOR: Enforce non-null groupState in GroupListing constructor#23324regarmukesh3g wants to merge 1 commit into
regarmukesh3g wants to merge 1 commit into
Conversation
The groupState parameter was not validated with Objects.requireNonNull, unlike the sibling type parameter in the same constructor and unlike the equivalent groupState field in ConsumerGroupListing. Add the missing check and cover both null cases with unit tests.
see-quick
approved these changes
Sep 1, 2026
see-quick
left a comment
see-quick
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, I wonder why Andrew didn't introduce it in https://github.com/apache/kafka/pull/17763/changes#diff-cc7bbaf7e3aedc6d4580c1d9ff876f24cd5897454c1072335dcb3269f5664287R49, but I assume that he overlooked it so LGTM, assuming test will pass 👍..
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
GroupListing's constructor validates thetypeparameter withObjects.requireNonNullbut not the siblinggroupStateparameter,even though both are
Optional<T>fields used identically throughoutthe class (
toString(),equals(),hashCode(), accessors).The omission was introduced when
groupStatewas added alongside thepre-existing
typefield in KAFKA-17949 (#17763) — the constructorpattern was copied but the
requireNonNullwrapper on the new fieldwas dropped.
For comparison, the equivalent (deprecated)
ConsumerGroupListingclass validates both of its analogous
Optionalfields (groupStateand
type) consistently.This PR adds the missing
Objects.requireNonNull(groupState)checkand adds unit tests covering the null case for both parameters.
Validation
GroupListingTesttests pass (including the two new ones)clients,tools,core,and
connect/mirrormodules always passOptional.of(...)orOptional.empty(), never a rawnull, forgroupState— so thischange introduces no behavioral regression
Reviewers: Maros Orsak maros.orsak159@gmail.com