From fdaecb9fb8785f6468855c721666e4539da73b60 Mon Sep 17 00:00:00 2001 From: jwilson Date: Wed, 1 Feb 2017 00:29:36 -0500 Subject: [PATCH] Use BigDecimal to encode exotic number types. Previously we'd retain whatever types the caller passed in. This was potentially problematic for non-immutable numeric types like AtomicInteger. --- .../java/com/squareup/moshi/JsonAdapter.java | 11 +- .../com/squareup/moshi/ObjectJsonWriter.java | 39 +++- .../squareup/moshi/ObjectJsonWriterTest.java | 201 ++++++++++++++++++ 3 files changed, 242 insertions(+), 9 deletions(-) diff --git a/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java index 50d4813..2b519d3 100644 --- a/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java @@ -18,6 +18,7 @@ package com.squareup.moshi; import java.io.IOException; import java.lang.annotation.Annotation; import java.lang.reflect.Type; +import java.math.BigDecimal; import java.util.Set; import okio.Buffer; import okio.BufferedSink; @@ -56,7 +57,15 @@ public abstract class JsonAdapter { /** * Encodes {@code value} as a Java model comprised of maps, lists, strings, numbers, booleans - * and nulls. The returned model is equivalent to calling {@link #toJson} to encode {@code value} + * and nulls. + * + *

Values encoded using {@code value(double)} or {@code value(long)} are modeled with the + * corresponding boxed type. Values encoded using {@code value(Number)} are modeled as a + * {@link Long} for boxed integer types ({@link Byte}, {@link Short}, {@link Integer}, and {@link + * Long}), as a {@link Double} for boxed floating point types ({@link Float} and {@link Double}), + * and as a {@link BigDecimal} for all other types. + * + *

