Jackson Databind Default serialisation for BigInteger should be as a JSON String

I am aware of the ability to configure Jackson to serialize BigInteger as a JSON String (https://github.com/FasterXML/jackson-databind/issues/1911) but I believe that it should be the default behaviour, with serialisation as a JSON Number configurable if required. The reason I believe this is that JSON Number does not have arbitrary length, which will lead to loss of precision during serialisation.

Supporting evidence: https://stackoverflow.com/a/39681707/323177 https://stackoverflow.com/a/38357877/323177 https://www.avioconsulting.com/blog/overcoming-javascript-numeric-precision-issues

Comment From: cowtowncoder

I disagree: I think number should be treated as number, unless configured to be serialized as JSON String (either globally or on per-field basis; both of which are possible). I think same is true for BigDecimal as well. This both from conceptual (what I think is Right) and practical (change would be major backwards incompatible breakage leading to dozens of bug reports that I would then need to address). In fact, I do not think there is much anything I can think of to suggest that such a change would be good thing.

But if you do want to convince me otherwise, your best bet would be to bring this up on mailing list, either jackson-dev (most likely) -- https://groups.google.com/forum/#!forum/jackson-dev -- or possible user list (https://groups.google.com/forum/#!forum/jackson-user).

Comment From: garretwilson

(I've removed my comment here as it was mistakenly added to an issue surrounding BigInteger and not BigDecimal. I've opened #5193.)

Comment From: pjfanning

we can't change behaviour without affecting existing users - like it or not, some of them might rely on the jackson works today
we could introduce a config (eg a SerializationFeature) that could be set be users if they want BigInteger (this issue) or BigDecimal (not this issue) to be serialized as a JSON string.
Jackson 3 could change the default but the risk is that Jackson 3 could be released before any changes for this issue are done.

Comment From: pjfanning

There is already com.fasterxml.jackson.core.json.JsonWriteFeature#WRITE_NUMBERS_AS_STRINGS

This is possibly too broad as it affects all numbers that are written to the JSON, not just BigInteger/BigDecimal.

So the option would be to add JsonWriteFeature#WRITE_BIG_INTEGERS_AS_STRINGS and JsonWriteFeature#WRITE_BIG_DECMALS_AS_STRINGS or possibly a single new feature (but what to call it).

Comment From: garretwilson

Oops, I'm sorry. The bug reporter form asked if I had searched for existing issues, and so I searched and accidentally found this issue, which was referring to BigInteger and not BigDecimal. I've opened a separate ticket #5193.

Comment From: pjfanning

This appears in some of our tests. So this is one option.

        mapper.configOverride(BigInteger.class)
            .setFormat(JsonFormat.Value.forShape(JsonFormat.Shape.STRING));

I'm adding this for users who run into this issue. I'm aware that some other users want us to change the default.

You can also annotate fields with @JsonFormat(shape=Shape.STRING).

Comment From: pjfanning

Plain String is controlled by this feature

https://github.com/FasterXML/jackson-core/blob/373f108a16e3f315e9df9eaecb482e43f9953621/src/main/java/com/fasterxml/jackson/core/JsonGenerator.java#L190-L204

Comment From: cowtowncoder

What @pjfanning show is THE recommended way to do configuration.

I am 100% against change to serialize Numbers (BigInteger or otherwise) as JSON Strings and consider it unlikely default will ever be changed for Jackson.

Will leave open only for sake of visibility.

Comment From: garretwilson

Plain String is controlled by this feature …

@pjfanning , could help me out here? I'm actually setting up read-only JSON readers and writers. First I construct an ObjectMapper using a builder:

return JsonMapper.builder().serializationInclusion(JsonInclude.Include.NON_ABSENT) //
    .withConfigOverride(BigDecimal.class, configOverride -> configOverride.setFormat(JsonFormat.Value.forShape(Shape.STRING))) //
    .enable(SerializationFeature.WRITE_BIGDECIMAL_AS_PLAIN) //

But here SerializationFeature.WRITE_BIGDECIMAL_AS_PLAIN is deprecated. How do I set up the reader and writer to use WRITE_BIGDECIMAL_AS_PLAIN if I'm not working in terms of a generator?

Comment From: pjfanning

Deprecated doesn't mean it shouldn't work - it's just a warning that support is likely to be removed in future. JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN is checked in jackson-databind serialization code too. You could try playing around with the settings to see what works. JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN is a better bet if it works for you since this is not deprecated.

Comment From: garretwilson

But I don't expose a generator. My design is to use the modern ObjectReader and ObjectWriter to provide a preconfigured way to read and write JSON; e.g.:

  static {
    OBJECT_MAPPER = createJsonObjectMapper();
    JSON_READER = OBJECT_MAPPER.reader();
    JSON_WRITER = OBJECT_MAPPER.writer();
  }

I guess I'll just go back to the way I was doing it before:

return JsonMapper.builder().serializationInclusion(JsonInclude.Include.NON_ABSENT) //
    .addModule(new SimpleModule() //
        .addSerializer(BigDecimal.class, new JsonSerializer<BigDecimal>() {
          @Override
          public void serialize(BigDecimal value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
            gen.writeString(value.toPlainString());
          }
        }) //
        .addKeySerializer(BigDecimal.class, new JsonSerializer<BigDecimal>() {
          @Override
          public void serialize(BigDecimal value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
            gen.writeFieldName(value.toPlainString());
          }
        })
  …

Eventually I'll put these in a reusable module.

Thanks anyway for the tips.

Comment From: pjfanning

This appears to compile for me:

JsonMapper.builder()
                .enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)
                .build();

Comment From: garretwilson

Ah, thanks, @pjfanning ! I had a SerializationFeature in my code for something else (disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)), so I was thinking I had to use the SerializationFeature here as well. It looks like enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN) works! 👏

Comment From: cowtowncoder

On deprecation of JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN -- while this will not be removed from 2.x ever, there is replacement (since this is format-agnostic setting):

StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN

using which removed deprecation warnings.

And since it is Streaming-level feature, it will change all writes with BigDecimal to use Plain notation, regardless of databind-level logical type (probably not much actual difference come to think of it now).

Comment From: garretwilson

using which removed deprecation warnings.

I think what was giving me deprecation warnings was SerializationFeature.WRITE_BIGDECIMAL_AS_PLAIN, not JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN, but what you indicate seems an even better flag, so I'll use it. Thanks.

Comment From: cowtowncoder

Ah. It is possible JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN not yet marked as deprecated. But it will be removed from 3.0, whereas StreamWriteFeature counterpart remains.