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);
@lyubomyr-shaydariv
Copy link
Contributor Author

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

The pull request addresses the same issue that's addressed in:

Related:

@danieleguiducci
Copy link

@danieleguiducci danieleguiducci commented Apr 25, 2018

Great!

@arouel
Copy link
Contributor

@arouel 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 Taipeek commented Aug 17, 2018

Still waiting for this to be released :)

@nonight89
Copy link

@nonight89 nonight89 commented Jan 25, 2019

Thank god! Finally you guys fix this! :)

@Catfriend1
Copy link

@Catfriend1 Catfriend1 commented Jan 26, 2019

lgtm

Catfriend1 added a commit to Catfriend1/syncthing-android that referenced this issue Jan 26, 2019
Catfriend1 pushed a commit to Catfriend1/syncthing-android that referenced this issue 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

@grieser-careoline grieser-careoline commented Aug 1, 2019

please, apply as soon as possible

@douglasom
Copy link

@douglasom douglasom commented Sep 1, 2019

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

@roycarser roycarser commented Jun 8, 2020

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
Contributor

@Marcono1234 Marcono1234 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.

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 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
Contributor

@Marcono1234 Marcono1234 left a comment

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
Contributor

@Marcono1234 Marcono1234 left a comment

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

@lyubomyr-shaydariv lyubomyr-shaydariv commented Jun 21, 2020

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

@googlebot googlebot commented Jun 22, 2020

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
Contributor

@Marcono1234 Marcono1234 commented Jun 22, 2020

@googlebot I consent.

@googlebot
Copy link

@googlebot googlebot commented Jun 22, 2020

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Member

@eamonnmcmanus eamonnmcmanus left a comment

Hi, Gson is in maintenance mode and for the most part we are not accepting new features like this. However, given the considerable demand, I think we can make an exception here. Thanks for putting this together!

I've run this code against the tests of all of Google's internal projects and I didn't see any problems. The new API also seems good.

I did have a few minor comments.

lyubomyr-shaydariv and others added 6 commits Oct 8, 2021
If the element has already been linked before, don't create a link for
every subsequent occurrence.

See also (slightly dated)
https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#inlinelinks
…pter

This encapsulates the logic a little bit better.
Additionally refactored factory created by NumberTypeAdapter to only create
TypeAdapter once and then have factory reuse that adapter for better
performance.
@erdemtpz
Copy link

@erdemtpz erdemtpz commented Oct 18, 2021

@googlebot I consent.

8 similar comments
@Bilecen
Copy link

@Bilecen Bilecen commented Oct 18, 2021

@googlebot I consent.

@kodkafali
Copy link

@kodkafali kodkafali commented Oct 18, 2021

@googlebot I consent.

@alperentoksoz
Copy link

@alperentoksoz alperentoksoz commented Oct 18, 2021

@googlebot I consent.

@osmantufekci
Copy link

@osmantufekci osmantufekci commented Oct 18, 2021

@googlebot I consent.

@yakupdmrr
Copy link

@yakupdmrr yakupdmrr commented Oct 18, 2021

@googlebot I consent.

@sorhanselcuk
Copy link

@sorhanselcuk sorhanselcuk commented Oct 18, 2021

@googlebot I consent.

@ogunozdemir
Copy link

@ogunozdemir ogunozdemir commented Oct 18, 2021

@googlebot I consent.

@mfg41
Copy link

@mfg41 mfg41 commented Oct 19, 2021

@googlebot I consent.

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

Successfully merging this pull request may close these issues.

None yet