Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support arbitrary Number implementation for Object and Number deserialization #1290

Merged
merged 6 commits into from Oct 9, 2021
Merged

Support arbitrary Number implementation for Object and Number deserialization #1290

merged 6 commits into from Oct 9, 2021

Conversation

lyubomyr-shaydariv
Copy link
Contributor

@lyubomyr-shaydariv lyubomyr-shaydariv commented Apr 17, 2018

RFC 8259, 6 Numbers allows numbers of arbitrary range and precision specifying that a particular implementation may set some limits for numeric values. As of Gson 2.8.4 uses the following deserialization strategies:

  • If the result type is declared as Object, Gson always returns Doubles for all encountered JSON numbers. In this case, the minimum and maximum hold number value fits the limits of Double which cannot hold exact Long values that exceed the Double exact precision limits (2^53+1).
  • ‎If the result type is declared as Number and there are no custom Number adapters, Gson always returns LazilyParsedNumbers that hold original string values parsing them to a requested type lazily allowing to use the whole range of Long. However, it does not allow to use numbers of arbitrary range and precision, and does not expose its hold string as a BigDecimal.

In order to fix these limitations and preserve backwards compatibility, some sort of "to-number" strategies might be accepted in GsonBuilder to override the default behavior of Gson. This pull-request introduces such a strategy interface to be used in the built-in Object and Number type adapters. There are also four strategies implemented in this PR (two standard to keep the backwards compatibility and two enhanced to overcome the limitations) using an enumeration:

  • ToNumberPolicy.DOUBLE that implements the default behavior of the Object type adapter returning Doubles only.
  • ToNumberPolicy.LAZILY_PARSED_NUMBER that implements the default behavior of the Number type adapter.
  • ToNumberPolicy.LONG_OR_DOUBLE that tries to parse a number as a Long, otherwise then tries to parse it as a Double, if the number cannot be parsed as a Long.
  • ToNumberPolicy.BIG_DECIMAL that can parse numbers of arbitrary range and precision.

Call-site is expected to extract proper values using the methods declared in java.lang.Number.

Examples of use:

Default behavior, backwards-compatible with the previous versions of Gson

Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(ToNumberPolicy.DOUBLE) // explicit default, may be omitted
  .create();
List<Object> actual = gson.fromJson("[null, 10, 10.0]", new TypeToken<List<Object>>() {}.getType());
List<Double> expected = Arrays.asList(null, 10.0, 10.0);
assertEquals(expected, actual);
Gson gson = new GsonBuilder()
    .setNumberToNumberStrategy(ToNumberPolicy.LAZILY_PARSED_NUMBER) // explicit default, may be omitted
    .create();
List<Number> actual = gson.fromJson("[null, 10, 10.0]", new TypeToken<List<Number>>() {}.getType());
List<Object> expected = Arrays.<Object>asList(null, new LazilyParsedNumber("10"), new LazilyParsedNumber("10.0"));
assertEquals(expected, actual);

Object-declared numbers are always LazilyParsedNumbers

Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(ToNumberPolicy.LAZILY_PARSED_NUMBER)
  .create();
List<Object> actual = gson.fromJson("[null, 10, 10.0]", new TypeToken<List<Object>>() {}.getType());
List<LazilyParsedNumber> expected = Arrays.asList(null, new LazilyParsedNumber("10"), new LazilyParsedNumber("10.0"));
assertEquals(expected, actual);

Number-declared numbers are always Doubles

Gson gson = new GsonBuilder()
  .setNumberToNumberStrategy(ToNumberPolicy.DOUBLE)
  .create();
List<Number> actual = gson.fromJson("[null, 10, 10.0]", new TypeToken<List<Number>>() {}.getType());
List<Double> expected = Arrays.asList(null, 10.0, 10.0);
assertEquals(expected, actual);

Object-declared numbers are either Long or Double

Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE)
  .create();
List<Object> actual = gson.fromJson("[null, 10, 10.0]", new TypeToken<List<Object>>() {}.getType());
List<? extends Number> expected = Arrays.asList(null, 10L, 10.0);
assertEquals(expected, actual);

Object-declared numbers are BigDecimals

Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(ToNumberPolicy.BIG_DECIMAL)
  .create();
List<Object> actual = gson.fromJson("[null, 3.141592653589793238462643383279, 1e400]", new TypeToken<List<Object>>() {}.getType());
List<BigDecimal> expected = Arrays.asList(null, new BigDecimal("3.141592653589793238462643383279"), new BigDecimal("1e400"));
assertEquals(expected, actual);

