From c20103424524dc0735405da4a16a7daad6436da8 Mon Sep 17 00:00:00 2001 From: PrimosK Date: Tue, 13 Jul 2021 13:37:29 +0200 Subject: [PATCH 1/8] Sprout for fix which downcasts parametrized nested `IJsonBackedObject` objects during deserialization. --- .../CollectionResponseSerializer.java | 3 +- .../graph/serializer/DefaultSerializer.java | 48 +------------ .../serializer/DerivedClassIdentifier.java | 59 ++++++++++++++++ .../FallbackTypeAdapterFactory.java | 69 +++++++++++++++++-- 4 files changed, 127 insertions(+), 52 deletions(-) create mode 100644 src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java diff --git a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java b/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java index 2f63fb66c..02baca3cf 100644 --- a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java @@ -86,7 +86,8 @@ public static BaseCollectionResponse deserialize(@Nonnull final JsonEle for(JsonElement sourceElement : sourceArray) { if(sourceElement.isJsonObject()) { final JsonObject sourceObject = sourceElement.getAsJsonObject(); - Class entityClass = serializer.getDerivedClass(sourceObject, baseEntityClass); + final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); + Class entityClass = derivedClassIdentifier.getDerivedClass(sourceObject, baseEntityClass); if(entityClass == null) { if(baseEntityClass == null) { logger.logError("Could not find target class for object " + sourceObject.toString(), null); diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index 2d9d5d2e8..a74b95955 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -22,12 +22,10 @@ package com.microsoft.graph.serializer; -import com.google.common.base.CaseFormat; import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; - import com.microsoft.graph.logger.ILogger; import java.io.IOException; @@ -38,9 +36,8 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Map.Entry; - +import java.util.Objects; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -108,7 +105,8 @@ public T deserializeObject(@Nonnull final JsonElement rawElement, @Nonnull f // If there is a derived class, try to get it and deserialize to it T jo = jsonObject; if (rawElement.isJsonObject()) { - final Class derivedClass = this.getDerivedClass(rawObject, clazz); + final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); + final Class derivedClass = derivedClassIdentifier.getDerivedClass(rawObject, clazz); if (derivedClass != null) jo = (T) gson.fromJson(rawElement, derivedClass); } @@ -310,46 +308,6 @@ private void addAdditionalDataFromManagerToJson(AdditionalDataManager additional } } - private final static String ODATA_TYPE_KEY = "@odata.type"; - /** - * Get the derived class for the given JSON object - * This covers scenarios in which the service may return one of several derived types - * of a base object, which it defines using the odata.type parameter - * - * @param jsonObject the raw JSON object of the response - * @param parentClass the parent class the derived class should inherit from - * @return the derived class if found, or null if not applicable - */ - @Nullable - public Class getDerivedClass(@Nonnull final JsonObject jsonObject, @Nullable final Class parentClass) { - Objects.requireNonNull(jsonObject, "parameter jsonObject cannot be null"); - //Identify the odata.type information if provided - if (jsonObject.get(ODATA_TYPE_KEY) != null) { - /** #microsoft.graph.user or #microsoft.graph.callrecords.callrecord */ - final String odataType = jsonObject.get(ODATA_TYPE_KEY).getAsString(); - final int lastDotIndex = odataType.lastIndexOf("."); - final String derivedType = (odataType.substring(0, lastDotIndex) + - ".models." + - CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, - odataType.substring(lastDotIndex + 1))) - .replace("#", "com."); - try { - Class derivedClass = Class.forName(derivedType); - //Check that the derived class inherits from the given parent class - if (parentClass == null || parentClass.isAssignableFrom(derivedClass)) { - return derivedClass; - } - return null; - } catch (ClassNotFoundException e) { - logger.logDebug("Unable to find a corresponding class for derived type " + derivedType + ". Falling back to parent class."); - //If we cannot determine the derived type to cast to, return null - //This may happen if the API and the SDK are out of sync - return null; - } - } - //If there is no defined OData type, return null - return null; - } /** * Gets the logger in use diff --git a/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java b/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java new file mode 100644 index 000000000..dbebd9981 --- /dev/null +++ b/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java @@ -0,0 +1,59 @@ +package com.microsoft.graph.serializer; + +import com.google.common.base.CaseFormat; +import com.google.gson.JsonObject; +import com.microsoft.graph.logger.ILogger; + +import java.util.Objects; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class DerivedClassIdentifier { + + private final static String ODATA_TYPE_KEY = "@odata.type"; + private final ILogger logger; + + public DerivedClassIdentifier(ILogger logger) { + this.logger = logger; + } + + /** + * Get the derived class for the given JSON object + * This covers scenarios in which the service may return one of several derived types + * of a base object, which it defines using the odata.type parameter + * + * @param jsonObject the raw JSON object of the response + * @param parentClass the parent class the derived class should inherit from + * @return the derived class if found, or null if not applicable + */ + @Nullable + public Class getDerivedClass(@Nonnull final JsonObject jsonObject, @Nullable final Class parentClass) { + Objects.requireNonNull(jsonObject, "parameter jsonObject cannot be null"); + //Identify the odata.type information if provided + if (jsonObject.get(ODATA_TYPE_KEY) != null) { + /** #microsoft.graph.user or #microsoft.graph.callrecords.callrecord */ + final String odataType = jsonObject.get(ODATA_TYPE_KEY).getAsString(); + final int lastDotIndex = odataType.lastIndexOf("."); + final String derivedType = (odataType.substring(0, lastDotIndex) + + ".models." + + CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, + odataType.substring(lastDotIndex + 1))) + .replace("#", "com."); + try { + Class derivedClass = Class.forName(derivedType); + //Check that the derived class inherits from the given parent class + if (parentClass == null || parentClass.isAssignableFrom(derivedClass)) { + return derivedClass; + } + return null; + } catch (ClassNotFoundException e) { + logger.logDebug("Unable to find a corresponding class for derived type " + derivedType + ". Falling back to parent class."); + //If we cannot determine the derived type to cast to, return null + //This may happen if the API and the SDK are out of sync + return null; + } + } + //If there is no defined OData type, return null + return null; + } +} diff --git a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java index 9c439c65d..00630a2d1 100644 --- a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java @@ -22,23 +22,26 @@ package com.microsoft.graph.serializer; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; - import com.google.common.base.CaseFormat; import com.google.gson.Gson; +import com.google.gson.JsonElement; import com.google.gson.TypeAdapter; import com.google.gson.TypeAdapterFactory; +import com.google.gson.internal.Streams; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; +import com.microsoft.graph.http.BaseCollectionPage; +import com.microsoft.graph.http.BaseCollectionResponse; import com.microsoft.graph.logger.ILogger; -import javax.annotation.Nullable; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Handles serialization/deserialization for special types (especially of @@ -89,16 +92,70 @@ public FallbackTypeAdapterFactory(@Nonnull final ILogger logger) { public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeToken type) { Objects.requireNonNull(type, "parameter type cannot be null"); final Class rawType = (Class) type.getRawType(); + if (rawType.isEnum()) { return new EnumTypeAdapter(rawType, logger); } else if (rawType == Void.class) { return (TypeAdapter) voidAdapter; + } else if (IJsonBackedObject.class.isAssignableFrom(type.getRawType())) { + + // Avoid overriding custom IJsonBackedObject type adapters defined in GsonFactory + if (BaseCollectionResponse.class.isAssignableFrom(rawType) || BaseCollectionPage.class.isAssignableFrom(rawType)) { + return null; + } + + final TypeAdapter delegatedAdapter = gson.getDelegateAdapter(this, type); + return (TypeAdapter) new ODataTypeParametrizedIJsonBackedObjectAdapter(gson, delegatedAdapter, type); } else { return null; } } + /** + * This adapter is responsible for deserialization of IJsonBackedObjects where service + * returns one of several derived types of a base object, which is defined using the + * odata.type parameter. If odata.type parameter is not found, the Gson default + * (delegated) type adapter is used. + */ + private class ODataTypeParametrizedIJsonBackedObjectAdapter extends TypeAdapter { + + private final Gson gson; + private final TypeAdapter delegatedAdapter; + private final TypeToken type; + + public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type) { + super(); + this.gson = gson; + this.delegatedAdapter = delegatedAdapter; + this.type = type; + } + + @Override + public void write(JsonWriter out, IJsonBackedObject value) + throws IOException + { + ((TypeAdapter)this.delegatedAdapter).write(out, value); + } + + @Override + public IJsonBackedObject read(JsonReader in) { + JsonElement jsonElement = Streams.parse(in); + + if (jsonElement.isJsonObject()) { + final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); + final Class derivedClass = derivedClassIdentifier.getDerivedClass(jsonElement.getAsJsonObject(), type.getRawType()); + + if (derivedClass != null) { + final TypeAdapter subTypeAdapter = gson.getDelegateAdapter(FallbackTypeAdapterFactory.this, TypeToken.get(derivedClass)); + return (IJsonBackedObject) subTypeAdapter.fromJsonTree(jsonElement); + } + } + + return (IJsonBackedObject) delegatedAdapter.fromJsonTree(jsonElement); + } + } + private static final class EnumTypeAdapter extends TypeAdapter { private final Map enumValues; From 7ecec8ef4d672a3088c988c3c11e8f73cc2837c2 Mon Sep 17 00:00:00 2001 From: PrimosK Date: Tue, 13 Jul 2021 15:16:47 +0200 Subject: [PATCH 2/8] Applied proposed changes. --- .../CollectionResponseSerializer.java | 4 +- .../graph/serializer/DefaultSerializer.java | 24 ++-- .../serializer/DerivedClassIdentifier.java | 119 +++++++++--------- .../FallbackTypeAdapterFactory.java | 11 +- 4 files changed, 83 insertions(+), 75 deletions(-) diff --git a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java b/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java index 02baca3cf..8bba95857 100644 --- a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java @@ -83,11 +83,11 @@ public static BaseCollectionResponse deserialize(@Nonnull final JsonEle logger.logDebug("could not find class" + baseEntityClassCanonicalName); } try { + final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); for(JsonElement sourceElement : sourceArray) { if(sourceElement.isJsonObject()) { final JsonObject sourceObject = sourceElement.getAsJsonObject(); - final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); - Class entityClass = derivedClassIdentifier.getDerivedClass(sourceObject, baseEntityClass); + Class entityClass = derivedClassIdentifier.identify(sourceObject, baseEntityClass); if(entityClass == null) { if(baseEntityClass == null) { logger.logError("Could not find target class for object " + sourceObject.toString(), null); diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index a74b95955..295220155 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -45,17 +45,23 @@ * The default serializer implementation for the SDK */ public class DefaultSerializer implements ISerializer { - private static final String graphResponseHeadersKey = "graphResponseHeaders"; + + private static final String GRAPH_RESPONSE_HEADERS_KEY = "graphResponseHeaders"; + + /** + * The logger + */ + private final ILogger logger; /** * The instance of the internal serializer */ private final Gson gson; - /** - * The logger - */ - private final ILogger logger; + /** + * Derived class identifier + */ + private final DerivedClassIdentifier derivedClassIdentifier; /** * Creates a DefaultSerializer @@ -65,6 +71,7 @@ public class DefaultSerializer implements ISerializer { public DefaultSerializer(@Nonnull final ILogger logger) { this.logger = Objects.requireNonNull(logger, "parameter logger cannot be null"); this.gson = GsonFactory.getGsonInstance(logger); + this.derivedClassIdentifier = new DerivedClassIdentifier(logger); } @Override @@ -105,8 +112,7 @@ public T deserializeObject(@Nonnull final JsonElement rawElement, @Nonnull f // If there is a derived class, try to get it and deserialize to it T jo = jsonObject; if (rawElement.isJsonObject()) { - final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); - final Class derivedClass = derivedClassIdentifier.getDerivedClass(rawObject, clazz); + final Class derivedClass = derivedClassIdentifier.identify(rawObject, clazz); if (derivedClass != null) jo = (T) gson.fromJson(rawElement, derivedClass); } @@ -121,7 +127,7 @@ public T deserializeObject(@Nonnull final JsonElement rawElement, @Nonnull f if (responseHeaders != null) { JsonElement convertedHeaders = gson.toJsonTree(responseHeaders); - jsonBackedObject.additionalDataManager().put(graphResponseHeadersKey, convertedHeaders); + jsonBackedObject.additionalDataManager().put(GRAPH_RESPONSE_HEADERS_KEY, convertedHeaders); } return jo; } else { @@ -302,7 +308,7 @@ private void addAdditionalDataFromJsonObjectToJson (final Object item, final Jso */ private void addAdditionalDataFromManagerToJson(AdditionalDataManager additionalDataManager, JsonObject jsonNode) { for (Map.Entry entry : additionalDataManager.entrySet()) { - if(!entry.getKey().equals(graphResponseHeadersKey)) { + if(!entry.getKey().equals(GRAPH_RESPONSE_HEADERS_KEY)) { jsonNode.add(entry.getKey(), entry.getValue()); } } diff --git a/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java b/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java index dbebd9981..6fbe1a4ba 100644 --- a/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java +++ b/src/main/java/com/microsoft/graph/serializer/DerivedClassIdentifier.java @@ -1,59 +1,60 @@ -package com.microsoft.graph.serializer; - -import com.google.common.base.CaseFormat; -import com.google.gson.JsonObject; -import com.microsoft.graph.logger.ILogger; - -import java.util.Objects; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -public class DerivedClassIdentifier { - - private final static String ODATA_TYPE_KEY = "@odata.type"; - private final ILogger logger; - - public DerivedClassIdentifier(ILogger logger) { - this.logger = logger; - } - - /** - * Get the derived class for the given JSON object - * This covers scenarios in which the service may return one of several derived types - * of a base object, which it defines using the odata.type parameter - * - * @param jsonObject the raw JSON object of the response - * @param parentClass the parent class the derived class should inherit from - * @return the derived class if found, or null if not applicable - */ - @Nullable - public Class getDerivedClass(@Nonnull final JsonObject jsonObject, @Nullable final Class parentClass) { - Objects.requireNonNull(jsonObject, "parameter jsonObject cannot be null"); - //Identify the odata.type information if provided - if (jsonObject.get(ODATA_TYPE_KEY) != null) { - /** #microsoft.graph.user or #microsoft.graph.callrecords.callrecord */ - final String odataType = jsonObject.get(ODATA_TYPE_KEY).getAsString(); - final int lastDotIndex = odataType.lastIndexOf("."); - final String derivedType = (odataType.substring(0, lastDotIndex) + - ".models." + - CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, - odataType.substring(lastDotIndex + 1))) - .replace("#", "com."); - try { - Class derivedClass = Class.forName(derivedType); - //Check that the derived class inherits from the given parent class - if (parentClass == null || parentClass.isAssignableFrom(derivedClass)) { - return derivedClass; - } - return null; - } catch (ClassNotFoundException e) { - logger.logDebug("Unable to find a corresponding class for derived type " + derivedType + ". Falling back to parent class."); - //If we cannot determine the derived type to cast to, return null - //This may happen if the API and the SDK are out of sync - return null; - } - } - //If there is no defined OData type, return null - return null; - } -} +package com.microsoft.graph.serializer; + +import com.google.common.base.CaseFormat; +import com.google.gson.JsonObject; +import com.microsoft.graph.logger.ILogger; + +import java.util.Objects; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class DerivedClassIdentifier { + + private final static String ODATA_TYPE_KEY = "@odata.type"; + + private final ILogger logger; + + public DerivedClassIdentifier(@Nonnull ILogger logger) { + this.logger = Objects.requireNonNull(logger, "logger parameter cannot be null");; + } + + /** + * Get the derived class for the given JSON object + * This covers scenarios in which the service may return one of several derived types + * of a base object, which it defines using the odata.type parameter + * + * @param jsonObject the raw JSON object of the response + * @param parentClass the parent class the derived class should inherit from + * @return the derived class if found, or null if not applicable + */ + @Nullable + public Class identify(@Nonnull final JsonObject jsonObject, @Nullable final Class parentClass) { + Objects.requireNonNull(jsonObject, "parameter jsonObject cannot be null"); + //Identify the odata.type information if provided + if (jsonObject.get(ODATA_TYPE_KEY) != null) { + /** #microsoft.graph.user or #microsoft.graph.callrecords.callrecord */ + final String odataType = jsonObject.get(ODATA_TYPE_KEY).getAsString(); + final int lastDotIndex = odataType.lastIndexOf("."); + final String derivedType = (odataType.substring(0, lastDotIndex) + + ".models." + + CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, + odataType.substring(lastDotIndex + 1))) + .replace("#", "com."); + try { + Class derivedClass = Class.forName(derivedType); + //Check that the derived class inherits from the given parent class + if (parentClass == null || parentClass.isAssignableFrom(derivedClass)) { + return derivedClass; + } + return null; + } catch (ClassNotFoundException e) { + logger.logDebug("Unable to find a corresponding class for derived type " + derivedType + ". Falling back to parent class."); + //If we cannot determine the derived type to cast to, return null + //This may happen if the API and the SDK are out of sync + return null; + } + } + //If there is no defined OData type, return null + return null; + } +} diff --git a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java index 00630a2d1..e0f23db41 100644 --- a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java @@ -123,12 +123,14 @@ private class ODataTypeParametrizedIJsonBackedObjectAdapter extends TypeAdapter< private final Gson gson; private final TypeAdapter delegatedAdapter; private final TypeToken type; + private final DerivedClassIdentifier derivedClassIdentifier; public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type) { super(); - this.gson = gson; - this.delegatedAdapter = delegatedAdapter; - this.type = type; + this.gson = Objects.requireNonNull(gson, "parameter gson cannot be null");; + this.delegatedAdapter = Objects.requireNonNull(delegatedAdapter, "object delegated adapted cannot be null"); + this.type = Objects.requireNonNull(type, "object type cannot be null"); + this.derivedClassIdentifier = new DerivedClassIdentifier(logger); } @Override @@ -143,8 +145,7 @@ public IJsonBackedObject read(JsonReader in) { JsonElement jsonElement = Streams.parse(in); if (jsonElement.isJsonObject()) { - final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); - final Class derivedClass = derivedClassIdentifier.getDerivedClass(jsonElement.getAsJsonObject(), type.getRawType()); + final Class derivedClass = derivedClassIdentifier.identify(jsonElement.getAsJsonObject(), type.getRawType()); if (derivedClass != null) { final TypeAdapter subTypeAdapter = gson.getDelegateAdapter(FallbackTypeAdapterFactory.this, TypeToken.get(derivedClass)); From 59a3098489194d6446c1830e4c231517bf7aca25 Mon Sep 17 00:00:00 2001 From: PrimosK Date: Thu, 15 Jul 2021 10:14:04 +0200 Subject: [PATCH 3/8] Fixes "Access to private field logger of class FallbackTypeAdapterFactory requires synthetic accessor". --- .../FallbackTypeAdapterFactory.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java index e0f23db41..ef8ec2d00 100644 --- a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java @@ -70,14 +70,14 @@ public void write(JsonWriter out, Void value) throws IOException { } @Override - public Void read(JsonReader in) throws IOException { + public Void read(JsonReader in) { return null; } }; /** - * Instanciates a new type adapter factory + * Instantiates a new type adapter factory * * @param logger logger to use for the factory */ @@ -94,7 +94,7 @@ public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeTo final Class rawType = (Class) type.getRawType(); if (rawType.isEnum()) { - return new EnumTypeAdapter(rawType, logger); + return new EnumTypeAdapter<>(rawType, logger); } else if (rawType == Void.class) { return (TypeAdapter) voidAdapter; } else if (IJsonBackedObject.class.isAssignableFrom(type.getRawType())) { @@ -104,8 +104,8 @@ public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeTo return null; } - final TypeAdapter delegatedAdapter = gson.getDelegateAdapter(this, type); - return (TypeAdapter) new ODataTypeParametrizedIJsonBackedObjectAdapter(gson, delegatedAdapter, type); + final TypeAdapter delegatedAdapter = (TypeAdapter) gson.getDelegateAdapter(this, type); + return (TypeAdapter) new ODataTypeParametrizedIJsonBackedObjectAdapter(gson, delegatedAdapter, (TypeToken) type, logger); } else { return null; @@ -121,13 +121,13 @@ public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeTo private class ODataTypeParametrizedIJsonBackedObjectAdapter extends TypeAdapter { private final Gson gson; - private final TypeAdapter delegatedAdapter; - private final TypeToken type; + private final TypeAdapter delegatedAdapter; + private final TypeToken type; private final DerivedClassIdentifier derivedClassIdentifier; - public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type) { + public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type, @Nonnull final ILogger logger) { super(); - this.gson = Objects.requireNonNull(gson, "parameter gson cannot be null");; + this.gson = Objects.requireNonNull(gson, "parameter gson cannot be null"); this.delegatedAdapter = Objects.requireNonNull(delegatedAdapter, "object delegated adapted cannot be null"); this.type = Objects.requireNonNull(type, "object type cannot be null"); this.derivedClassIdentifier = new DerivedClassIdentifier(logger); @@ -137,7 +137,7 @@ public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnul public void write(JsonWriter out, IJsonBackedObject value) throws IOException { - ((TypeAdapter)this.delegatedAdapter).write(out, value); + this.delegatedAdapter.write(out, value); } @Override @@ -153,7 +153,7 @@ public IJsonBackedObject read(JsonReader in) { } } - return (IJsonBackedObject) delegatedAdapter.fromJsonTree(jsonElement); + return delegatedAdapter.fromJsonTree(jsonElement); } } From ba9993b8805505481b182a2bb71f996bf275e8ea Mon Sep 17 00:00:00 2001 From: PrimosK Date: Mon, 19 Jul 2021 08:25:10 +0200 Subject: [PATCH 4/8] Improves the logic which makes sure that custom IJsonBackedObject type adapters defined in GsonFactory are not overridden. --- .../graph/serializer/FallbackTypeAdapterFactory.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java index ef8ec2d00..549f28d04 100644 --- a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java @@ -28,12 +28,11 @@ import com.google.gson.TypeAdapter; import com.google.gson.TypeAdapterFactory; import com.google.gson.internal.Streams; +import com.google.gson.internal.bind.ReflectiveTypeAdapterFactory; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; -import com.microsoft.graph.http.BaseCollectionPage; -import com.microsoft.graph.http.BaseCollectionResponse; import com.microsoft.graph.logger.ILogger; import java.io.IOException; @@ -99,12 +98,13 @@ public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeTo return (TypeAdapter) voidAdapter; } else if (IJsonBackedObject.class.isAssignableFrom(type.getRawType())) { + final TypeAdapter delegatedAdapter = (TypeAdapter) gson.getDelegateAdapter(this, type); + // Avoid overriding custom IJsonBackedObject type adapters defined in GsonFactory - if (BaseCollectionResponse.class.isAssignableFrom(rawType) || BaseCollectionPage.class.isAssignableFrom(rawType)) { + if (!(delegatedAdapter instanceof ReflectiveTypeAdapterFactory.Adapter)) { return null; } - final TypeAdapter delegatedAdapter = (TypeAdapter) gson.getDelegateAdapter(this, type); return (TypeAdapter) new ODataTypeParametrizedIJsonBackedObjectAdapter(gson, delegatedAdapter, (TypeToken) type, logger); } else { From 1305d4247d16c34413a130f40f1ae6aef72d3ac9 Mon Sep 17 00:00:00 2001 From: PrimosK Date: Mon, 19 Jul 2021 08:55:15 +0200 Subject: [PATCH 5/8] Added unit test covering case in PR #249. --- .../microsoft/graph/models/MessageStub.java | 20 +++++++ .../MessagesCollectionResponseStub.java | 10 ++++ .../microsoft/graph/models/ReactionStub.java | 21 ++++++++ .../graph/models/SubReactionStub1.java | 10 ++++ .../graph/models/SubReactionStub2.java | 10 ++++ .../graph/models/TestIJsonBackedObject.java | 24 +++++++++ .../FallbackTypeAdapterFactoryTest.java | 54 +++++++++++++++++++ 7 files changed, 149 insertions(+) create mode 100644 src/test/java/com/microsoft/graph/models/MessageStub.java create mode 100644 src/test/java/com/microsoft/graph/models/MessagesCollectionResponseStub.java create mode 100644 src/test/java/com/microsoft/graph/models/ReactionStub.java create mode 100644 src/test/java/com/microsoft/graph/models/SubReactionStub1.java create mode 100644 src/test/java/com/microsoft/graph/models/SubReactionStub2.java create mode 100644 src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java create mode 100644 src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java diff --git a/src/test/java/com/microsoft/graph/models/MessageStub.java b/src/test/java/com/microsoft/graph/models/MessageStub.java new file mode 100644 index 000000000..886bcd21b --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/MessageStub.java @@ -0,0 +1,20 @@ +package com.microsoft.graph.models; + +import com.google.gson.annotations.Expose; +import com.google.gson.annotations.SerializedName; + +/** + * This class shouldn't be moved as it's location in this + * particular package is tightly coupled with the logic present at: + * com/microsoft/graph/serializer/DerivedClassIdentifier.java:38 + */ +public class MessageStub extends TestIJsonBackedObject { + + @SerializedName("name") + @Expose() + public String name; + + @SerializedName("reaction") + @Expose() + public ReactionStub reaction; +} diff --git a/src/test/java/com/microsoft/graph/models/MessagesCollectionResponseStub.java b/src/test/java/com/microsoft/graph/models/MessagesCollectionResponseStub.java new file mode 100644 index 000000000..b30418346 --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/MessagesCollectionResponseStub.java @@ -0,0 +1,10 @@ +package com.microsoft.graph.models; + +import com.microsoft.graph.http.BaseCollectionResponse; + +public class MessagesCollectionResponseStub extends BaseCollectionResponse { + + public MessagesCollectionResponseStub() { + } + +} diff --git a/src/test/java/com/microsoft/graph/models/ReactionStub.java b/src/test/java/com/microsoft/graph/models/ReactionStub.java new file mode 100644 index 000000000..bdcfb63bc --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/ReactionStub.java @@ -0,0 +1,21 @@ +package com.microsoft.graph.models; + +import com.google.gson.annotations.Expose; +import com.google.gson.annotations.SerializedName; +import org.jetbrains.annotations.Nullable; + +/** + * This class shouldn't be moved as it's location in this + * particular package is tightly coupled with the logic present at: + * com/microsoft/graph/serializer/DerivedClassIdentifier.java:38 + */ +public class ReactionStub extends TestIJsonBackedObject { + + /** + * the OData type of the object as returned by the service + */ + @SerializedName("@odata.type") + @Expose + @Nullable + public String oDataType; +} diff --git a/src/test/java/com/microsoft/graph/models/SubReactionStub1.java b/src/test/java/com/microsoft/graph/models/SubReactionStub1.java new file mode 100644 index 000000000..c2e7cc051 --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/SubReactionStub1.java @@ -0,0 +1,10 @@ +package com.microsoft.graph.models; + +/** + * This class shouldn't be moved as it's location in this + * particular package is tightly coupled with the logic present at: + * com/microsoft/graph/serializer/DerivedClassIdentifier.java:38 + */ +public class SubReactionStub1 extends ReactionStub { + +} diff --git a/src/test/java/com/microsoft/graph/models/SubReactionStub2.java b/src/test/java/com/microsoft/graph/models/SubReactionStub2.java new file mode 100644 index 000000000..33246dbd8 --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/SubReactionStub2.java @@ -0,0 +1,10 @@ +package com.microsoft.graph.models; + +/** + * This class shouldn't be moved as it's location in this + * particular package is tightly coupled with the logic present at: + * com/microsoft/graph/serializer/DerivedClassIdentifier.java:38 + */ +public class SubReactionStub2 extends ReactionStub { + +} diff --git a/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java b/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java new file mode 100644 index 000000000..bd0c4a135 --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java @@ -0,0 +1,24 @@ +package com.microsoft.graph.models; + +import com.google.gson.JsonObject; +import com.microsoft.graph.serializer.AdditionalDataManager; +import com.microsoft.graph.serializer.IJsonBackedObject; +import com.microsoft.graph.serializer.ISerializer; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static org.mockito.Mockito.mock; + +public abstract class TestIJsonBackedObject implements IJsonBackedObject { + + @Override + public void setRawObject(@NotNull ISerializer serializer, @NotNull JsonObject json) { + // Do nothing + } + + @Nullable + @Override + public AdditionalDataManager additionalDataManager() { + return mock(AdditionalDataManager.class); + } +} diff --git a/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java b/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java new file mode 100644 index 000000000..2200023a9 --- /dev/null +++ b/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java @@ -0,0 +1,54 @@ +package com.microsoft.graph.serializer; + +import com.google.gson.Gson; +import com.google.gson.JsonObject; +import com.google.gson.reflect.TypeToken; +import com.microsoft.graph.http.BaseCollectionResponse; +import com.microsoft.graph.logger.ILogger; +import com.microsoft.graph.models.MessageStub; +import com.microsoft.graph.models.MessagesCollectionResponseStub; +import com.microsoft.graph.models.SubReactionStub1; +import com.microsoft.graph.models.SubReactionStub2; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Type; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +public class FallbackTypeAdapterFactoryTest { + + /** + * This test covers the scenario of objects not being + * properly deserialized (@odata.type not taken into account): + * https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/249 + */ + @Test + public void testDeserializationOfNestedODataTypeAnnotatedObjects() { + final ILogger logger = mock(ILogger.class); + final Gson gsonInstance = GsonFactory.getGsonInstance(logger); + + final Type listType = new TypeToken(){}.getType(); + final String testJsonResponse = + "{\"value\": " + + " [" + + " {\"name\": \"parent1\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}}," + + " {\"name\": \"parent2\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub2\"}}" + + " ]" + + "}"; + + final BaseCollectionResponse baseCollectionResponse = gsonInstance.fromJson(testJsonResponse, listType); + + assertNotNull(baseCollectionResponse.value); + + final MessageStub messageStub1 = baseCollectionResponse.value.get(0); + final MessageStub messageStub2 = baseCollectionResponse.value.get(1); + + assertTrue(messageStub1.reaction instanceof SubReactionStub1); + assertTrue(messageStub2.reaction instanceof SubReactionStub2); + } + +} From 3b6be7c395cef0d7106b04f4aac21f034486c1da Mon Sep 17 00:00:00 2001 From: PrimosK Date: Wed, 21 Jul 2021 07:29:01 +0200 Subject: [PATCH 6/8] Moved `ODataTypeParametrizedIJsonBackedObjectAdapter` to it's own file. --- ...peParametrizedIJsonBackedTypedAdapter.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 src/main/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapter.java diff --git a/src/main/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapter.java b/src/main/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapter.java new file mode 100644 index 000000000..4b2620d5b --- /dev/null +++ b/src/main/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapter.java @@ -0,0 +1,63 @@ +package com.microsoft.graph.serializer; + +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.google.gson.TypeAdapter; +import com.google.gson.internal.Streams; +import com.google.gson.reflect.TypeToken; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; +import com.microsoft.graph.logger.ILogger; + +import java.io.IOException; +import java.util.Objects; +import javax.annotation.Nonnull; + +/** + * This adapter is responsible for deserialization of IJsonBackedObjects where service + * returns one of several derived types of a base object, which is defined using the + * odata.type parameter. If odata.type parameter is not found, the Gson default + * (delegated) type adapter is used. + */ +class ODataTypeParametrizedIJsonBackedTypedAdapter extends TypeAdapter { + + private final FallbackTypeAdapterFactory fallbackTypeAdapterFactory; + private final Gson gson; + private final TypeAdapter delegatedAdapter; + private final TypeToken type; + private final DerivedClassIdentifier derivedClassIdentifier; + + public ODataTypeParametrizedIJsonBackedTypedAdapter(FallbackTypeAdapterFactory fallbackTypeAdapterFactory, @Nonnull Gson gson, + @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type, @Nonnull final ILogger logger) + { + super(); + this.fallbackTypeAdapterFactory = fallbackTypeAdapterFactory; + this.gson = Objects.requireNonNull(gson, "parameter gson cannot be null"); + this.delegatedAdapter = Objects.requireNonNull(delegatedAdapter, "object delegated adapted cannot be null"); + this.type = Objects.requireNonNull(type, "object type cannot be null"); + this.derivedClassIdentifier = new DerivedClassIdentifier(logger); + } + + @Override + public void write(JsonWriter out, IJsonBackedObject value) + throws IOException + { + this.delegatedAdapter.write(out, value); + } + + @Override + public IJsonBackedObject read(JsonReader in) { + JsonElement jsonElement = Streams.parse(in); + + if (jsonElement.isJsonObject()) { + final Class derivedClass = derivedClassIdentifier.identify(jsonElement.getAsJsonObject(), type.getRawType()); + + if (derivedClass != null) { + final TypeAdapter subTypeAdapter = gson.getDelegateAdapter(fallbackTypeAdapterFactory, TypeToken.get(derivedClass)); + return (IJsonBackedObject) subTypeAdapter.fromJsonTree(jsonElement); + } + } + + return delegatedAdapter.fromJsonTree(jsonElement); + } +} From 88eca4232605bdecc16b0b026a5a31702a33d0e6 Mon Sep 17 00:00:00 2001 From: PrimosK Date: Wed, 21 Jul 2021 07:45:21 +0200 Subject: [PATCH 7/8] Increased unit tests coverage. --- .../FallbackTypeAdapterFactory.java | 49 +-------- .../microsoft/graph/models/MessageStub.java | 4 +- .../microsoft/graph/models/ReactionsStub.java | 21 ++++ .../FallbackTypeAdapterFactoryTest.java | 54 ---------- ...rametrizedIJsonBackedTypedAdapterTest.java | 101 ++++++++++++++++++ 5 files changed, 125 insertions(+), 104 deletions(-) create mode 100644 src/test/java/com/microsoft/graph/models/ReactionsStub.java delete mode 100644 src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java create mode 100644 src/test/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapterTest.java diff --git a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java index 549f28d04..849e8750b 100644 --- a/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java @@ -24,10 +24,8 @@ import com.google.common.base.CaseFormat; import com.google.gson.Gson; -import com.google.gson.JsonElement; import com.google.gson.TypeAdapter; import com.google.gson.TypeAdapterFactory; -import com.google.gson.internal.Streams; import com.google.gson.internal.bind.ReflectiveTypeAdapterFactory; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; @@ -105,58 +103,13 @@ public TypeAdapter create(@Nonnull final Gson gson, @Nonnull final TypeTo return null; } - return (TypeAdapter) new ODataTypeParametrizedIJsonBackedObjectAdapter(gson, delegatedAdapter, (TypeToken) type, logger); + return (TypeAdapter) new ODataTypeParametrizedIJsonBackedTypedAdapter(this, gson, delegatedAdapter, (TypeToken) type, logger); } else { return null; } } - /** - * This adapter is responsible for deserialization of IJsonBackedObjects where service - * returns one of several derived types of a base object, which is defined using the - * odata.type parameter. If odata.type parameter is not found, the Gson default - * (delegated) type adapter is used. - */ - private class ODataTypeParametrizedIJsonBackedObjectAdapter extends TypeAdapter { - - private final Gson gson; - private final TypeAdapter delegatedAdapter; - private final TypeToken type; - private final DerivedClassIdentifier derivedClassIdentifier; - - public ODataTypeParametrizedIJsonBackedObjectAdapter(@Nonnull Gson gson, @Nonnull TypeAdapter delegatedAdapter, @Nonnull final TypeToken type, @Nonnull final ILogger logger) { - super(); - this.gson = Objects.requireNonNull(gson, "parameter gson cannot be null"); - this.delegatedAdapter = Objects.requireNonNull(delegatedAdapter, "object delegated adapted cannot be null"); - this.type = Objects.requireNonNull(type, "object type cannot be null"); - this.derivedClassIdentifier = new DerivedClassIdentifier(logger); - } - - @Override - public void write(JsonWriter out, IJsonBackedObject value) - throws IOException - { - this.delegatedAdapter.write(out, value); - } - - @Override - public IJsonBackedObject read(JsonReader in) { - JsonElement jsonElement = Streams.parse(in); - - if (jsonElement.isJsonObject()) { - final Class derivedClass = derivedClassIdentifier.identify(jsonElement.getAsJsonObject(), type.getRawType()); - - if (derivedClass != null) { - final TypeAdapter subTypeAdapter = gson.getDelegateAdapter(FallbackTypeAdapterFactory.this, TypeToken.get(derivedClass)); - return (IJsonBackedObject) subTypeAdapter.fromJsonTree(jsonElement); - } - } - - return delegatedAdapter.fromJsonTree(jsonElement); - } - } - private static final class EnumTypeAdapter extends TypeAdapter { private final Map enumValues; diff --git a/src/test/java/com/microsoft/graph/models/MessageStub.java b/src/test/java/com/microsoft/graph/models/MessageStub.java index 886bcd21b..b6d6621a6 100644 --- a/src/test/java/com/microsoft/graph/models/MessageStub.java +++ b/src/test/java/com/microsoft/graph/models/MessageStub.java @@ -10,9 +10,9 @@ */ public class MessageStub extends TestIJsonBackedObject { - @SerializedName("name") + @SerializedName("body") @Expose() - public String name; + public String body; @SerializedName("reaction") @Expose() diff --git a/src/test/java/com/microsoft/graph/models/ReactionsStub.java b/src/test/java/com/microsoft/graph/models/ReactionsStub.java new file mode 100644 index 000000000..458bda2cc --- /dev/null +++ b/src/test/java/com/microsoft/graph/models/ReactionsStub.java @@ -0,0 +1,21 @@ +package com.microsoft.graph.models; + +import com.google.gson.annotations.Expose; +import com.google.gson.annotations.SerializedName; +import org.jetbrains.annotations.Nullable; + +import java.util.List; + +/** + * This class shouldn't be moved as it's location in this + * particular package is tightly coupled with the logic present at: + * com/microsoft/graph/serializer/DerivedClassIdentifier.java:38 + */ +public class ReactionsStub extends TestIJsonBackedObject { + + @SerializedName("reactions") + @Expose + @Nullable + public List reactions; + +} diff --git a/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java b/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java deleted file mode 100644 index 2200023a9..000000000 --- a/src/test/java/com/microsoft/graph/serializer/FallbackTypeAdapterFactoryTest.java +++ /dev/null @@ -1,54 +0,0 @@ -package com.microsoft.graph.serializer; - -import com.google.gson.Gson; -import com.google.gson.JsonObject; -import com.google.gson.reflect.TypeToken; -import com.microsoft.graph.http.BaseCollectionResponse; -import com.microsoft.graph.logger.ILogger; -import com.microsoft.graph.models.MessageStub; -import com.microsoft.graph.models.MessagesCollectionResponseStub; -import com.microsoft.graph.models.SubReactionStub1; -import com.microsoft.graph.models.SubReactionStub2; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import org.junit.jupiter.api.Test; - -import java.lang.reflect.Type; - -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; - -public class FallbackTypeAdapterFactoryTest { - - /** - * This test covers the scenario of objects not being - * properly deserialized (@odata.type not taken into account): - * https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/249 - */ - @Test - public void testDeserializationOfNestedODataTypeAnnotatedObjects() { - final ILogger logger = mock(ILogger.class); - final Gson gsonInstance = GsonFactory.getGsonInstance(logger); - - final Type listType = new TypeToken(){}.getType(); - final String testJsonResponse = - "{\"value\": " + - " [" + - " {\"name\": \"parent1\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}}," + - " {\"name\": \"parent2\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub2\"}}" + - " ]" + - "}"; - - final BaseCollectionResponse baseCollectionResponse = gsonInstance.fromJson(testJsonResponse, listType); - - assertNotNull(baseCollectionResponse.value); - - final MessageStub messageStub1 = baseCollectionResponse.value.get(0); - final MessageStub messageStub2 = baseCollectionResponse.value.get(1); - - assertTrue(messageStub1.reaction instanceof SubReactionStub1); - assertTrue(messageStub2.reaction instanceof SubReactionStub2); - } - -} diff --git a/src/test/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapterTest.java b/src/test/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapterTest.java new file mode 100644 index 000000000..e235941f7 --- /dev/null +++ b/src/test/java/com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapterTest.java @@ -0,0 +1,101 @@ +package com.microsoft.graph.serializer; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import com.microsoft.graph.http.BaseCollectionResponse; +import com.microsoft.graph.logger.ILogger; +import com.microsoft.graph.models.MessageStub; +import com.microsoft.graph.models.MessagesCollectionResponseStub; +import com.microsoft.graph.models.ReactionStub; +import com.microsoft.graph.models.ReactionsStub; +import com.microsoft.graph.models.SubReactionStub1; +import com.microsoft.graph.models.SubReactionStub2; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Type; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * Covers the scenario of objects not being properly deserialized + * (@odata.type not taken into account): + * https://github.com/microsoftgraph/msgraph-sdk-java-core/pull/249 + */ +public class ODataTypeParametrizedIJsonBackedTypedAdapterTest { + + final ILogger logger = mock(ILogger.class); + final Gson gsonInstance = GsonFactory.getGsonInstance(logger); + + @Test + public void testDeserializationOfObjectWithODataTypeProperty() { + // Given + final String testJsonResponse = + "{\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}"; + + // When + ReactionStub reaction = gsonInstance.fromJson(testJsonResponse, ReactionStub.class); + + // Then + assertTrue(reaction instanceof SubReactionStub1); + } + + @Test + public void testDeserializationOfPropertyWithODataTypeProperty() { + // Given + final String testJsonResponse = + "{\"body\": \"message1\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub2\"}}"; + + // When + MessageStub message = gsonInstance.fromJson(testJsonResponse, MessageStub.class); + + // Then + assertTrue(message.reaction instanceof SubReactionStub2); + } + + @Test + public void testDeserializationOfCollectionPropertyContainingObjectsWithODataTypeProperty() { + // Given + final String testJsonResponse = + "{ reactions : [" + + "{\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}," + + "{\"@odata.type\": \"#microsoft.graph.subReactionStub2\"}," + + "{\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}," + + "]}"; + + // When + ReactionsStub reactions = gsonInstance.fromJson(testJsonResponse, ReactionsStub.class); + + // Then + assertNotNull(reactions.reactions); + assertTrue(reactions.reactions.get(0) instanceof SubReactionStub1); + assertTrue(reactions.reactions.get(1) instanceof SubReactionStub2); + assertTrue(reactions.reactions.get(2) instanceof SubReactionStub1); + } + + @Test + public void testDeserializationOfNestedODataTypeAnnotatedObjects() { + // Given + final Type listType = new TypeToken(){}.getType(); + final String testJsonResponse = + "{\"value\": " + + " [" + + " {\"body\": \"message1\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}}," + + " {\"body\": \"message2\",\"reaction\": {\"@odata.type\": \"#microsoft.graph.subReactionStub2\"}}" + + " ]" + + "}"; + + // When + final BaseCollectionResponse baseCollectionResponse = gsonInstance.fromJson(testJsonResponse, listType); + + // Then + assertNotNull(baseCollectionResponse.value); + + final MessageStub messageStub1 = baseCollectionResponse.value.get(0); + final MessageStub messageStub2 = baseCollectionResponse.value.get(1); + + assertTrue(messageStub1.reaction instanceof SubReactionStub1); + assertTrue(messageStub2.reaction instanceof SubReactionStub2); + } +} From 69df140e0a96d79a0e0712e17f8a7859d2569fce Mon Sep 17 00:00:00 2001 From: PrimosK Date: Wed, 21 Jul 2021 11:04:23 +0200 Subject: [PATCH 8/8] Removed DerivedClassIdentifier and related code from: - DefaultSerializer - CollectionResponseDeserializer ... as this is now taken care by ODataTypeParametrizedJsonBackedTypeAdapter. Tests covering this change: - com.microsoft.graph.serializer.DefaultSerializerTest.testDeserializationOfObjectWithODataTypeProperty - com.microsoft.graph.serializer.ODataTypeParametrizedIJsonBackedTypedAdapterTest.testDeserializationOfNestedODataTypeAnnotatedObjects --- .../serializer/CollectionPageSerializer.java | 3 +- ...va => CollectionResponseDeserializer.java} | 17 +++------ .../graph/serializer/DefaultSerializer.java | 18 ++-------- .../graph/serializer/GsonFactory.java | 2 +- .../graph/models/TestIJsonBackedObject.java | 12 +++++-- .../serializer/DefaultSerializerTest.java | 36 +++++++++++++++++++ 6 files changed, 54 insertions(+), 34 deletions(-) rename src/main/java/com/microsoft/graph/serializer/{CollectionResponseSerializer.java => CollectionResponseDeserializer.java} (87%) create mode 100644 src/test/java/com/microsoft/graph/serializer/DefaultSerializerTest.java diff --git a/src/main/java/com/microsoft/graph/serializer/CollectionPageSerializer.java b/src/main/java/com/microsoft/graph/serializer/CollectionPageSerializer.java index dd305b532..424503af1 100644 --- a/src/main/java/com/microsoft/graph/serializer/CollectionPageSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/CollectionPageSerializer.java @@ -24,7 +24,6 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -112,7 +111,7 @@ public static > BaseCollectionPage final Class responseClass = Class.forName(responseClassCanonicalName); final JsonObject responseJson = new JsonObject(); responseJson.add("value", json); - final BaseCollectionResponse response = CollectionResponseSerializer.deserialize(responseJson, responseClass, logger); + final BaseCollectionResponse response = CollectionResponseDeserializer.deserialize(responseJson, responseClass, logger); /** eg: com.microsoft.graph.requests.AttachmentCollectionRequestBuilder */ final String responseBuilderCanonicalName = responseClassCanonicalName .substring(0, responseClassCanonicalName.length() - responseLength) + "RequestBuilder"; diff --git a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java b/src/main/java/com/microsoft/graph/serializer/CollectionResponseDeserializer.java similarity index 87% rename from src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java rename to src/main/java/com/microsoft/graph/serializer/CollectionResponseDeserializer.java index 8bba95857..5b9c5c5a1 100644 --- a/src/main/java/com/microsoft/graph/serializer/CollectionResponseSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/CollectionResponseDeserializer.java @@ -37,13 +37,13 @@ import com.microsoft.graph.http.BaseCollectionResponse; import com.microsoft.graph.logger.ILogger; -/** Specialized serializer to handle collection responses */ -public class CollectionResponseSerializer { +/** Specialized de-serializer to handle collection responses */ +public class CollectionResponseDeserializer { private static DefaultSerializer serializer; /** * Not available for instantiation */ - private CollectionResponseSerializer() {} + private CollectionResponseDeserializer() {} /** * Deserializes the JsonElement * @@ -83,19 +83,10 @@ public static BaseCollectionResponse deserialize(@Nonnull final JsonEle logger.logDebug("could not find class" + baseEntityClassCanonicalName); } try { - final DerivedClassIdentifier derivedClassIdentifier = new DerivedClassIdentifier(logger); for(JsonElement sourceElement : sourceArray) { if(sourceElement.isJsonObject()) { final JsonObject sourceObject = sourceElement.getAsJsonObject(); - Class entityClass = derivedClassIdentifier.identify(sourceObject, baseEntityClass); - if(entityClass == null) { - if(baseEntityClass == null) { - logger.logError("Could not find target class for object " + sourceObject.toString(), null); - continue; - } else - entityClass = baseEntityClass; // it is possible the odata type is absent or we can't find the derived type (not in SDK yet) - } - final T1 targetObject = (T1)serializer.deserializeObject(sourceObject, entityClass); + final T1 targetObject = (T1)serializer.deserializeObject(sourceObject, baseEntityClass); ((IJsonBackedObject)targetObject).setRawObject(serializer, sourceObject); list.add(targetObject); } else if (sourceElement.isJsonPrimitive()) { diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index 295220155..3ea6cf36d 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -58,10 +58,6 @@ public class DefaultSerializer implements ISerializer { */ private final Gson gson; - /** - * Derived class identifier - */ - private final DerivedClassIdentifier derivedClassIdentifier; /** * Creates a DefaultSerializer @@ -71,7 +67,6 @@ public class DefaultSerializer implements ISerializer { public DefaultSerializer(@Nonnull final ILogger logger) { this.logger = Objects.requireNonNull(logger, "parameter logger cannot be null"); this.gson = GsonFactory.getGsonInstance(logger); - this.derivedClassIdentifier = new DerivedClassIdentifier(logger); } @Override @@ -108,16 +103,7 @@ public T deserializeObject(@Nonnull final JsonElement rawElement, @Nonnull f if (jsonObject instanceof IJsonBackedObject) { logger.logDebug("Deserializing type " + clazz.getSimpleName()); final JsonObject rawObject = rawElement.isJsonObject() ? rawElement.getAsJsonObject() : null; - - // If there is a derived class, try to get it and deserialize to it - T jo = jsonObject; - if (rawElement.isJsonObject()) { - final Class derivedClass = derivedClassIdentifier.identify(rawObject, clazz); - if (derivedClass != null) - jo = (T) gson.fromJson(rawElement, derivedClass); - } - - final IJsonBackedObject jsonBackedObject = (IJsonBackedObject) jo; + final IJsonBackedObject jsonBackedObject = (IJsonBackedObject) jsonObject; if(rawElement.isJsonObject()) { jsonBackedObject.setRawObject(this, rawObject); @@ -129,7 +115,7 @@ public T deserializeObject(@Nonnull final JsonElement rawElement, @Nonnull f JsonElement convertedHeaders = gson.toJsonTree(responseHeaders); jsonBackedObject.additionalDataManager().put(GRAPH_RESPONSE_HEADERS_KEY, convertedHeaders); } - return jo; + return jsonObject; } else { logger.logDebug("Deserializing a non-IJsonBackedObject type " + clazz.getSimpleName()); return jsonObject; diff --git a/src/main/java/com/microsoft/graph/serializer/GsonFactory.java b/src/main/java/com/microsoft/graph/serializer/GsonFactory.java index 4a040cc99..1b24e1c08 100644 --- a/src/main/java/com/microsoft/graph/serializer/GsonFactory.java +++ b/src/main/java/com/microsoft/graph/serializer/GsonFactory.java @@ -239,7 +239,7 @@ public JsonElement serialize(final BaseCollectionPage src, public BaseCollectionResponse deserialize(final JsonElement json, final Type typeOfT, final JsonDeserializationContext context) throws JsonParseException { - return CollectionResponseSerializer.deserialize(json, typeOfT, logger); + return CollectionResponseDeserializer.deserialize(json, typeOfT, logger); } }; diff --git a/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java b/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java index bd0c4a135..b5a80c12c 100644 --- a/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java +++ b/src/test/java/com/microsoft/graph/models/TestIJsonBackedObject.java @@ -11,14 +11,22 @@ public abstract class TestIJsonBackedObject implements IJsonBackedObject { + AdditionalDataManager additionalDataManager = mock(AdditionalDataManager.class); + + String rawObject; + @Override public void setRawObject(@NotNull ISerializer serializer, @NotNull JsonObject json) { - // Do nothing + this.rawObject = json.toString(); } @Nullable @Override public AdditionalDataManager additionalDataManager() { - return mock(AdditionalDataManager.class); + return additionalDataManager; + } + + public String getRawObject() { + return rawObject; } } diff --git a/src/test/java/com/microsoft/graph/serializer/DefaultSerializerTest.java b/src/test/java/com/microsoft/graph/serializer/DefaultSerializerTest.java new file mode 100644 index 000000000..459e9b14a --- /dev/null +++ b/src/test/java/com/microsoft/graph/serializer/DefaultSerializerTest.java @@ -0,0 +1,36 @@ +package com.microsoft.graph.serializer; + +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.microsoft.graph.logger.ILogger; +import com.microsoft.graph.models.ReactionStub; +import com.microsoft.graph.models.SubReactionStub1; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +public class DefaultSerializerTest { + + final ILogger logger = mock(ILogger.class); + Gson gson = GsonFactory.getGsonInstance(logger); + DefaultSerializer defaultSerializer = new DefaultSerializer(logger); + + @Test + public void testDeserializationOfObjectWithODataTypeProperty() { + // Given + final String testJsonResponse = + "{\"@odata.type\": \"#microsoft.graph.subReactionStub1\"}"; + + // When + ReactionStub reaction = defaultSerializer.deserializeObject(testJsonResponse, ReactionStub.class); + + // Then + assertTrue(reaction instanceof SubReactionStub1); + assertEquals("{\"@odata.type\":\"#microsoft.graph.subReactionStub1\"}", reaction.getRawObject()); + Mockito.verify(reaction.additionalDataManager()).setAdditionalData(gson.fromJson(testJsonResponse, JsonElement.class).getAsJsonObject()); + } + +}