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

Incompatibility with "Schema" based designs

Open cyberphone opened this issue 6 years ago • 58 comments

This issue is related to #33

AFAIK all existing schema based designs presume that the type of an element stays the same regardless of its value. The variable type concept for numbers used by jsonb/jsonp breaks that.

Have you performed any interoperability tests with other JSON tools?

cyberphone avatar Jan 07 '19 05:01 cyberphone

@cyberphone I am not up on whats going on, which elements are we talking about? Can you please provide an example?

bravehorsie avatar Feb 13 '19 07:02 bravehorsie

@bravehorsie If I understood it correctly, the jsonb/jsonp solution uses a variable serialization method of values in order to automatically cope with I-JSON requirements: https://github.com/cyberphone/I-JSON-Number-System#java-json-b This is incompatible with OAS as well as most other systems as well.

cyberphone avatar Feb 13 '19 09:02 cyberphone

@cyberphone OpenAPI and ECMA Script generally doesn't support numeric types longer than IEEE754. JSONB/JSONP on the other hand has to deal with numbers outside of that precision. Is there any other type defined by ECMA Script suitable to be used for BigDecimal/BigInteger besides String? Do you suggest to write these types always as String regardless of the fact if its value "fits" or is outside of ECMAScript's Number type / IEEE754?

bravehorsie avatar Feb 18 '19 11:02 bravehorsie

@bravehorsie OpenAPI do support long: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#dataTypes Long always serializes as JSON Number and thus should be incompatible with a system using a variable serialization scheme. Your interpretation of I-JSON is correct but pretty illogical for declarative models. IMO, the only viable solution is either a global option like I-JSON or annotations that permit doing this with ease.

However, it may get worse depending how the following pans out: https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme-05#section-3.1 "An additional constraint is that parsed JSON String data MUST NOT be altered during subsequent serializations"

cyberphone avatar Feb 18 '19 13:02 cyberphone

I guess it is ok since openapi is about data of the application which can be enforced through an adapter if needed/desired, same applies to jsonschema. Note that this last one is particular in early stages (even if "old") and I don't think we should consider more than being compati-able (being compatible without enforcing anything for it) in the context of JSON-B otherwise we'll get into troubles (like breaking API).

rmannibucau avatar Feb 18 '19 13:02 rmannibucau

@cyberphone I see, it doesn't say anything about double precision, just that it is a 64bit number. That makes their long also not parsable by ES JSON.parse(). There is already an I-JSON option in the API, however it doesn't affect serializing numbers in any way. I-JSON states:

I-JSON messages SHOULD NOT include numbers that express greater magnitude or precision than an IEEE 754 double precision number provides, for example, 1E400 or 3.141592653589793238462643383279. ... For applications that require the exact interchange of numbers with greater magnitude or precision, it is RECOMMENDED to encode them in JSON string values.

https://tools.ietf.org/html/rfc7493#section-2.2 It doesn't say anything about type conversion / mapping, just that numbers (I understand that as number values) of greater precision should be written as strings.

@rmannibucau My first thought on adaptive notation problem was that it would be pretty awkward to have java POJOs being serialized with their long id values as strings no matter of value. On the other hand having an id in JSON either as a number or as a string depending on value is also not the best option.

bravehorsie avatar Feb 18 '19 15:02 bravehorsie

@bravehorsie while a round trip works I think it works even if not perfect (yes it is linked to javascript world historical habits). AFAIK JSON-B is friendly with that behavior and does its best here, no?

rmannibucau avatar Feb 18 '19 16:02 rmannibucau

@bravehorsie @rmannibucau If you take the currently shipping version of Chrome and feed it with

JSON.stringify(BigInt("5"))

it returns "Uncaught TypeError: Do not know how to serialize a BigInt".

OTOH, your solution for dealing with BigInteger serialization differs (to the best of my knowledge) from every other implementation out there.

That is "we" (the industry) obviously have a problem.

I'm looking for workable (and hopefully) future proof changes to address this issue. That's all.

cyberphone avatar Feb 19 '19 04:02 cyberphone

Afaik it is common to use a number when possible and switch to string when not. It is what I saw and used since 6-7 years so not sure "we" have a problem. Some language libs didnt solve this issue but it is their problem and not main impls I think.

rmannibucau avatar Feb 19 '19 05:02 rmannibucau