Custom bytes-only:

Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(new ToNumberStrategy() {
    @Override
    public Byte toNumber(final JsonReader in)
            throws IOException {
      return (byte) in.nextInt();
    }
  })
  .create();
List<Object> actual = gson.fromJson("[null, 10, 20, 30]", new TypeToken<List<Object>>() {}.getType());
List<Byte> expected = Arrays.asList(null, (byte) 10, (byte) 20, (byte) 30);
assertEquals(expected, actual);

Custom deserialization does not affect Byte-declared deserialization:

ToNumberStrategy fail = new ToNumberStrategy() {
    @Override
    public Byte toNumber(final JsonReader in) {
      throw new AssertionError();
    }
  };
Gson gson = new GsonBuilder()
  .setObjectToNumberStrategy(fail)
  .setNumberToNumberStrategy(fail)
  .create();
List<Object> actual = gson.fromJson("[null, 10, 20, 30]", new TypeToken<List<Byte>>() {}.getType());
List<Byte> expected = Arrays.asList(null, (byte) 10, (byte) 20, (byte) 30);
assertEquals(expected, actual);

@xavix-yo
Copy link

Great!

@arouel
Copy link
Contributor

arouel commented May 4, 2018

Are there plans to merge and release it?

@lyubomyr-shaydariv lyubomyr-shaydariv changed the title Object and Number type adapters number deserialization can be configured Support arbitrary Number implementation for deserialization May 4, 2018
@lyubomyr-shaydariv lyubomyr-shaydariv changed the title Support arbitrary Number implementation for deserialization Support arbitrary Number implementation during deserialization May 4, 2018
@Taipeek
Copy link

Taipeek commented Aug 17, 2018

Still waiting for this to be released :)

@nonight89
Copy link

Thank god! Finally you guys fix this! :)

@Catfriend1
Copy link

lgtm

Catfriend1 added a commit to Catfriend1/syncthing-android that referenced this pull request Jan 26, 2019
Catfriend1 pushed a commit to Catfriend1/syncthing-android that referenced this pull request Jan 27, 2019
* WIP

* Revert "WIP"

This reverts commit 98b34c4.

* WIP

* Revert "WIP"

This reverts commit 3b9fc96.

* Add de/serializer for MinDiskFree

* Move MinDiskFree out of Folder

* Move MinDiskFree out of Folder (2)

* Revert "Move MinDiskFree out of Folder (2)"

This reverts commit 65f87db.

* Revert "Move MinDiskFree out of Folder"

This reverts commit b71350b.

* Revert "Add de/serializer for MinDiskFree"

This reverts commit 5827426.

* RestApi: Add MinDiskFreeSerializer, MinDiskFreeDeserializer

* Revert "RestApi: Add MinDiskFreeSerializer, MinDiskFreeDeserializer"

This reverts commit 3922f24.

* Test

* Revert "Test"

This reverts commit 3550095.

* FolderActivity/DeviceActivity: Fix restApi unavailable in onCreate()

* Model/Folder#MinDiskFree: Initialize members (fixes #277)

* ConfigXml#getFolders: Add MinDiskFree (fixes #277)

