From 24881eb5e83515638dc3203c4296c8fe0c67684b Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 08:52:06 -0700 Subject: [PATCH 1/6] QueryParam Backcompat --- .../internal/util/QueryParamsAdapter.java | 61 ++++++++--- .../internal/util/QueryParamsAdapterTest.java | 103 ++++++++++++++++++ 2 files changed, 149 insertions(+), 15 deletions(-) create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java diff --git a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java index 121e176f35..4ad0392339 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java +++ b/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> 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>> { +public class QueryParamsAdapter extends TypeAdapter>> { + + private static final String TAG = QueryParamsAdapter.class.getSimpleName(); private static final Gson mGson; @@ -56,24 +65,25 @@ public class QueryParamsAdapter extends TypeAdapter> queryParams) throws IOException { + public void write(final JsonWriter out, final List> queryParams) throws IOException { out.beginObject(); - for (final Map.Entry keyValuePair : queryParams) { - out.name(keyValuePair.getKey()); - out.value(keyValuePair.getValue()); + for (final Pair keyValuePair : queryParams) { + out.name(keyValuePair.first); + out.value(keyValuePair.second); } + out.endObject(); } @Override - public List> read(final JsonReader in) throws IOException { + public List> read(final JsonReader in) throws IOException { in.beginObject(); - final List> result = new ArrayList<>(); + final List> result = new ArrayList<>(); while (in.hasNext()) { final String key = in.nextName(); final String value = in.nextString(); - final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(key, value); + final Pair keyValuePair = new Pair<>(key, value); result.add(keyValuePair); } in.endObject(); @@ -81,22 +91,43 @@ public List> read(final JsonReader in) throws IOExcept } public static String _toJson(final List> extraQueryStringParameters) { - return mGson.toJson(extraQueryStringParameters, getListType()); + final List> extraQpPairs = new ArrayList<>(); + for (final Map.Entry entry: extraQueryStringParameters) { + extraQpPairs.add(new Pair(entry.getKey(), entry.getValue())); + } + return mGson.toJson(extraQpPairs, getPairListType()); } - public static List> _fromJson(final String jsonString) { + public static List> _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> extraQpPairs = mGson.fromJson(jsonString, getPairListType()); + final List> extraQpMapEntries = new ArrayList<>(); + for (final Pair entry: extraQpPairs) { + if (!StringUtil.isEmpty(entry.first)) { + extraQpMapEntries.add(new AbstractMap.SimpleEntry(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> + * @return a Type object representing the type of the query params in this case List> */ - 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(); } } diff --git a/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java b/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java new file mode 100644 index 0000000000..24cfb86d36 --- /dev/null +++ b/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> result = QueryParamsAdapter._fromJson(null); + Assert.assertEquals(0, result.size()); + } + + @Test + public void testConvertFromEmptyJsonString() throws Exception { + final List> result = QueryParamsAdapter._fromJson(""); + Assert.assertEquals(0, result.size()); + } + + @Test + public void testConvertToJson() throws Exception { + final List> input = new ArrayList<>(); + input.add(new AbstractMap.SimpleEntry("eqp1", "1")); + input.add(new AbstractMap.SimpleEntry("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> expected = new ArrayList<>(); + expected.add(new AbstractMap.SimpleEntry("eqp1", "1")); + expected.add(new AbstractMap.SimpleEntry("eqp2", "2")); + + Assert.assertEquals(expected, QueryParamsAdapter._fromJson(input)); + } + + @Test + public void testConvertFromIncorrectJson() throws Exception { + // This is what we get if we serialized List 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()); + } + } +} \ No newline at end of file From 92ae93b14541ab37e6785c75f61ae239b9007a21 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 10:29:23 -0700 Subject: [PATCH 2/6] kill the adapter --- .../internal/util/QueryParamsAdapter.java | 37 ++----------------- 1 file changed, 3 insertions(+), 34 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java index 4ad0392339..2fbcdf7255 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java @@ -43,51 +43,20 @@ import java.util.Map; /** - * Class to serialize and deserialize query parameters from List> to json String + * Class to serialize and deserialize query parameters from List> to json String * 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>> { +public class QueryParamsAdapter { private static final String TAG = QueryParamsAdapter.class.getSimpleName(); private static final Gson mGson; static { - final GsonBuilder gsonBuilder = new GsonBuilder(); - gsonBuilder.registerTypeAdapter( - QueryParamsAdapter.class, - new QueryParamsAdapter() - ); - mGson = gsonBuilder.create(); - } - - @Override - public void write(final JsonWriter out, final List> queryParams) throws IOException { - out.beginObject(); - - for (final Pair keyValuePair : queryParams) { - out.name(keyValuePair.first); - out.value(keyValuePair.second); - } - - out.endObject(); - } - - @Override - public List> read(final JsonReader in) throws IOException { - in.beginObject(); - final List> result = new ArrayList<>(); - while (in.hasNext()) { - final String key = in.nextName(); - final String value = in.nextString(); - final Pair keyValuePair = new Pair<>(key, value); - result.add(keyValuePair); - } - in.endObject(); - return result; + mGson = new GsonBuilder().create(); } public static String _toJson(final List> extraQueryStringParameters) { From e248ba85671032e2bd7f9a26ae093bf2cfd7c84f Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 10:51:04 -0700 Subject: [PATCH 3/6] newline --- .../identity/common/internal/util/QueryParamsAdapterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java b/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java index 24cfb86d36..d6c529490d 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java +++ b/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java @@ -100,4 +100,4 @@ public void testConvertFromTruncatedJson(){ Assert.assertEquals(ClientException.JSON_PARSE_FAILURE, e.getErrorCode()); } } -} \ No newline at end of file +} From 403307990cc163a10eb13cfdf1a1927d4c840fe0 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 13:15:40 -0700 Subject: [PATCH 4/6] Rely on serializer completely --- .../request/MsalBrokerRequestAdapter.java | 2 +- .../internal/util/QueryParamsAdapter.java | 102 --------- .../common/java/util/QueryParamsAdapter.java | 203 ++++++++++++++++++ .../java}/util/QueryParamsAdapterTest.java | 65 ++++-- 4 files changed, 255 insertions(+), 117 deletions(-) delete mode 100644 common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java create mode 100644 common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java rename {common/src/test/java/com/microsoft/identity/common/internal => common4j/src/test/com/microsoft/identity/common/java}/util/QueryParamsAdapterTest.java (67%) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/request/MsalBrokerRequestAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/request/MsalBrokerRequestAdapter.java index 905d3fa7fb..6503ab6770 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/request/MsalBrokerRequestAdapter.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/request/MsalBrokerRequestAdapter.java @@ -52,7 +52,7 @@ import com.microsoft.identity.common.java.commands.parameters.RemoveAccountCommandParameters; import com.microsoft.identity.common.java.ui.BrowserDescriptor; import com.microsoft.identity.common.internal.util.BrokerProtocolVersionUtil; -import com.microsoft.identity.common.internal.util.QueryParamsAdapter; +import com.microsoft.identity.common.java.util.QueryParamsAdapter; import com.microsoft.identity.common.internal.util.StringUtil; import com.microsoft.identity.common.java.authorities.AzureActiveDirectoryAuthority; import com.microsoft.identity.common.java.authscheme.AuthenticationSchemeFactory; diff --git a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java deleted file mode 100644 index 2fbcdf7255..0000000000 --- a/common/src/main/java/com/microsoft/identity/common/internal/util/QueryParamsAdapter.java +++ /dev/null @@ -1,102 +0,0 @@ -// 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 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; -import java.util.AbstractMap; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -/** - * Class to serialize and deserialize query parameters from List> to json String - * 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 { - - private static final String TAG = QueryParamsAdapter.class.getSimpleName(); - - private static final Gson mGson; - - static { - mGson = new GsonBuilder().create(); - } - - public static String _toJson(final List> extraQueryStringParameters) { - final List> extraQpPairs = new ArrayList<>(); - for (final Map.Entry entry: extraQueryStringParameters) { - extraQpPairs.add(new Pair(entry.getKey(), entry.getValue())); - } - return mGson.toJson(extraQpPairs, getPairListType()); - } - - public static List> _fromJson(final String jsonString) - throws ClientException{ - final String methodName = ":_fromJson"; - - if (TextUtils.isEmpty(jsonString)) { - return new ArrayList<>(); - } - - try { - final List> extraQpPairs = mGson.fromJson(jsonString, getPairListType()); - final List> extraQpMapEntries = new ArrayList<>(); - for (final Pair entry: extraQpPairs) { - if (!StringUtil.isEmpty(entry.first)) { - extraQpMapEntries.add(new AbstractMap.SimpleEntry(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> - */ - private static Type getPairListType() { - return TypeToken.getParameterized(List.class, TypeToken.getParameterized(Pair.class, String.class, String.class).getRawType()).getType(); - } -} diff --git a/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java new file mode 100644 index 0000000000..15b3e4aae0 --- /dev/null +++ b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java @@ -0,0 +1,203 @@ +// 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.java.util; + +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.java.logging.Logger; + +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import lombok.AllArgsConstructor; + +/** + * Class to serialize and deserialize query parameters from List> to json String + * 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. + */ +@AllArgsConstructor +public class QueryParamsAdapter extends TypeAdapter>> { + + private static final String TAG = QueryParamsAdapter.class.getSimpleName(); + + private static final Gson mGson; + + /** + * If enabled, this will write a json string in the 'proper' format. + * i.e. {"eqp1":"1","eqp2","2"}. + * + * Otherwise, it will write in the backcompat 'List' format. + * i.e. [{"first":"eqp1","second":"1"},{"first":"eqp2","second":"2"}] + * */ + boolean mWriteProperFormat = false; + + static { + final GsonBuilder gsonBuilder = new GsonBuilder(); + gsonBuilder.registerTypeAdapter( + getListType(), + // Turn off the "proper" format for now, because it'll break backcompat. + new QueryParamsAdapter(false) + ); + mGson = gsonBuilder.create(); + } + + @Override + public void write(final JsonWriter out, final List> queryParams) throws IOException { + if (mWriteProperFormat) { + // The "proper" format. We don't use it now because it'll break backcompat. The older broker doesn't support it. + // i.e. {"eqp1":"1","eqp2","2"}. + writeProperFormat(out, queryParams); + } else { + // Backcompat to support the old "List>" format + // i.e. [{"first":"eqp1","second":"1"},{"first":"eqp2","second":"2"}] + writeListPairFormat(out, queryParams); + } + } + + private void writeProperFormat(final JsonWriter out, final List> queryParams) throws IOException { + out.beginObject(); + for (final Map.Entry keyValuePair : queryParams) { + out.name(keyValuePair.getKey()); + out.value(keyValuePair.getValue()); + } + out.endObject(); + } + + private void writeListPairFormat(final JsonWriter out, final List> queryParams) throws IOException { + out.beginArray(); + for (final Map.Entry keyValuePair : queryParams) { + out.beginObject(); + out.name("first"); + out.value(keyValuePair.getKey()); + out.name("second"); + out.value(keyValuePair.getValue()); + out.endObject(); + } + out.endArray(); + } + + @Override + public List> read(final JsonReader in) throws IOException { + final List> result = new ArrayList<>(); + + switch (in.peek()) { + // Backcompat to support the old "List>" format + // i.e. [{"first":"eqp1","second":"1"},{"first":"eqp2","second":"2"}] + case BEGIN_ARRAY: + return readListPairFormat(in); + + // i.e. {"eqp1":"1","eqp2","2"}, the "proper" one. + // We don't use it now because it's not compatible with the old Broker. + case BEGIN_OBJECT: + return readProperFormat(in); + } + + return result; + } + + private List> readProperFormat(final JsonReader in) throws IOException { + final List> result = new ArrayList<>(); + + in.beginObject(); + while (in.hasNext()) { + final String key = in.nextName(); + final String value = in.nextString(); + final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(key, value); + result.add(keyValuePair); + } + in.endObject(); + return result; + } + + private List> readListPairFormat(final JsonReader in) throws IOException { + final List> result = new ArrayList<>(); + + in.beginArray(); + while (in.hasNext()) { + in.beginObject(); + + String key = ""; + String value = ""; + + while (in.hasNext()) { + final String name = in.nextName(); + if (StringUtil.equalsIgnoreCase(name, "first")) { + key = in.nextString(); + } else if (StringUtil.equalsIgnoreCase(name, "second")) { + value = in.nextString(); + } else{ + throw new JsonSyntaxException("Unexpected NAME field: " + name); + } + } + + final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(key, value); + result.add(keyValuePair); + in.endObject(); + } + in.endArray(); + return result; + } + + public static String _toJson(final List> extraQueryStringParameters) { + return mGson.toJson(extraQueryStringParameters, getListType()); + } + + public static List> _fromJson(final String jsonString) + throws ClientException{ + final String methodName = ":_fromJson"; + + if (StringUtil.isNullOrEmpty(jsonString)) { + return new ArrayList<>(); + } + + try { + return mGson.fromJson(jsonString, getListType()); + } 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> + */ + public static Type getListType() { + return TypeToken.getParameterized(List.class, TypeToken.getParameterized(Map.Entry.class, String.class, String.class).getRawType()).getType(); + } +} diff --git a/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java b/common4j/src/test/com/microsoft/identity/common/java/util/QueryParamsAdapterTest.java similarity index 67% rename from common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java rename to common4j/src/test/com/microsoft/identity/common/java/util/QueryParamsAdapterTest.java index d6c529490d..2ceca964b5 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/util/QueryParamsAdapterTest.java +++ b/common4j/src/test/com/microsoft/identity/common/java/util/QueryParamsAdapterTest.java @@ -21,23 +21,54 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -package com.microsoft.identity.common.internal.util; +package com.microsoft.identity.common.java.util; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; 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 org.junit.runners.JUnit4; import java.util.AbstractMap; import java.util.ArrayList; import java.util.List; import java.util.Map; -@RunWith(RobolectricTestRunner.class) +@RunWith(JUnit4.class) public class QueryParamsAdapterTest { + @Test + public void testConvertToJson() throws Exception { + final List> input = new ArrayList<>(); + input.add(new AbstractMap.SimpleEntry("eqp1", "1")); + input.add(new AbstractMap.SimpleEntry("eqp2", "2")); + + final String expected = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]"; + + Assert.assertEquals(expected, QueryParamsAdapter._toJson(input)); + } + + @Test + public void testConvertToProperJsonFormat() throws Exception{ + final String expected = "{\"eqp1\":\"1\",\"eqp2\":\"2\"}"; + + final List> input = new ArrayList<>(); + input.add(new AbstractMap.SimpleEntry("eqp1", "1")); + input.add(new AbstractMap.SimpleEntry("eqp2", "2")); + + final GsonBuilder gsonBuilder = new GsonBuilder(); + gsonBuilder.registerTypeAdapter( + QueryParamsAdapter.getListType(), + new QueryParamsAdapter(true) + ); + final Gson gson = gsonBuilder.create(); + + Assert.assertEquals(expected, gson.toJson(input, QueryParamsAdapter.getListType())); + } + @Test public void testConvertFromNullJsonString() throws Exception { final List> result = QueryParamsAdapter._fromJson(null); @@ -51,19 +82,19 @@ public void testConvertFromEmptyJsonString() throws Exception { } @Test - public void testConvertToJson() throws Exception { - final List> input = new ArrayList<>(); - input.add(new AbstractMap.SimpleEntry("eqp1", "1")); - input.add(new AbstractMap.SimpleEntry("eqp2", "2")); + public void testConvertFromJson() throws Exception { + final String input = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]"; - final String expected = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]"; + final List> expected = new ArrayList<>(); + expected.add(new AbstractMap.SimpleEntry("eqp1", "1")); + expected.add(new AbstractMap.SimpleEntry("eqp2", "2")); - Assert.assertEquals(expected, QueryParamsAdapter._toJson(input)); + Assert.assertEquals(expected, QueryParamsAdapter._fromJson(input)); } @Test - public void testConvertFromJson() throws Exception { - final String input = "[{\"first\":\"eqp1\",\"second\":\"1\"},{\"first\":\"eqp2\",\"second\":\"2\"}]"; + public void testConvertFromProperJsonFormat() throws Exception{ + final String input = "{\"eqp1\": \"1\", \"eqp2\": \"2\"}"; final List> expected = new ArrayList<>(); expected.add(new AbstractMap.SimpleEntry("eqp1", "1")); @@ -73,15 +104,21 @@ public void testConvertFromJson() throws Exception { } @Test - public void testConvertFromIncorrectJson() throws Exception { + public void testConvertFromUnsupportedJson() throws Exception { // This is what we get if we serialized List directly. + // the difference from List is first->key and second->value. final String input = "[{\"key\":\"eqp1\",\"value\":\"1\"},{\"key\":\"eqp2\",\"value\":\"2\"}]"; - Assert.assertEquals(0, QueryParamsAdapter._fromJson(input).size()); + try { + QueryParamsAdapter._fromJson(input); + Assert.fail(); + } catch (final ClientException e){ + Assert.assertEquals(ClientException.JSON_PARSE_FAILURE, e.getErrorCode()); + } } @Test public void testConvertFromMalformedJson(){ - final String input = "[{\"eqp1\", \"1\"}, {\"eqp2\", \"2\"}]"; + final String input = "[{\"eqp1\"}, {\"eqp2\", \"2\"}]"; try { QueryParamsAdapter._fromJson(input); Assert.fail(); From a72be42bc5b9c6d78704c76408b60ae48c7a08a9 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 13:17:08 -0700 Subject: [PATCH 5/6] Clean up --- .../identity/common/java/util/QueryParamsAdapter.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java index 15b3e4aae0..37b96e6d54 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java +++ b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java @@ -110,9 +110,7 @@ private void writeListPairFormat(final JsonWriter out, final List> read(final JsonReader in) throws IOException { - final List> result = new ArrayList<>(); - + public List> read(final JsonReader in) throws IOException { switch (in.peek()) { // Backcompat to support the old "List>" format // i.e. [{"first":"eqp1","second":"1"},{"first":"eqp2","second":"2"}] @@ -125,7 +123,7 @@ public List> read(final JsonReader in) throws IOExcept return readProperFormat(in); } - return result; + return new ArrayList<>(); } private List> readProperFormat(final JsonReader in) throws IOException { From 687d80625f857278fdd532db647fafe5877789dd Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol Date: Fri, 8 Oct 2021 13:35:33 -0700 Subject: [PATCH 6/6] Address comments --- .../common/java/util/QueryParamsAdapter.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java index 37b96e6d54..f84b42343d 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java +++ b/common4j/src/main/com/microsoft/identity/common/java/util/QueryParamsAdapter.java @@ -110,7 +110,7 @@ private void writeListPairFormat(final JsonWriter out, final List> read(final JsonReader in) throws IOException { + public List> read(final JsonReader in) throws IOException { switch (in.peek()) { // Backcompat to support the old "List>" format // i.e. [{"first":"eqp1","second":"1"},{"first":"eqp2","second":"2"}] @@ -131,9 +131,7 @@ private List> readProperFormat(final JsonReader in) th in.beginObject(); while (in.hasNext()) { - final String key = in.nextName(); - final String value = in.nextString(); - final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(key, value); + final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(in.nextName(), in.nextString()); result.add(keyValuePair); } in.endObject(); @@ -161,18 +159,29 @@ private List> readListPairFormat(final JsonReader in) } } - final Map.Entry keyValuePair = new AbstractMap.SimpleEntry<>(key, value); - result.add(keyValuePair); + result.add(new AbstractMap.SimpleEntry<>(key, value)); in.endObject(); } in.endArray(); return result; } + /** + * Serializes a List>. + * + * @param extraQueryStringParameters an object to serialize. + * @return a serialized string. + * */ public static String _toJson(final List> extraQueryStringParameters) { return mGson.toJson(extraQueryStringParameters, getListType()); } + /** + * Deerializes a string into a List>. + * + * @param jsonString a string to deserialize. + * @return a deserialized object. + * */ public static List> _fromJson(final String jsonString) throws ClientException{ final String methodName = ":_fromJson"; @@ -191,9 +200,9 @@ public static List> _fromJson(final String jsonString) } /** - * Create a Type for the List of query params + * Create a Type for the List of query params. * - * @return a Type object representing the type of the query params in this case List> + * @return a Type object representing the type of the query params in this case List> */ public static Type getListType() { return TypeToken.getParameterized(List.class, TypeToken.getParameterized(Map.Entry.class, String.class, String.class).getRawType()).getType();