From 75363c6cbebf893f6c0d9c383c1b54a86d4538ad Mon Sep 17 00:00:00 2001 From: nazar-kutz Date: Tue, 19 Jan 2021 12:59:31 +0200 Subject: [PATCH] Fix Import / Export bugs with Online Routing Code improvements --- .../onlinerouting/OnlineRoutingHelper.java | 68 +++----------- .../onlinerouting/OnlineRoutingUtils.java | 91 +++++++++++++++++++ .../engine/GraphhopperEngine.java | 4 +- .../engine/OnlineRoutingEngine.java | 17 +--- .../ui/OnlineRoutingEngineFragment.java | 23 ++--- .../backup/OnlineRoutingSettingsItem.java | 43 +++++---- 6 files changed, 143 insertions(+), 103 deletions(-) create mode 100644 OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingUtils.java diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java index b28d98d55f..9b3d042699 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java +++ b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingHelper.java @@ -3,31 +3,24 @@ package net.osmand.plus.onlinerouting; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.google.gson.Gson; -import com.google.gson.reflect.TypeToken; - import net.osmand.PlatformUtil; import net.osmand.data.LatLon; import net.osmand.osm.io.NetworkUtils; import net.osmand.plus.OsmandApplication; import net.osmand.plus.Version; -import net.osmand.plus.onlinerouting.engine.EngineType; import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine; import net.osmand.plus.settings.backend.OsmandSettings; import net.osmand.util.Algorithms; import org.apache.commons.logging.Log; -import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; -import java.lang.reflect.Type; import java.net.HttpURLConnection; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -36,10 +29,6 @@ public class OnlineRoutingHelper { private static final Log LOG = PlatformUtil.getLog(OnlineRoutingHelper.class); - private static final String ITEMS = "items"; - private static final String TYPE = "type"; - private static final String PARAMS = "params"; - private OsmandApplication app; private OsmandSettings settings; private Map cachedEngines; @@ -56,7 +45,7 @@ public class OnlineRoutingHelper { } @NonNull - public List getEnginesExceptMentioned(@Nullable String ... excludeKeys) { + public List getEnginesExceptMentionedKeys(@Nullable String... excludeKeys) { List engines = getEngines(); if (excludeKeys != null) { for (String key : excludeKeys) { @@ -72,6 +61,16 @@ public class OnlineRoutingHelper { return cachedEngines.get(stringKey); } + @Nullable + public OnlineRoutingEngine getEngineByName(@Nullable String name) { + for (OnlineRoutingEngine engine : getEngines()) { + if (Algorithms.objectEquals(engine.getName(app), name)) { + return engine; + } + } + return null; + } + @NonNull public List calculateRouteOnline(@NonNull OnlineRoutingEngine engine, @NonNull List path) throws IOException, JSONException { @@ -155,7 +154,7 @@ public class OnlineRoutingHelper { if (!Algorithms.isEmpty(jsonString)) { try { JSONObject json = new JSONObject(jsonString); - readFromJson(json, engines); + OnlineRoutingUtils.readFromJson(json, engines); } catch (JSONException | IllegalArgumentException e) { LOG.debug("Error when reading engines from JSON ", e); } @@ -167,7 +166,7 @@ public class OnlineRoutingHelper { if (!Algorithms.isEmpty(cachedEngines)) { try { JSONObject json = new JSONObject(); - writeToJson(json, getEngines()); + OnlineRoutingUtils.writeToJson(json, getEngines()); settings.ONLINE_ROUTING_ENGINES.set(json.toString()); } catch (JSONException e) { LOG.debug("Error when writing engines to JSON ", e); @@ -176,45 +175,4 @@ public class OnlineRoutingHelper { settings.ONLINE_ROUTING_ENGINES.set(null); } } - - public static void readFromJson(@NonNull JSONObject json, - @NonNull List engines) throws JSONException { - if (!json.has("items")) { - return; - } - Gson gson = new Gson(); - Type typeToken = new TypeToken>() { - }.getType(); - JSONArray itemsJson = json.getJSONArray(ITEMS); - for (int i = 0; i < itemsJson.length(); i++) { - JSONObject object = itemsJson.getJSONObject(i); - if (object.has(TYPE) && object.has(PARAMS)) { - EngineType type = EngineType.getTypeByName(object.getString(TYPE)); - String paramsString = object.getString(PARAMS); - HashMap params = gson.fromJson(paramsString, typeToken); - OnlineRoutingEngine engine = OnlineRoutingFactory.createEngine(type, params); - if (!Algorithms.isEmpty(engine.getStringKey())) { - engines.add(engine); - } - } - } - } - - public static void writeToJson(@NonNull JSONObject json, - @NonNull List engines) throws JSONException { - JSONArray jsonArray = new JSONArray(); - Gson gson = new Gson(); - Type type = new TypeToken>() { - }.getType(); - for (OnlineRoutingEngine engine : engines) { - if (Algorithms.isEmpty(engine.getStringKey())) { - continue; - } - JSONObject jsonObject = new JSONObject(); - jsonObject.put(TYPE, engine.getType().name()); - jsonObject.put(PARAMS, gson.toJson(engine.getParams(), type)); - jsonArray.put(jsonObject); - } - json.put(ITEMS, jsonArray); - } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingUtils.java b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingUtils.java new file mode 100644 index 0000000000..90eb4ea83c --- /dev/null +++ b/OsmAnd/src/net/osmand/plus/onlinerouting/OnlineRoutingUtils.java @@ -0,0 +1,91 @@ +package net.osmand.plus.onlinerouting; + +import android.content.Context; + +import androidx.annotation.NonNull; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; + +import net.osmand.plus.onlinerouting.engine.EngineType; +import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine; +import net.osmand.util.Algorithms; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + +import java.lang.reflect.Type; +import java.util.HashMap; +import java.util.List; + +public class OnlineRoutingUtils { + + private static final String ITEMS = "items"; + private static final String TYPE = "type"; + private static final String PARAMS = "params"; + + public static void generateUniqueName(@NonNull Context ctx, + @NonNull OnlineRoutingEngine engine, + @NonNull List otherEngines) { + engine.remove(EngineParameter.NAME_INDEX); + if (hasNameDuplicate(ctx, engine.getName(ctx), otherEngines)) { + int index = 0; + do { + engine.put(EngineParameter.NAME_INDEX, String.valueOf(++index)); + } while (hasNameDuplicate(ctx, engine.getName(ctx), otherEngines)); + } + } + + public static boolean hasNameDuplicate(@NonNull Context ctx, + @NonNull String engineName, + @NonNull List otherEngines) { + for (OnlineRoutingEngine engine : otherEngines) { + if (Algorithms.objectEquals(engine.getName(ctx), engineName)) { + return true; + } + } + return false; + } + + public static void readFromJson(@NonNull JSONObject json, + @NonNull List engines) throws JSONException { + if (!json.has("items")) { + return; + } + Gson gson = new Gson(); + Type typeToken = new TypeToken>() { + }.getType(); + JSONArray itemsJson = json.getJSONArray(ITEMS); + for (int i = 0; i < itemsJson.length(); i++) { + JSONObject object = itemsJson.getJSONObject(i); + if (object.has(TYPE) && object.has(PARAMS)) { + EngineType type = EngineType.getTypeByName(object.getString(TYPE)); + String paramsString = object.getString(PARAMS); + HashMap params = gson.fromJson(paramsString, typeToken); + OnlineRoutingEngine engine = OnlineRoutingFactory.createEngine(type, params); + if (!Algorithms.isEmpty(engine.getStringKey())) { + engines.add(engine); + } + } + } + } + + public static void writeToJson(@NonNull JSONObject json, + @NonNull List engines) throws JSONException { + JSONArray jsonArray = new JSONArray(); + Gson gson = new Gson(); + Type type = new TypeToken>() { + }.getType(); + for (OnlineRoutingEngine engine : engines) { + if (Algorithms.isEmpty(engine.getStringKey())) { + continue; + } + JSONObject jsonObject = new JSONObject(); + jsonObject.put(TYPE, engine.getType().name()); + jsonObject.put(PARAMS, gson.toJson(engine.getParams(), type)); + jsonArray.put(jsonObject); + } + json.put(ITEMS, jsonArray); + } +} diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java index d676e1e154..df9381352b 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java +++ b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/GraphhopperEngine.java @@ -64,11 +64,11 @@ public class GraphhopperEngine extends OnlineRoutingEngine { .append('&'); } String vehicle = get(EngineParameter.VEHICLE_KEY); - if (isEmpty(vehicle)) { + if (!isEmpty(vehicle)) { sb.append("vehicle=").append(vehicle); } String apiKey = get(EngineParameter.API_KEY); - if (isEmpty(apiKey)) { + if (!isEmpty(apiKey)) { sb.append('&').append("key=").append(apiKey); } } diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java index e3f2d416ce..511f31cc67 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java +++ b/OsmAnd/src/net/osmand/plus/onlinerouting/engine/OnlineRoutingEngine.java @@ -52,23 +52,16 @@ public abstract class OnlineRoutingEngine implements Cloneable { @NonNull public String getName(@NonNull Context ctx) { - String customName = get(EngineParameter.CUSTOM_NAME); - if (customName != null) { - return customName; - } else { - return getStandardName(ctx); + String name = get(EngineParameter.CUSTOM_NAME); + if (name == null) { + name = getStandardName(ctx); } - } - - @NonNull - private String getStandardName(@NonNull Context ctx) { - String base = getBaseName(ctx); String index = get(EngineParameter.NAME_INDEX); - return !isEmpty(index) ? base + " " + index : base; + return !isEmpty(index) ? name + " " + index : name; } @NonNull - public String getBaseName(@NonNull Context ctx) { + public String getStandardName(@NonNull Context ctx) { String vehicleTitle = getSelectedVehicleName(ctx); if (isEmpty(vehicleTitle)) { return getType().getTitle(); diff --git a/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java b/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java index 4c0bca61a6..adfbfb1bc0 100644 --- a/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java +++ b/OsmAnd/src/net/osmand/plus/onlinerouting/ui/OnlineRoutingEngineFragment.java @@ -40,6 +40,7 @@ import net.osmand.plus.mapcontextmenu.other.HorizontalSelectionAdapter.Horizonta import net.osmand.plus.onlinerouting.EngineParameter; import net.osmand.plus.onlinerouting.OnlineRoutingFactory; import net.osmand.plus.onlinerouting.OnlineRoutingHelper; +import net.osmand.plus.onlinerouting.OnlineRoutingUtils; import net.osmand.plus.onlinerouting.VehicleType; import net.osmand.plus.onlinerouting.ui.OnlineRoutingCard.OnTextChangedListener; import net.osmand.plus.onlinerouting.engine.EngineType; @@ -243,6 +244,7 @@ public class OnlineRoutingEngineFragment extends BaseOsmAndFragment { public void onTextChanged(boolean changedByUser, @NonNull String text) { if (changedByUser) { engine.put(EngineParameter.CUSTOM_NAME, text); + engine.remove(EngineParameter.NAME_INDEX); checkCustomNameUnique(engine); } } @@ -448,18 +450,13 @@ public class OnlineRoutingEngineFragment extends BaseOsmAndFragment { private void generateUniqueNameIfNeeded() { if (engine.get(EngineParameter.CUSTOM_NAME) == null) { - engine.remove(EngineParameter.NAME_INDEX); - if (hasNameDuplicate(engine.getName(app))) { - int index = 0; - do { - engine.put(EngineParameter.NAME_INDEX, String.valueOf(++index)); - } while (hasNameDuplicate(engine.getName(app))); - } + List cachedEngines = helper.getEnginesExceptMentionedKeys(editedEngineKey); + OnlineRoutingUtils.generateUniqueName(app, engine, cachedEngines); } } private void checkCustomNameUnique(@NonNull OnlineRoutingEngine engine) { - if (hasNameDuplicate(engine.getName(app))) { + if (hasNameDuplicate(engine)) { nameCard.showFieldBoxError(getString(R.string.message_name_is_already_exists)); saveButton.setEnabled(false); } else { @@ -468,13 +465,9 @@ public class OnlineRoutingEngineFragment extends BaseOsmAndFragment { } } - private boolean hasNameDuplicate(@NonNull String name) { - for (OnlineRoutingEngine engine : helper.getEnginesExceptMentioned(editedEngineKey)) { - if (Algorithms.objectEquals(engine.getName(app), name)) { - return true; - } - } - return false; + private boolean hasNameDuplicate(@NonNull OnlineRoutingEngine engine) { + List cachedEngines = helper.getEnginesExceptMentionedKeys(editedEngineKey); + return OnlineRoutingUtils.hasNameDuplicate(app, engine.getName(app), cachedEngines); } private void onSaveEngine() { diff --git a/OsmAnd/src/net/osmand/plus/settings/backend/backup/OnlineRoutingSettingsItem.java b/OsmAnd/src/net/osmand/plus/settings/backend/backup/OnlineRoutingSettingsItem.java index cf08142393..6da9eb590e 100644 --- a/OsmAnd/src/net/osmand/plus/settings/backend/backup/OnlineRoutingSettingsItem.java +++ b/OsmAnd/src/net/osmand/plus/settings/backend/backup/OnlineRoutingSettingsItem.java @@ -8,9 +8,10 @@ import androidx.annotation.Nullable; 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.OnlineRoutingUtils; import net.osmand.plus.onlinerouting.engine.OnlineRoutingEngine; import net.osmand.plus.onlinerouting.OnlineRoutingHelper; +import net.osmand.util.Algorithms; import org.json.JSONException; import org.json.JSONObject; @@ -20,7 +21,8 @@ import java.util.List; public class OnlineRoutingSettingsItem extends CollectionSettingsItem { - private OnlineRoutingHelper onlineRoutingHelper; + private OnlineRoutingHelper helper; + private List otherEngines; public OnlineRoutingSettingsItem(@NonNull OsmandApplication app, @NonNull List items) { super(app, null, items); @@ -37,8 +39,9 @@ public class OnlineRoutingSettingsItem extends CollectionSettingsItem(onlineRoutingHelper.getEngines()); + helper = app.getOnlineRoutingHelper(); + existingItems = new ArrayList<>(helper.getEngines()); + otherEngines = new ArrayList<>(existingItems); } @NonNull @@ -67,12 +70,18 @@ public class OnlineRoutingSettingsItem extends CollectionSettingsItem