make the access token non-nullable in RouteOptions

This refactors the access token in the RouteOptions to be non-nullable again by first deserializing the json into a generic data structure, appending the provided token, and only then transforming into the type-safe RouteOptions object.

Author: Łukasz Paczos

services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/DirectionsRoute.java

@@ -5,6 +5,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonObject;
 import com.google.gson.TypeAdapter;
 import com.google.gson.annotations.SerializedName;
 import com.mapbox.api.directions.v5.DirectionsAdapterFactory;
@@ -165,26 +166,66 @@ public static TypeAdapter<DirectionsRoute> typeAdapter(Gson gson) {
 
   /**
    * Create a new instance of this class by passing in a formatted valid JSON String.
+   * <p>
+   * Use this method if the provided serialized route was not obtained by this library.
+   * Alternatively, use {@link #fromJson(String, String)}.
+   * <p>
+   * If you're using the provided route with the Mapbox Navigation SDK, also see
+   * {@link #fromJson(String, RouteOptions, String)}.
    *
    * @param json a formatted valid JSON string defining a GeoJson Directions Route
    * @return a new instance of this class defined by the values passed inside this static factory
    *   method
-   * @see #fromJson(String, RouteOptions, String)
-   * @since 3.0.0
    */
   public static DirectionsRoute fromJson(@NonNull String json) {
     GsonBuilder gson = new GsonBuilder();
+    JsonObject jsonObject = gson.create().fromJson(json, JsonObject.class);
+    if (jsonObject.has("routeOptions")) {
+      throw new IllegalArgumentException(
+        "Provided serialized route contains RouteOptions. "
+          + "Use DirectionsRoute#fromJson(json, accessToken) instead."
+      );
+    }
+    gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
+    gson.registerTypeAdapter(Point.class, new PointAsCoordinatesTypeAdapter());
+    return gson.create().fromJson(jsonObject, DirectionsRoute.class);
+  }
+
+  /**
+   * Create a new instance of this class by passing in a formatted valid JSON String.
+   * <p>
+   * Use this method if the provided serialized route was obtained by this library.
+   * This means that it includes {@link RouteOptions} and you need to supply a Mapbox Access Token.
+   * Alternatively, use {@link #fromJson(String)}.
+   *
+   * @param json        a formatted valid JSON string defining a GeoJson Directions Route
+   * @param accessToken a Mapbox Access Token
+   * @return a new instance of this class defined by the values passed inside this static factory
+   *   method
+   */
+  public static DirectionsRoute fromJson(@NonNull String json, @NonNull String accessToken) {
+    GsonBuilder gson = new GsonBuilder();
+    JsonObject jsonObject = gson.create().fromJson(json, JsonObject.class);
+    if (jsonObject.has("routeOptions")) {
+      JsonObject routeOptions = jsonObject.getAsJsonObject("routeOptions");
+      routeOptions.addProperty("access_token", accessToken);
+    } else {
+      throw new IllegalArgumentException(
+        "Provided serialized route does not contain RouteOptions. "
+          + "Use DirectionsRoute#fromJson(json) instead."
+      );
+    }
     gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
     gson.registerTypeAdapter(Point.class, new PointAsCoordinatesTypeAdapter());
-    return gson.create().fromJson(json, DirectionsRoute.class);
+    return gson.create().fromJson(jsonObject, DirectionsRoute.class);
   }
 
   /**
    * Create a new instance of this class by passing in a formatted valid JSON String.
    * <p>
    * The parameters of {@link RouteOptions} that were used to make the original route request
    * as well as the {@link String} UUID of the original response are needed
-   * by the Navigation SDK to support correct rerouting and route refreshing.
+   * by the Mapbox Navigation SDK to support correct rerouting and route refreshing.
    *
    * @param json         a formatted valid JSON string defining a GeoJson Directions Route
    * @param routeOptions options that were used during the original route request
@@ -198,10 +239,7 @@ public static DirectionsRoute fromJson(@NonNull String json) {
   public static DirectionsRoute fromJson(
     @NonNull String json, @Nullable RouteOptions routeOptions, @Nullable String requestUuid
   ) {
-    GsonBuilder gson = new GsonBuilder();
-    gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
-    gson.registerTypeAdapter(Point.class, new PointAsCoordinatesTypeAdapter());
-    return gson.create().fromJson(json, DirectionsRoute.class)
+    return fromJson(json)
       .toBuilder()
       .routeOptions(routeOptions)
       .requestUuid(requestUuid)
@@ -223,6 +261,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder distance(@NonNull Double distance);
 
     /**
@@ -232,6 +271,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder duration(@NonNull Double duration);
 
     /**
@@ -244,6 +284,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 5.5.0
      */
+    @NonNull
     public abstract Builder durationTypical(@Nullable Double durationTypical);
 
     /**
@@ -253,6 +294,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder geometry(@Nullable String geometry);
 
     /**
@@ -262,6 +304,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder weight(@Nullable Double weight);
 
     /**
@@ -273,6 +316,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder weightName(@Nullable String weightName);
 
     /**
@@ -282,6 +326,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder legs(@Nullable List<RouteLeg> legs);
 
     /**
@@ -292,6 +337,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.0.0
      */
+    @NonNull
     public abstract Builder routeOptions(@Nullable RouteOptions routeOptions);
 
     /**
@@ -302,6 +348,7 @@ public abstract static class Builder {
      * @return this builder for chaining options together
      * @since 3.1.0
      */
+    @NonNull
     public abstract Builder voiceLanguage(@Nullable String voiceLanguage);
 
     /**
@@ -313,6 +360,7 @@ public abstract static class Builder {
     @NonNull
     public abstract Builder requestUuid(@Nullable String requestUuid);
 
+    @NonNull
     abstract Builder routeIndex(String routeIndex);
 
     /**
@@ -321,6 +369,7 @@ public abstract static class Builder {
      * @return a new {@link DirectionsRoute} using the provided values in this builder
      * @since 3.0.0
      */
+    @NonNull
     public abstract DirectionsRoute build();
   }
 }