* ConfigXml: Write back minDiskFree (fixes #277)

* Ignore notices about updating gradle dependencies

* ConfigXml: Make number parsing more safe

* FolderActivity#initFolder: Add new Folder.MinDiskFree

* Handle minDiskFree.value as String instead of float

* Revert "Handle minDiskFree.value as String instead of float"

This reverts commit 0552cfc.

* WIP

* Revert "WIP"

This reverts commit 0a3df91.

* RestApi: Avoid creating duplicate Gson() instances

* Model/Folder: Use Integer instead of Float

See gson glitch:
google/gson#1290
google/gson#968

* Try MinDiskFree.value as Long instead of Integer

* Revert "Try MinDiskFree.value as Long instead of Integer"

This reverts commit d358862.

* Revert "Model/Folder: Use Integer instead of Float"

This reverts commit ca3931b.

* Update model/Options: MinHomeDiskFree (fixes #277)
@grieser-careoline
Copy link

please, apply as soon as possible

@douglasom
Copy link

Is this PR ever going to be applied? If not with this enhancement, what is the approach recommended by the Gson team for deserializing numbers?

If I'm deserializing as a Map<String, Object> and I have some Integer values how to avoid for them to be interpreted as a Double?

@lyubomyr-shaydariv lyubomyr-shaydariv changed the title Support arbitrary Number implementation during deserialization Support arbitrary Number implementation for Object and Number deserialization Apr 13, 2020
@roycarser
Copy link

is this PR acceptable now ?

gson/src/main/java/com/google/gson/GsonBuilder.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/GsonBuilder.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberStrategy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberStrategy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberStrategy.java Outdated Show resolved Hide resolved
@Marcono1234
Copy link
Collaborator

Could you please not force push next time? It requires looking at all the files again to find out what changed and it does not show any of the previous review comments in the code anymore.

For a clean commit history, all the commits can then later be squashed when the pull request is merged.

@lyubomyr-shaydariv
Copy link
Contributor Author

lyubomyr-shaydariv commented Jun 19, 2020

:)

Could you please not force push next time? It requires looking at all the files again to find out what changed and it does not show any of the previous review comments in the code anymore.

https://github.com/lyubomyr-shaydariv/gson/commits/to-number-strategy%2Bhistory

For a clean commit history, all the commits can then later be squashed when the pull request is merged.

It depends on how the repository maintainers do merges. From what I see in the git log graph, squashes are not a must. and I'd like to keep it as clean as possible.

Copy link
Collaborator

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more minor things, but it looks pretty good 👍

gson/src/main/java/com/google/gson/GsonBuilder.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/GsonBuilder.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/Gson.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
Copy link
Collaborator

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, still some more minor things which could possibly be improved (if you like).

gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberPolicy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberStrategy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/ToNumberStrategy.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/Gson.java Outdated Show resolved Hide resolved
@lyubomyr-shaydariv
Copy link
Contributor Author

Sorry, still some more minor things which could possibly be improved (if you like).

No problem, please provide either an inline diff (or a patch as a comment) or provide a patch on top of the "+history" branch as a PR into my repository (so that I could merge/cherry-pick your suggestions on top of the branch and then I could squash all these changes into this branch) addressing the unresolved comments (3 remaining as I could resolve the first two only). Thank you.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@Marcono1234
Copy link
Collaborator

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@erdemtpz
Copy link

@googlebot I consent.

8 similar comments
@Bilecen
Copy link

Bilecen commented Oct 18, 2021

@googlebot I consent.

@kodkafali
Copy link

@googlebot I consent.

@alperentoksoz
Copy link

@googlebot I consent.

@osmantufekci
Copy link

@googlebot I consent.

@yakupdmrr
Copy link

@googlebot I consent.

@sorhanselcuk
Copy link

@googlebot I consent.

@ogunozdemir
Copy link

@googlebot I consent.

@mfg41
Copy link

mfg41 commented Oct 19, 2021

@googlebot I consent.

@rmirabelle
Copy link

I don't understand this "feature" of Gson.

Assuming objects of this shape:

data class MyThing(
    val answer: Any? = null
)

And assuming incoming JSON like so:

[
    {
        "answer": "1"
    },
    {
        "answer": 1
    }
    {
        "answer": 1.0
    },
    {
        "answer": null
    }
]

The very obvious solution to the above is for Gson to see the target type of Any?, then infer the destination type based on the JSON value, setting the answer values to String, Int, Double and null respectively. This doesn't strike me as even marginally difficult.

QUITE unfortunately, Gson still doesn't do this and is happy to corrupt incoming JSON values, requiring that AFTER conversion, we manually inspect the resulting object and manually correct all of Gson's errors. To compound this issue, I cannot craft my own custom converter either, because that too, would require selecting a single type only.

@Marcono1234
Copy link
Collaborator

Marcono1234 commented Sep 14, 2022

The very obvious solution to the above is for Gson to see the target type of Any?, then infer the destination type based on the JSON value, setting the answer values to String, Int, Double and null respectively.

When you tell Gson to deserialize a Java Object / Kotlin Any and it finds a JSON number it deserializes it by default as Double regardless of the format of the number. I assume for historical reasons that could not be changed which is why this PR here was proposed to allow customizing the behavior.

The ToNumberStrategy allows you to implement the functionality you want. A very simple implementation which just checks if the number contains a . could look like this:

val gson = GsonBuilder()
    .setObjectToNumberStrategy { reader ->
        // Read the JSON number as string to inspect its format
        val numberAsString = reader.nextString()

        if (numberAsString.contains('.')) numberAsString.toDouble()
            else numberAsString.toInt()
    }
    .create()

You could also have a look at the implementation of Gson's ToNumberPolicy.LONG_OR_DOUBLE which implements the same concept except for Long and Double, and also rejects non-finite values and produces more helpful exception messages when parsing the number fails.

@rmirabelle
Copy link

@Marcono1234 Thanks for the very thoughtful response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet