Skip to content

fixes a serialization issue for additional data #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Parent: ** fix ** fix for LargFileUploadTask constructor
merged 5 commits into from
Jun 11, 2021
Merged

fixes a serialization issue for additional data #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
739b59e
- fixes a bug where passing null to additional data manager could lea…
baywet Jun 11, 2021
cde5553
- fixes a bug where the serializer would only look at first level pro…
baywet Jun 11, 2021
2b532c0
- code linting
baywet Jun 11, 2021
98e44d9
- more code linting
baywet Jun 11, 2021
0f3fb83
- pr feedback
baywet Jun 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions src/main/java/com/microsoft/graph/serializer/AdditionalDataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
// ------------------------------------------------------------------------------
// Copyright (c) 2017 Microsoft Corporation
//
//
// 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, sub-license, 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
Expand Down Expand Up @@ -38,12 +38,12 @@
public class AdditionalDataManager extends HashMap<String, JsonElement> {

private static final long serialVersionUID = 8641634955796941429L;

private final transient IJsonBackedObject jsonBackedObject;

/**
* Instanciates a new additional data manager from the json backed object
*
*
* @param jsonBackedObject the object to read values from
*/
public AdditionalDataManager(@Nullable final IJsonBackedObject jsonBackedObject) {
Expand All @@ -56,15 +56,15 @@ public AdditionalDataManager(@Nullable final IJsonBackedObject jsonBackedObject)
*
* @param json the raw JSON to set as additionalData
*/
final void setAdditionalData(JsonObject json) {
final void setAdditionalData(final JsonObject json) {
// Get the names of all the fields on this object's hierarchy
Set<String> objectFields = getFields();
final Set<String> objectFields = getFields();

// Get the keys on this JSON
Set<String> jsonKeys = getJsonKeys(json);
final Set<String> jsonKeys = getJsonKeys(json);

// Get all keys present in JSON and *NOT* present in fields
Set<String> additionalDataKeys = new HashSet<>(jsonKeys);
final Set<String> additionalDataKeys = new HashSet<>(jsonKeys);
additionalDataKeys.removeAll(objectFields);

// set the additionalData
Expand All @@ -73,23 +73,25 @@ final void setAdditionalData(JsonObject json) {
}
}

private Set<String> getJsonKeys(JsonObject json) {
Set<String> keys = new HashSet<>();
Set<Map.Entry<String, JsonElement>> entries = json.entrySet();
private Set<String> getJsonKeys(final JsonObject json) {
final Set<String> keys = new HashSet<>();
final Set<Map.Entry<String, JsonElement>> entries = json.entrySet();
for (Map.Entry<String, JsonElement> entry : entries) {
keys.add(entry.getKey());
}
return keys;
}

private Set<String> getFields() {
Field[] fields = jsonBackedObject.getClass().getFields();
Set<String> serializingFields = new HashSet<>();
for (Field field : fields) {
SerializedName serializedName;
if (null != (serializedName = field.getAnnotation(SerializedName.class))
&& null != field.getAnnotation(Expose.class)) {
serializingFields.add(serializedName.value());
final Set<String> serializingFields = new HashSet<>();
if(jsonBackedObject != null ) {
final Field[] fields = jsonBackedObject.getClass().getFields();
for (Field field : fields) {
final SerializedName serializedName = field.getAnnotation(SerializedName.class);
if (null != serializedName
&& null != field.getAnnotation(Expose.class)) {
serializingFields.add(serializedName.value());
}
}
}
return serializingFields;
Expand Down
141 changes: 53 additions & 88 deletions src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,109 +223,74 @@ else if (fieldObject instanceof IJsonBackedObject) {
public <T> String serializeObject(@Nonnull final T serializableObject) {
Objects.requireNonNull(serializableObject, "parameter serializableObject cannot be null");
logger.logDebug("Serializing type " + serializableObject.getClass().getSimpleName());
JsonElement outJsonTree = gson.toJsonTree(serializableObject);

if (serializableObject instanceof IJsonBackedObject) {
outJsonTree = getDataFromAdditionalDataManager(outJsonTree, serializableObject);
} else if (outJsonTree.isJsonObject()) {
final Field[] fields = serializableObject.getClass().getDeclaredFields();
JsonObject outJson = outJsonTree.getAsJsonObject();
for(Field field : fields) {
if(outJson.has(field.getName())) {
final Type[] interfaces = field.getType().getGenericInterfaces();
for(Type interfaceType : interfaces) {
if(interfaceType == IJsonBackedObject.class && outJson.get(field.getName()).isJsonObject()) {
try {
final JsonElement outdatedValue = outJson.remove(field.getName());
outJson.add(field.getName(), getDataFromAdditionalDataManager(outdatedValue.getAsJsonObject(), field.get(serializableObject)));
} catch (IllegalAccessException ex ) {
logger.logDebug("Couldn't access prop" + field.getName());
}
break;
}
}
}
}
}

return outJsonTree.toString();
}
private <T> JsonElement getDataFromAdditionalDataManager(JsonElement outJsonTree, final T serializableObject) {
final IJsonBackedObject serializableJsonObject = (IJsonBackedObject) serializableObject;
final AdditionalDataManager additionalData = serializableJsonObject.additionalDataManager();

// If the item is a valid Graph object, add its additional data
if (outJsonTree.isJsonObject()) {
final JsonObject outJson = outJsonTree.getAsJsonObject();

addAdditionalDataFromManagerToJson(additionalData, outJson);
getChildAdditionalData(serializableJsonObject, outJson);

return outJson;
} else {
return outJsonTree;
final JsonElement outJsonTree = gson.toJsonTree(serializableObject);
if(outJsonTree != null) {
getChildAdditionalData(serializableObject, outJsonTree);
return outJsonTree.toString();
}
return "";
}

/**
* Recursively populates additional data for each child object
*
* @param serializableObject the child to get additional data for
* @param outJson the serialized output JSON to add to
*/
@SuppressWarnings("unchecked")
private void getChildAdditionalData(final IJsonBackedObject serializableObject, final JsonObject outJson) {
if(outJson == null)
return;
// Use reflection to iterate through fields for eligible Graph children
for (java.lang.reflect.Field field : serializableObject.getClass().getFields()) {
try {
final Object fieldObject = field.get(serializableObject);
final JsonElement fieldJsonElement = outJson.get(field.getName());
if(fieldObject == null || fieldJsonElement == null)
continue;

// If the object is a HashMap, iterate through its children
if (fieldObject instanceof Map && fieldJsonElement.isJsonObject()) {
final Map<String, Object> serializableChildren = (Map<String, Object>) fieldObject;
final Iterator<Entry<String, Object>> it = serializableChildren.entrySet().iterator();
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();

while (it.hasNext()) {
final Map.Entry<String, Object> pair = (Map.Entry<String, Object>)it.next();
final Object child = pair.getValue();
final JsonElement childJsonElement = fieldJsonObject.get(pair.getKey().toString());
// If the item is a valid Graph object, add its additional data
addAdditionalDataFromJsonElementToJson(child, childJsonElement);
}
}
// If the object is a list of Graph objects, iterate through elements
else if (fieldObject instanceof List && fieldJsonElement.isJsonArray()) {
final JsonArray fieldArrayValue = fieldJsonElement.getAsJsonArray();
final List<?> fieldObjectList = (List<?>) fieldObject;
for (int index = 0; index < fieldObjectList.size(); index++) {
final Object item = fieldObjectList.get(index);
final JsonElement itemJsonElement = fieldArrayValue.get(index);
addAdditionalDataFromJsonElementToJson(item, itemJsonElement);
}
}
// If the object is a valid Graph object, add its additional data
addAdditionalDataFromJsonElementToJson(fieldObject, fieldJsonElement);
} catch (IllegalArgumentException | IllegalAccessException e) {
logger.logError("Unable to access child fields of " + serializableObject.getClass().getSimpleName(), e);
}
}
}
private void getChildAdditionalData(final Object serializableObject, final JsonElement outJson) {
if(outJson == null || serializableObject == null || !outJson.isJsonObject())
return;
final JsonObject outJsonObject = outJson.getAsJsonObject();
// Use reflection to iterate through fields for eligible Graph children
for (java.lang.reflect.Field field : serializableObject.getClass().getFields()) {
try {
final Object fieldObject = field.get(serializableObject);
final JsonElement fieldJsonElement = outJsonObject.get(field.getName());
if(fieldObject == null || fieldJsonElement == null)
continue;

// If the object is a HashMap, iterate through its children
if (fieldObject instanceof Map && fieldJsonElement.isJsonObject()) {
final Map<String, Object> serializableChildren = (Map<String, Object>) fieldObject;
final Iterator<Entry<String, Object>> it = serializableChildren.entrySet().iterator();
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();

while (it.hasNext()) {
final Map.Entry<String, Object> pair = (Map.Entry<String, Object>)it.next();
final Object child = pair.getValue();
final JsonElement childJsonElement = fieldJsonObject.get(pair.getKey().toString());
// If the item is a valid Graph object, add its additional data
getChildAdditionalData(child, childJsonElement);
}
}
// If the object is a list of Graph objects, iterate through elements
else if (fieldObject instanceof List && fieldJsonElement.isJsonArray()) {
final JsonArray fieldArrayValue = fieldJsonElement.getAsJsonArray();
final List<?> fieldObjectList = (List<?>) fieldObject;
for (int index = 0; index < fieldObjectList.size(); index++) {
final Object item = fieldObjectList.get(index);
final JsonElement itemJsonElement = fieldArrayValue.get(index);
getChildAdditionalData(item, itemJsonElement);
}
} else if(fieldJsonElement.isJsonObject()) {
// If the object is a valid Graph object, add its additional data
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();
addAdditionalDataFromJsonObjectToJson(fieldObject, fieldJsonObject);
}
} catch (IllegalArgumentException | IllegalAccessException e) {
logger.logError("Unable to access child fields of " + serializableObject.getClass().getSimpleName(), e);
}
}
}

/**
* Add each non-transient additional data property to the given JSON node
*
* @param item the object containing additional data
* @param itemJsonElement the JSON node to add the additional data properties to
* @param itemJsonObject the JSON node to add the additional data properties to
*/
private void addAdditionalDataFromJsonElementToJson (final Object item, final JsonElement itemJsonElement) {
if (item instanceof IJsonBackedObject && itemJsonElement.isJsonObject()) {
final JsonObject itemJsonObject = itemJsonElement.getAsJsonObject();
private void addAdditionalDataFromJsonObjectToJson (final Object item, final JsonObject itemJsonObject) {
if (item instanceof IJsonBackedObject && itemJsonObject != null) {
final IJsonBackedObject serializableItem = (IJsonBackedObject) item;
final AdditionalDataManager itemAdditionalData = serializableItem.additionalDataManager();
addAdditionalDataFromManagerToJson(itemAdditionalData, itemJsonObject);
Expand Down
41 changes: 29 additions & 12 deletions src/test/java/com/microsoft/graph/content/BatchRequestContentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -14,6 +15,7 @@
import java.util.ArrayList;

import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
import com.microsoft.graph.authentication.IAuthenticationProvider;
import com.microsoft.graph.core.BaseClient;
import com.microsoft.graph.core.IBaseClient;
Expand All @@ -37,12 +39,12 @@
import okhttp3.Response;
import okhttp3.ResponseBody;

public class BatchRequestContentTest {
class BatchRequestContentTest {

String testurl = "http://graph.microsoft.com/me";

@Test
public void testBatchRequestContentCreation() throws MalformedURLException {
void testBatchRequestContentCreation() throws MalformedURLException {
BatchRequestContent requestContent = new BatchRequestContent();
for (int i = 0; i < 5; i++) {
IHttpRequest requestStep = mock(IHttpRequest.class);
Expand All @@ -53,7 +55,7 @@ public void testBatchRequestContentCreation() throws MalformedURLException {
}

@Test
public void testGetBatchRequestContent() throws MalformedURLException {
void testGetBatchRequestContent() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
BatchRequestContent requestContent = new BatchRequestContent();
Expand All @@ -65,7 +67,7 @@ public void testGetBatchRequestContent() throws MalformedURLException {
}

@Test
public void testGetBatchRequestContentWithHeader() throws MalformedURLException {
void testGetBatchRequestContentWithHeader() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
when(requestStep.getHeaders()).thenReturn(Arrays.asList(new HeaderOption("testkey", "testvalue")));
Expand All @@ -78,7 +80,7 @@ public void testGetBatchRequestContentWithHeader() throws MalformedURLException
}

@Test
public void testRemoveBatchRequesStepWithId() throws MalformedURLException {
void testRemoveBatchRequesStepWithId() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
BatchRequestContent requestContent = new BatchRequestContent();
Expand All @@ -90,7 +92,7 @@ public void testRemoveBatchRequesStepWithId() throws MalformedURLException {
}

@Test
public void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws MalformedURLException {
void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
BatchRequestContent requestContent = new BatchRequestContent();
Expand All @@ -109,7 +111,7 @@ public void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws M
}

@Test
public void defensiveProgrammingTests() {
void defensiveProgrammingTests() {
assertThrows(NullPointerException.class, () -> {
new BatchRequestContent().addBatchRequestStep(null);
}, "should throw argument exception");
Expand All @@ -135,7 +137,7 @@ public void defensiveProgrammingTests() {
}

@Test
public void executeBatchTest() throws Throwable {
void executeBatchTest() throws Throwable {
final BatchRequestContent content = new BatchRequestContent();
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
Expand Down Expand Up @@ -164,7 +166,7 @@ public void executeBatchTest() throws Throwable {
}

@Test
public void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException {
void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
when(requestStep.getHttpMethod()).thenReturn(HttpMethod.DELETE);
Expand All @@ -174,13 +176,14 @@ public void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException
}

@Test
public void doesNotThrowWhenTryingToRemoveRequestFromNull() {
void doesNotThrowWhenTryingToRemoveRequestFromNull() {
final BatchRequestContent batchRequest = new BatchRequestContent();
batchRequest.removeBatchRequestStepWithId("id");
assertNull(batchRequest.requests);
}

@Test
public void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
final BatchRequestContent batchRequest = new BatchRequestContent();
Expand All @@ -194,7 +197,7 @@ public void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
}

@Test
public void addsContentTypeForBodies() throws MalformedURLException {
void addsContentTypeForBodies() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
final BatchRequestContent batchRequest = new BatchRequestContent();
Expand Down Expand Up @@ -235,4 +238,18 @@ public AdditionalDataManager additionalDataManager() {
assertNotNull(step4.headers);
assertEquals("application/octet-stream", step4.headers.get("content-type"));
}
@Test
void serializesAdditionalData() throws MalformedURLException {
IHttpRequest requestStep = mock(IHttpRequest.class);
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
final BatchRequestContent batchRequest = new BatchRequestContent();
final String bindValue = "https://somebindvalue";
final BatchRequestTestBody body = new BatchRequestTestBody(); // using a dynamic implementation doesn't work as "this" maps to the current test class
body.additionalDataManager().put("teamsApp@odata.bind", new JsonPrimitive(bindValue));
batchRequest.addBatchRequestStep(requestStep, HttpMethod.POST, body);
final ISerializer serializer = new DefaultSerializer(mock(ILogger.class));
final String result = serializer.serializeObject(batchRequest);
assertNotNull(result);
assertTrue(result.contains(bindValue));
}
}
Loading