Skip to content
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.

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

Make it compatible with Retrofit #67

Closed
feinstein opened this issue Jan 4, 2018 · 14 comments
Closed

Make it compatible with Retrofit #67

feinstein opened this issue Jan 4, 2018 · 14 comments

Comments

@feinstein
Copy link

I am having issues using this library with Retrofit and GSON.

Since some of the classes, like ZoneId are abstract and GSON can't create them, I am getting exceptions on Retrofit when I try do deserialize my webservice JSON response.

It would be great to have an adapter that could be plugged into Retrofit to provide GSON the ability to deal with this library.

@JakeWharton
Copy link
Owner

Looks like https://github.com/aaronhe42/ThreeTen-Backport-Gson-Adapter exists.

Regardless, though, this is a request for ThreeTenBP and not this library which merely repackages its timezone database.

@feinstein
Copy link
Author

I found that adapter, but I thought that maybe since you are one of the developers of Retrofit and this library, maybe you would want to make something streamlined for Retrofit, but I will use that adapter anyways, thank you Jake.

@JakeWharton
Copy link
Owner

JakeWharton commented Jan 4, 2018 via email

@feinstein
Copy link
Author

I know, but since it's so easy to add a Converter on Retrofit, I was hoping there could be an easy plug and play solution for this as well, like a wrapper around the GsonConverterFactory, or something similar.... Not that registering an adapter with GSON is a great deal of work, just not as simple as it could be.

@feinstein
Copy link
Author

FYI: The https://github.com/aaronhe42/ThreeTen-Backport-Gson-Adapter doesn't work.

It calls GSON JsonElement.getAsString() which is basically an UnsupportedOperationException.

So now I will have to write my own adapter :/

@JakeWharton
Copy link
Owner

JakeWharton commented Jan 5, 2018 via email

@feinstein
Copy link
Author

When I was searching for JSON solutions I decided to not use Moshi because it appeared to be 2x slower than GSON, at least from this benchmark: https://medium.com/@IlyaEremin/android-json-parsers-comparison-2017-8b5221721e31

Maybe the benchmark was flawed?

I will try Moshi then, if it fixes my issue I can cope with the slower performance. Thank you for the recommendation Jake!

@JakeWharton
Copy link
Owner

JakeWharton commented Jan 5, 2018 via email

@feinstein
Copy link
Author

Oh, that makes perfect sense!

And yes, adapter examples would be most welcome :D

@JakeWharton
Copy link
Owner

JakeWharton commented Jan 22, 2018

public @Rfc1123 OffsetDateTime createdAt;
public final class ModelAdapters {
  @FromJson //
  public @Rfc1123 OffsetDateTime rfc1123OffsetDateTimeFromJson(String value) {
    return OffsetDateTime.parse(value, RFC_1123_DATE_TIME);
  }

  @ToJson //
  public String rfc1123OffsetDateTimeToJson(@Rfc1123 OffsetDateTime value) {
    return RFC_1123_DATE_TIME.format(value);
  }
}
Moshi moshi = new Moshi.Builder() //
    .add(new ModelAdapters()) //
    .build();

// ...

.addConverterFactory(MoshiConverterFactory.create(moshi)) //

@feinstein
Copy link
Author

Thanks Jake.

I already have it working but I made it differently. I used Jackson's Java time module on my server, so it will convert automatically a ZonedDateTime to a timestamp, like an Instant. Then I made a Moshi adapter to convert that timestamp back to a ZonedDateTime.

The only caveat though is that the timestamp is sent as a double with nanoseconds as the decimals. When Moshi grabs this double with will crash on Parsing, since it's a big double and somehow appears a scientific notation in the string behind the covers, so it won't convert it to a string automatically either, so I had to use a JsonReader as my Adapter's input, in order to get the timestamp double as a string.

It would be nice to have the Moshi adapters to automatically pass numeric values as strings if the adapter has a string parameter, like the JsonReader does.

@JakeWharton
Copy link
Owner

JakeWharton commented Jan 22, 2018 via email

@feinstein
Copy link
Author

Sure, I will make it later today.

@feinstein
Copy link
Author

I tried to reproduce the issue and roll back my code, but now it works, so I have no idea what was going on haha.

Even the @FromJson ZonedDateTime fromJson(String timestamp) Adapter that would crash saying timestamp isn't a String, but a double is now passing the original numeric value as a String so I can simplify my code and ditch the JsonReader apparently.

Thank you for your help @JakeWharton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants