Skip to content

Commit 53f42a1

Browse files
karussellkarussell
authored
a new IllegalArgumentException mapper (graphhopper#1407)
* a new IllegalArgumentException mapper, EncodingManager no longer necessary in RouteResource * avoid toLowerCase, wasn't there before * findbugs needs one bug exclusion * corrected error response for wrong point format * include supported list again * fixed i18n bug graphhopper#1409 * avoid passing through * include suggestion from @michaz and catch NumberFormatException
1 parent e465265 commit 53f42a1

File tree

11 files changed

+143
-81
lines changed

11 files changed

+143
-81
lines changed

‎api/src/main/java/com/graphhopper/util/shapes/GHPoint.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,19 @@ public static GHPoint fromJson(double[] xy) {
5353

5454
private static GHPoint fromString(String str, boolean lonLatOrder) {
5555
String[] fromStrs = str.split(",");
56-
if (fromStrs.length == 2) {
56+
if (fromStrs.length != 2)
57+
throw new IllegalArgumentException("Cannot parse point '" + str + "'");
58+
59+
try {
5760
double fromLat = Double.parseDouble(fromStrs[0]);
5861
double fromLon = Double.parseDouble(fromStrs[1]);
5962
if (lonLatOrder) {
6063
return new GHPoint(fromLon, fromLat);
6164
} else {
6265
return new GHPoint(fromLat, fromLon);
6366
}
64-
} else {
65-
throw new IllegalArgumentException(str);
67+
} catch (NumberFormatException ex) {
68+
throw new IllegalArgumentException("Cannot parse point '" + str + "'");
6669
}
6770
}
6871

@@ -91,8 +94,7 @@ public boolean equals(Object obj) {
9194
if (obj == null)
9295
return false;
9396

94-
@SuppressWarnings("unchecked")
95-
final GHPoint other = (GHPoint) obj;
97+
@SuppressWarnings("unchecked") final GHPoint other = (GHPoint) obj;
9698
return NumHelper.equalsEps(lat, other.lat) && NumHelper.equalsEps(lon, other.lon);
9799
}
98100

‎core/files/findbugs-exclude.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
<FindBugsFilter>
33
<!-- documentation about findbug's rule match clauses can be found here:
44
http://findbugs.sourceforge.net/manual/filter.html -->
5+
56
</FindBugsFilter>

‎core/src/main/java/com/graphhopper/GraphHopper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -977,8 +977,7 @@ public List<Path> calcPaths(GHRequest request, GHResponse ghRsp) {
977977
readLock.lock();
978978
try {
979979
if (!encodingManager.supports(vehicle))
980-
throw new IllegalArgumentException("Vehicle " + vehicle + " unsupported. "
981-
+ "Supported are: " + getEncodingManager());
980+
throw new IllegalArgumentException("Vehicle not supported: " + vehicle + ". Supported are: " + encodingManager.toString());
982981

983982
HintsMap hints = request.getHints();
984983
String tModeStr = hints.get("traversal_mode", traversalMode.toString());

‎web-bundle/src/main/java/com/graphhopper/http/GHPointConverterProvider.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Licensed to GraphHopper GmbH under one or more contributor
3+
* license agreements. See the NOTICE file distributed with this work for
4+
* additional information regarding copyright ownership.
5+
*
6+
* GraphHopper GmbH licenses this file to you under the Apache License,
7+
* Version 2.0 (the "License"); you may not use this file except in
8+
* compliance with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package com.graphhopper.http;
20+
21+
import com.graphhopper.MultiException;
22+
import com.graphhopper.util.shapes.GHPoint;
23+
24+
import javax.ws.rs.WebApplicationException;
25+
import javax.ws.rs.core.Response;
26+
import javax.ws.rs.ext.ParamConverter;
27+
import javax.ws.rs.ext.ParamConverterProvider;
28+
import java.lang.annotation.Annotation;
29+
import java.lang.reflect.Type;
30+
31+
public class GHPointConverterProvider implements ParamConverterProvider {
32+
33+
@Override
34+
public <T> ParamConverter<T> getConverter(Class<T> rawType, Type genericType, Annotation[] annotations) {
35+
if (rawType.equals(GHPoint.class)) {
36+
return new ParamConverter<T>() {
37+
@Override
38+
public T fromString(String value) {
39+
try {
40+
return (T) GHPoint.fromString(value);
41+
} catch (IllegalArgumentException ex) {
42+
throw new WebApplicationException(Response.status(Response.Status.BAD_REQUEST)
43+
.entity(new MultiException(ex))
44+
.build());
45+
}
46+
}
47+
48+
@Override
49+
public String toString(T value) {
50+
return value.toString();
51+
}
52+
};
53+
}
54+
return null;
55+
}
56+
}
57+

‎web-bundle/src/main/java/com/graphhopper/http/GraphHopperBundle.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,9 @@ public void run(GraphHopperBundleConfiguration configuration, Environment enviro
197197
environment.jersey().register(new MultiExceptionMapper());
198198
environment.jersey().register(new MultiExceptionGPXMessageBodyWriter());
199199

200+
environment.jersey().register(new IllegalArgumentExceptionMapper());
201+
environment.jersey().register(new GHPointConverterProvider());
202+
200203
if (configuration.getGraphHopperConfiguration().has("gtfs.file")) {
201204
// switch to different API implementation when using Pt
202205
runPtGraphHopper(configuration.getGraphHopperConfiguration(), environment);

‎web-bundle/src/main/java/com/graphhopper/http/IllegalArgumentExceptionMapper.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.graphhopper.http;
2+
3+
import com.graphhopper.MultiException;
4+
5+
import javax.ws.rs.core.Response;
6+
import javax.ws.rs.ext.ExceptionMapper;
7+
8+
public class IllegalArgumentExceptionMapper implements ExceptionMapper<IllegalArgumentException> {
9+
10+
@Override
11+
public Response toResponse(IllegalArgumentException e) {
12+
return Response.status(Response.Status.BAD_REQUEST)
13+
.entity(new MultiException(e))
14+
.build();
15+
}
16+
}

‎web-bundle/src/main/java/com/graphhopper/http/MultiExceptionMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@
2828
public class MultiExceptionMapper implements ExceptionMapper<MultiException> {
2929
@Override
3030
public Response toResponse(MultiException exception) {
31-
return Response.status(400).entity(exception).build();
31+
return Response.status(Response.Status.BAD_REQUEST).entity(exception).build();
3232
}
3333
}

‎web-bundle/src/main/java/com/graphhopper/resources/IsochroneResource.java

Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.graphhopper.resources;
22

3-
import com.fasterxml.jackson.databind.node.ArrayNode;
43
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
54
import com.fasterxml.jackson.databind.node.ObjectNode;
65
import com.graphhopper.GraphHopper;
@@ -13,7 +12,6 @@
1312
import com.graphhopper.storage.index.LocationIndex;
1413
import com.graphhopper.storage.index.QueryResult;
1514
import com.graphhopper.util.StopWatch;
16-
import com.graphhopper.util.exceptions.GHException;
1715
import com.graphhopper.util.shapes.GHPoint;
1816
import org.slf4j.Logger;
1917
import org.slf4j.LoggerFactory;
@@ -30,8 +28,6 @@
3028
import java.util.HashMap;
3129
import java.util.List;
3230

33-
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
34-
3531
@Path("isochrone")
3632
public class IsochroneResource {
3733

@@ -62,22 +58,22 @@ public Response doGet(
6258
@QueryParam("distance_limit") @DefaultValue("-1") double distanceInMeter) {
6359

6460
if (buckets > 20 || buckets < 1)
65-
throwArgExc("Number of buckets has to be in the range [1, 20]");
61+
throw new IllegalArgumentException("Number of buckets has to be in the range [1, 20]");
6662

6763
if (point == null)
68-
throwArgExc("point parameter cannot be null");
64+
throw new IllegalArgumentException("point parameter cannot be null");
6965

7066
StopWatch sw = new StopWatch().start();
7167

7268
if (!encodingManager.supports(vehicle))
73-
throwArgExc("vehicle not supported:" + vehicle);
69+
throw new IllegalArgumentException("vehicle not supported:" + vehicle);
7470

7571
FlagEncoder encoder = encodingManager.getEncoder(vehicle);
7672
EdgeFilter edgeFilter = DefaultEdgeFilter.allEdges(encoder);
7773
LocationIndex locationIndex = graphHopper.getLocationIndex();
7874
QueryResult qr = locationIndex.findClosest(point.lat, point.lon, edgeFilter);
7975
if (!qr.isValid())
80-
throwArgExc("Point not found:" + point);
76+
throw new IllegalArgumentException("Point not found:" + point);
8177

8278
Graph graph = graphHopper.getGraphHopperStorage();
8379
QueryGraph queryGraph = new QueryGraph(graph);
@@ -92,31 +88,31 @@ public Response doGet(
9288
if (distanceInMeter > 0) {
9389
double maxMeter = 50 * 1000;
9490
if (distanceInMeter > maxMeter)
95-
throwArgExc("Specify a limit of less than " + maxMeter / 1000f + "km");
91+
throw new IllegalArgumentException("Specify a limit of less than " + maxMeter / 1000f + "km");
9692
if (buckets > (distanceInMeter / 500))
97-
throwArgExc("Specify buckets less than the number of explored kilometers");
93+
throw new IllegalArgumentException("Specify buckets less than the number of explored kilometers");
9894

9995
isochrone.setDistanceLimit(distanceInMeter);
10096
} else {
10197

10298
long maxSeconds = 80 * 60;
10399
if (timeLimitInSeconds > maxSeconds)
104-
throwArgExc("Specify a limit of less than " + maxSeconds + " seconds");
100+
throw new IllegalArgumentException("Specify a limit of less than " + maxSeconds + " seconds");
105101
if (buckets > (timeLimitInSeconds / 60))
106-
throwArgExc("Specify buckets less than the number of explored minutes");
102+
throw new IllegalArgumentException("Specify buckets less than the number of explored minutes");
107103

108104
isochrone.setTimeLimit(timeLimitInSeconds);
109105
}
110106

111107
List<List<Double[]>> list = isochrone.searchGPS(qr.getClosestNode(), buckets);
112108
if (isochrone.getVisitedNodes() > graphHopper.getMaxVisitedNodes() / 5) {
113-
throwArgExc("Server side reset: too many junction nodes would have to explored (" + isochrone.getVisitedNodes() + "). Let us know if you need this increased.");
109+
throw new IllegalArgumentException("Server side reset: too many junction nodes would have to explored (" + isochrone.getVisitedNodes() + "). Let us know if you need this increased.");
114110
}
115111

116112
int counter = 0;
117113
for (List<Double[]> tmp : list) {
118114
if (tmp.size() < 2) {
119-
throwArgExc("Too few points found for bucket " + counter + ". "
115+
throw new IllegalArgumentException("Too few points found for bucket " + counter + ". "
120116
+ "Please try a different 'point', a smaller 'buckets' count or a larger 'time_limit'. "
121117
+ "And let us know if you think this is a bug!");
122118
}
@@ -149,7 +145,7 @@ public Response doGet(
149145
}
150146
calcRes = polyList;
151147
} else {
152-
throw new WebApplicationException(jsonErrorResponse(Collections.singletonList(new IllegalArgumentException("type not supported:" + resultStr))));
148+
throw new IllegalArgumentException("type not supported:" + resultStr);
153149
}
154150

155151
logger.info("took: " + sw.getSeconds() + ", visited nodes:" + isochrone.getVisitedNodes() + ", " + uriInfo.getQueryParameters());
@@ -158,33 +154,6 @@ public Response doGet(
158154
.build();
159155
}
160156

161-
private void throwArgExc(String msg) {
162-
throw new WebApplicationException(jsonErrorResponse(Collections.singletonList(new IllegalArgumentException(msg))));
163-
}
164-
165-
166-
private Response jsonErrorResponse(List<Throwable> errors) {
167-
ObjectNode json = JsonNodeFactory.instance.objectNode();
168-
json.put("message", getMessage(errors.get(0)));
169-
ArrayNode errorHintList = json.putArray("hints");
170-
for (Throwable t : errors) {
171-
ObjectNode error = errorHintList.addObject();
172-
error.put("message", getMessage(t));
173-
error.put("details", t.getClass().getName());
174-
if (t instanceof GHException) {
175-
((GHException) t).getDetails().forEach(error::putPOJO);
176-
}
177-
}
178-
return Response.status(SC_BAD_REQUEST).entity(json).build();
179-
}
180-
181-
private String getMessage(Throwable t) {
182-
if (t.getMessage() == null)
183-
return t.getClass().getSimpleName();
184-
else
185-
return t.getMessage();
186-
}
187-
188157
private Response jsonSuccessResponse(Object result, float took) {
189158
ObjectNode json = JsonNodeFactory.instance.objectNode();
190159
json.putPOJO("polygons", result);

‎web-bundle/src/main/java/com/graphhopper/resources/RouteResource.java

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.graphhopper.routing.util.EncodingManager;
2626
import com.graphhopper.routing.util.HintsMap;
2727
import com.graphhopper.util.Constants;
28+
import com.graphhopper.util.Helper;
2829
import com.graphhopper.util.Parameters;
2930
import com.graphhopper.util.StopWatch;
3031
import com.graphhopper.util.shapes.GHPoint;
@@ -37,7 +38,10 @@
3738
import javax.ws.rs.*;
3839
import javax.ws.rs.container.ContainerRequestContext;
3940
import javax.ws.rs.core.*;
40-
import java.util.*;
41+
import java.util.ArrayList;
42+
import java.util.Collections;
43+
import java.util.List;
44+
import java.util.Map;
4145

4246
import static com.graphhopper.util.Parameters.Routing.*;
4347

@@ -54,13 +58,11 @@ public class RouteResource {
5458
private static final Logger logger = LoggerFactory.getLogger(RouteResource.class);
5559

5660
private final GraphHopperAPI graphHopper;
57-
private final EncodingManager encodingManager;
5861
private final Boolean hasElevation;
5962

6063
@Inject
61-
public RouteResource(GraphHopperAPI graphHopper, EncodingManager encodingManager, @Named("hasElevation") Boolean hasElevation) {
64+
public RouteResource(GraphHopperAPI graphHopper, @Named("hasElevation") Boolean hasElevation) {
6265
this.graphHopper = graphHopper;
63-
this.encodingManager = encodingManager;
6466
this.hasElevation = hasElevation;
6567
}
6668

@@ -70,7 +72,7 @@ public Response doGet(
7072
@Context HttpServletRequest httpReq,
7173
@Context UriInfo uriInfo,
7274
@Context ContainerRequestContext rc,
73-
@QueryParam(WAY_POINT_MAX_DISTANCE)@DefaultValue("1") double minPathPrecision,
75+
@QueryParam(WAY_POINT_MAX_DISTANCE) @DefaultValue("1") double minPathPrecision,
7476
@QueryParam("point") List<GHPoint> requestPoints,
7577
@QueryParam("type") @DefaultValue("json") String type,
7678
@QueryParam(INSTRUCTIONS) @DefaultValue("true") boolean instructions,
@@ -94,22 +96,15 @@ public Response doGet(
9496

9597
StopWatch sw = new StopWatch().start();
9698

97-
if(requestPoints.isEmpty()) {
98-
throw new MultiException(new IllegalArgumentException("You have to pass at least one point"));
99-
}
100-
101-
if (!encodingManager.supports(vehicleStr)) {
102-
throw new MultiException(new IllegalArgumentException("Vehicle not supported: " + vehicleStr));
103-
} else if (enableElevation && !hasElevation) {
104-
throw new MultiException(new IllegalArgumentException("Elevation not supported!"));
105-
} else if (favoredHeadings.size() > 1 && favoredHeadings.size() != requestPoints.size()) {
106-
throw new MultiException(new IllegalArgumentException("The number of 'heading' parameters must be <= 1 "
107-
+ "or equal to the number of points (" + requestPoints.size() + ")"));
108-
}
109-
110-
if (pointHints.size() > 0 && pointHints.size() != requestPoints.size()) {
111-
throw new MultiException(new IllegalArgumentException("If you pass " + POINT_HINT + ", you need to pass a hint for every point, empty hints will be ignored"));
112-
}
99+
if (requestPoints.isEmpty())
100+
throw new IllegalArgumentException("You have to pass at least one point");
101+
if (enableElevation && !hasElevation)
102+
throw new IllegalArgumentException("Elevation not supported!");
103+
if (favoredHeadings.size() > 1 && favoredHeadings.size() != requestPoints.size())
104+
throw new IllegalArgumentException("The number of 'heading' parameters must be <= 1 "
105+
+ "or equal to the number of points (" + requestPoints.size() + ")");
106+
if (pointHints.size() > 0 && pointHints.size() != requestPoints.size())
107+
throw new IllegalArgumentException("If you pass " + POINT_HINT + ", you need to pass a hint for every point, empty hints will be ignored");
113108

114109
GHRequest request;
115110
if (favoredHeadings.size() > 0) {
@@ -126,7 +121,7 @@ public Response doGet(
126121
}
127122

128123
initHints(request.getHints(), uriInfo.getQueryParameters());
129-
request.setVehicle(encodingManager.getEncoder(vehicleStr).toString()).
124+
request.setVehicle(vehicleStr).
130125
setWeighting(weighting).
131126
setAlgorithm(algoStr).
132127
setLocale(localeStr).
@@ -165,9 +160,10 @@ public Response doGet(
165160
}
166161
}
167162

168-
private static Response.ResponseBuilder gpxSuccessResponseBuilder(GHResponse ghRsp, String timeString, String trackName, boolean enableElevation, boolean withRoute, boolean withTrack, boolean withWayPoints, String version) {
163+
private static Response.ResponseBuilder gpxSuccessResponseBuilder(GHResponse ghRsp, String timeString, String
164+
trackName, boolean enableElevation, boolean withRoute, boolean withTrack, boolean withWayPoints, String version) {
169165
if (ghRsp.getAll().size() > 1) {
170-
throw new MultiException(new IllegalArgumentException("Alternatives are currently not yet supported for GPX"));
166+
throw new IllegalArgumentException("Alternatives are currently not yet supported for GPX");
171167
}
172168

173169
long time = timeString != null ? Long.parseLong(timeString) : System.currentTimeMillis();

‎web/src/test/java/com/graphhopper/http/isochrone/IsochroneResourceIT.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.junit.ClassRule;
1414
import org.junit.Test;
1515

16+
import javax.ws.rs.core.Response;
1617
import java.io.File;
1718
import java.util.List;
1819

@@ -92,6 +93,11 @@ public void requestReverseFlow() throws Exception {
9293
}
9394

9495
@Test
96+
public void requestBadRequest() {
97+
Response response = app.client().target("http://localhost:8080/route?point=-1.816719,51.557148").request().buildGet().invoke();
98+
assertEquals(400, response.getStatus());
99+
}
100+
95101
public void requestWithShortest() throws Exception {
96102
IsochroneResponse rsp = client.isochroneGet("42.509644,1.540554", "no_key_necessary", 130,
97103
-1, "car", 1, false, "shortest");

0 commit comments

Comments
 (0)