From b4ecc09891e772b652ff01a308f0fda711871d82 Mon Sep 17 00:00:00 2001 From: "FAREAST\\vidadhee" Date: Thu, 16 Aug 2018 17:20:19 +0530 Subject: [PATCH 1/7] Added a test case to verify multi-level additional data setter and added a comment to explain the IllegalArgumentException --- .../microsoft/graph/serializer/DefaultSerializer.java | 3 ++- .../graph/serializer/AdditionalDataTests.java | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index 69ca75ccef3..46e0a26856a 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -148,7 +148,8 @@ else if (fieldObject != null && fieldObject instanceof IJsonBackedObject) { setChildAdditionalData((IJsonBackedObject) fieldObject,rawJson.get(field.getName()).getAsJsonObject()); } } catch (IllegalArgumentException | IllegalAccessException e) { - logger.logError("Unable to access child fields of " + serializedObject.getClass().getSimpleName(), e); + //Not throwing the IllegalArgumentException as the Serialized Object would still be usable even if the additional data is not set. + logger.logError("Unable to set child fields of " + serializedObject.getClass().getSimpleName(), e); } } } diff --git a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java index c620945e550..41d253fd517 100644 --- a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java +++ b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java @@ -5,8 +5,10 @@ import org.junit.Before; import org.junit.Test; +import com.google.gson.JsonElement; import com.google.gson.JsonPrimitive; import com.microsoft.graph.logger.DefaultLogger; +import com.microsoft.graph.models.extensions.Drive; import com.microsoft.graph.models.extensions.Entity; import com.microsoft.graph.models.extensions.PlannerAssignment; import com.microsoft.graph.models.extensions.PlannerAssignments; @@ -48,6 +50,15 @@ public void testChildAdditionalData() { assertEquals("{\"manager\":{\"id\":\"1\",\"additionalData\":\"additionalValue\"},\"id\":\"2\"}", serializedObject); } + + @Test + public void testChildAdditionalDataDeserialization() { + String source = "{\"@odata.context\":\"https://graph.microsoft.com/v1.0/$metadata#drives/$entity\",\"id\":\"8bf6ae90006c4a4c\",\"driveType\":\"personal\",\"owner\":{\"user\":{\"displayName\":\"Peter\",\"id\":\"8bf6ae90006c4a4c\",\"email\":\"petertest@onmicrosoft.com\"}},\"quota\":{\"deleted\":1485718314,\"remaining\":983887466461,\"state\":\"normal\",\"total\":1142461300736,\"used\":158573834275}}"; + Drive result = serializer.deserializeObject(source, Drive.class); + JsonElement email = result.owner.user.additionalDataManager().get("email"); + + assertEquals("\"petertest@onmicrosoft.com\"",email.toString()); + } @Test public void testSkipTransientData() { From 59fd088ddbf47469bce43dc418cec37339247f4a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 9 Sep 2020 15:48:22 -0400 Subject: [PATCH 2/7] - removes duplicate test and adjusts faulty unit test --- .../graph/serializer/AdditionalDataTests.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java index 2b636fe7bd8..1fdf12f786b 100644 --- a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java +++ b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java @@ -58,15 +58,6 @@ public void testChildAdditionalData() { assertEquals("{\"manager\":{\"id\":\"1\",\"additionalData\":\"additionalValue\"},\"id\":\"2\"}", serializedObject); } - @Test - public void testChildAdditionalDataDeserialization() { - String source = "{\"@odata.context\":\"https://graph.microsoft.com/v1.0/$metadata#drives/$entity\",\"id\":\"8bf6ae90006c4a4c\",\"driveType\":\"personal\",\"owner\":{\"user\":{\"displayName\":\"Peter\",\"id\":\"8bf6ae90006c4a4c\",\"email\":\"petertest@onmicrosoft.com\"}},\"quota\":{\"deleted\":1485718314,\"remaining\":983887466461,\"state\":\"normal\",\"total\":1142461300736,\"used\":158573834275}}"; - Drive result = serializer.deserializeObject(source, Drive.class); - JsonElement email = result.owner.user.additionalDataManager().get("email"); - - assertEquals("\"petertest@onmicrosoft.com\"",email.toString()); - } - @Test public void testSkipTransientData() { Entity entity = new Entity(); @@ -77,7 +68,7 @@ public void testSkipTransientData() { String serializedObject = serializer.serializeObject(entity); - assertEquals("{\"id\":\"1\",\"@odata.type\":\"entity\"}", serializedObject); + assertEquals("{\"id\":\"1\",\"@odata.nextLink\":\"1\",\"@odata.type\":\"entity\"}", serializedObject); } @Test From 0059f66f374619151a6c2ed1f3c23162b64e037b Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 9 Sep 2020 15:52:26 -0400 Subject: [PATCH 3/7] - light refactoring for clarity and defensive programming --- .../graph/serializer/DefaultSerializer.java | 100 ++++++++++-------- 1 file changed, 55 insertions(+), 45 deletions(-) diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index e17c62be26a..92111bf3ccb 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -118,60 +118,70 @@ public T deserializeObject(final String inputString, final Class clazz, M * @param serializedObject the parent object whose children will be iterated to set additional data * @param rawJson the raw json */ - @SuppressWarnings("unchecked") - private void setChildAdditionalData(IJsonBackedObject serializedObject, JsonObject rawJson) { + private void setChildAdditionalData(final IJsonBackedObject serializedObject, final JsonObject rawJson) { // Use reflection to iterate through fields for eligible Graph children - for (java.lang.reflect.Field field : serializedObject.getClass().getFields()) { - try { - Object fieldObject = field.get(serializedObject); - - // If the object is a HashMap, iterate through its children - if (fieldObject instanceof HashMap) { - HashMap serializableChildren = (HashMap) fieldObject; - Iterator> it = serializableChildren.entrySet().iterator(); + if(rawJson != null) { + for (java.lang.reflect.Field field : serializedObject.getClass().getFields()) { + try { + if(field != null) { + final Object fieldObject = field.get(serializedObject); + if (fieldObject instanceof HashMap) { + // If the object is a HashMap, iterate through its children + @SuppressWarnings("unchecked") + final HashMap serializableChildren = (HashMap) fieldObject; + final Iterator> it = serializableChildren.entrySet().iterator(); - while (it.hasNext()) { - Map.Entry pair = (Map.Entry)it.next(); - Object child = pair.getValue(); + while (it.hasNext()) { + final Map.Entry pair = (Map.Entry)it.next(); + final Object child = pair.getValue(); - // If the item is a valid Graph object, set its additional data - if (child instanceof IJsonBackedObject) { - AdditionalDataManager childAdditionalDataManager = ((IJsonBackedObject) child).additionalDataManager(); - if(rawJson != null && field != null && rawJson.get(field.getName()) != null && rawJson.get(field.getName()).isJsonObject() - && rawJson.get(field.getName()).getAsJsonObject().get(pair.getKey()).isJsonObject()) { - childAdditionalDataManager.setAdditionalData(rawJson.get(field.getName()).getAsJsonObject().get(pair.getKey()).getAsJsonObject()); - setChildAdditionalData((IJsonBackedObject) child,rawJson.get(field.getName()).getAsJsonObject().get(pair.getKey()).getAsJsonObject()); + // If the item is a valid Graph object, set its additional data + if (child instanceof IJsonBackedObject) { + final AdditionalDataManager childAdditionalDataManager = ((IJsonBackedObject) child).additionalDataManager(); + final JsonElement fieldElement = rawJson.get(field.getName()); + if(fieldElement != null && fieldElement.isJsonObject() + && fieldElement.getAsJsonObject().get(pair.getKey()) != null + && fieldElement.getAsJsonObject().get(pair.getKey()).isJsonObject()) { + childAdditionalDataManager.setAdditionalData(fieldElement.getAsJsonObject().get(pair.getKey()).getAsJsonObject()); + setChildAdditionalData((IJsonBackedObject) child,fieldElement.getAsJsonObject().get(pair.getKey()).getAsJsonObject()); + } + } } } - } - } - // If the object is a list of Graph objects, iterate through elements - else if (fieldObject instanceof List && rawJson != null) { - final JsonElement collectionJson = rawJson.get(field.getName()); - final List fieldObjectList = (List) fieldObject; - if (collectionJson.isJsonArray() && ((JsonArray)collectionJson).size() == fieldObjectList.size()) { - final JsonArray rawJsonArray = (JsonArray) collectionJson; - for (int i = 0; i < fieldObjectList.size(); i++) { - final Object element = fieldObjectList.get(i); - if (element instanceof IJsonBackedObject) { - final JsonElement elementRawJson = rawJsonArray.get(i); - setChildAdditionalData((IJsonBackedObject) element, elementRawJson.getAsJsonObject()); + // If the object is a list of Graph objects, iterate through elements + else if (fieldObject instanceof List) { + final JsonElement collectionJson = rawJson.get(field.getName()); + final List fieldObjectList = (List) fieldObject; + if (collectionJson != null && collectionJson.isJsonArray()) { + final JsonArray rawJsonArray = (JsonArray) collectionJson; + if(rawJsonArray.size() == fieldObjectList.size()) { + for (int i = 0; i < fieldObjectList.size(); i++) { + final Object element = fieldObjectList.get(i); + if (element instanceof IJsonBackedObject) { + final JsonElement elementRawJson = rawJsonArray.get(i); + if(elementRawJson != null) { + setChildAdditionalData((IJsonBackedObject) element, elementRawJson.getAsJsonObject()); + } + } + } + } + } + } + // If the object is a valid Graph object, set its additional data + else if (fieldObject instanceof IJsonBackedObject) { + final IJsonBackedObject serializedChild = (IJsonBackedObject) fieldObject; + final AdditionalDataManager childAdditionalDataManager = serializedChild.additionalDataManager(); + final JsonElement fieldElement = rawJson.get(field.getName()); + if(fieldElement != null && fieldElement.isJsonObject()) { + childAdditionalDataManager.setAdditionalData(fieldElement.getAsJsonObject()); + setChildAdditionalData((IJsonBackedObject) fieldObject,fieldElement.getAsJsonObject()); } } } + } catch (IllegalArgumentException | IllegalAccessException e) { + //Not throwing the IllegalArgumentException as the Serialized Object would still be usable even if the additional data is not set. + logger.logError("Unable to set child fields of " + serializedObject.getClass().getSimpleName(), e); } - // If the object is a valid Graph object, set its additional data - else if (fieldObject != null && fieldObject instanceof IJsonBackedObject) { - IJsonBackedObject serializedChild = (IJsonBackedObject) fieldObject; - AdditionalDataManager childAdditionalDataManager = serializedChild.additionalDataManager(); - if(rawJson != null && field != null && rawJson.get(field.getName()) != null && rawJson.get(field.getName()).isJsonObject()) { - childAdditionalDataManager.setAdditionalData(rawJson.get(field.getName()).getAsJsonObject()); - setChildAdditionalData((IJsonBackedObject) fieldObject,rawJson.get(field.getName()).getAsJsonObject()); - } - } - } catch (IllegalArgumentException | IllegalAccessException e) { - //Not throwing the IllegalArgumentException as the Serialized Object would still be usable even if the additional data is not set. - logger.logError("Unable to set child fields of " + serializedObject.getClass().getSimpleName(), e); } } } From cd27f494f40acd9c0ff505b012b40ee31552c1a9 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 17 Sep 2020 15:16:44 -0400 Subject: [PATCH 4/7] - adds logging for edge cases to help with potential troubleshooting --- .../graph/serializer/DefaultSerializer.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java index 28648ac4ad9..d2529573e5a 100644 --- a/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java +++ b/src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java @@ -154,17 +154,19 @@ else if (fieldObject instanceof List) { final List fieldObjectList = (List) fieldObject; if (collectionJson != null && collectionJson.isJsonArray()) { final JsonArray rawJsonArray = (JsonArray) collectionJson; - if(rawJsonArray.size() == fieldObjectList.size()) { - for (int i = 0; i < fieldObjectList.size(); i++) { - final Object element = fieldObjectList.get(i); - if (element instanceof IJsonBackedObject) { - final JsonElement elementRawJson = rawJsonArray.get(i); - if(elementRawJson != null) { - setChildAdditionalData((IJsonBackedObject) element, elementRawJson.getAsJsonObject()); - } + final Integer fieldObjectListSize = fieldObjectList.size(); + final Integer rawJsonArraySize = rawJsonArray.size(); + for (int i = 0; i < fieldObjectListSize && i < rawJsonArraySize; i++) { + final Object element = fieldObjectList.get(i); + if (element instanceof IJsonBackedObject) { + final JsonElement elementRawJson = rawJsonArray.get(i); + if(elementRawJson != null) { + setChildAdditionalData((IJsonBackedObject) element, elementRawJson.getAsJsonObject()); } } } + if (rawJsonArraySize != fieldObjectListSize) + logger.logDebug("rawJsonArray has a size of " + rawJsonArraySize + " and fieldObjectList of " + fieldObjectListSize); } } // If the object is a valid Graph object, set its additional data @@ -181,6 +183,7 @@ else if (fieldObject instanceof IJsonBackedObject) { } catch (IllegalArgumentException | IllegalAccessException e) { //Not throwing the IllegalArgumentException as the Serialized Object would still be usable even if the additional data is not set. logger.logError("Unable to set child fields of " + serializedObject.getClass().getSimpleName(), e); + logger.logDebug(rawJson.getAsString()); } } } From 4831f581d541905adfc94e625249fd2f5dc9e23d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 21 Sep 2020 12:57:34 -0400 Subject: [PATCH 5/7] - adds a unit test for hashmap properties --- .../graph/serializer/AdditionalDataTests.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java index e367ada7331..3b6a0782ef8 100644 --- a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java +++ b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java @@ -1,6 +1,7 @@ package com.microsoft.graph.serializer; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -138,4 +139,14 @@ public void testChildAdditionalDataDeserialization() { assertEquals("\"petertest@onmicrosoft.com\"",email.toString()); } + + @Test + public void testHashMapProperties() { + final String source = "{\"description\": \"Task details properties:\nchecklist:Sub items\nreferences:Related links\",\"previewType\": \"automatic\",\"references\": {\"https%3A//developer%2Emicrosoft%2Ecom/en-us/graph/graph-explorer\": {\"@odata.type\": \"#microsoft.graph.plannerExternalReference\",\"alias\": \"Graph Explorer\",\"type\": \"Other\",\"previewPriority\": \"0009005706180391122\",\"lastModifiedBy\": {\"user\": {\"id\": \"fbab97d0-4932-4511-b675-204639209557\"}},\"lastModifiedDateTime\": \"2017-04-24T22:52:29.814Z\"}},\"checklist\": {\"d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff\": {\"@odata.type\": \"#microsoft.graph.plannerChecklistItem\",\"isChecked\": false,\"title\": \"Try reading task details\",\"orderHint\": \"8587094707721254251P]\",\"lastModifiedBy\": {\"user\": {\"id\": \"e396de0e-4812-4fcb-9f9e-0358744df343\"}},\"lastModifiedDateTime\": \"2017-04-14T02:16:14.866Z\"}},\"id\": \"gcrYAaAkgU2EQUvpkNNXLGQAGTtu\"}"; + final PlannerTaskDetails taskDetails = serializer.deserializeObject(source, PlannerTaskDetails.class); + assertNotNull(taskDetails); + assertNotNull(taskDetails.checklist); + assertFalse(taskDetails.checklist.isEmpty()); + assertTrue(taskDetails.checklist.get("d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff").title.equals("Try reading task details")); + } } From 4150c14ac850206313a2b88504c28cb16c6bddf8 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 21 Sep 2020 13:18:15 -0400 Subject: [PATCH 6/7] - updates unit test to check for additional properties in child elements --- .../com/microsoft/graph/serializer/AdditionalDataTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java index 3b6a0782ef8..9da27b06779 100644 --- a/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java +++ b/src/test/java/com/microsoft/graph/serializer/AdditionalDataTests.java @@ -142,11 +142,12 @@ public void testChildAdditionalDataDeserialization() { @Test public void testHashMapProperties() { - final String source = "{\"description\": \"Task details properties:\nchecklist:Sub items\nreferences:Related links\",\"previewType\": \"automatic\",\"references\": {\"https%3A//developer%2Emicrosoft%2Ecom/en-us/graph/graph-explorer\": {\"@odata.type\": \"#microsoft.graph.plannerExternalReference\",\"alias\": \"Graph Explorer\",\"type\": \"Other\",\"previewPriority\": \"0009005706180391122\",\"lastModifiedBy\": {\"user\": {\"id\": \"fbab97d0-4932-4511-b675-204639209557\"}},\"lastModifiedDateTime\": \"2017-04-24T22:52:29.814Z\"}},\"checklist\": {\"d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff\": {\"@odata.type\": \"#microsoft.graph.plannerChecklistItem\",\"isChecked\": false,\"title\": \"Try reading task details\",\"orderHint\": \"8587094707721254251P]\",\"lastModifiedBy\": {\"user\": {\"id\": \"e396de0e-4812-4fcb-9f9e-0358744df343\"}},\"lastModifiedDateTime\": \"2017-04-14T02:16:14.866Z\"}},\"id\": \"gcrYAaAkgU2EQUvpkNNXLGQAGTtu\"}"; + final String source = "{\"description\": \"Task details properties:\nchecklist:Sub items\nreferences:Related links\",\"previewType\": \"automatic\",\"references\": {\"https%3A//developer%2Emicrosoft%2Ecom/en-us/graph/graph-explorer\": {\"@odata.type\": \"#microsoft.graph.plannerExternalReference\",\"alias\": \"Graph Explorer\",\"type\": \"Other\",\"previewPriority\": \"0009005706180391122\",\"lastModifiedBy\": {\"user\": {\"id\": \"fbab97d0-4932-4511-b675-204639209557\"}},\"lastModifiedDateTime\": \"2017-04-24T22:52:29.814Z\"}},\"checklist\": {\"d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff\": {\"@odata.type\": \"#microsoft.graph.plannerChecklistItem\",\"isChecked\": false,\"title\": \"Try reading task details\",\"orderHint\": \"8587094707721254251P]\",\"lastModifiedBy\": {\"user\": {\"id\": \"e396de0e-4812-4fcb-9f9e-0358744df343\", \"customProp\": \"somestring\"}},\"lastModifiedDateTime\": \"2017-04-14T02:16:14.866Z\"}},\"id\": \"gcrYAaAkgU2EQUvpkNNXLGQAGTtu\"}"; final PlannerTaskDetails taskDetails = serializer.deserializeObject(source, PlannerTaskDetails.class); assertNotNull(taskDetails); assertNotNull(taskDetails.checklist); assertFalse(taskDetails.checklist.isEmpty()); assertTrue(taskDetails.checklist.get("d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff").title.equals("Try reading task details")); + assertTrue(taskDetails.checklist.get("d280ed1a-9f6b-4f9c-a962-fb4d00dc50ff").lastModifiedBy.user.additionalDataManager().get("customProp").getAsString().equals("somestring")); } } From d6881da9c5677949394dc7b5a63ebaa563bcc69e Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 21 Sep 2020 13:18:49 -0400 Subject: [PATCH 7/7] - add suppress warnings --- .../java/com/microsoft/graph/serializer/MockSerializer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/microsoft/graph/serializer/MockSerializer.java b/src/test/java/com/microsoft/graph/serializer/MockSerializer.java index d76ba9d4ff0..3b73ee29dcf 100644 --- a/src/test/java/com/microsoft/graph/serializer/MockSerializer.java +++ b/src/test/java/com/microsoft/graph/serializer/MockSerializer.java @@ -50,6 +50,7 @@ public MockSerializer(final Object deserializeReturn, final String serializeRetu } @Override + @SuppressWarnings("unchecked") public T deserializeObject(final String inputString, final Class clazz) { //noinspection unchecked return (T) mDeserializeReturn; @@ -61,8 +62,8 @@ public String serializeObject(final T serializableObject) { } @Override + @SuppressWarnings("unchecked") public T deserializeObject(String inputString, Class clazz, Map> responseHeaders) { - // TODO Auto-generated method stub return (T) mDeserializeReturn; } }