jakartaee / jsonb-api

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

Deprecating vs completely specifying JsonAdapter #341

Open mkarg opened 1 year ago

mkarg commented 1 year ago

The JSON-B specification lacks several details about the use of JsonAdapter. As a result, the existing implementations behave differently. As a consequence, existing applications not to apply workarounds to work on all implementations unchanged.

There are two proposals currently how to fix that in a future version of the specification:

To get broad input from the community, everybody (users and vendors) are invited to discuss the pros and cons of these proposals.

To come to a final decision, committers are invited to vote for / agains these proposals.

mkarg commented 1 year ago

@m0mus @rmannibacau I like to invite you into this discussion about the future of JsonbAdapter.

mkarg commented 1 year ago

To start the discussion, I like to forward some opinions I collected when discussing with other users:

rmannibucau commented 1 year ago

Without rephrasing what I already wrote Id like to emphasis that this statement is wrong or should be perceived as such to kot biase this topic thinking:

Adapters are very simple and intuitive to use, while Serializers are much more complex and enforces the need to understand JSON-P

Technically this is true of adapters too and serializer context makes it intuitive (jsonb api like) and simple.

Also note we can make a deserializer+serializer an api (codec like) if it makes it easier but it avoids to duplicate the concept hiding thz limitation in the api.

mkarg commented 1 year ago

Let's agree to disagree. I already explained the reason why application programmers (me included) perceive adapters to be intuitive and simple, but do not perceive serializers to be. All arguments are said and will not change just because you personally feel different about it. We need to find more arguments instead of saying an argument told by a user would be "wrong". It is his perception, and there is nothing "wrong" about his perception, wether we share it or not.

BTW, here is the link to the Jakarta EE docs mentioned by Mark Struberg: https://jakartaee.github.io/jakartaee-platform/CompatibilityRequirements. Let me quote two items from the contained FAQ:

Q2. Is it permissible to remove an existing interface from an existing specification? No, because this would violate rules 2 and 3 above. Note that it is also not permissible to mark it as deprecated.

Q4. Is it permissible to remove an existing method from an existing interface? No, because this would violates rules 2 and 3 above.

Note that Jakarta EE officially is not applying SemVer, but there is an undecided request to do so in future: https://github.com/jakartaee/jakartaee-platform/issues/449.

rmannibucau commented 1 year ago

Not saying perception is wrong, just it is not factual and mainly due to a wrong knowledge we can easily fix so we should give this point much weight in the deprecation path since user will win, at least with a simpler and lighter api to learn (note not a single user understand adapters today).

mkarg commented 1 year ago

" (note not a single user understand adapters today)"

I personally know of application programmers that pretty well understand and successfully use adapters, hence I cannot see how you come to this general statement?

rmannibucau commented 1 year ago

Do they understand they are limited to jsonp types as of today in jsonb spec state and therefore are just serializers with a return type instead of a callback? All users i talked too, including some vendor implementors thought it was a pre-serialization mapping (this process being undefined or vendor specific with > 1 adapters matching)...so api is often overevaluated by users, when you understand it is just a jsonp top layer it becomes useless but almost nobody get to that point to my knowledge.

mkarg commented 1 year ago

Do they understand they are limited to jsonp types as of today in jsonb spec state...?

Actually today's spec does not limit adapters to JSON-P types. As you know, Yasson does support mapping to non-JSON-P types (proven today in https://github.com/jakartaee/jsonb-api/issues/337, and you confirmed recently that it is not unambiguously told in the adapter spec that adapters are for JSON-P types, not for JSON-B types). Hence there is a difference what you want adapters to be understood like, and what de-facto adapters are doing in Yasson - which is intuitive and understood by the users I talked to. And while Jakarta EE does not have the idea of "reference implementations" anymore, people still accept Yasson's behavior as authoritative (besides bugs); BTW The Yasson Readme even (incorrectly) supports this wrong perception, by saying, it would be an "official reference implementation"...:

I understand your point, but note that others have a different angle of view.

rmannibucau commented 1 year ago

