kamikat / moshi-jsonapi

JSON API v1.0 Specification in Moshi.
MIT License
156 stars 34 forks source link

Double value should keep decimal portion calling fromJsonValue on JsonAdapter<Document> #96

Closed jasonbekolay closed 5 years ago

jasonbekolay commented 5 years ago

Currently just a reproduction in a failing test.

The cause of of this issue is in the dump(JsonReader reader, JsonWriter writer) method in MoshiHelper

case NUMBER:
  try {
      writer.value(reader.nextLong());
  } catch (Exception ignored) {
      writer.value(reader.nextDouble());
  }
  break;

Typically this code is called with an instance JsonUtf8Reader and everything works fine in that case. In the case of the failing test in this PR, it's called with an instance of JsonValueReader instead. In JsonValueReader.nextLong()it essentially casts a double down to a long instead of throwing an exception.

I'm happy to implement a solution but would appreciate some guidance. The dump method could be changed to something like this:

case NUMBER:
  Double doubleValue = reader.nextDouble();
  if (Math.floor(doubleValue) == doubleValue) {
    writer.value((long)doubleValue);
  } else {
    writer.value(doubleValue);
  }
  break;

I would also be open to the thought that JsonValueReader should throw an exception if you call nextLong() when the value has a fractional part. If that is the preferred path, I can create an issue in the main moshi repo.

Thank you for moshi-jsonapi! Your work on it is much appreciated by the team at Yapp!

kamikat commented 5 years ago

Thanks for help improving moshi-jsonapi.

We can just follow behavior of moshi.

So just go ahead :-)

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.2%) to 82.147% when pulling 4b746f9fdb70cdc584d0ebe7ec418555d3987495 on jasonbekolay:bug/double-fraction-lost-in-dump into 2cd5a56f522216c941eda7b87d4f842a74a033e3 on kamikat:master.

jasonbekolay commented 5 years ago

Thank you for the feedback 😀 I've added a commit with a fix. If you have further feedback, I'm happy to make modifications.