Bearing formatting fix (#1121)

* bearing formatting fix

* fix bearings parsing

* use string to store RouteOptions.waypointTargets

Author: korshaknn

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

@@ -481,13 +481,10 @@ public String waypointNames() {
    * Must be used with {@link RouteOptions#steps()} = true.
    * @return  a list of Points representing coordinate pairs for drop-off locations
    * @since 4.3.0
-   * @deprecated use {@link #waypointTargetsList()}
    */
+  @SerializedName("waypoint_targets")
   @Nullable
-  @Deprecated
-  public String waypointTargets() {
-    return FormatUtils.formatPointsList(waypointTargetsList());
-  }
+  public abstract String waypointTargets();
 
   /**
    * A list of points used to specify drop-off
@@ -499,9 +496,10 @@ public String waypointTargets() {
    * Must be used with {@link RouteOptions#steps()} = true.
    * @return  a list of Points representing coordinate pairs for drop-off locations
    */
-  @SerializedName("waypoint_targets")
   @Nullable
-  public abstract List<Point> waypointTargetsList();
+  public List<Point> waypointTargetsList() {
+    return ParseUtils.parseToPoints(waypointTargets());
+  }
 
   /**
    * To be used to specify settings for use with the walking profile.
@@ -1029,16 +1027,8 @@ public Builder waypointNames(@NonNull String waypointNames) {
      * @param waypointTargets list of coordinate pairs for drop-off locations (;)
      * @return this builder for chaining options together
      * @since 4.3.0
-     * @deprecated use {@link #waypointTargetsList(List)}
      */
-    @Deprecated
-    public Builder waypointTargets(@NonNull String waypointTargets) {
-      List<Point> targets = ParseUtils.parseToPoints(waypointTargets);
-      if (targets != null) {
-        waypointTargetsList(targets);
-      }
-      return this;
-    }
+    public abstract Builder waypointTargets(@NonNull String waypointTargets);
 
     /**
      * A list of coordinate pairs used to specify drop-off
@@ -1054,7 +1044,10 @@ public Builder waypointTargets(@NonNull String waypointTargets) {
      * @param waypointTargets list of Points for drop-off locations
      * @return this builder for chaining options together
      */
-    public abstract Builder waypointTargetsList(@NonNull List<Point> waypointTargets);
+    public Builder waypointTargetsList(@NonNull List<Point> waypointTargets) {
+      waypointTargets(FormatUtils.formatPointsList(waypointTargets));
+      return this;
+    }
 
     /**
      * To be used to specify settings for use with the walking profile.

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

@@ -125,12 +125,26 @@ public static String formatBearings(@Nullable List<List<Double>> bearings) {
 
     List<String> bearingsToJoin = new ArrayList<>();
     for (List<Double> bearing : bearings) {
-      if (bearing == null || bearing.size() == 0) {
+      if (bearing == null) {
         bearingsToJoin.add(null);
       } else {
-        bearingsToJoin.add(String.format(Locale.US, "%s,%s",
-            formatCoordinate(bearing.get(0)),
-            formatCoordinate(bearing.get(1))));
+        if (bearing.size() != 2) {
+          throw new RuntimeException("Bearing size should be 2.");
+        }
+
+        Double angle = bearing.get(0);
+        Double tolerance = bearing.get(1);
+        if (angle == null || tolerance == null) {
+          bearingsToJoin.add(null);
+        } else {
+          if (angle < 0 || angle > 360 || tolerance < 0 || tolerance > 360) {
+            throw new RuntimeException("Angle and tolerance have to be from 0 to 360.");
+          }
+
+          bearingsToJoin.add(String.format(Locale.US, "%s,%s",
+              formatCoordinate(angle),
+              formatCoordinate(tolerance)));
+        }
       }
     }
     return join(";", bearingsToJoin);

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

@@ -138,7 +138,7 @@ public static List<List<Double>> parseToListOfListOfDoubles(@Nullable String ori
     }
 
     List<List<Double>> result = new ArrayList<>();
-    String[] pairs = original.split(SEMICOLON);
+    String[] pairs = original.split(SEMICOLON, -1);
     for (String pair : pairs) {
       if (pair.isEmpty()) {
         result.add(null);

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

@@ -16,6 +16,8 @@
 
 public class RouteOptionsTest {
 
+  private static final String ROUTE_OPTIONS_JSON = "{\"baseUrl\":\"base_url\",\"user\":\"user\",\"profile\":\"profile\",\"coordinates\":[[1.0,2.0],[3.0,4.0]],\"access_token\":\"token\",\"uuid\":\"requestUuid\",\"waypoint_targets\":\";33,44;55,66\"}";
+
   @Test
   public void toBuilder() {
     RouteOptions routeOptions = routeOptions();
@@ -91,12 +93,14 @@ public void bearingsString() {
     bearing.add(5.1);
     bearing.add(7.4);
 
-    assertEquals(";5.1,7.4", routeOptions.bearings());
+    assertEquals(";5.1,7.4;;", routeOptions.bearings());
 
     List<List<Double>> bearings = routeOptions.bearingsList();
-    assertEquals(2, bearings.size());
+    assertEquals(4, bearings.size());
     assertEquals(null, bearings.get(0));
     assertEquals(bearing, bearings.get(1));
+    assertEquals(null, bearings.get(2));
+    assertEquals(null, bearings.get(3));
   }
 
   @Test
@@ -255,7 +259,7 @@ public void waypointTargetsString() {
         .waypointTargets(targetsStr)
         .build();
 
-    assertEquals("1.2,3.4;;;5.65,7.123", routeOptions.waypointTargets());
+    assertEquals("1.2,3.4;;;5.65,7.123;;;", routeOptions.waypointTargets());
 
     List<Point> targets = routeOptions.waypointTargetsList();
     assertEquals(4, targets.size());
@@ -320,6 +324,50 @@ public void annotationsList() {
     assertEquals("congestion;distance;maxspeed;speed", routeOptions.annotations());
   }
 
+  @Test
+  public void waypointTargetsStringToJson() {
+    RouteOptions options = routeOptions().toBuilder()
+        .waypointTargets(";33,44;55,66")
+        .build();
+
+    String json = options.toJson();
+
+    assertEquals(ROUTE_OPTIONS_JSON, json);
+  }
+
+  @Test
+  public void waypointTargetsListToJson() {
+    List<Point> waypointTargets = new ArrayList<>();
+    waypointTargets.add(null);
+    waypointTargets.add(Point.fromLngLat(33, 44));
+    waypointTargets.add(Point.fromLngLat(55, 66));
+
+    RouteOptions options = routeOptions().toBuilder()
+        .waypointTargetsList(waypointTargets)
+        .build();
+
+    String json = options.toJson();
+
+    assertEquals(ROUTE_OPTIONS_JSON, json);
+  }
+
+  @Test
+  public void waypointTargetsStringFromJson() {
+    RouteOptions options = RouteOptions.fromJson(ROUTE_OPTIONS_JSON);
+
+    assertEquals(";33,44;55,66", options.waypointTargets());
+  }
+
+  @Test
+  public void waypointTargetsListFromJson() {
+    RouteOptions options = RouteOptions.fromJson(ROUTE_OPTIONS_JSON);
+
+    assertEquals(3, options.waypointTargetsList().size());
+    assertEquals(null, options.waypointTargetsList().get(0));
+    assertEquals(Point.fromLngLat(33, 44), options.waypointTargetsList().get(1));
+    assertEquals(Point.fromLngLat(55, 66), options.waypointTargetsList().get(2));
+  }
+
   private RouteOptions routeOptions() {
     List<Point> coordinates = new ArrayList<>();
     coordinates.add(Point.fromLngLat(1.0, 2.0));

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

@@ -59,7 +59,7 @@ private List<DirectionsRoute> generateRouteOptions(Response<DirectionsResponse>
           .waypointNamesList(ParseUtils.parseToStrings(mapboxDirections.waypointNames()))
           .waypointTargetsList(ParseUtils.parseToPoints(mapboxDirections.waypointTargets()))
           .continueStraight(mapboxDirections.continueStraight())
-          .annotations(mapboxDirections.annotation())
+          .annotationsList(ParseUtils.parseToStrings(mapboxDirections.annotation()))
           .approachesList(ParseUtils.parseToStrings(mapboxDirections.approaches()))
           .bearingsList(ParseUtils.parseToListOfListOfDoubles(mapboxDirections.bearing()))
           .alternatives(mapboxDirections.alternatives())

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

@@ -658,11 +658,7 @@ public Builder addAnnotation(@AnnotationCriteria @NonNull String annotation) {
      */
     public Builder addBearing(@Nullable @FloatRange(from = 0, to = 360) Double angle,
                               @Nullable @FloatRange(from = 0, to = 360) Double tolerance) {
-      if (angle == null || tolerance == null) {
-        bearings.add(new ArrayList<Double>());
-      } else {
-        bearings.add(Arrays.asList(angle, tolerance));
-      }
+      bearings.add(Arrays.asList(angle, tolerance));
       return this;
     }
 
@@ -679,24 +675,7 @@ public Builder addBearing(@Nullable @FloatRange(from = 0, to = 360) Double angle
      * @return this builder for chaining options together
      */
     public Builder bearings(@NonNull List<List<Double>> bearings) {
-      List<List<Double>> newBearings = new ArrayList<>();
-      for (List<Double> bearing : bearings) {
-        if (bearing.size() != 2) {
-          throw new ServicesException("Bearing size should be 2.");
-        }
-        Double angle = bearing.get(0);
-        Double tolerance = bearing.get(1);
-        if (angle == null || tolerance == null) {
-          newBearings.add(new ArrayList<Double>());
-        } else {
-          if (angle < 0 || angle > 360 || tolerance < 0 || tolerance > 360) {
-            throw new ServicesException("Angle and tolerance have to be from 0 to 360.");
-          }
-          newBearings.add(Arrays.asList(angle, tolerance));
-        }
-      }
-
-      this.bearings = newBearings;
+      this.bearings = bearings;
       return this;
     }
 

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

@@ -8,6 +8,7 @@
 import com.mapbox.api.directions.v5.models.DirectionsRoute;
 import com.mapbox.api.directions.v5.models.LegAnnotation;
 import com.mapbox.api.directions.v5.models.RouteOptions;
+import com.mapbox.api.directions.v5.utils.ParseUtils;
 import com.mapbox.core.TestUtils;
 import com.mapbox.core.exceptions.ServicesException;
 import com.mapbox.geojson.Point;
@@ -470,6 +471,54 @@ public void addBearing_doesGetFormattedInUrlCorrectly() throws Exception {
       directions.cloneCall().request().url().queryParameter("bearings"));
   }
 
+  @Test
+  public void addNullBearings_doesGetFormattedInUrlCorrectly() throws Exception {
+    List<Double> bearing1 = new ArrayList<>();
+    bearing1.add(45d);
+    bearing1.add(90d);
+
+    List<Double> bearing2 = new ArrayList<>();
+    bearing2.add(2d);
+    bearing2.add(90d);
+
+    List<List<Double>> bearings = new ArrayList<>();
+    bearings.add(null);
+    bearings.add(null);
+    bearings.add(bearing1);
+    bearings.add(bearing2);
+    bearings.add(null);
+
+    MapboxDirections directions = MapboxDirections.builder()
+        .destination(Point.fromLngLat(13.4930, 9.958))
+        .origin(Point.fromLngLat(1.234, 2.345))
+        .bearings(bearings)
+        .accessToken(ACCESS_TOKEN)
+        .build();
+    assertEquals(";;45,90;2,90;",
+        directions.cloneCall().request().url().queryParameter("bearings"));
+  }
+
+  @Test
+  public void checksParseToListOfListOfDoublesEmptyTrailing() {
+    List<List<Double>> bearings = ParseUtils.parseToListOfListOfDoubles(";;45,90;2,90;");
+
+    assertEquals(5, bearings.size());
+
+    List<Double> bearing1 = new ArrayList<>();
+    bearing1.add(45d);
+    bearing1.add(90d);
+
+    List<Double> bearing2 = new ArrayList<>();
+    bearing2.add(2d);
+    bearing2.add(90d);
+
+    assertEquals(null, bearings.get(0));
+    assertEquals(null, bearings.get(1));
+    assertEquals(bearing1, bearings.get(2));
+    assertEquals(bearing2, bearings.get(3));
+    assertEquals(null, bearings.get(4));
+  }
+
   @Test
   public void bearing_doesGetFormattedInUrlCorrectly() throws Exception {
     List<List<Double>> bearings = new ArrayList<>();

services-geojson/src/main/java/com/mapbox/geojson/BaseCoordinatesTypeAdapter.java

@@ -25,8 +25,11 @@
 abstract class BaseCoordinatesTypeAdapter<T> extends TypeAdapter<T> {
 
 
-  protected void writePoint(JsonWriter out, Point value) throws  IOException {
-    writePointList(out, value.coordinates());
+  protected void writePoint(JsonWriter out, Point point) throws  IOException {
+    if (point == null) {
+      return;
+    }
+    writePointList(out, point.coordinates());
   }
 
   protected Point readPoint(JsonReader in) throws IOException {