Yasson has not compliant features as johnzon has...but if you respect the spec you are limited to jsonp, this was clarified in multiple tickets as the rest not being defined so clearly unsupported by the spec (vendors are always doing at least the spec). Yasson is kot authoritative but the ri, spec is the pdf, javadoc and api, period, this is what jakarta mention as authoritative. So still, if perception can be discussed, facts remain the same and perception does not help much. Once again we can fix any misdoc/comm.

mkarg commented 1 year ago

I think we understood and respect each other's experience and collected feedback, so it is time to hear what others think how to proceed. Let's hear what the Yasson committers say, and let's collect some feedback from the broad community. It makes not much sense to decide just on the base of your and mine original claims, just as it makes not much sense to claim that it would be irrelevant what application programmed perceived (aka "misunderstood"). In the end, it is the application programmers who we do this spec for, not us.

Update: I invited people on Twitter to join our discussion.

rmannibucau commented 1 year ago

Agree on that statement, just highlighted that perception is sometimes wrong due to past experience or bad doc so not a real decision key.

mkarg commented 1 year ago

IMHO decision key for any API should be user expectations. The problem is that most people's expectations are tightly coupled to some kind of extrapolation of their perception of that past. This is why I came up with "perception". We cannot erase perception to get actually unbiased opinions. Hence we need a very good replacement to convince people to give up their biased opinion.

rmannibucau commented 1 year ago

Reasoning like that you will end up with guava like bundles, really hope we dont think like that, just lead to mess and error prone api. From my window it looks an empty argument and a bad way to try to defend something technically not accurate. I strongly think people can learn and are not dumb to just copy stackoverflow (or we dont care cause they wouldnt understand any solution right?) so, once again, not something to take into account for any decision for the future if we care where we go.

mkarg commented 1 year ago

No comment from me on how dumb or lazy people are / are not, but we should take into consideration that talking from a business angle, replacing APIs means modifying existing applications eventually, which is costs, possibly high costs.

Edit: BTW, I do not defend something technically accurate. I plea for making it accurate, but not by replacing it.

njr-11 commented 1 year ago

BTW, here is the link to the Jakarta EE docs mentioned by Mark Struberg: https://jakartaee.github.io/jakartaee-platform/CompatibilityRequirements. Let me quote two items from the contained FAQ:

Q2. Is it permissible to remove an existing interface from an existing specification? No, because this would violate rules 2 and 3 above. Note that it is also not permissible to mark it as deprecated.

Q4. Is it permissible to remove an existing method from an existing interface? No, because this would violates rules 2 and 3 above.

It is too selective of a reading of the document to conclude from the above that deprecation is forbidden in Jakarta EE. The same document also has a section on deprecation, seemingly contradictory to the above, which states:

  1. A specification project may decide to designate a method or interface as being “deprecated” when its use is no longer recommended or when the specification project considers that a different interface or method should be used instead. This can be stated in the specification and in the relevant javadoc.

We should raise this issue to the Jakarta EE platform @ivargrimstad @edburns @Emily-Jiang to clarify the stance on deprecation and get the document updated. It looks like the document also needs an update in a different section as well, where it alludes to being written prior to Jakarta EE 9,

XXX - This section needs to be updated with new rules for the actual removal of APIs and specifications from the Jakarta EE platform as anticipated in Jakarta EE 9, similar to what’s done for the Java SE platform.

Please note that none of what I said above should be considered as an argument for or against JsonbAdapter.

mkarg commented 1 year ago

+1 for making that dokument perfectly bullet-proof by the Platform team, but for the time being we are on the safe side if we take the FAQ's pretty unambiguous answer for granted.

Edit: Officially asked platform team to discuss this issue, see https://github.com/jakartaee/jakartaee-platform/issues/683.

mkarg commented 1 year ago

Good news: The Platform and Specification Committees agreed that in their next version of the mentioned document they will officially allow us to deprecate and remove features according SemVer rules. :+1:

Verdent commented 1 year ago

I would say, that adapters are reasonably easy to use and I can see the reasoning for having them, so deprecation would not be the way I think this should go. On the other hand, I think they are not fully described in the spec and that is something we should improve. Simply fully document adapter behavior and desired limitations.