jakartaee / jsonb-api

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

Harmonize JSONB / JAXB TypeAdapter behavior for Collections #261

Open KeyBridge opened 4 years ago

KeyBridge commented 4 years ago

JSONB type adapter logic differs from JAXB in an important way for collections.

In JAXB when applied to a collection an XmlJavaTypeAdapter is applied to each entry in the collection.
whereas
In JSONB when applied to a collection an JsonbTypeAdapter is applied to the Collection as a whole.

The JAXB strategy leads to a simpler code base as only the object in question requires a transformer and the same transformer can be applied to a single field or a collection. The JSONB strategy requires separate adapters for single instances and for collections.

This request is that the JsonbTypeAdapter processor is adjusted to also recognize collections. For example:

Example class code:

  @JsonbTypeAdapter(JsonMyObjectAdapter.class)
  private Collection<MyObject> myObjects;

Current JSONB adapter code pattern:

public class JsonMyObjectAdapter implements JsonbAdapter<Collection<MyObject>, String> {
   ...

Desired JSONB adapter code pattern:

public class JsonMyObjectAdapter implements JsonbAdapter<MyObject, String> {
   ...
rmannibucau commented 4 years ago

In Johnzon we went a bit more advanced supporting both by checking the generic type (looping types not supported but it is ok since it break java too at some point). This makes it very smooth and working in all cases without treating collection as a particular type in user code.

kaqqao commented 3 years ago

I'd honestly hope this will one day become (if #66 gets implemented):

private Collection<@JsonbTypeAdapter(JsonMyObjectAdapter.class) MyObject> myObjects;

Removes all ambiguity and enables infinitely more advanced usage. I don't know why this spec still, after 7 years, pretends AnnotatedType doesn't exist.

Imagine a worse scenario:

@JsonbTypeAdapter(JsonMyObjectAdapter.class)
private Collection<Optional<Set<MyObject>>> myObjects;

What does the adapter apply to now? Every single combination of types requires a dedicated adapter implementation... no composability whatsoever. Now compare that to a type-usage level annotation which composes perfectly and unambiguously.

And more generally, it enables far more powerful (de)serializers to exist. E.g. need a deserializer that decodes stings as it goes? Easy: List<@Decoded Key> decodedKeys.

rmannibucau commented 3 years ago

Well, I'd say that bean validation went this way and it is actually never used, dev tend to not like this over verbosity and java goes against it with var and diamond feature. Supporting item adapters through field/property adapter is doable and sufficient. It is likely a good simplicity/verbosity compromise IMHO.

In term of composability: you can compose adapters to make them another one and use it, it is the common solution (we often see MyAdapter.List pattern).

Indeed just my 2cts but avoiding overcomplexifying the spec is indeed sane IMHO.

kaqqao commented 3 years ago

Well, I'd say that bean validation went this way and it is actually never used

It's most definitely used where I live...

dev tend to not like this over verbosity

How is the position of the annotation influencing verbosity? It's the same annotation, just moved to the place it actually applies to. Or did you mean the verbosity of the spec? That is the trade off, true. I'd argue it makes the logic much clearer, but the spec will indeed become more cumbersome if it attempts to stay compatible with the original behavior, which is likely...

In term of composability: you can compose adapters to make them another one and use it, it is the common solution (we often see MyAdapter.List pattern).

So in my example I need MyAdapter.Collection.Optional.Set? And another for MyAdapter.Set.Optional.Collection? That's the opposite of composability. And isn't this infinitely more verbose than moving the annotation?

rmannibucau commented 3 years ago

@kaqqao when I meant verbosity it is about inline verbosity, java is not used to that and very few framework took that path which means java dev are not used to that, tooling is not very friendly to that and even java language is not very friendly when you really use it (combine bean validation + jsonb + an xml mapper in your diamond and your are not lisible anymore compared to decorating the field which is well tooled, supported and used by dev. Don't get me wrong, I fully agree we can support it but I see it as a feature which does not help the dev at the end. About the optional example, as mentionned, we can make the field decorator matching either the item type or collection. Forgetting one second the optional case it mean Myadapter will support My type, Collection, Set, List, Map<String, My>. Since JSON-B already supports Optional I guess we can make it supporting it the same (ie unwrapping at serialization time). Note that optional is not encouraged since it is undefined if it gets empty() or null (both have pro and cons - don't fully recall out of my head but think it gets null right now so not always container friendly).