Merge pull request #10728 from osmandapp/OnlineRoutingRefactoring

refactor server wrong response checking

OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java

@@ -9,6 +9,7 @@ import net.osmand.osm.io.NetworkUtils;
 import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.Version;
 import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine;
+import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine.OnlineRoutingResponse;
 import net.osmand.plus.settings.backend.OsmandSettings;
 import net.osmand.util.Algorithms;
 

OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingResponse.java

@@ -1,24 +0,0 @@
-package net.osmand.plus.onlinerouting;
-
-import net.osmand.Location;
-import net.osmand.plus.routing.RouteDirectionInfo;
-
-import java.util.List;
-
-public class OnlineRoutingResponse {
-	private List<Location> route;
-	private List<RouteDirectionInfo> directions;
-
-	public OnlineRoutingResponse(List<Location> route, List<RouteDirectionInfo> directions) {
-		this.route = route;
-		this.directions = directions;
-	}
-
-	public List<Location> getRoute() {
-		return route;
-	}
-
-	public List<RouteDirectionInfo> getDirections() {
-		return directions;
-	}
-}

OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java

@@ -8,7 +8,6 @@ import net.osmand.data.LatLon;
 import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.R;
 import net.osmand.plus.onlinerouting.EngineParameter;
-import net.osmand.plus.onlinerouting.OnlineRoutingResponse;
 import net.osmand.plus.onlinerouting.VehicleType;
 import net.osmand.plus.routing.RouteDirectionInfo;
 import net.osmand.router.TurnType;