I'm talking about things that could be regarded as standard. You won't find a single standard out there utilizing JSON for information interchange which builds on the serialization scheme used by JSON-B/P tools. For in-house solutions you can do whatever works for you but that is not my interest.

Feel free ignoring (and closing) this issue but I'm pretty sure it will pop up some day again 😀

cyberphone avatar Feb 19 '19 05:02 cyberphone

@cyberphone point is jakartaa is not about defining json standards, it just follows them and ensure it works for users.

rmannibucau avatar Feb 19 '19 05:02 rmannibucau

@rmannibucau That I fully understand. What I'm questioning is the interpretation of the standard. Lossy long variables doesn't sound very cool.

I can't imagine a JSON schema designer adding a BigIntger would ever consider "variable" as a proper serialization format unless he/she is working on an in-house design. That it "usually works" is great but sounds awfully "hackish" in my ears.

I leave it there.

cyberphone avatar Feb 19 '19 06:02 cyberphone

Hmm, dont mix two things, jsonschema users will put strong for big numbers to type it or alternatively - and it is jsonschema compatible - an anyOf with string and number so JSON-B by itself is compatible with that.

rmannibucau avatar Feb 19 '19 06:02 rmannibucau

"Schema" was on a conceptual level like in IETF standards using JSON where the serialization of each element is described in detail.

cyberphone avatar Feb 19 '19 06:02 cyberphone

Ok, issue is then than bug numbers are out of the definition of these schema where only primitives, structures and arrays exist so they would fall into string category naturally and it is trivial to enforce it with an adapter ;)

rmannibucau avatar Feb 19 '19 06:02 rmannibucau

@rmannibucau JSONB does its best was also my initial thought. I have tried to use JSONP to parse an adaptive notation.

JSON: {"outOfRange":"850006245121695744"}

Both: javax.json.stream.JsonParser#getLong and javax.json.stream.JsonParser#getBigDecimal will fail reading the value with:

java.lang.IllegalStateException: JsonParser#getBigDecimal/getLong() is valid only for VALUE_NUMBER parser state. But current parser state is VALUE_STRING

I can imagine someone using JSONP on his embedded device client to read response generated by JSONB which will mostly have a number in the ID field, but suddenly in the worst possible moment the ID is returned as a String. Considering that both technologies are JakartaEE it definitely does not feel as we are doing our best. Also how server side devs should document ID field in their rest API? Sometimes number, sometimes string?

While enforcing strings with an adapter is trivial it is not intuitive for users not aware of the problem. A setting for JsonbConfig or annotation parameter with proper javadoc description should be more educational.

When JSONP API have methods for writing BigDecimal and Long types, should not double precision problem be solved also on JSONP side? Currently it prints numbers outside of 64bit IEEE754 as Number type.

bravehorsie avatar Feb 19 '19 08:02 bravehorsie

@bravehorsie yes cause the correct json representation is

{"outOfRange":850006245121695744}

Here is one roundtrip using jsonp/jsonb:

public class Main {
    public static void main(final String[] args) throws Exception {
        final String json = "{\"outOfRange\":850006245121695744.1}";
        final JsonParser parser = Json.createParser(new StringReader(json));
        parser.next(); // start object
        parser.next(); // key
        parser.next(); // value (NUMBER)
        System.out.println("from parser: " + parser.getBigDecimal());
        parser.close();

        try (final Jsonb jsonb = JsonbBuilder.create()) {
            System.out.println("mapped: " + jsonb.fromJson(json, Mapped.class).bigDecimal);
            System.out.println("serialized: " + jsonb.toJson(json, Mapped.class));
        }
    }

    public static class Mapped {
        protected final BigDecimal bigDecimal;

        @JsonbCreator
        public Mapped(@JsonbProperty("outOfRange") BigDecimal bigDecimal) {
            this.bigDecimal = bigDecimal;
        }
    }
}

And output is:

from parser: 850006245121695744.1
mapped: 850006245121695744.1
serialized: {"outOfRange":850006245121695744.1} 

Also I wonder if this topic is not belonging to JSON-P only. JSON-B only delegates to JSON-P here since it is typed in JSON-P API.

rmannibucau avatar Feb 19 '19 08:02 rmannibucau

@rmannibucau Looks like JSONB implementation used in your roundtrip is not conformant to JSONB spec section 3.16, otherwise you would get printed:

serialized: {"outOfRange":"850006245121695744.1"}

bravehorsie avatar Feb 19 '19 09:02 bravehorsie

