QueryParam Backcompat

common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java

@@ -23,13 +23,17 @@
 package com.microsoft.identity.common.internal.util;
 
 import android.text.TextUtils;
+import android.util.Pair;
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonSyntaxException;
 import com.google.gson.TypeAdapter;
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonWriter;
+import com.microsoft.identity.common.java.exception.ClientException;
+import com.microsoft.identity.common.logging.Logger;
 
 import java.io.IOException;
 import java.lang.reflect.Type;
@@ -40,9 +44,14 @@
 
 /**
  * Class to serialize and deserialize query parameters from List<Pair<String, String>> to json String
- * and vice versa
+ * and vice versa.
+ *
+ * NOTE: Even we no longer use Pair (Since it's android-only), we are keeping this the same
+ *       to maintain backcompat with serialized value from older common that still uses it.
  */
-public class QueryParamsAdapter extends TypeAdapter<List<Map.Entry<String, String>>> {
+public class QueryParamsAdapter extends TypeAdapter<List<Pair<String, String>>> {
+
+    private static final String TAG = QueryParamsAdapter.class.getSimpleName();
 
     private static final Gson mGson;
 
@@ -56,47 +65,69 @@ public class QueryParamsAdapter extends TypeAdapter<List<Map.Entry<String, Strin
     }
 
     @Override
-    public void write(final JsonWriter out, final List<Map.Entry<String, String>> queryParams) throws IOException {
+    public void write(final JsonWriter out, final List<Pair<String, String>> queryParams) throws IOException {
         out.beginObject();
 
-        for (final Map.Entry<String, String> keyValuePair : queryParams) {
-            out.name(keyValuePair.getKey());
-            out.value(keyValuePair.getValue());
+        for (final Pair<String, String> keyValuePair : queryParams) {
+            out.name(keyValuePair.first);
+            out.value(keyValuePair.second);
         }
+
         out.endObject();
     }
 
     @Override
-    public List<Map.Entry<String, String>> read(final JsonReader in) throws IOException {
+    public List<Pair<String, String>> read(final JsonReader in) throws IOException {
         in.beginObject();
-        final List<Map.Entry<String, String>> result = new ArrayList<>();
+        final List<Pair<String, String>> result = new ArrayList<>();
         while (in.hasNext()) {
             final String key = in.nextName();
             final String value = in.nextString();
-            final Map.Entry<String, String> keyValuePair = new AbstractMap.SimpleEntry<>(key, value);
+            final Pair<String, String> keyValuePair = new Pair<>(key, value);
             result.add(keyValuePair);
         }
         in.endObject();
         return result;
     }
 
     public static String _toJson(final List<Map.Entry<String, String>> extraQueryStringParameters) {
-        return mGson.toJson(extraQueryStringParameters, getListType());
+        final List<Pair<String, String>> extraQpPairs = new ArrayList<>();
+        for (final Map.Entry<String, String> entry: extraQueryStringParameters) {
+            extraQpPairs.add(new Pair<String, String>(entry.getKey(), entry.getValue()));
+        }
+        return mGson.toJson(extraQpPairs, getPairListType());
     }
 
-    public static List<Map.Entry<String, String>> _fromJson(final String jsonString) {
+    public static List<Map.Entry<String, String>> _fromJson(final String jsonString)
+            throws ClientException{
+        final String methodName = ":_fromJson";
+
         if (TextUtils.isEmpty(jsonString)) {
             return new ArrayList<>();
         }
-        return mGson.fromJson(jsonString, getListType());
+
+        try {
+            final List<Pair<String, String>> extraQpPairs = mGson.fromJson(jsonString, getPairListType());
+            final List<Map.Entry<String, String>> extraQpMapEntries = new ArrayList<>();
+            for (final Pair<String, String> entry: extraQpPairs) {
+                if (!StringUtil.isEmpty(entry.first)) {
+                    extraQpMapEntries.add(new AbstractMap.SimpleEntry<String, String>(entry.first, entry.second));
+                }
+            }
+            return extraQpMapEntries;
+        } catch (final JsonSyntaxException e) {
+            final String errorMessage = "malformed json string:" + jsonString;
+            Logger.error(TAG + methodName, errorMessage, e);
+            throw new ClientException(ClientException.JSON_PARSE_FAILURE, errorMessage, e);
+        }
     }
 
     /**
      * Create a Type for the List of query params
      *
-     * @return a Type object representing the type of the query params in this case List<Map.Entry<String, String>>
+     * @return a Type object representing the type of the query params in this case List<Pair<String, String>>
      */
-    private static Type getListType() {
-        return TypeToken.getParameterized(List.class, TypeToken.getParameterized(Map.Entry.class, String.class, String.class).getRawType()).getType();
+    private static Type getPairListType() {
+        return TypeToken.getParameterized(List.class, TypeToken.getParameterized(Pair.class, String.class, String.class).getRawType()).getType();
     }
 }

common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java

@@ -0,0 +1,103 @@
+// Copyright (c) Microsoft Corporation.
+// All rights reserved.
+//
+// This code is licensed under the MIT License.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files(the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions :
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package com.microsoft.identity.common.internal.util;
+
+import com.microsoft.identity.common.java.exception.ClientException;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+@RunWith(RobolectricTestRunner.class)
+public class QueryParamsAdapterTest {
+
+    @Test
+    public void testConvertFromNullJsonString() throws Exception {
+        final List<Map.Entry<String, String>> result = QueryParamsAdapter._fromJson(null);
+        Assert.assertEquals(0, result.size());
+    }
+
+    @Test
+    public void testConvertFromEmptyJsonString() throws Exception {
+        final List<Map.Entry<String, String>> result = QueryParamsAdapter._fromJson("");
+        Assert.assertEquals(0, result.size());
+    }
+
+    @Test
+    public void testConvertToJson() throws Exception {
+        final List<Map.Entry<String, String>> input = new ArrayList<>();
+        input.add(new AbstractMap.SimpleEntry<String, String>("eqp1", "1"));
+        input.add(new AbstractMap.SimpleEntry<String, String>("eqp2", "2"));
+
+        final String expected = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]";
+
+        Assert.assertEquals(expected, QueryParamsAdapter._toJson(input));
+    }
+
+    @Test
+    public void testConvertFromJson() throws Exception {
+        final String input = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]";
+
+        final List<Map.Entry<String, String>> expected = new ArrayList<>();
+        expected.add(new AbstractMap.SimpleEntry<String, String>("eqp1", "1"));
+        expected.add(new AbstractMap.SimpleEntry<String, String>("eqp2", "2"));
+
+        Assert.assertEquals(expected, QueryParamsAdapter._fromJson(input));
+    }
+
+    @Test
+    public void testConvertFromIncorrectJson() throws Exception {
+        // This is what we get if we serialized List<Map.Entry> directly.
+        final String input = "[{\"key\":\"eqp1\",\"value\":\"1\"},{\"key\":\"eqp2\",\"value\":\"2\"}]";
+        Assert.assertEquals(0, QueryParamsAdapter._fromJson(input).size());
+    }
+
+    @Test
+    public void testConvertFromMalformedJson(){
+        final String input = "[{\"eqp1\", \"1\"}, {\"eqp2\", \"2\"}]";
+        try {
+            QueryParamsAdapter._fromJson(input);
+            Assert.fail();
+        } catch (final ClientException e){
+            Assert.assertEquals(ClientException.JSON_PARSE_FAILURE, e.getErrorCode());
+        }
+    }
+
+    @Test
+    public void testConvertFromTruncatedJson(){
+        final String input = "[{\"key1\"";
+        try {
+            QueryParamsAdapter._fromJson(input);
+            Assert.fail();
+        } catch (final ClientException e){
+            Assert.assertEquals(ClientException.JSON_PARSE_FAILURE, e.getErrorCode());
+        }
+    }
+}