@@ -84,12 +83,9 @@ public class GraphhopperEngine extends OnlineRoutingEngine {
 
 	@Nullable
 	@Override
-	public OnlineRoutingResponse parseServerResponse(@NonNull String content,
+	public OnlineRoutingResponse parseServerResponse(@NonNull JSONObject root,
 	                                                 @NonNull OsmandApplication app,
 	                                                 boolean leftSideNavigation) throws JSONException {
-		JSONObject obj = new JSONObject(content);
-		JSONObject root = obj.getJSONArray("paths").getJSONObject(0);
-
 		String encoded = root.getString("points");
 		List<LatLon> points = GeoPolylineParserUtil.parse(encoded, GeoPolylineParserUtil.PRECISION_5);
 		if (isEmpty(points)) return null;
@@ -216,14 +212,15 @@ public class GraphhopperEngine extends OnlineRoutingEngine {
 		return id != null ? TurnType.valueOf(id, leftSide) : null;
 	}
 
+	@NonNull
 	@Override
-	public boolean parseServerMessage(@NonNull StringBuilder sb,
+	protected String getErrorMessageKey() {
-	                                  @NonNull String content) throws JSONException {
+		return "message";
-		JSONObject obj = new JSONObject(content);
+	}
-		if (obj.has("message")) {
+
-			String message = obj.getString("message");
+	@NonNull
-			sb.append(message);
+	@Override
-		}
+	protected String getRootArrayKey() {
-		return obj.has("paths");
+		return "paths";
 	}
 }

OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java

@@ -12,12 +12,14 @@ import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.R;
 import net.osmand.plus.onlinerouting.EngineParameter;
 import net.osmand.plus.onlinerouting.OnlineRoutingFactory;
-import net.osmand.plus.onlinerouting.OnlineRoutingResponse;
 import net.osmand.plus.onlinerouting.VehicleType;
+import net.osmand.plus.routing.RouteDirectionInfo;
 import net.osmand.plus.routing.RouteProvider;
 import net.osmand.util.Algorithms;
 
+import org.json.JSONArray;
 import org.json.JSONException;
+import org.json.JSONObject;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -98,10 +100,31 @@ public abstract class OnlineRoutingEngine implements Cloneable {
 	@NonNull
 	public abstract String getStandardUrl();
 
+	public OnlineRoutingResponse parseServerResponse(@NonNull String content,
+	                                                 @NonNull OsmandApplication app,
+	                                                 boolean leftSideNavigation) throws JSONException {
+		JSONObject root = parseRootResponseObject(content);
+		return root != null ? parseServerResponse(root, app, leftSideNavigation) : null;
+	}
+
 	@Nullable
-	public abstract OnlineRoutingResponse parseServerResponse(@NonNull String content,
+	protected abstract OnlineRoutingResponse parseServerResponse(@NonNull JSONObject root,
-	                                                          @NonNull OsmandApplication app,
+	                                                             @NonNull OsmandApplication app,
-	                                                          boolean leftSideNavigation) throws JSONException;
+	                                                             boolean leftSideNavigation) throws JSONException;
+
+	@Nullable
+	protected JSONObject parseRootResponseObject(@NonNull String content) throws JSONException {
+		JSONObject fullJSON = new JSONObject(content);
+		String responseArrayKey = getRootArrayKey();
+		JSONArray array = null;
+		if (fullJSON.has(responseArrayKey)) {
+			array = fullJSON.getJSONArray(responseArrayKey);
+		}
+		return array != null && array.length() > 0 ? array.getJSONObject(0) : null;
+	}
+
+	@NonNull
+	protected abstract String getRootArrayKey();
 
 	@NonNull
 	protected List<Location> convertRouteToLocationsList(@NonNull List<LatLon> route) {
@@ -161,7 +184,7 @@ public abstract class OnlineRoutingEngine implements Cloneable {
 		return allowedParameters.contains(key);
 	}
 
-	protected void allowParameters(@NonNull EngineParameter ... allowedParams) {
+	protected void allowParameters(@NonNull EngineParameter... allowedParams) {
 		allowedParameters.addAll(Arrays.asList(allowedParams));
 	}
 
@@ -193,8 +216,19 @@ public abstract class OnlineRoutingEngine implements Cloneable {
 		return CUSTOM_VEHICLE;
 	}
 
-	public abstract boolean parseServerMessage(@NonNull StringBuilder sb,
+	public boolean checkServerResponse(@NonNull StringBuilder errorMessage,
-	                                           @NonNull String content) throws JSONException;
+	                                   @NonNull String content) throws JSONException {
+		JSONObject obj = new JSONObject(content);
+		String messageKey = getErrorMessageKey();
+		if (obj.has(messageKey)) {
+			String message = obj.getString(messageKey);
+			errorMessage.append(message);
+		}
+		return obj.has(getRootArrayKey());
+	}
+
+	@NonNull
+	protected abstract String getErrorMessageKey();
 
 	@NonNull
 	@Override
@@ -202,11 +236,6 @@ public abstract class OnlineRoutingEngine implements Cloneable {
 		return OnlineRoutingFactory.createEngine(getType(), getParams());
 	}
 
-	@NonNull
-	public static String generateKey() {
-		return ONLINE_ROUTING_ENGINE_PREFIX + System.currentTimeMillis();
-	}
-
 	@Override
 	public boolean equals(Object o) {
 		if (this == o) return true;
@@ -216,4 +245,27 @@ public abstract class OnlineRoutingEngine implements Cloneable {
 		if (getType() != engine.getType()) return false;
 		return Algorithms.objectEquals(getParams(), engine.getParams());
 	}
+
+	@NonNull
+	public static String generateKey() {
+		return ONLINE_ROUTING_ENGINE_PREFIX + System.currentTimeMillis();
+	}
+
+	public static class OnlineRoutingResponse {
+		private List<Location> route;
+		private List<RouteDirectionInfo> directions;
+
+		public OnlineRoutingResponse(List<Location> route, List<RouteDirectionInfo> directions) {
+			this.route = route;
+			this.directions = directions;
+		}
+
+		public List<Location> getRoute() {
+			return route;
+		}
+
+		public List<RouteDirectionInfo> getDirections() {
+			return directions;
+		}
+	}
 }

OsmAnd/src/net/osmand/plus/onlinerouting/engine/OrsEngine.java

@@ -8,7 +8,6 @@ import net.osmand.data.LatLon;
 import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.R;
 import net.osmand.plus.onlinerouting.EngineParameter;
-import net.osmand.plus.onlinerouting.OnlineRoutingResponse;
 import net.osmand.plus.onlinerouting.VehicleType;
 
 import org.json.JSONArray;
@@ -81,12 +80,10 @@ public class OrsEngine extends OnlineRoutingEngine {
 
 	@Nullable
 	@Override
-	public OnlineRoutingResponse parseServerResponse(@NonNull String content,
+	public OnlineRoutingResponse parseServerResponse(@NonNull JSONObject root,
 	                                                 @NonNull OsmandApplication app,
 	                                                 boolean leftSideNavigation) throws JSONException {
-		JSONObject obj = new JSONObject(content);
+		JSONArray array = root.getJSONObject("geometry").getJSONArray("coordinates");
-		JSONArray array = obj.getJSONArray("features").getJSONObject(0)
-				.getJSONObject("geometry").getJSONArray("coordinates");
 		List<LatLon> points = new ArrayList<>();
 		for (int i = 0; i < array.length(); i++) {
 			JSONArray point = array.getJSONArray(i);
@@ -101,14 +98,15 @@ public class OrsEngine extends OnlineRoutingEngine {
 		return null;
 	}
 
+	@NonNull
 	@Override
-	public boolean parseServerMessage(@NonNull StringBuilder sb,
+	protected String getErrorMessageKey() {
-	                                  @NonNull String content) throws JSONException {
+		return "error";
-		JSONObject obj = new JSONObject(content);
+	}
-		if (obj.has("error")) {
+
-			String message = obj.getString("error");
+	@NonNull
-			sb.append(message);
+	@Override
-		}
+	protected String getRootArrayKey() {
-		return obj.has("features");
+		return "features";
 	}
 }

OsmAnd/src/net/osmand/plus/onlinerouting/engine/OsrmEngine.java

@@ -8,7 +8,6 @@ import net.osmand.data.LatLon;
 import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.R;
 import net.osmand.plus.onlinerouting.EngineParameter;
-import net.osmand.plus.onlinerouting.OnlineRoutingResponse;
 import net.osmand.plus.onlinerouting.VehicleType;
 import net.osmand.plus.routing.RouteCalculationResult;
 import net.osmand.plus.routing.RouteDirectionInfo;
@@ -77,19 +76,17 @@ public class OsrmEngine extends OnlineRoutingEngine {
 
 	@Nullable
 	@Override
-	public OnlineRoutingResponse parseServerResponse(@NonNull String content,
+	public OnlineRoutingResponse parseServerResponse(@NonNull JSONObject root,
 	                                                 @NonNull OsmandApplication app,
 	                                                 boolean leftSideNavigation) throws JSONException {
-		JSONObject obj = new JSONObject(content);
+		String encodedPoints = root.getString("geometry");
-		JSONObject routeInfo = obj.getJSONArray("routes").getJSONObject(0);
-		String encodedPoints = routeInfo.getString("geometry");
 		List<LatLon> points = GeoPolylineParserUtil.parse(encodedPoints, GeoPolylineParserUtil.PRECISION_5);
 		if (isEmpty(points)) return null;
 
 		List<Location> route = convertRouteToLocationsList(points);
 		List<RouteDirectionInfo> directions = new ArrayList<>();
 		int startSearchingId = 0;
-		JSONArray legs = routeInfo.getJSONArray("legs");
+		JSONArray legs = root.getJSONArray("legs");
 		for (int i = 0; i < legs.length(); i++) {
 			JSONObject leg = legs.getJSONObject(i);
 			if (!leg.has("steps")) continue;
@@ -226,14 +223,15 @@ public class OsrmEngine extends OnlineRoutingEngine {
 		return id != null ? TurnType.valueOf(id, leftSide) : null;
 	}
 
+	@NonNull
 	@Override
-	public boolean parseServerMessage(@NonNull StringBuilder sb,
+	protected String getErrorMessageKey() {
-	                                  @NonNull String content) throws JSONException {
+		return "message";
-		JSONObject obj = new JSONObject(content);
+	}
-		if (obj.has("message")) {
+
-			String message = obj.getString("message");
+	@NonNull
-			sb.append(message);
+	@Override
-		}
+	protected String getRootArrayKey() {
-		return obj.has("routes");
+		return "routes";
 	}
 }

OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java

@@ -458,15 +458,15 @@ public class OnlineRoutingEngineFragment extends BaseOsmAndFragment {
 		new Thread(new Runnable() {
 			@Override
 			public void run() {
-				StringBuilder message = new StringBuilder();
+				StringBuilder errorMessage = new StringBuilder();
 				boolean resultOk = false;
 				try {
 					String response = helper.makeRequest(exampleCard.getEditedText());
-					resultOk = requestedEngine.parseServerMessage(message, response);
+					resultOk = requestedEngine.checkServerResponse(errorMessage, response);
 				} catch (IOException | JSONException e) {
-					message.append(e.toString());
+					errorMessage.append(e.toString());
 				}
-				showTestResults(resultOk, message.toString(), location);
+				showTestResults(resultOk, errorMessage.toString(), location);
 			}
 		}).start();
 	}

OsmAnd/src/net/osmand/plus/routing/RouteProvider.java

@@ -21,7 +21,7 @@ import net.osmand.data.LocationPoint;
 import net.osmand.data.WptLocationPoint;
 import net.osmand.plus.OsmandApplication;
 import net.osmand.plus.onlinerouting.OnlineRoutingHelper;
-import net.osmand.plus.onlinerouting.OnlineRoutingResponse;
+import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine.OnlineRoutingResponse;
 import net.osmand.plus.settings.backend.OsmandSettings;
 import net.osmand.plus.settings.backend.CommonPreference;
 import net.osmand.plus.R;