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

Adapter specification incomplete #240

Open
martijndwars opened this issue May 8, 2020 · 4 comments
Open

Adapter specification incomplete #240

martijndwars opened this issue May 8, 2020 · 4 comments
Labels
documentation Involves the spec or javadocs

Comments

@martijndwars
Copy link

From the specification:

There are two ways how to register JsonbAdapter:

  1. Using JsonbConfig::withAdapters method;
  2. Annotating a class field with JsonbTypeAdapter annotation.

However, the JsonbTypeAdapter annotation has as possible targets ANNOTATION_TYPE , TYPE, FIELD, METHOD, which means that you can also annotate a class itself with this annotation and not just a class field.

I'm being a bit pedantic, but I thought I'd create an issue so this can be clarified in future versions of the specification.

@aguibert aguibert added the documentation Involves the spec or javadocs label May 8, 2020
@aguibert
Copy link
Contributor

aguibert commented May 8, 2020

hi @martijndwars, would you mind proposing a PR to clarify this in the spec?

@martijndwars
Copy link
Author

Before starting on this, there are some other things that I'd like to include in the spec as well, even if it is just to make explicit that these things are left unspecified (i.e. up to the implementer):

  • If you register an adapter for a "special" type (e.g. LocalDate), does the adapter take precedence over the normal rules?
  • The specification does not mention how often should adapters should be applied. E.g. if there is an Adapter<A, A>, should the adapter be applied once, twice, ad infinitum? I think Yasson applies the adapter twice but then stops.
  • The rules for matching an Adapter<A, B> to a runtime type T. I think Yasson requires subtyping for raw types, and for generic types it needs the raw type to be a subtype but the type arguments to be equal.
  • When a type matches multiple adapters, which adapter takes precedence? (I believe you changed this behavior in Yasson recently?).

What is your opinion on this?

@rmannibucau
Copy link

Hi @martijndwars ,

We should do a pass with all implementations (I know yasson and johnzon, not sure there is another one) I think because we can't break too much users on these topics IMHO.

Now, assuming we are "free", I'd say:

  1. Special types are overriden (maybe except for map impls?)
  2. I'd let it loop in the spec - implementations can protect themselves indeed - to say it is user specific (typically if the user returns the input instance it should stop looking but if he keeps copying it it will loop as he requested)
  3. I'd emphasis exact matching is recommended and isAssignableFrom supported
  4. What about using annotations then the JsonbConfig order?

@oliviercailloux
Copy link

oliviercailloux commented Sep 12, 2022

I don’t think it’s being pedantic, this issue seems of importance for predictability of the resulting json (which is crucial to about any application I guess).

I’d like an annotation on a type to act as a default only.
If I provide a converter via the JsonbConfig it should override the type annotation.
If I annotate a field it should override both the type annotation and the JsonbConfig provided converter.

Relatedly, can I provide both an adapter and a serializer for some type? Which will be used then?

Another question about possible loops: what if I provide adapters A to B and B to C and C to A? If the types are incompatible, then (I believe that) it is impossible that the conversion works. Is the runtime allowed to complain early in that case?

I believe that such loops should be simply forbidden, for simplicity, and the implementation to fail whenever they want (ideally they would fail as soon as possible, of course). I think that the value of fast failure would greatly override any possible benefit of very edge cases where a loop would actually be useful (if this exists).

Related: #169.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Involves the spec or javadocs
Projects
None yet
Development

No branches or pull requests

4 participants