Skip to content

Commit 670f02a

Browse files
baywetbaywet
authored
Merge pull request #232 from microsoftgraph/bugfix/batch-serialization
fixes a serialization issue for additional data
2 parents 11397dc + 0f3fb83 commit 670f02a

File tree

4 files changed

+133
-119
lines changed

4 files changed

+133
-119
lines changed

‎src/main/java/com/microsoft/graph/serializer/AdditionalDataManager.java‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
// ------------------------------------------------------------------------------
22
// Copyright (c) 2017 Microsoft Corporation
3-
//
3+
//
44
// Permission is hereby granted, free of charge, to any person obtaining a copy
55
// of this software and associated documentation files (the "Software"), to deal
66
// in the Software without restriction, including without limitation the rights
77
// to use, copy, modify, merge, publish, distribute, sub-license, and/or sell
88
// copies of the Software, and to permit persons to whom the Software is
99
// furnished to do so, subject to the following conditions:
10-
//
10+
//
1111
// The above copyright notice and this permission notice shall be included in
1212
// all copies or substantial portions of the Software.
13-
//
13+
//
1414
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
1515
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
1616
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -38,12 +38,12 @@
3838
public class AdditionalDataManager extends HashMap<String, JsonElement> {
3939

4040
private static final long serialVersionUID = 8641634955796941429L;
41-
41+
4242
private final transient IJsonBackedObject jsonBackedObject;
4343

4444
/**
4545
* Instanciates a new additional data manager from the json backed object
46-
*
46+
*
4747
* @param jsonBackedObject the object to read values from
4848
*/
4949
public AdditionalDataManager(@Nullable final IJsonBackedObject jsonBackedObject) {
@@ -56,15 +56,15 @@ public AdditionalDataManager(@Nullable final IJsonBackedObject jsonBackedObject)
5656
*
5757
* @param json the raw JSON to set as additionalData
5858
*/
59-
final void setAdditionalData(JsonObject json) {
59+
final void setAdditionalData(final JsonObject json) {
6060
// Get the names of all the fields on this object's hierarchy
61-
Set<String> objectFields = getFields();
61+
final Set<String> objectFields = getFields();
6262

6363
// Get the keys on this JSON
64-
Set<String> jsonKeys = getJsonKeys(json);
64+
final Set<String> jsonKeys = getJsonKeys(json);
6565

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

7070
// set the additionalData
@@ -73,23 +73,25 @@ final void setAdditionalData(JsonObject json) {
7373
}
7474
}
7575

76-
private Set<String> getJsonKeys(JsonObject json) {
77-
Set<String> keys = new HashSet<>();
78-
Set<Map.Entry<String, JsonElement>> entries = json.entrySet();
76+
private Set<String> getJsonKeys(final JsonObject json) {
77+
final Set<String> keys = new HashSet<>();
78+
final Set<Map.Entry<String, JsonElement>> entries = json.entrySet();
7979
for (Map.Entry<String, JsonElement> entry : entries) {
8080
keys.add(entry.getKey());
8181
}
8282
return keys;
8383
}
8484

8585
private Set<String> getFields() {
86-
Field[] fields = jsonBackedObject.getClass().getFields();
87-
Set<String> serializingFields = new HashSet<>();
88-
for (Field field : fields) {
89-
SerializedName serializedName;
90-
if (null != (serializedName = field.getAnnotation(SerializedName.class))
91-
&& null != field.getAnnotation(Expose.class)) {
92-
serializingFields.add(serializedName.value());
86+
final Set<String> serializingFields = new HashSet<>();
87+
if(jsonBackedObject != null ) {
88+
final Field[] fields = jsonBackedObject.getClass().getFields();
89+
for (Field field : fields) {
90+
final SerializedName serializedName = field.getAnnotation(SerializedName.class);
91+
if (null != serializedName
92+
&& null != field.getAnnotation(Expose.class)) {
93+
serializingFields.add(serializedName.value());
94+
}
9395
}
9496
}
9597
return serializingFields;

‎src/main/java/com/microsoft/graph/serializer/DefaultSerializer.java‎

Lines changed: 53 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -223,109 +223,74 @@ else if (fieldObject instanceof IJsonBackedObject) {
223223
public <T> String serializeObject(@Nonnull final T serializableObject) {
224224
Objects.requireNonNull(serializableObject, "parameter serializableObject cannot be null");
225225
logger.logDebug("Serializing type " + serializableObject.getClass().getSimpleName());
226-
JsonElement outJsonTree = gson.toJsonTree(serializableObject);
227-
228-
if (serializableObject instanceof IJsonBackedObject) {
229-
outJsonTree = getDataFromAdditionalDataManager(outJsonTree, serializableObject);
230-
} else if (outJsonTree.isJsonObject()) {
231-
final Field[] fields = serializableObject.getClass().getDeclaredFields();
232-
JsonObject outJson = outJsonTree.getAsJsonObject();
233-
for(Field field : fields) {
234-
if(outJson.has(field.getName())) {
235-
final Type[] interfaces = field.getType().getGenericInterfaces();
236-
for(Type interfaceType : interfaces) {
237-
if(interfaceType == IJsonBackedObject.class && outJson.get(field.getName()).isJsonObject()) {
238-
try {
239-
final JsonElement outdatedValue = outJson.remove(field.getName());
240-
outJson.add(field.getName(), getDataFromAdditionalDataManager(outdatedValue.getAsJsonObject(), field.get(serializableObject)));
241-
} catch (IllegalAccessException ex ) {
242-
logger.logDebug("Couldn't access prop" + field.getName());
243-
}
244-
break;
245-
}
246-
}
247-
}
248-
}
249-
}
250-
251-
return outJsonTree.toString();
252-
}
253-
private <T> JsonElement getDataFromAdditionalDataManager(JsonElement outJsonTree, final T serializableObject) {
254-
final IJsonBackedObject serializableJsonObject = (IJsonBackedObject) serializableObject;
255-
final AdditionalDataManager additionalData = serializableJsonObject.additionalDataManager();
256-
257-
// If the item is a valid Graph object, add its additional data
258-
if (outJsonTree.isJsonObject()) {
259-
final JsonObject outJson = outJsonTree.getAsJsonObject();
260-
261-
addAdditionalDataFromManagerToJson(additionalData, outJson);
262-
getChildAdditionalData(serializableJsonObject, outJson);
263-
264-
return outJson;
265-
} else {
266-
return outJsonTree;
226+
final JsonElement outJsonTree = gson.toJsonTree(serializableObject);
227+
if(outJsonTree != null) {
228+
getChildAdditionalData(serializableObject, outJsonTree);
229+
return outJsonTree.toString();
267230
}
231+
return "";
268232
}
269-
270233
/**
271234
* Recursively populates additional data for each child object
272235
*
273236
* @param serializableObject the child to get additional data for
274237
* @param outJson the serialized output JSON to add to
275238
*/
276239
@SuppressWarnings("unchecked")
277-
private void getChildAdditionalData(final IJsonBackedObject serializableObject, final JsonObject outJson) {
278-
if(outJson == null)
279-
return;
280-
// Use reflection to iterate through fields for eligible Graph children
281-
for (java.lang.reflect.Field field : serializableObject.getClass().getFields()) {
282-
try {
283-
final Object fieldObject = field.get(serializableObject);
284-
final JsonElement fieldJsonElement = outJson.get(field.getName());
285-
if(fieldObject == null || fieldJsonElement == null)
286-
continue;
287-
288-
// If the object is a HashMap, iterate through its children
289-
if (fieldObject instanceof Map && fieldJsonElement.isJsonObject()) {
290-
final Map<String, Object> serializableChildren = (Map<String, Object>) fieldObject;
291-
final Iterator<Entry<String, Object>> it = serializableChildren.entrySet().iterator();
292-
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();
293-
294-
while (it.hasNext()) {
295-
final Map.Entry<String, Object> pair = (Map.Entry<String, Object>)it.next();
296-
final Object child = pair.getValue();
297-
final JsonElement childJsonElement = fieldJsonObject.get(pair.getKey().toString());
298-
// If the item is a valid Graph object, add its additional data
299-
addAdditionalDataFromJsonElementToJson(child, childJsonElement);
300-
}
301-
}
302-
// If the object is a list of Graph objects, iterate through elements
303-
else if (fieldObject instanceof List && fieldJsonElement.isJsonArray()) {
304-
final JsonArray fieldArrayValue = fieldJsonElement.getAsJsonArray();
305-
final List<?> fieldObjectList = (List<?>) fieldObject;
306-
for (int index = 0; index < fieldObjectList.size(); index++) {
307-
final Object item = fieldObjectList.get(index);
308-
final JsonElement itemJsonElement = fieldArrayValue.get(index);
309-
addAdditionalDataFromJsonElementToJson(item, itemJsonElement);
310-
}
311-
}
312-
// If the object is a valid Graph object, add its additional data
313-
addAdditionalDataFromJsonElementToJson(fieldObject, fieldJsonElement);
314-
} catch (IllegalArgumentException | IllegalAccessException e) {
315-
logger.logError("Unable to access child fields of " + serializableObject.getClass().getSimpleName(), e);
316-
}
317-
}
318-
}
240+
private void getChildAdditionalData(final Object serializableObject, final JsonElement outJson) {
241+
if(outJson == null || serializableObject == null || !outJson.isJsonObject())
242+
return;
243+
final JsonObject outJsonObject = outJson.getAsJsonObject();
244+
// Use reflection to iterate through fields for eligible Graph children
245+
for (java.lang.reflect.Field field : serializableObject.getClass().getFields()) {
246+
try {
247+
final Object fieldObject = field.get(serializableObject);
248+
final JsonElement fieldJsonElement = outJsonObject.get(field.getName());
249+
if(fieldObject == null || fieldJsonElement == null)
250+
continue;
251+
252+
// If the object is a HashMap, iterate through its children
253+
if (fieldObject instanceof Map && fieldJsonElement.isJsonObject()) {
254+
final Map<String, Object> serializableChildren = (Map<String, Object>) fieldObject;
255+
final Iterator<Entry<String, Object>> it = serializableChildren.entrySet().iterator();
256+
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();
257+
258+
while (it.hasNext()) {
259+
final Map.Entry<String, Object> pair = (Map.Entry<String, Object>)it.next();
260+
final Object child = pair.getValue();
261+
final JsonElement childJsonElement = fieldJsonObject.get(pair.getKey().toString());
262+
// If the item is a valid Graph object, add its additional data
263+
getChildAdditionalData(child, childJsonElement);
264+
}
265+
}
266+
// If the object is a list of Graph objects, iterate through elements
267+
else if (fieldObject instanceof List && fieldJsonElement.isJsonArray()) {
268+
final JsonArray fieldArrayValue = fieldJsonElement.getAsJsonArray();
269+
final List<?> fieldObjectList = (List<?>) fieldObject;
270+
for (int index = 0; index < fieldObjectList.size(); index++) {
271+
final Object item = fieldObjectList.get(index);
272+
final JsonElement itemJsonElement = fieldArrayValue.get(index);
273+
getChildAdditionalData(item, itemJsonElement);
274+
}
275+
} else if(fieldJsonElement.isJsonObject()) {
276+
// If the object is a valid Graph object, add its additional data
277+
final JsonObject fieldJsonObject = fieldJsonElement.getAsJsonObject();
278+
addAdditionalDataFromJsonObjectToJson(fieldObject, fieldJsonObject);
279+
}
280+
} catch (IllegalArgumentException | IllegalAccessException e) {
281+
logger.logError("Unable to access child fields of " + serializableObject.getClass().getSimpleName(), e);
282+
}
283+
}
284+
}
319285

320286
/**
321287
* Add each non-transient additional data property to the given JSON node
322288
*
323289
* @param item the object containing additional data
324-
* @param itemJsonElement the JSON node to add the additional data properties to
290+
* @param itemJsonObject the JSON node to add the additional data properties to
325291
*/
326-
private void addAdditionalDataFromJsonElementToJson (final Object item, final JsonElement itemJsonElement) {
327-
if (item instanceof IJsonBackedObject && itemJsonElement.isJsonObject()) {
328-
final JsonObject itemJsonObject = itemJsonElement.getAsJsonObject();
292+
private void addAdditionalDataFromJsonObjectToJson (final Object item, final JsonObject itemJsonObject) {
293+
if (item instanceof IJsonBackedObject && itemJsonObject != null) {
329294
final IJsonBackedObject serializableItem = (IJsonBackedObject) item;
330295
final AdditionalDataManager itemAdditionalData = serializableItem.additionalDataManager();
331296
addAdditionalDataFromManagerToJson(itemAdditionalData, itemJsonObject);

‎src/test/java/com/microsoft/graph/content/BatchRequestContentTest.java‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.junit.jupiter.api.Assertions.assertNotNull;
55
import static org.junit.jupiter.api.Assertions.assertNull;
66
import static org.junit.jupiter.api.Assertions.assertThrows;
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
78
import static org.mockito.ArgumentMatchers.any;
89
import static org.mockito.Mockito.mock;
910
import static org.mockito.Mockito.when;
@@ -14,6 +15,7 @@
1415
import java.util.ArrayList;
1516

1617
import com.google.gson.JsonObject;
18+
import com.google.gson.JsonPrimitive;
1719
import com.microsoft.graph.authentication.IAuthenticationProvider;
1820
import com.microsoft.graph.core.BaseClient;
1921
import com.microsoft.graph.core.IBaseClient;
@@ -37,12 +39,12 @@
3739
import okhttp3.Response;
3840
import okhttp3.ResponseBody;
3941

40-
public class BatchRequestContentTest {
42+
class BatchRequestContentTest {
4143

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

4446
@Test
45-
public void testBatchRequestContentCreation() throws MalformedURLException {
47+
void testBatchRequestContentCreation() throws MalformedURLException {
4648
BatchRequestContent requestContent = new BatchRequestContent();
4749
for (int i = 0; i < 5; i++) {
4850
IHttpRequest requestStep = mock(IHttpRequest.class);
@@ -53,7 +55,7 @@ public void testBatchRequestContentCreation() throws MalformedURLException {
5355
}
5456

5557
@Test
56-
public void testGetBatchRequestContent() throws MalformedURLException {
58+
void testGetBatchRequestContent() throws MalformedURLException {
5759
IHttpRequest requestStep = mock(IHttpRequest.class);
5860
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
5961
BatchRequestContent requestContent = new BatchRequestContent();
@@ -65,7 +67,7 @@ public void testGetBatchRequestContent() throws MalformedURLException {
6567
}
6668

6769
@Test
68-
public void testGetBatchRequestContentWithHeader() throws MalformedURLException {
70+
void testGetBatchRequestContentWithHeader() throws MalformedURLException {
6971
IHttpRequest requestStep = mock(IHttpRequest.class);
7072
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
7173
when(requestStep.getHeaders()).thenReturn(Arrays.asList(new HeaderOption("testkey", "testvalue")));
@@ -78,7 +80,7 @@ public void testGetBatchRequestContentWithHeader() throws MalformedURLException
7880
}
7981

8082
@Test
81-
public void testRemoveBatchRequesStepWithId() throws MalformedURLException {
83+
void testRemoveBatchRequesStepWithId() throws MalformedURLException {
8284
IHttpRequest requestStep = mock(IHttpRequest.class);
8385
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
8486
BatchRequestContent requestContent = new BatchRequestContent();
@@ -90,7 +92,7 @@ public void testRemoveBatchRequesStepWithId() throws MalformedURLException {
9092
}
9193

9294
@Test
93-
public void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws MalformedURLException {
95+
void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws MalformedURLException {
9496
IHttpRequest requestStep = mock(IHttpRequest.class);
9597
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
9698
BatchRequestContent requestContent = new BatchRequestContent();
@@ -109,7 +111,7 @@ public void testRemoveBatchRequesStepWithIdByAddingMultipleBatchSteps() throws M
109111
}
110112

111113
@Test
112-
public void defensiveProgrammingTests() {
114+
void defensiveProgrammingTests() {
113115
assertThrows(NullPointerException.class, () -> {
114116
new BatchRequestContent().addBatchRequestStep(null);
115117
}, "should throw argument exception");
@@ -135,7 +137,7 @@ public void defensiveProgrammingTests() {
135137
}
136138

137139
@Test
138-
public void executeBatchTest() throws Throwable {
140+
void executeBatchTest() throws Throwable {
139141
final BatchRequestContent content = new BatchRequestContent();
140142
IHttpRequest requestStep = mock(IHttpRequest.class);
141143
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
@@ -164,7 +166,7 @@ public void executeBatchTest() throws Throwable {
164166
}
165167

166168
@Test
167-
public void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException {
169+
void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException {
168170
IHttpRequest requestStep = mock(IHttpRequest.class);
169171
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
170172
when(requestStep.getHttpMethod()).thenReturn(HttpMethod.DELETE);
@@ -174,13 +176,14 @@ public void usesHttpMethodFromRequestIfAlreadySet() throws MalformedURLException
174176
}
175177

176178
@Test
177-
public void doesNotThrowWhenTryingToRemoveRequestFromNull() {
179+
void doesNotThrowWhenTryingToRemoveRequestFromNull() {
178180
final BatchRequestContent batchRequest = new BatchRequestContent();
179181
batchRequest.removeBatchRequestStepWithId("id");
182+
assertNull(batchRequest.requests);
180183
}
181184

182185
@Test
183-
public void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
186+
void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
184187
IHttpRequest requestStep = mock(IHttpRequest.class);
185188
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
186189
final BatchRequestContent batchRequest = new BatchRequestContent();
@@ -194,7 +197,7 @@ public void doesNotRemoveDependsOnWhenNotEmpty() throws MalformedURLException {
194197
}
195198

196199
@Test
197-
public void addsContentTypeForBodies() throws MalformedURLException {
200+
void addsContentTypeForBodies() throws MalformedURLException {
198201
IHttpRequest requestStep = mock(IHttpRequest.class);
199202
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
200203
final BatchRequestContent batchRequest = new BatchRequestContent();
@@ -235,4 +238,18 @@ public AdditionalDataManager additionalDataManager() {
235238
assertNotNull(step4.headers);
236239
assertEquals("application/octet-stream", step4.headers.get("content-type"));
237240
}
241+
@Test
242+
void serializesAdditionalData() throws MalformedURLException {
243+
IHttpRequest requestStep = mock(IHttpRequest.class);
244+
when(requestStep.getRequestUrl()).thenReturn(new URL(testurl));
245+
final BatchRequestContent batchRequest = new BatchRequestContent();
246+
final String bindValue = "https://somebindvalue";
247+
final BatchRequestTestBody body = new BatchRequestTestBody(); // using a dynamic implementation doesn't work as "this" maps to the current test class
248+
body.additionalDataManager().put("[email protected]", new JsonPrimitive(bindValue));
249+
batchRequest.addBatchRequestStep(requestStep, HttpMethod.POST, body);
250+
final ISerializer serializer = new DefaultSerializer(mock(ILogger.class));
251+
final String result = serializer.serializeObject(batchRequest);
252+
assertNotNull(result);
253+
assertTrue(result.contains(bindValue));
254+
}
238255
}

0 commit comments

Comments
 (0)