Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototype support for handling absence #1412

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

ZacSweers
Copy link
Collaborator

@ZacSweers ZacSweers commented Oct 30, 2021

(edit: see following comments, this was generalized away from just Optional)

This is a prototype of what Optional support would look like. This implements it in two parts:

  1. A new OptionalJsonAdapter + associated factory. This essentially works by associating nulls with Optional.absent() and used for properties that are either present or explicitly null.
  2. Reflective support in ClassJsonAdapter and KotlinJsonAdapter for supporting defaulting absent values to Optional.empty().

Open questions:

  1. Do we really want to support this in Kotlin? If so, how do we handle the following:
    a. Optional<String>?
    b. Do we support this in code gen too?
  2. Do we support just j.u.Optional or do we want to do something more generic (to support Guava, KOptional, etc)? How would that look if so?
  3. Do we want to generify absent properties in general? Maybe an optional JsonAdapter.ifAbsent() or separate AbsenceHandler interface?
  • This would obviate 1 and 2 as we just need to support this API and delegate to whatever adapters folks bring.

@ZacSweers
Copy link
Collaborator Author

Pushed an absence handler API demo instead, which I think came out pretty clean all things considered!

@ZacSweers ZacSweers changed the title Prototype support for Optional Prototype support for handling absence Oct 31, 2021
@ZacSweers
Copy link
Collaborator Author

Should say I'm not thrilled about the of adding this directly to the JsonAdapter API, but without a clear/easy way to access delegated adapters it's not clear how to unpack an opt-in interface's APIs

@lassem
Copy link

lassem commented Nov 16, 2021

In Kotlin I'd use this to fall back to any default arguments on data classes. In our existing implementation this is achieved via a work around utilising a custom @DefaultIfNull annotation and deserialising via Map first, which of course is additional overhead.

@ZacSweers
Copy link
Collaborator Author

@lassem we already do that in kotlin. This would only be for Java or for kotlin cases where you don't provide a default and simply want it to coerce to Optional.empty()

@lassem
Copy link

lassem commented Nov 16, 2021

That is true. However, the aforementioned @DefaultIfNull annotation would also use the default if the json specifically said null

@ZacSweers
Copy link
Collaborator Author

I don't see how it would since an absent key would not be present in a map. In any case let's keep this PR on topic

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