remove legacy turn_costs for isochrone too (#2018)

* remove legacy turn_costs for isochrone too * add test case that was red and is now green
graphhopper · Apr 26, 2020 · d428c4b · d428c4b
1 parent ad7cf96
commit d428c4b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 12 deletions.
diff --git a/web-bundle/src/main/java/com/graphhopper/resources/IsochroneResource.java b/web-bundle/src/main/java/com/graphhopper/resources/IsochroneResource.java
@@ -48,6 +48,7 @@
 
 import static com.graphhopper.resources.IsochroneResource.ResponseType.geojson;
 import static com.graphhopper.resources.RouteResource.errorIfLegacyParameters;
+import static com.graphhopper.resources.RouteResource.removeLegacyParameters;
 import static com.graphhopper.routing.util.TraversalMode.EDGE_BASED;
 import static com.graphhopper.routing.util.TraversalMode.NODE_BASED;
 
@@ -75,7 +76,7 @@ enum ResponseType {json, geojson}
     public Response doGet(
             @Context UriInfo uriInfo,
             @QueryParam("profile") String profileName,
-            @QueryParam("buckets") @Range(min=1,max=20) @DefaultValue("1") IntParam nBuckets,
+            @QueryParam("buckets") @Range(min = 1, max = 20) @DefaultValue("1") IntParam nBuckets,
             @QueryParam("reverse_flow") @DefaultValue("false") boolean reverseFlow,
             @QueryParam("point") @NotNull GHPointParam point,
             @QueryParam("time_limit") @DefaultValue("600") LongParam timeLimitInSeconds,
@@ -90,8 +91,7 @@ public Response doGet(
         hintsMap.putObject(Parameters.Landmark.DISABLE, true);
         if (Helper.isEmpty(profileName)) {
             profileName = profileResolver.resolveProfile(hintsMap).getName();
-            hintsMap.remove("weighting");
-            hintsMap.remove("vehicle");
+            removeLegacyParameters(hintsMap);
         }
         errorIfLegacyParameters(hintsMap);
 
@@ -117,7 +117,7 @@ public Response doGet(
         ShortestPathTree shortestPathTree = new ShortestPathTree(queryGraph, weighting, reverseFlow, traversalMode);
 
         double limit;
-        if (weightLimit.get() > 0){
+        if (weightLimit.get() > 0) {
             limit = weightLimit.get();
             shortestPathTree.setWeightLimit(limit + Math.max(limit * 0.14, 2_000));
         } else if (distanceLimitInMeter.get() > 0) {
@@ -136,7 +136,7 @@ public Response doGet(
         Collection<ConstraintVertex> sites = new ArrayList<>();
         shortestPathTree.search(qr.getClosestNode(), label -> {
             double exploreValue;
-            if (weightLimit.get() > 0){
+            if (weightLimit.get() > 0) {
                 exploreValue = label.weight;
             } else if (distanceLimitInMeter.get() > 0) {
                 exploreValue = label.distance;

diff --git a/web-bundle/src/main/java/com/graphhopper/resources/RouteResource.java b/web-bundle/src/main/java/com/graphhopper/resources/RouteResource.java
@@ -105,7 +105,7 @@ public Response doGet(
         if (Helper.isEmpty(profileName)) {
             enableEdgeBasedIfThereAreCurbsides(curbsides, request);
             profileName = profileResolver.resolveProfile(request.getHints()).getName();
-            removeLegacyParameters(request);
+            removeLegacyParameters(request.getHints());
         }
         errorIfLegacyParameters(request.getHints());
         request.setPoints(points).
@@ -161,7 +161,7 @@ public Response doPost(@NotNull GHRequest request, @Context HttpServletRequest h
         if (Helper.isEmpty(request.getProfile())) {
             enableEdgeBasedIfThereAreCurbsides(request.getCurbsides(), request);
             request.setProfile(profileResolver.resolveProfile(request.getHints()).getName());
-            removeLegacyParameters(request);
+            removeLegacyParameters(request.getHints());
         }
         errorIfLegacyParameters(request.getHints());
         GHResponse ghResponse = graphHopper.route(request);
@@ -219,12 +219,12 @@ public static void errorIfLegacyParameters(PMap hints) {
                     " You used 'turn_costs=" + hints.getBool("turn_costs", false) + "'");
     }
 
-    private void removeLegacyParameters(GHRequest request) {
+    public static void removeLegacyParameters(PMap hints) {
         // these parameters should only be used to resolve the profile, but should not be passed to GraphHopper
-        request.getHints().remove("weighting");
-        request.getHints().remove("vehicle");
-        request.getHints().remove("edge_based");
-        request.getHints().remove("turn_costs");
+        hints.remove("weighting");
+        hints.remove("vehicle");
+        hints.remove("edge_based");
+        hints.remove("turn_costs");
     }
 
     private static Response.ResponseBuilder gpxSuccessResponseBuilder(GHResponse ghRsp, String timeString, String

diff --git a/web/src/test/java/com/graphhopper/http/resources/IsochroneResourceTest.java b/web/src/test/java/com/graphhopper/http/resources/IsochroneResourceTest.java
@@ -202,6 +202,18 @@ public void profileWithLegacyParametersNotAllowed() {
         assertNotAllowed("&profile=fast_car&vehicle=car", "Since you are using the 'profile' parameter, do not use the 'vehicle' parameter. You used 'vehicle=car'");
     }
 
+    @Test
+    public void queryWithLegacyParameter() {
+        Response rsp = clientTarget(app, "/isochrone")
+                .queryParam("point", "42.508932,1.528516")
+                .queryParam("turn_costs", "false")
+                .queryParam("type", "geojson")
+                .request().buildGet().invoke();
+        JsonFeatureCollection featureCollection = rsp.readEntity(JsonFeatureCollection.class);
+        Geometry polygon0 = featureCollection.getFeatures().get(0).getGeometry();
+        assertEquals(330, polygon0.getCoordinates().length, 20);
+    }
+
     @Test
     public void missingPoint() {
         Response rsp = clientTarget(app, "/isochrone").request().buildGet().invoke();