jakartaee / jsonb-api

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

Update jsonb.adoc - Annotate adapter on a class #340

Open mkarg opened 1 year ago

mkarg commented 1 year ago

It would be rather comfortable if we could allow to annotate any POJO with @JsonbTypeAdapter, so ANY use of that POJO in ANY location (as field content, in collections, as keys or values in maps) would trigger the use of the named adapter class.

mkarg commented 1 year ago

@m0mus @rmannibucau I would kindly invite you into the discussion of this feature proposal. :-)

rmannibucau commented 1 year ago

What about deprecating adapters in favor of (de)serializers? I always found it inconsistent and misleading in their usage cause they have hidden limitations even if idea looks great upfront.

mkarg commented 1 year ago

What about deprecating adapters in favor of (de)serializers? I always found it inconsistent and misleading in their usage cause they have hidden limitations even if idea looks great upfront.

From the user's perspective I need to say that most people I met really love adapters but dislike serializers, so we should keep them but remove the limitations. In this particular case, there not even seems to be a technical constraint, as e. g. Yasson already supports in some situations.

BTW, according to the Jakarta EE WG's documents it is forbidden to remove existing features.

rmannibucau commented 1 year ago

we should keep them but remove the limitations

At some point it will not be possible, like adapters loop handling or defining if a chain should be supported (forbidden today), it also probably goes way too far in terms of mapping and will have deep implications when mixed with cdi too much which looks wrong from a design perspective so shouldnt be allowed even if technically we can go a bit further.

I need to say that most people I met really love adapters but dislike serializers

So they probably dislike adapters since you can code adapters as (de)serializer (it is one line) but not the opposite so maybe a doc issue or comm issue but technically it is not just lower level, it is lower+same level (context).

BTW, according to the Jakarta EE WG's documents it is forbidden to remove existing features.

Fully agree but also fully inconsistent with Jakarta EE WG acts since it is at eclipse, all releases broke and removed features. That said I never said to drop without warning, just to deprecate in next release with a remove for N+2 or 3 which is allowed by jakarta (was already by EE).

So summary looks like:

this is why I think working on them is making the spec more complex with no user gain

m0mus commented 1 year ago

Thanks for your PR @mkarg. I checked the source code, @JsonbTypeAdapter can be placed on type, field, method, and parameter. @JsonbTypeAdapter on a method is uncovered my the doc. It would be good to add it. Also, it may make sense updating the JavaDoc accordingly.

rmannibucau commented 1 year ago

@m0mus what about dropping adapters so freezing the features as this (ie we don't care about class case), seems logicial with the high level api of (de)serializer today, just miss a serial+deserial API but since it includes the mapping API, serializer is included easily too and avoids multiple entry points which is always a nightmare for end users to pick the right API. I know we discussed it for 1.0 but the defintion concurrency of the two API didn't finish/timed very well but since we have now this feedback we can fix it and deprecate adapter concept in favor of a clean mapping. This also makes the view concept quite pointless (keeping the API minimal and light is always good for such multifacet library IMHO). Can finish a PR this way if it helps.