From a71b33fe822630a2e777d1602cb01cacb21696a0 Mon Sep 17 00:00:00 2001 From: Nikita Korshak Date: Thu, 20 Feb 2020 11:21:28 +0300 Subject: [PATCH 1/3] bearing formatting fix --- .../api/directions/v5/utils/FormatUtils.java | 22 ++++++++++++--- .../api/directions/v5/MapboxDirections.java | 25 ++--------------- .../directions/v5/MapboxDirectionsTest.java | 27 +++++++++++++++++++ .../geojson/BaseCoordinatesTypeAdapter.java | 7 +++-- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/FormatUtils.java b/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/FormatUtils.java index fc395fe84..62bcd1651 100644 --- a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/FormatUtils.java +++ b/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/FormatUtils.java @@ -125,12 +125,26 @@ public static String formatBearings(@Nullable List> bearings) { List bearingsToJoin = new ArrayList<>(); for (List 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); diff --git a/services-directions/src/main/java/com/mapbox/api/directions/v5/MapboxDirections.java b/services-directions/src/main/java/com/mapbox/api/directions/v5/MapboxDirections.java index 05daa70a3..371799a38 100644 --- a/services-directions/src/main/java/com/mapbox/api/directions/v5/MapboxDirections.java +++ b/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()); - } 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> bearings) { - List> newBearings = new ArrayList<>(); - for (List 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()); - } 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; } diff --git a/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java b/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java index 045a13b1e..ff14bc097 100644 --- a/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java +++ b/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java @@ -470,6 +470,33 @@ public void addBearing_doesGetFormattedInUrlCorrectly() throws Exception { directions.cloneCall().request().url().queryParameter("bearings")); } + @Test + public void addNullBearings_doesGetFormattedInUrlCorrectly() throws Exception { + List bearing1 = new ArrayList<>(); + bearing1.add(45d); + bearing1.add(90d); + + List bearing2 = new ArrayList<>(); + bearing2.add(2d); + bearing2.add(90d); + + List> 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 bearing_doesGetFormattedInUrlCorrectly() throws Exception { List> bearings = new ArrayList<>(); diff --git a/services-geojson/src/main/java/com/mapbox/geojson/BaseCoordinatesTypeAdapter.java b/services-geojson/src/main/java/com/mapbox/geojson/BaseCoordinatesTypeAdapter.java index 274dbfe36..319a05e0c 100644 --- a/services-geojson/src/main/java/com/mapbox/geojson/BaseCoordinatesTypeAdapter.java +++ b/services-geojson/src/main/java/com/mapbox/geojson/BaseCoordinatesTypeAdapter.java @@ -25,8 +25,11 @@ abstract class BaseCoordinatesTypeAdapter extends TypeAdapter { - 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 { From 377949a65501b48acf100272aa138b2008883326 Mon Sep 17 00:00:00 2001 From: Nikita Korshak Date: Fri, 21 Feb 2020 12:45:55 +0300 Subject: [PATCH 2/3] fix bearings parsing --- .../api/directions/v5/utils/ParseUtils.java | 2 +- .../v5/models/RouteOptionsTest.java | 6 +++-- .../v5/DirectionsResponseFactory.java | 2 +- .../directions/v5/MapboxDirectionsTest.java | 22 +++++++++++++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/ParseUtils.java b/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/ParseUtils.java index 7ed57b1f5..d781437eb 100644 --- a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/ParseUtils.java +++ b/services-directions-models/src/main/java/com/mapbox/api/directions/v5/utils/ParseUtils.java @@ -138,7 +138,7 @@ public static List> parseToListOfListOfDoubles(@Nullable String ori } List> result = new ArrayList<>(); - String[] pairs = original.split(SEMICOLON); + String[] pairs = original.split(SEMICOLON, -1); for (String pair : pairs) { if (pair.isEmpty()) { result.add(null); diff --git a/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java b/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java index 5f55aa4de..e877d15e8 100644 --- a/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java +++ b/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java @@ -91,12 +91,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> 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 diff --git a/services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsResponseFactory.java b/services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsResponseFactory.java index a11a30161..55e08aa5c 100644 --- a/services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsResponseFactory.java +++ b/services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsResponseFactory.java @@ -59,7 +59,7 @@ private List generateRouteOptions(Response .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()) diff --git a/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java b/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java index ff14bc097..e60d69c51 100644 --- a/services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java +++ b/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; @@ -497,6 +498,27 @@ public void addNullBearings_doesGetFormattedInUrlCorrectly() throws Exception { directions.cloneCall().request().url().queryParameter("bearings")); } + @Test + public void checksParseToListOfListOfDoublesEmptyTrailing() { + List> bearings = ParseUtils.parseToListOfListOfDoubles(";;45,90;2,90;"); + + assertEquals(5, bearings.size()); + + List bearing1 = new ArrayList<>(); + bearing1.add(45d); + bearing1.add(90d); + + List 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> bearings = new ArrayList<>(); From b95befc2cb4532947b0aefad57a2f57dfde4cb68 Mon Sep 17 00:00:00 2001 From: Nikita Korshak Date: Mon, 24 Feb 2020 14:27:20 +0300 Subject: [PATCH 3/3] use string to store RouteOptions.waypointTargets --- .../directions/v5/models/RouteOptions.java | 27 ++++------- .../v5/models/RouteOptionsTest.java | 48 ++++++++++++++++++- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java b/services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java index 5f28b8032..f2f72af7a 100644 --- a/services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java +++ b/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 waypointTargetsList(); + public List 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 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 waypointTargets); + public Builder waypointTargetsList(@NonNull List waypointTargets) { + waypointTargets(FormatUtils.formatPointsList(waypointTargets)); + return this; + } /** * To be used to specify settings for use with the walking profile. diff --git a/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java b/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java index e877d15e8..05cab475c 100644 --- a/services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java +++ b/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(); @@ -257,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 targets = routeOptions.waypointTargetsList(); assertEquals(4, targets.size()); @@ -322,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 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 coordinates = new ArrayList<>(); coordinates.add(Point.fromLngLat(1.0, 2.0));