if too few curbsides are specified do not throw IndexOutOfBoundsExcep…

…tion for POST requests
graphhopper · Feb 21, 2020 · 4c5d589 · 4c5d589
1 parent fcc17d9
commit 4c5d589
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 26 deletions.
diff --git a/core/src/main/java/com/graphhopper/routing/template/ViaRoutingTemplate.java b/core/src/main/java/com/graphhopper/routing/template/ViaRoutingTemplate.java
@@ -44,6 +44,7 @@
 import static com.graphhopper.util.EdgeIterator.ANY_EDGE;
 import static com.graphhopper.util.EdgeIterator.NO_EDGE;
 import static com.graphhopper.util.Parameters.Curbsides.CURBSIDE_ANY;
+import static com.graphhopper.util.Parameters.Routing.CURBSIDE;
 
 /**
  * Implementation of calculating a route with multiple via points.
@@ -140,35 +141,37 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
             // calculate paths
             List<Path> tmpPathList;
             if (!directions.isEmpty()) {
-                assert ghRequest.getCurbsides().size() == directions.size();
-                if (!(algo instanceof BidirRoutingAlgorithm)) {
+                if (ghRequest.getCurbsides().size() != ghRequest.getPoints().size())
+                    throw new IllegalArgumentException("If you pass " + CURBSIDE + ", you need to pass exactly one curbside for every point, empty curbsides will be ignored");
+
+                if (!(algo instanceof BidirRoutingAlgorithm))
                     throw new IllegalArgumentException("To make use of the " + Routing.CURBSIDE + " parameter you need a bidirectional algorithm, got: " + algo.getName());
-                } else {
-                    final String fromCurbside = ghRequest.getCurbsides().get(placeIndex - 1);
-                    final String toCurbside = ghRequest.getCurbsides().get(placeIndex);
-                    int sourceOutEdge = DirectionResolverResult.getOutEdge(directions.get(placeIndex - 1), fromCurbside);
-                    int targetInEdge = DirectionResolverResult.getInEdge(directions.get(placeIndex), toCurbside);
-                    sourceOutEdge = ignoreThrowOrAcceptImpossibleCurbsides(sourceOutEdge, placeIndex - 1, forceCurbsides);
-                    targetInEdge = ignoreThrowOrAcceptImpossibleCurbsides(targetInEdge, placeIndex, forceCurbsides);
-
-                    if (fromQResult.getClosestNode() == toQResult.getClosestNode()) {
-                        // special case where we go from one point back to itself. for example going from a point A
-                        // with curbside right to the same point with curbside right is interpreted as 'being there
-                        // already' -> empty path. Similarly if the curbside for the start/target is not even specified
-                        // there is no need to drive a loop. However, going from point A/right to point A/left (or the
-                        // other way around) means we need to drive some kind of loop to get back to the same location
-                        // (arriving on the other side of the road).
-                        if (Helper.isEmpty(fromCurbside) || Helper.isEmpty(toCurbside) || fromCurbside.equals(CURBSIDE_ANY) ||
-                                toCurbside.equals(CURBSIDE_ANY) || fromCurbside.equals(toCurbside)) {
-                            // we just disable start/target edge constraints to get an empty path
-                            sourceOutEdge = ANY_EDGE;
-                            targetInEdge = ANY_EDGE;
-                        }
+
+                final String fromCurbside = ghRequest.getCurbsides().get(placeIndex - 1);
+                final String toCurbside = ghRequest.getCurbsides().get(placeIndex);
+                int sourceOutEdge = DirectionResolverResult.getOutEdge(directions.get(placeIndex - 1), fromCurbside);
+                int targetInEdge = DirectionResolverResult.getInEdge(directions.get(placeIndex), toCurbside);
+                sourceOutEdge = ignoreThrowOrAcceptImpossibleCurbsides(sourceOutEdge, placeIndex - 1, forceCurbsides);
+                targetInEdge = ignoreThrowOrAcceptImpossibleCurbsides(targetInEdge, placeIndex, forceCurbsides);
+
+                if (fromQResult.getClosestNode() == toQResult.getClosestNode()) {
+                    // special case where we go from one point back to itself. for example going from a point A
+                    // with curbside right to the same point with curbside right is interpreted as 'being there
+                    // already' -> empty path. Similarly if the curbside for the start/target is not even specified
+                    // there is no need to drive a loop. However, going from point A/right to point A/left (or the
+                    // other way around) means we need to drive some kind of loop to get back to the same location
+                    // (arriving on the other side of the road).
+                    if (Helper.isEmpty(fromCurbside) || Helper.isEmpty(toCurbside) || fromCurbside.equals(CURBSIDE_ANY) ||
+                            toCurbside.equals(CURBSIDE_ANY) || fromCurbside.equals(toCurbside)) {
+                        // we just disable start/target edge constraints to get an empty path
+                        sourceOutEdge = ANY_EDGE;
+                        targetInEdge = ANY_EDGE;
                     }
-                    // todo: enable curbside feature for alternative routes as well ?
-                    tmpPathList = Collections.singletonList(((BidirRoutingAlgorithm) algo)
-                            .calcPath(fromQResult.getClosestNode(), toQResult.getClosestNode(), sourceOutEdge, targetInEdge));
                 }
+                // todo: enable curbside feature for alternative routes as well ?
+                tmpPathList = Collections.singletonList(((BidirRoutingAlgorithm) algo)
+                        .calcPath(fromQResult.getClosestNode(), toQResult.getClosestNode(), sourceOutEdge, targetInEdge));
+
             } else {
                 tmpPathList = algo.calcPaths(fromQResult.getClosestNode(), toQResult.getClosestNode());
             }

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,6 +105,8 @@ public Response doGet(
         if (favoredHeadings.size() > 1 && favoredHeadings.size() != requestPoints.size())
             throw new IllegalArgumentException("The number of 'heading' parameters must be <= 1 "
                     + "or equal to the number of points (" + requestPoints.size() + ")");
+
+        // TODO these checks should be only necessary once in the core, e.g. pointHints problems are currently ignored for POST requests
         if (pointHints.size() > 0 && pointHints.size() != requestPoints.size())
             throw new IllegalArgumentException("If you pass " + POINT_HINT + ", you need to pass exactly one hint for every point, empty hints will be ignored");
         if (curbsides.size() > 0 && curbsides.size() != requestPoints.size())