@bravehorsie hmm this means either your JSON-B impl (yasson I assume) does not pass directly to JSON-P the BigDecimal or your JSON-P implementation is not JSON compliant or looses the "number" type of a BigDecimal. If we agree JSON-B does nothing here except using JSON-P, can we move this topic to JSON-P which has all it needs to solve it cleanly?

rmannibucau avatar Feb 19 '19 09:02 rmannibucau

@rmannibucau 👍 A JSONB impl that is conformant to JSONB spec 1.0 can't pass to JSONP without intervention, especially when JSONP specification does not describe big numbers in any way.

Looks like we can agree that big numbers should be moved to JSONP specification, however JSONB has to have something to pass the setting inside JSONP. JSONB users don't interact with JSONP directly, but should have control over big numbers handling. Also JSONB spec section 3.16 would be redundant and should be updated accordingly.

bravehorsie avatar Feb 19 '19 09:02 bravehorsie

@bravehorsie JSON-P has BigDecimal/BigInteger API so while JSON-B expects these to be the representation of big numbers then JSON-P is the one, if not adapters sounds like the place to use. In other words we can say that users of JSON-B interact indirectly with JSON-P through the type they choose in their mapping which implies some code path in the JSON-B impl which must respect all "primitives" available in JSON-P API (https://docs.oracle.com/javaee/7/api/javax/json/stream/JsonGenerator.html#write-java.math.BigDecimal- and https://docs.oracle.com/javaee/7/api/javax/json/stream/JsonParser.html#getBigDecimal-- or the reader/writer equivalent).

rmannibucau avatar Feb 19 '19 09:02 rmannibucau

@rmannibucau That is how it should be done if JSON-P would honour numbers out of IEEE754 double precision range. However it currently doesn't and so result produced by your roundtrip example is in contradiction with rules forced by JSONB 3.16 explicitly.

Please note, after fixing both specifications and leaving big numbers to JSONP, the original problem of adaptive notation would still persist. That is why I think an additional configuration capabilities regarding big numbers are necessary to provide.

bravehorsie avatar Feb 19 '19 10:02 bravehorsie

@bravehorsie hmm, johnzon does respect that (JSON-P part), is it a bug in the RI or a spec bug? Some customization can be needed in JSON-P but I still think JSON-B must stay out of that game otherwise you will get incompatibilities between JSON-P and JSON-B which would be worse than anything else since they are really meant to be interoperable.

rmannibucau avatar Feb 19 '19 12:02 rmannibucau

@rmannibucau I've got your point, lets move to original problem of adaptive notation and its solution.

Some customization can be needed in JSON-P but I still think JSON-B must stay out of that game otherwise you will get incompatibilities between JSON-P and JSON-B which would be worse than anything else since they are really meant to be interoperable.

Thanks for pointing that out. Can you please provide an example of such incompatibility?

bravehorsie avatar Feb 19 '19 13:02 bravehorsie

@bravehorsie JSONB 3.16 specifies than JSONB must serialize instead of saying JSONB must delegate to JSONP the serialization passing to it a BigInteger or BigDecimal instance. In other words JSONB never serialize or deserialize anything, it just binds JSONP/user loaded values to Java/JSONP.

rmannibucau avatar Feb 19 '19 13:02 rmannibucau

@rmannibucau How JSONP will handle big numbers? Will it have the same adaptive notation as JSONB does today?

bravehorsie avatar Feb 20 '19 09:02 bravehorsie

@bravehorsie seems so yes (https://github.com/eclipse-ee4j/jsonp/blob/9892c0658f9d55b819d39039a364df37d1d60202/api/src/main/java/javax/json/stream/JsonGenerator.java#L256), note sure where the spec is with eclipse migration but it is the only real place it can be handled

rmannibucau avatar Feb 20 '19 09:02 rmannibucau

@rmannibucau Than the original problem reported by @cyberphone still persists, it is just moved one level down isn't it?

bravehorsie avatar Feb 20 '19 10:02 bravehorsie

@bravehorsie yep which is already a huge win since we /2 the number of divergences and inconsistencies.

rmannibucau avatar Feb 20 '19 11:02 rmannibucau

@rmannibucau So how should developers document a REST API which maps java Long for id, or BigDecimal for an amount? Should they write: "Most of the time as a number, occasionally as string"? Isn't it exactly what adaptive notation causes?

bravehorsie avatar Feb 20 '19 11:02 bravehorsie