jsonb-api icon indicating copy to clipboard operation
jsonb-api copied to clipboard

Adapter specification incomplete

Open martijndwars opened this issue 4 years ago • 4 comments

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.

martijndwars avatar May 08 '20 20:05 martijndwars

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

aguibert avatar May 08 '20 20:05 aguibert

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?

martijndwars avatar Jun 09 '20 08:06 martijndwars

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?

rmannibucau avatar Jun 09 '20 08:06 rmannibucau

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.

oliviercailloux avatar Sep 12 '22 08:09 oliviercailloux