kamikat / moshi-jsonapi

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

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

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 think it 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!

jasonbekolay commented 5 years ago

Sorry, meant to create as a draft PR. Closing and recreating.