The returned model is equivalent to calling {@link #toJson} to encode {@code value} * as a JSON string, and then parsing that string without any particular type. */ public final Object toJsonObject(T value) { diff --git a/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java b/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java index 237d462..289e765 100644 --- a/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java +++ b/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java @@ -16,6 +16,7 @@ package com.squareup.moshi; import java.io.IOException; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -24,6 +25,8 @@ import static com.squareup.moshi.JsonScope.EMPTY_ARRAY; import static com.squareup.moshi.JsonScope.EMPTY_DOCUMENT; import static com.squareup.moshi.JsonScope.EMPTY_OBJECT; import static com.squareup.moshi.JsonScope.NONEMPTY_DOCUMENT; +import static java.lang.Double.NEGATIVE_INFINITY; +import static java.lang.Double.POSITIVE_INFINITY; /** Writes JSON by building a Java object comprising maps, lists, and JSON primitives. */ final class ObjectJsonWriter extends JsonWriter { @@ -131,7 +134,16 @@ final class ObjectJsonWriter extends JsonWriter { } @Override public JsonWriter value(double value) throws IOException { - return value(Double.valueOf(value)); + if (!lenient + && (Double.isNaN(value) || value == NEGATIVE_INFINITY || value == POSITIVE_INFINITY)) { + throw new IllegalArgumentException("Numeric values must be finite, but was " + value); + } + if (promoteValueToName) { + return name(Double.toString(value)); + } + add(value); + pathIndices[stackSize - 1]++; + return this; } @Override public JsonWriter value(long value) throws IOException { @@ -144,16 +156,27 @@ final class ObjectJsonWriter extends JsonWriter { } @Override public JsonWriter value(Number value) throws IOException { - if (!lenient) { - double d = value.doubleValue(); - if (d == Double.POSITIVE_INFINITY || d == Double.NEGATIVE_INFINITY || Double.isNaN(d)) { - throw new IllegalArgumentException("Numeric values must be finite, but was " + value); - } + // If it's trivially converted to a long, do that. + if (value instanceof Byte + || value instanceof Short + || value instanceof Integer + || value instanceof Long) { + return value(value.longValue()); } + + // If it's trivially converted to a double, do that. + if (value instanceof Float || value instanceof Double) { + return value(value.doubleValue()); + } + + // Everything else gets converted to a BigDecimal. + BigDecimal bigDecimalValue = value instanceof BigDecimal + ? ((BigDecimal) value) + : new BigDecimal(value.toString()); if (promoteValueToName) { - return name(value.toString()); + return name(bigDecimalValue.toString()); } - add(value); + add(bigDecimalValue); pathIndices[stackSize - 1]++; return this; } diff --git a/moshi/src/test/java/com/squareup/moshi/ObjectJsonWriterTest.java b/moshi/src/test/java/com/squareup/moshi/ObjectJsonWriterTest.java index 878ae55..774fb92 100644 --- a/moshi/src/test/java/com/squareup/moshi/ObjectJsonWriterTest.java +++ b/moshi/src/test/java/com/squareup/moshi/ObjectJsonWriterTest.java @@ -16,8 +16,13 @@ package com.squareup.moshi; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -66,5 +71,201 @@ public final class ObjectJsonWriterTest { "Map key 'a' has multiple values at path $.a: 1 and 2"); } } + + @Test public void valueLongEmitsLong() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.beginArray(); + writer.value(Long.MIN_VALUE); + writer.value(-1L); + writer.value(0L); + writer.value(1L); + writer.value(Long.MAX_VALUE); + writer.endArray(); + + List numbers = Arrays.asList( + Long.MIN_VALUE, + -1L, + 0L, + 1L, + Long.MAX_VALUE); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + @Test public void valueDoubleEmitsDouble() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.setLenient(true); + writer.beginArray(); + writer.value(-2147483649.0d); + writer.value(-2147483648.0d); + writer.value(-1.0d); + writer.value(0.0d); + writer.value(1.0d); + writer.value(2147483647.0d); + writer.value(2147483648.0d); + writer.value(9007199254740991.0d); + writer.value(9007199254740992.0d); + writer.value(9007199254740994.0d); + writer.value(9223372036854776832.0d); + writer.value(-0.5d); + writer.value(-0.0d); + writer.value(0.5d); + writer.value(9.22337203685478e18); + writer.value(Double.NEGATIVE_INFINITY); + writer.value(Double.MIN_VALUE); + writer.value(Double.MIN_NORMAL); + writer.value(-Double.MIN_NORMAL); + writer.value(Double.MAX_VALUE); + writer.value(Double.POSITIVE_INFINITY); + writer.value(Double.NaN); + writer.endArray(); + + List numbers = Arrays.asList( + -2147483649.0d, + -2147483648.0d, + -1.0d, + 0.0d, + 1.0d, + 2147483647.0d, + 2147483648.0d, + 9007199254740991.0d, + 9007199254740992.0d, + 9007199254740994.0d, + 9223372036854775807.0d, + -0.5d, + -0.0d, + 0.5d, + 9.22337203685478e18, + Double.NEGATIVE_INFINITY, + Double.MIN_VALUE, + Double.MIN_NORMAL, + -Double.MIN_NORMAL, + Double.MAX_VALUE, + Double.POSITIVE_INFINITY, + Double.NaN); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + @Test public void primitiveIntegerTypesEmitLong() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.beginArray(); + writer.value(new Byte(Byte.MIN_VALUE)); + writer.value(new Short(Short.MIN_VALUE)); + writer.value(new Integer(Integer.MIN_VALUE)); + writer.value(new Long(Long.MIN_VALUE)); + writer.endArray(); + + List numbers = Arrays.asList( + -128L, + -32768L, + -2147483648L, + -9223372036854775808L); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + @Test public void primitiveFloatingPointTypesEmitDouble() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.beginArray(); + writer.value(new Float(0.5f)); + writer.value(new Double(0.5d)); + writer.endArray(); + + List numbers = Arrays.asList( + 0.5d, + 0.5d); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + @Test public void otherNumberTypesEmitBigDecimal() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.beginArray(); + writer.value(new AtomicInteger(-2147483648)); + writer.value(new AtomicLong(-9223372036854775808L)); + writer.value(new BigInteger("-9223372036854775808")); + writer.value(new BigInteger("-1")); + writer.value(new BigInteger("0")); + writer.value(new BigInteger("1")); + writer.value(new BigInteger("9223372036854775807")); + writer.value(new BigDecimal("-9223372036854775808")); + writer.value(new BigDecimal("-1")); + writer.value(new BigDecimal("0")); + writer.value(new BigDecimal("1")); + writer.value(new BigDecimal("9223372036854775807")); + writer.value(new BigInteger("-9223372036854775809")); + writer.value(new BigInteger("9223372036854775808")); + writer.value(new BigDecimal("-9223372036854775809")); + writer.value(new BigDecimal("9223372036854775808")); + writer.value(new BigDecimal("0.5")); + writer.value(new BigDecimal("100000e15")); + writer.value(new BigDecimal("0.0000100e-10")); + writer.endArray(); + + List numbers = Arrays.asList( + new BigDecimal("-2147483648"), + new BigDecimal("-9223372036854775808"), + new BigDecimal("-9223372036854775808"), + new BigDecimal("-1"), + new BigDecimal("0"), + new BigDecimal("1"), + new BigDecimal("9223372036854775807"), + new BigDecimal("-9223372036854775808"), + new BigDecimal("-1"), + new BigDecimal("0"), + new BigDecimal("1"), + new BigDecimal("9223372036854775807"), + new BigDecimal("-9223372036854775809"), + new BigDecimal("9223372036854775808"), + new BigDecimal("-9223372036854775809"), + new BigDecimal("9223372036854775808"), + new BigDecimal("0.5"), + new BigDecimal("100000e15"), + new BigDecimal("0.0000100e-10")); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + @Test public void valueCustomNumberTypeEmitsLongOrBigDecimal() throws Exception { + ObjectJsonWriter writer = new ObjectJsonWriter(); + writer.beginArray(); + writer.value(stringNumber("-9223372036854775809")); + writer.value(stringNumber("-9223372036854775808")); + writer.value(stringNumber("0.5")); + writer.value(stringNumber("1.0")); + writer.endArray(); + + List numbers = Arrays.asList( + new BigDecimal("-9223372036854775809"), + new BigDecimal("-9223372036854775808"), + new BigDecimal("0.5"), + new BigDecimal("1.0")); + assertThat((List) writer.root()).isEqualTo(numbers); + } + + /** + * Returns an instance of number whose {@link #toString} is {@code s}. Using the standard number + * methods like {@link Number#doubleValue} are awkward because they may truncate or discard + * precision. + */ + private Number stringNumber(final String s) { + return new Number() { + @Override public int intValue() { + throw new AssertionError(); + } + + @Override public long longValue() { + throw new AssertionError(); + } + + @Override public float floatValue() { + throw new AssertionError(); + } + + @Override public double doubleValue() { + throw new AssertionError(); + } + + @Override public String toString() { + return s; + } + }; + } }