Skip to content

KAFKA-9514; The protocol generator generated useless condition when a field is made nullable and flexible version is used#8793

Merged
cmccabe merged 1 commit intoapache:trunkfrom
dajac:KAFKA-9514
Jun 4, 2020
Merged

KAFKA-9514; The protocol generator generated useless condition when a field is made nullable and flexible version is used#8793
cmccabe merged 1 commit intoapache:trunkfrom
dajac:KAFKA-9514

Conversation

@dajac
Copy link
Copy Markdown
Member

@dajac dajac commented Jun 3, 2020

The protocol generator generates useless conditions when a field of type string is made nullable after the request has been converted to using optional fields.

As an example, we have make the field ProtocolName nullable in the JoinGroupResponse. The JoinGroupResponse supports optional fields since version 6 and the field is nullable since version 7. Under these conditions, the generator generates the following code:

if (protocolName == null) {
 if (_version >= 7) {
   if (_version >= 6) {
     _writable.writeUnsignedVarint(0);
   } else {
     _writable.writeShort((short) -1);
  }
 } else {
   throw new NullPointerException();
 }
}

spotbugs raises an UC_USELESS_CONDITION because _version >= 6 is always true.

This PR fixes the bug by propagating the outer versions to the underlying VersionConditional so it can generate the code accordingly.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

… field is made nullable and flexible version is used
@dajac dajac changed the title KAFKA-9514; The protocol generator generated useless condition when afield is made nullable and flexible version is used KAFKA-9514; The protocol generator generated useless condition when a field is made nullable and flexible version is used Jun 3, 2020
@ijuma ijuma requested a review from cmccabe June 3, 2020 17:25
@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Jun 3, 2020

LGTM. Thanks, @dajac

@cmccabe cmccabe merged commit 21362ad into apache:trunk Jun 4, 2020
ijuma added a commit that referenced this pull request Jun 4, 2020
…use_all_dns_ips-as-default

* apache-github/trunk:
  KAFKA-9788; Use distinct names for transaction and group load time sensors (#8784)
  KAFKA-9514; The protocol generator generated useless condition when a field is made nullable and flexible version is used (#8793)
  MINOR: Update to Gradle 6.5 and tweak build jvm config (#8751)
  MINOR: Upgrade spotbugs and spotbugsPlugin (#8790)
  KAFKA-10089 The stale ssl engine factory is not closed after reconfigure (#8792)
  KAFKA-10080; Fix race condition on txn completion which can cause duplicate appends (#8782)
  KAFKA-10084: Fix EosTestDriver end offset (#8785)
  KAFKA-10083: fix failed testReassignmentWithRandomSubscriptionsAndChanges tests (#8786)
@dajac dajac deleted the KAFKA-9514 branch June 4, 2020 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants