Skip to content

Safe cast long to double #73

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Safe cast long to double #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28aa07d
Safe cast long to double
jhaber Jun 22, 2020
3ed8d1a
Use exception that exists in Jackson 2.9
jhaber Jun 22, 2020
51dda2b
Remove throws clause
jhaber Jun 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 21 additions & 1 deletion ...n/java/com/hubspot/jackson/datatype/protobuf/builtin/deserializers/ValueDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
143 changes: 58 additions & 85 deletions src/test/java/com/hubspot/jackson/datatype/protobuf/builtin/ValueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@

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;
import java.util.Map.Entry;

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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -228,14 +217,10 @@ public void itReadsStructValue() throws IOException {
assertThat(valueWrapper.hasValue()).isTrue();

Value value = valueWrapper.getValue();
switch (value.getKindCase()) {
case STRUCT_VALUE:
Entry<String, Value> 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<String, Value> entry = entry("key", Value.newBuilder().setStringValue("value").build());
assertThat(value.getStructValue().getFieldsMap()).containsExactly(entry);
}

@Test
Expand All @@ -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
Expand All @@ -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<String, Value> 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<String, Value> 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
Expand All @@ -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());
}
}