From cfa202c3d4d459e0cf67b6f4310f1ea76e64935a Mon Sep 17 00:00:00 2001 From: nazar-kutz Date: Mon, 1 Feb 2021 12:44:49 +0200 Subject: [PATCH] refactor server wrong response checking move OnlineRoutingResponse class to OnlineRoutingEngine --- .../onlinerouting/OnlineRoutingHelper.java | 1 + .../onlinerouting/OnlineRoutingResponse.java | 24 ------ .../engine/GraphhopperEngine.java | 23 +++--- .../engine/OnlineRoutingEngine.java | 76 ++++++++++++++++--- .../plus/onlinerouting/engine/OrsEngine.java | 24 +++--- .../plus/onlinerouting/engine/OsrmEngine.java | 26 +++---- .../ui/OnlineRoutingEngineFragment.java | 8 +- .../osmand/plus/routing/RouteProvider.java | 2 +- 8 files changed, 103 insertions(+), 81 deletions(-) delete mode 100644 OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingResponse.java diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java index ab56cade13..de36b87449 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java +++ b/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; diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingResponse.java b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingResponse.java deleted file mode 100644 index 02a6a8f53c..0000000000 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingResponse.java +++ /dev/null @@ -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 route; - private List directions; - - public OnlineRoutingResponse(List route, List directions) { - this.route = route; - this.directions = directions; - } - - public List getRoute() { - return route; - } - - public List getDirections() { - return directions; - } -} diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java index 20bbfaa70e..930ebcab8d 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java +++ b/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 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, - @NonNull String content) throws JSONException { - JSONObject obj = new JSONObject(content); - if (obj.has("message")) { - String message = obj.getString("message"); - sb.append(message); - } - return obj.has("paths"); + protected String getErrorMessageKey() { + return "message"; + } + + @NonNull + @Override + protected String getRootArrayKey() { + return "paths"; } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java index 24e5db7d5d..0752282c70 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java +++ b/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, - @NonNull OsmandApplication app, - boolean leftSideNavigation) throws JSONException; + protected abstract OnlineRoutingResponse parseServerResponse(@NonNull JSONObject root, + @NonNull OsmandApplication app, + 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 convertRouteToLocationsList(@NonNull List 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, - @NonNull String content) throws JSONException; + public boolean checkServerResponse(@NonNull StringBuilder errorMessage, + @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 route; + private List directions; + + public OnlineRoutingResponse(List route, List directions) { + this.route = route; + this.directions = directions; + } + + public List getRoute() { + return route; + } + + public List getDirections() { + return directions; + } + } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OrsEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OrsEngine.java index 5f8c2a5108..ed9168ccf9 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OrsEngine.java +++ b/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 = obj.getJSONArray("features").getJSONObject(0) - .getJSONObject("geometry").getJSONArray("coordinates"); + JSONArray array = root.getJSONObject("geometry").getJSONArray("coordinates"); List 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, - @NonNull String content) throws JSONException { - JSONObject obj = new JSONObject(content); - if (obj.has("error")) { - String message = obj.getString("error"); - sb.append(message); - } - return obj.has("features"); + protected String getErrorMessageKey() { + return "error"; + } + + @NonNull + @Override + protected String getRootArrayKey() { + return "features"; } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OsrmEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OsrmEngine.java index 2e892d2aa6..4d75b2b440 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OsrmEngine.java +++ b/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); - JSONObject routeInfo = obj.getJSONArray("routes").getJSONObject(0); - String encodedPoints = routeInfo.getString("geometry"); + String encodedPoints = root.getString("geometry"); List points = GeoPolylineParserUtil.parse(encodedPoints, GeoPolylineParserUtil.PRECISION_5); if (isEmpty(points)) return null; List route = convertRouteToLocationsList(points); List 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, - @NonNull String content) throws JSONException { - JSONObject obj = new JSONObject(content); - if (obj.has("message")) { - String message = obj.getString("message"); - sb.append(message); - } - return obj.has("routes"); + protected String getErrorMessageKey() { + return "message"; + } + + @NonNull + @Override + protected String getRootArrayKey() { + return "routes"; } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java b/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java index 84856139b2..008bb75852 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java +++ b/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(); } diff --git a/OsmAnd/src/net/osmand/plus/routing/RouteProvider.java b/OsmAnd/src/net/osmand/plus/routing/RouteProvider.java index 0d2564eb3e..b3b2a11ab6 100644 --- a/OsmAnd/src/net/osmand/plus/routing/RouteProvider.java +++ b/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;