services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java

@@ -6,6 +6,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.gson.TypeAdapter;
 import com.google.gson.annotations.SerializedName;
@@ -367,15 +368,12 @@ public List<String> annotationsList() {
 
   /**
    * A valid Mapbox access token used to making the request.
-   * <p>
-   * Avoiding to provide a token will most-likely result in a failure, however,
-   * it's annotated as nullable to prevent serialization of tokens.
    *
    * @return a string representing the Mapbox access token
    */
   @SerializedName("access_token")
   @Ignore(Ignore.Type.SERIALIZATION)
-  @Nullable
+  @NonNull
   public abstract String accessToken();
 
   /**
@@ -674,42 +672,36 @@ public static TypeAdapter<RouteOptions> typeAdapter(Gson gson) {
   }
 
   /**
-   * Create a new instance of this class by passing in a formatted valid JSON String.
-   * <p>
-   * The Mapbox Access Token that was part of the original object was not serialized and needs
-   * to be provided again.
-   * The options will not be valid for a request without a Mapbox Access Token.
+   * Create a new instance of this class by passing in a formatted valid JSON String
+   * with a Mapbox Access Token.
    *
    * @param json        a formatted valid JSON string defining a RouteOptions
-   * @param accessToken a Mapbox Access Token
+   * @param accessToken a Mapbox Access Token since {@link #toJson()} does not serialize the token
    * @return a new instance of this class defined by the values passed inside this static factory
    *   method
    * @see #fromUrl(URL)
    */
   @NonNull
-  public static RouteOptions fromJson(@NonNull String json, @Nullable String accessToken) {
-    return fromJson(json).toBuilder().accessToken(accessToken).build();
+  public static RouteOptions fromJson(@NonNull String json, @NonNull String accessToken) {
+    GsonBuilder gson = new GsonBuilder();
+
+    JsonObject jsonObject = gson.create().fromJson(json, JsonObject.class);
+    jsonObject.addProperty("access_token", accessToken);
+
+    return fromJsonElement(jsonObject);
   }
 
   /**
-   * Create a new instance of this class by passing in a formatted valid JSON String.
+   * This takes the currently defined values found inside this instance and converts it to a json
+   * string.
    * <p>
-   * The Mapbox Access Token that was part of the original object was not serialized and needs
-   * to be provided again.
-   * The options will not be valid for a request without a Mapbox Access Token so make sure to
-   * provide a token with {@link #fromJson(String, String)}
-   * or rebuild the options with {@link #toBuilder()}.
+   * The access token field is not serialized when using this method.
    *
-   * @param json a formatted valid JSON string defining a RouteOptions
-   * @return a new instance of this class defined by the values passed inside this static factory
-   *   method
-   * @see #fromUrl(URL)
+   * @return a JSON string
    */
-  @NonNull
-  public static RouteOptions fromJson(@NonNull String json) {
-    GsonBuilder gson = new GsonBuilder();
-    gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
-    return gson.create().fromJson(json, RouteOptions.class);
+  @Override
+  public String toJson() {
+    return super.toJson();
   }
 
   /**
@@ -744,7 +736,21 @@ public static RouteOptions fromUrl(@NonNull URL url) {
       }
     }
 
-    return fromJson(optionsJson.toString());
+    return fromJsonString(optionsJson.toString());
+  }
+
+  @NonNull
+  private static RouteOptions fromJsonString(@NonNull String json) {
+    GsonBuilder gson = new GsonBuilder();
+    gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
+    return gson.create().fromJson(json, RouteOptions.class);
+  }
+
+  @NonNull
+  private static RouteOptions fromJsonElement(@NonNull JsonElement jsonElement) {
+    GsonBuilder gson = new GsonBuilder();
+    gson.registerTypeAdapterFactory(DirectionsAdapterFactory.create());
+    return gson.create().fromJson(jsonElement, RouteOptions.class);
   }
 
   /**

services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/DirectionsRouteTest.java

@@ -1,23 +1,27 @@
 package com.mapbox.api.directions.v5.models;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import com.mapbox.api.directions.v5.DirectionsCriteria;
 import com.mapbox.core.TestUtils;
 import com.mapbox.geojson.Point;
-
-import org.junit.Test;
-
 import java.util.ArrayList;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import org.hamcrest.junit.ExpectedException;
+import org.junit.Rule;
+import org.junit.Test;
 
 public class DirectionsRouteTest extends TestUtils {
 
-  private static final String DIRECTIONS_V5_VOICE_BANNER_FIXTURE = "directions_v5_voice_banner.json";
-  private static final String DIRECTIONS_V5_VOICE_INVALID_FIXTURE = "directions_v5_voice_invalid.json";
+  private static final String DIRECTIONS_V5_VOICE_BANNER_FIXTURE =
+    "directions_v5_voice_banner.json";
+  private static final String DIRECTIONS_V5_VOICE_INVALID_FIXTURE =
+    "directions_v5_voice_invalid.json";
   private static final int FIRST_ROUTE = 0;
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void sanity() throws Exception {
     DirectionsRoute route = DirectionsRoute.builder()
@@ -73,4 +77,62 @@ public void directionsRoute_doesContainOptionsAndUuid() throws Exception {
     assertEquals(options, route.routeOptions());
     assertEquals(uuid, route.requestUuid());
   }
+
+  @Test
+  public void directionsRoute_json_withOptionsAndUUID_roundTripping() throws Exception {
+    String json = loadJsonFixture("directions_v5-with-closure_precision_6.json");
+    RouteOptions options = RouteOptions.builder()
+      .profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC)
+      .coordinatesList(new ArrayList<Point>() {{
+        add(Point.fromLngLat(1.0, 1.0));
+        add(Point.fromLngLat(2.0, 2.0));
+      }})
+      .accessToken("token")
+      .build();
+    String uuid = "123";
+    DirectionsRoute route = DirectionsRoute.fromJson(json, options, uuid);
+
+    String newRouteJson = route.toJson();
+
+    DirectionsRoute newRoute = DirectionsRoute.fromJson(newRouteJson, "token");
+
+    assertEquals(route, newRoute);
+  }
+
+  @Test
+  public void directionsRoute_json_invalid_with_options() throws Exception {
+    thrown.expect(RuntimeException.class);
+    thrown.expectMessage(
+      "Provided serialized route contains RouteOptions. "
+        + "Use DirectionsRoute#fromJson(json, accessToken) instead.");
+    String json = loadJsonFixture("directions_v5-with-closure_precision_6.json");
+    RouteOptions options = RouteOptions.builder()
+      .profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC)
+      .coordinatesList(new ArrayList<Point>() {{
+        add(Point.fromLngLat(1.0, 1.0));
+        add(Point.fromLngLat(2.0, 2.0));
+      }})
+      .accessToken("token")
+      .build();
+    String uuid = "123";
+    DirectionsRoute route = DirectionsRoute.fromJson(json, options, uuid);
+
+    String newRouteJson = route.toJson();
+
+    DirectionsRoute.fromJson(newRouteJson);
+  }
+
+  @Test
+  public void directionsRoute_json_invalid_without_options() throws Exception {
+    thrown.expect(RuntimeException.class);
+    thrown.expectMessage(
+      "Provided serialized route does not contain RouteOptions. "
+        + "Use DirectionsRoute#fromJson(json) instead.");
+    String json = loadJsonFixture("directions_v5-with-closure_precision_6.json");
+    DirectionsRoute route = DirectionsRoute.fromJson(json);
+
+    String newRouteJson = route.toJson();
+
+    DirectionsRoute.fromJson(newRouteJson, "token");
+  }
 }