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

Add fromJson to all API response models #606

Closed
cammace opened this issue Oct 16, 2017 · 6 comments
Closed

Add fromJson to all API response models #606

cammace opened this issue Oct 16, 2017 · 6 comments
Labels
Archived Issue archived due to lack of activity

Comments

@cammace
Copy link
Contributor

cammace commented Oct 16, 2017

Now that we are using AutoValue, the response objects which get created during the serialization of JSON require a type adapter factory which can often be confusing in times when you want to deserialize a JSON string into a DirectionsResponse for example. Doing so without adding the type adapter factory will result in:

Failed to invoke public com.mapbox.directions.v5.models.DirectionsRoute() with no args

To make this easier, I'm proposing we add a static factory method to all API model classes which would looks something like:

public static DirectionsResponse fromJson(String json) {
  return GsonBuilder().registerTypeAdapterFactory(DirectionsAdapterFactory.create()).create()
      .fromJson(directionsRoute, DirectionsResponse.class);
}

cc: @zugaldia

@zugaldia
Copy link
Member

I like it. If I understand it correctly, this would bring functionality consistent with what we already provide for other types like GeoJSON (https://github.com/mapbox/mapbox-java/blob/master/mapbox/libjava-geojson/src/main/java/com/mapbox/services/commons/geojson/Feature.java#L130-L142). Correct?

@cammace
Copy link
Contributor Author

cammace commented Oct 17, 2017

Exactly, this is inspired by the trouble I ran into when debugging this. AutoValue Gson type adapter factory documentation for reference.

@lilykaiser
Copy link

Is this still in progress?

@jethromayuk
Copy link

This would be great.

@zugaldia
Copy link
Member

zugaldia commented Aug 1, 2018

This landed for directions models with #854.

@jethro-2018 could you share a bit more about your use case (service, serialization needed, ...)? Thanks!

@stale stale bot added the Archived Issue archived due to lack of activity label Jan 28, 2019
@stale
Copy link

stale bot commented Jan 28, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Archived Issue archived due to lack of activity
Projects
None yet
Development

No branches or pull requests

4 participants