jakartaee / jsonb-api

Jakarta JSON Binding
https://eclipse-ee4j.github.io/jsonb-api/
Other
78 stars 39 forks source link

Adapter specification incomplete #240

Open martijndwars opened 4 years ago

martijndwars commented 4 years ago

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 commented 4 years ago

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

martijndwars commented 4 years ago

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):

What is your opinion on this?

rmannibucau commented 4 years ago

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 commented 2 years ago

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.