diff --git a/.travis.yml b/.travis.yml index 57789a3..f791238 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ jdk: sudo: false -script: mvn -B -q verify +script: mvn -B -q verify $JACKSON_VERSIONS env: - # use Jackson versions from POM diff --git a/src/main/java/com/hubspot/jackson/datatype/protobuf/builtin/deserializers/ValueDeserializer.java b/src/main/java/com/hubspot/jackson/datatype/protobuf/builtin/deserializers/ValueDeserializer.java index e3037a3..e5a85e7 100644 --- a/src/main/java/com/hubspot/jackson/datatype/protobuf/builtin/deserializers/ValueDeserializer.java +++ b/src/main/java/com/hubspot/jackson/datatype/protobuf/builtin/deserializers/ValueDeserializer.java @@ -2,7 +2,9 @@ import java.io.IOException; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationContext; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.NullValue; @@ -36,8 +38,11 @@ protected void populate( builder.setStringValue(parser.getText()); return; case VALUE_NUMBER_INT: + long longValue = parser.getLongValue(); + builder.setNumberValue(safeCast(longValue, context)); + return; case VALUE_NUMBER_FLOAT: - builder.setNumberValue(parser.getValueAsDouble()); + builder.setNumberValue(parser.getDoubleValue()); return; case VALUE_TRUE: builder.setBoolValue(true); @@ -60,4 +65,19 @@ protected void populate( public Value.Builder getNullValue(DeserializationContext ctxt) { return Value.newBuilder().setNullValue(NullValue.NULL_VALUE); } + + double safeCast(long longValue, DeserializationContext context) throws JsonProcessingException { + double doubleValue = (double) longValue; + + if (Double.valueOf(doubleValue).longValue() == longValue) { + return doubleValue; + } else { + String message = String.format( + "Number %d can not be represented as a double without loss of precision", + longValue + ); + + throw new JsonParseException(context.getParser(), message); + } + } } diff --git a/src/test/java/com/hubspot/jackson/datatype/protobuf/builtin/ValueTest.java b/src/test/java/com/hubspot/jackson/datatype/protobuf/builtin/ValueTest.java index 3d8c409..9ace403 100644 --- a/src/test/java/com/hubspot/jackson/datatype/protobuf/builtin/ValueTest.java +++ b/src/test/java/com/hubspot/jackson/datatype/protobuf/builtin/ValueTest.java @@ -2,8 +2,8 @@ import static com.hubspot.jackson.datatype.protobuf.util.ObjectMapperHelper.camelCase; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.assertj.core.api.Assertions.entry; -import static org.assertj.core.api.Assertions.fail; import java.io.IOException; import java.util.Map; @@ -11,11 +11,13 @@ import org.junit.Test; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; import com.google.protobuf.ListValue; import com.google.protobuf.NullValue; import com.google.protobuf.Struct; import com.google.protobuf.Value; +import com.google.protobuf.Value.KindCase; import com.hubspot.jackson.datatype.protobuf.util.BuiltInProtobufs.HasValue; public class ValueTest { @@ -148,13 +150,8 @@ public void itReadsNullValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case NULL_VALUE: - assertThat(value.getNullValue()).isEqualTo(NullValue.NULL_VALUE); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.NULL_VALUE); + assertThat(value.getNullValue()).isEqualTo(NullValue.NULL_VALUE); } @Test @@ -164,13 +161,8 @@ public void itReadsIntegralValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case NUMBER_VALUE: - assertThat(value.getNumberValue()).isEqualTo(1.0d); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.NUMBER_VALUE); + assertThat(value.getNumberValue()).isEqualTo(1.0d); } @Test @@ -180,13 +172,20 @@ public void itReadsFloatingPointValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case NUMBER_VALUE: - assertThat(value.getNumberValue()).isEqualTo(1.5d); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.NUMBER_VALUE); + assertThat(value.getNumberValue()).isEqualTo(1.5d); + } + + @Test + public void itThrowsOnOutOfBounds() { + // this number can't be represented exactly by a Java double + String json = "{\"value\":-8747832031878802303}"; + + Throwable t = catchThrowable(() -> camelCase().readValue(json, HasValue.class)); + + assertThat(t) + .isInstanceOf(JsonParseException.class) + .hasMessageContaining("-8747832031878802303"); } @Test @@ -196,13 +195,8 @@ public void itReadsStringValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case STRING_VALUE: - assertThat(value.getStringValue()).isEqualTo("test"); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.STRING_VALUE); + assertThat(value.getStringValue()).isEqualTo("test"); } @Test @@ -212,13 +206,8 @@ public void itReadsBooleanValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case BOOL_VALUE: - assertThat(value.getBoolValue()).isTrue(); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.BOOL_VALUE); + assertThat(value.getBoolValue()).isTrue(); } @Test @@ -228,14 +217,10 @@ public void itReadsStructValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); - switch (value.getKindCase()) { - case STRUCT_VALUE: - Entry entry = entry("key", Value.newBuilder().setStringValue("value").build()); - assertThat(value.getStructValue().getFieldsMap()).containsExactly(entry); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.STRUCT_VALUE); + + Entry entry = entry("key", Value.newBuilder().setStringValue("value").build()); + assertThat(value.getStructValue().getFieldsMap()).containsExactly(entry); } @Test @@ -245,14 +230,10 @@ public void itReadsListValue() throws IOException { assertThat(valueWrapper.hasValue()).isTrue(); Value value = valueWrapper.getValue(); + assertThat(value.getKindCase()).isEqualTo(KindCase.LIST_VALUE); + ListValue list = ListValue.newBuilder().addValues(Value.newBuilder().setStringValue("test").build()).build(); - switch (value.getKindCase()) { - case LIST_VALUE: - assertThat(value.getListValue()).isEqualTo(list); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getListValue()).isEqualTo(list); } @Test @@ -261,23 +242,19 @@ public void itReadsMixedStruct() throws IOException { HasValue message = camelCase().readValue(json, HasValue.class); assertThat(message.hasValue()).isTrue(); Value value = message.getValue(); - switch (value.getKindCase()) { - case STRUCT_VALUE: - Map map = value.getStructValue().getFieldsMap(); - Value nested = Value.newBuilder().setStringValue("nested").build(); - Struct nestedStruct = Struct.newBuilder().putFields("key", nested).build(); - ListValue list = ListValue.newBuilder().addValues(nested).build(); - assertThat(map.size()).isEqualTo(6); - assertThat(map.get("null")).isEqualTo(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); - assertThat(map.get("number")).isEqualTo(Value.newBuilder().setNumberValue(1.5).build()); - assertThat(map.get("string")).isEqualTo(Value.newBuilder().setStringValue("test").build()); - assertThat(map.get("boolean")).isEqualTo(Value.newBuilder().setBoolValue(true).build()); - assertThat(map.get("struct")).isEqualTo(Value.newBuilder().setStructValue(nestedStruct).build()); - assertThat(map.get("list")).isEqualTo(Value.newBuilder().setListValue(list).build()); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.STRUCT_VALUE); + + Map map = value.getStructValue().getFieldsMap(); + Value nested = Value.newBuilder().setStringValue("nested").build(); + Struct nestedStruct = Struct.newBuilder().putFields("key", nested).build(); + ListValue list = ListValue.newBuilder().addValues(nested).build(); + assertThat(map.size()).isEqualTo(6); + assertThat(map.get("null")).isEqualTo(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); + assertThat(map.get("number")).isEqualTo(Value.newBuilder().setNumberValue(1.5).build()); + assertThat(map.get("string")).isEqualTo(Value.newBuilder().setStringValue("test").build()); + assertThat(map.get("boolean")).isEqualTo(Value.newBuilder().setBoolValue(true).build()); + assertThat(map.get("struct")).isEqualTo(Value.newBuilder().setStructValue(nestedStruct).build()); + assertThat(map.get("list")).isEqualTo(Value.newBuilder().setListValue(list).build()); } @Test @@ -286,22 +263,18 @@ public void itReadsMixedListValue() throws IOException { HasValue message = camelCase().readValue(json, HasValue.class); assertThat(message.hasValue()).isTrue(); Value value = message.getValue(); - switch (value.getKindCase()) { - case LIST_VALUE: - ListValue list = value.getListValue(); - Value nested = Value.newBuilder().setStringValue("nested").build(); - Struct struct = Struct.newBuilder().putFields("key", nested).build(); - ListValue nestedList = ListValue.newBuilder().addValues(nested).build(); - assertThat(list.getValuesCount()).isEqualTo(6); - assertThat(list.getValues(0)).isEqualTo(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); - assertThat(list.getValues(1)).isEqualTo(Value.newBuilder().setNumberValue(1.5).build()); - assertThat(list.getValues(2)).isEqualTo(Value.newBuilder().setStringValue("test").build()); - assertThat(list.getValues(3)).isEqualTo(Value.newBuilder().setBoolValue(true).build()); - assertThat(list.getValues(4)).isEqualTo(Value.newBuilder().setStructValue(struct).build()); - assertThat(list.getValues(5)).isEqualTo(Value.newBuilder().setListValue(nestedList).build()); - break; - default: - fail("Unexpected value kind: " + value.getKindCase()); - } + assertThat(value.getKindCase()).isEqualTo(KindCase.LIST_VALUE); + + ListValue list = value.getListValue(); + Value nested = Value.newBuilder().setStringValue("nested").build(); + Struct struct = Struct.newBuilder().putFields("key", nested).build(); + ListValue nestedList = ListValue.newBuilder().addValues(nested).build(); + assertThat(list.getValuesCount()).isEqualTo(6); + assertThat(list.getValues(0)).isEqualTo(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); + assertThat(list.getValues(1)).isEqualTo(Value.newBuilder().setNumberValue(1.5).build()); + assertThat(list.getValues(2)).isEqualTo(Value.newBuilder().setStringValue("test").build()); + assertThat(list.getValues(3)).isEqualTo(Value.newBuilder().setBoolValue(true).build()); + assertThat(list.getValues(4)).isEqualTo(Value.newBuilder().setStructValue(struct).build()); + assertThat(list.getValues(5)).isEqualTo(Value.newBuilder().setListValue(nestedList).build()); } }