Skip to content

MINOR: Support java.util.Optional in the auto-generated protocol#9085

Closed
dajac wants to merge 1 commit intoapache:trunkfrom
dajac:minor-optional
Closed

MINOR: Support java.util.Optional in the auto-generated protocol#9085
dajac wants to merge 1 commit intoapache:trunkfrom
dajac:minor-optional

Conversation

@dajac
Copy link
Copy Markdown
Member

@dajac dajac commented Jul 27, 2020

The AK protocol often relies on sentinel value (e.g. null or -1) to indicate that a field was not provided. Usually, these sentinels are turned into java.util.Optional that is empty when the field is equal to the sentinel. Until now, we were doing this transformation while mapping the Struct to internal Request or Response classes. With the generated protocol, we would like to get rid of these internal classes and also avoid spending time to construct them.

This PR is an attempt to support java.util.Optional in the auto-generated classes. A field can be marked as optional that indicates to the generator that its type must be wrapped in an Optional.

When an optional is written, its internal value is used or its default if the optional is undefined. When an option is read, the value read is used or it defaults to an empty optional if the default value is read.

I did a deep integration by changing all the places where values are written or read to do the above conversions. I just wrote it as a proof of concept to discuss this further. The code may be simplified.

An alternative to this would be to only provide getters and setters which convert the internal value to an Optional and vice versa. That would limit the scope of the change. The downside would be that every time the getters or the setters are called, it requires to do the conversion.

Committer Checklist (excluded from commit message)

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

@dajac
Copy link
Copy Markdown
Member Author

dajac commented Feb 4, 2022

PR is outdated. Closing it.

@dajac dajac closed this Feb 4, 2022
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.

1 participant