Ignore previously serialized fields for compatibility with old JSONs (#1447)

Author: vadzim-vys

CHANGELOG.md

@@ -3,6 +3,7 @@
 Mapbox welcomes participation and contributions from everyone.
 
 ### main
+- Fixed `RouteOptions#toUrl` for a case when `RouteOptions` was deserialized from a json generated by an old version of mapbox-java. [#1447](https://github.com/mapbox/mapbox-java/pull/1447)
 
 ### v6.5.0 - May 25, 2022
 - No additional changes

services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java

@@ -17,14 +17,18 @@
 import com.mapbox.api.directions.v5.utils.ParseUtils;
 import com.mapbox.auto.value.gson.SerializableJsonElement;
 import com.mapbox.geojson.Point;
+
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Defines route request parameters.
@@ -39,6 +43,20 @@ public abstract class RouteOptions extends DirectionsJsonObject {
   private static final String UTF_8 = "UTF-8";
   private static final String ACCESS_TOKEN_URL_PARAM_NAME = "access_token";
 
+  /**
+   *  Fields that aren't supposed to be serialized.
+   * - <b>access_token</b> - token must not be a part of RouteOptions,
+   *   it is provided via {@link RouteOptions#toUrl}
+   * - <b>uuid</b> - legacy field, skipped for backward compatibility
+   */
+  private static final Set<String> DEPRECATED_SERIALIZED_FIELDS = Collections.unmodifiableSet(
+    new HashSet<String>() {
+      {
+        add(ACCESS_TOKEN_URL_PARAM_NAME);
+        add("uuid");
+      }
+    });
+
   /**
    * Build a new instance of {@link RouteOptions} and sets default values for:
    * <ul>
@@ -968,6 +986,9 @@ public URL toUrl(@NonNull String accessToken) {
     if (unrecognized != null) {
       for (Map.Entry<String, SerializableJsonElement> entry : unrecognized.entrySet()) {
         JsonElement element = entry.getValue().getElement();
+        if (DEPRECATED_SERIALIZED_FIELDS.contains(entry.getKey())) {
+          continue;
+        }
         if (element.isJsonPrimitive()) {
           appendQueryParameter(sb, entry.getKey(), element.getAsString());
         } else {

services-directions-models/src/test/java/com/mapbox/api/directions/v5/models/RouteOptionsTest.java

@@ -1,11 +1,5 @@
 package com.mapbox.api.directions.v5.models;
 
-import static com.google.gson.JsonParser.parseString;
-import static com.mapbox.api.directions.v5.utils.Asserts.assertContains;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonObject;
@@ -14,6 +8,8 @@
 import com.mapbox.api.directions.v5.utils.MutateJsonUtil;
 import com.mapbox.core.TestUtils;
 import com.mapbox.geojson.Point;
+import org.junit.Test;
+
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -24,7 +20,16 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Set;
-import org.junit.Test;
+
+import static com.google.gson.JsonParser.parseString;
+import static com.mapbox.api.directions.v5.utils.Asserts.assertContains;
+import static com.mapbox.api.directions.v5.utils.Asserts.assertContainsExactCount;
+import static com.mapbox.api.directions.v5.utils.Asserts.assertNoDuplicatedParameters;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 public class RouteOptionsTest extends TestUtils {
   /**
@@ -713,6 +718,45 @@ public void routeOptionsWithUnrecognizedObjectsAndArrays() throws IOException {
     RouteOptions.fromJson(mutatedRouteOptionsJson.toString()).toUrl("testToken");
   }
 
+  @Test
+  public void routeOptionsDoNotAddAccessTokenFromJsonToUrl() {
+    Gson gson = new GsonBuilder().create();
+    JsonObject routeOptionsJson = gson.fromJson(optionsJson, JsonObject.class);
+    routeOptionsJson.addProperty("access_token", "wrong");
+    String routeOptionsJsonString = routeOptionsJson.toString();
+
+    URL url = RouteOptions.fromJson(routeOptionsJsonString).toUrl("right");
+
+    String query = url.getQuery();
+    assertContainsExactCount(query, "access_token", 1);
+    assertContains(query, "access_token=right");
+  }
+
+  @Test
+  public void routeOptionsDoNotAddUUIDFromJsonToUrl() {
+    Gson gson = new GsonBuilder().create();
+    JsonObject routeOptionsJson = gson.fromJson(optionsJson, JsonObject.class);
+    routeOptionsJson.addProperty("uuid", "test");
+    String routeOptionsJsonString = routeOptionsJson.toString();
+
+    URL url = RouteOptions.fromJson(routeOptionsJsonString).toUrl("test");
+
+    assertFalse(
+      "url shouldn't contain uuid: " + url,
+      url.getQuery().contains("uuid")
+    );
+  }
+
+  @Test
+  public void sanityCheckForPreviouslySerializedRouteResponse() throws IOException {
+    String serializedDirectionResponse = loadJsonFixture("directions_v5_serialized_with_legacy_fields.json");
+    DirectionsResponse response = DirectionsResponse.fromJson(serializedDirectionResponse);
+
+    URL url = response.routes().get(0).routeOptions().toUrl("token");
+
+    assertNoDuplicatedParameters(url);
+  }
+
   @Test
   public void emptyExcludeObjectsCleansUpExcludes() {
     RouteOptions routeOptions = routeOptions().toBuilder()

services-directions-models/src/test/java/com/mapbox/api/directions/v5/utils/Asserts.java

@@ -1,7 +1,11 @@
 package com.mapbox.api.directions.v5.utils;
 
+import java.net.URL;
+import java.util.HashSet;
 import java.util.Set;
 
+import static com.mapbox.api.directions.v5.utils.StringUtil.countSubstrings;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -26,4 +30,34 @@ public static <T> void assertContains(Set<T> set, T object) {
       set.contains(object)
     );
   }
+
+  public static void assertContainsExactCount(String string, String substring, int count) {
+    int substringsFound = countSubstrings(string, substring);
+    assertEquals(
+      "String \"" + string + "\" contains " +
+        substringsFound + " \"" + substring + "\"",
+      substringsFound,
+      count
+    );
+  }
+
+  /***
+   * Verifies that URL doesn't contain duplicated query parameters.
+   * Examples:
+   * https://api.mapbox.com/test?a=1&a=2 - invalid
+   * https://api.mapbox.com/test?a=1&b=2 - valid
+   * @param url
+   */
+  public static void assertNoDuplicatedParameters(URL url) {
+    String query = url.getQuery();
+    Set<String> queryParameters = new HashSet<>();
+    for (String param : query.split("&")) {
+      String parameterName = param.split("=")[0];
+      assertFalse(
+        "parameter \"" + parameterName + "\" is duplicated in query response",
+        queryParameters.contains(parameterName)
+      );
+      queryParameters.add(parameterName);
+    }
+  }
 }

services-directions-models/src/test/java/com/mapbox/api/directions/v5/utils/CountSubstringsTest.java

@@ -0,0 +1,44 @@
+package com.mapbox.api.directions.v5.utils;
+
+import org.junit.Test;
+
+import static com.mapbox.api.directions.v5.utils.StringUtil.countSubstrings;
+import static org.junit.Assert.assertEquals;
+
+public class CountSubstringsTest {
+
+  @Test
+  public void zeroSubstrings() {
+    int result = countSubstrings("test", "zero");
+    assertEquals(0, result);
+  }
+
+  @Test
+  public void oneSubstring() {
+    int result = countSubstrings("test-one-substring", "one");
+    assertEquals(1, result);
+  }
+
+  @Test
+  public void twoSubstring() {
+    int result = countSubstrings("twotest-two-substring", "two");
+    assertEquals(2, result);
+  }
+
+  @Test
+  public void substringsAtTheEnd() {
+    int result = countSubstrings("testheyheyhey", "hey");
+    assertEquals(3, result);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void emptySubstring() {
+    countSubstrings("test", "");
+  }
+
+
+  @Test(expected = IllegalArgumentException.class)
+  public void emptyString() {
+    countSubstrings("", "test");
+  }
+}

services-directions-models/src/test/java/com/mapbox/api/directions/v5/utils/StringUtil.java

@@ -0,0 +1,26 @@
+package com.mapbox.api.directions.v5.utils;
+
+public final class StringUtil {
+  /***
+   * Counts how many times the substring is present in the string.
+   * See examples in {@link CountSubstringsTest}
+   * @param string
+   * @param substring
+   * @return
+   */
+  public static int countSubstrings(String string, String substring) {
+    if (substring == null || substring.isEmpty()) {
+      throw new IllegalArgumentException("substring can't be null or empty");
+    }
+    if (string == null || string.isEmpty()) {
+      throw new IllegalArgumentException("string can't be null or empty");
+    }
+    int nextIndex = -1;
+    int substringsFound = -1;
+    do {
+      substringsFound++;
+      nextIndex = string.indexOf(substring, nextIndex + 1);
+    } while (nextIndex != -1);
+    return substringsFound;
+  }
+}