Skip to content

Commit 96fa18d

Browse files
baywetbaywet
committed
- adds defensive programming
- switches to json objects to avoid too much string manipulations - fixes a bug where the service root would be appended twice - adds missing unit tests
1 parent 7b54d3b commit 96fa18d

File tree

6 files changed

+212
-70
lines changed

6 files changed

+212
-70
lines changed

‎src/main/java/com/microsoft/graph/content/MSBatchRequestContent.java

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package com.microsoft.graph.content;
22

33
import java.io.IOException;
4-
import java.util.Arrays;
4+
import java.util.HashSet;
55
import java.util.LinkedHashMap;
66
import java.util.List;
77
import java.util.Map;
@@ -43,20 +43,14 @@ public class MSBatchRequestContent {
4343
*
4444
* @param batchRequestStepsArray List of batch steps for batching
4545
*/
46-
public MSBatchRequestContent(@Nonnull final List<MSBatchRequestStep> batchRequestStepsArray) {
47-
if (batchRequestStepsArray.size() > MAX_NUMBER_OF_REQUESTS)
46+
public MSBatchRequestContent(@Nonnull final MSBatchRequestStep... batchRequestStepsArray) {
47+
if (batchRequestStepsArray.length > MAX_NUMBER_OF_REQUESTS)
4848
throw new IllegalArgumentException("Number of batch request steps cannot exceed " + MAX_NUMBER_OF_REQUESTS);
4949

5050
this.batchRequestStepsHashMap = new LinkedHashMap<>();
5151
for (final MSBatchRequestStep requestStep : batchRequestStepsArray)
52-
addBatchRequestStep(requestStep);
53-
}
54-
55-
/**
56-
* Creates empty batch request content
57-
*/
58-
public MSBatchRequestContent() {
59-
this.batchRequestStepsHashMap = new LinkedHashMap<>();
52+
if(requestStep != null)
53+
addBatchRequestStep(requestStep);
6054
}
6155

6256
/**
@@ -66,6 +60,8 @@ public MSBatchRequestContent() {
6660
* given
6761
*/
6862
public boolean addBatchRequestStep(@Nonnull final MSBatchRequestStep batchRequestStep) {
63+
if(batchRequestStep == null)
64+
throw new IllegalArgumentException("batchRequestStep parameter cannot be null");
6965
if (batchRequestStepsHashMap.containsKey(batchRequestStep.getRequestId()) ||
7066
batchRequestStepsHashMap.size() >= MAX_NUMBER_OF_REQUESTS)
7167
return false;
@@ -81,11 +77,13 @@ public boolean addBatchRequestStep(@Nonnull final MSBatchRequestStep batchReques
8177
*/
8278
@Nonnull
8379
public String addBatchRequestStep(@Nonnull final Request request, @Nullable final String... arrayOfDependsOnIds) {
80+
if(request == null)
81+
throw new IllegalArgumentException("request parameter cannot be null");
8482
String requestId;
8583
do {
8684
requestId = Integer.toString(ThreadLocalRandom.current().nextInt(1, Integer.MAX_VALUE));
8785
} while(batchRequestStepsHashMap.keySet().contains(requestId));
88-
if(addBatchRequestStep(new MSBatchRequestStep(requestId, request, Arrays.asList(arrayOfDependsOnIds))))
86+
if(addBatchRequestStep(new MSBatchRequestStep(requestId, request, arrayOfDependsOnIds)))
8987
return requestId;
9088
else
9189
throw new IllegalArgumentException("unable to add step to batch. Number of batch request steps cannot exceed " + MAX_NUMBER_OF_REQUESTS);
@@ -103,29 +101,30 @@ public boolean removeBatchRequestStepWithId(@Nonnull final String requestId) {
103101
batchRequestStepsHashMap.remove(requestId);
104102
removed = true;
105103
for (final Map.Entry<String, MSBatchRequestStep> steps : batchRequestStepsHashMap.entrySet()) {
106-
if (steps.getValue() != null && steps.getValue().getArrayOfDependsOnIds() != null) {
107-
while (steps.getValue().getArrayOfDependsOnIds().remove(requestId))
104+
if (steps.getValue() != null && steps.getValue().getDependsOnIds() != null) {
105+
while (steps.getValue().getDependsOnIds().remove(requestId))
108106
;
109107
}
110108
}
111109
}
112110
return removed;
113111
}
114112

115-
/**
116-
* @return Batch request content's json as String
117-
*/
118-
@Nonnull
119-
public String getBatchRequestContent() {
113+
private JsonObject getBatchRequestContentAsJson() {
120114
final JsonObject batchRequestContentMap = new JsonObject();
121115
final JsonArray batchContentArray = new JsonArray();
122116
for (final Map.Entry<String, MSBatchRequestStep> requestStep : batchRequestStepsHashMap.entrySet()) {
123117
batchContentArray.add(getBatchRequestObjectFromRequestStep(requestStep.getValue()));
124118
}
125119
batchRequestContentMap.add("requests", batchContentArray);
126-
127-
final String content = batchRequestContentMap.toString();
128-
return content;
120+
return batchRequestContentMap;
121+
}
122+
/**
123+
* @return Batch request content's json as String
124+
*/
125+
@Nonnull
126+
public String getBatchRequestContent() {
127+
return getBatchRequestContentAsJson().toString();
129128
}
130129

131130
/**
@@ -135,12 +134,14 @@ public String getBatchRequestContent() {
135134
* @throws ClientException when the batch couldn't be executed because of client issues.
136135
*/
137136
@Nonnull
138-
public MSBatchResponseContent execute(@Nonnull final IBaseClient client) throws ClientException {
139-
try {
140-
return executeAsync(client).get();
141-
} catch (Exception ex) {
142-
throw new ClientException("Batch failed to execute", ex);
143-
}
137+
public MSBatchResponseContent execute(@Nonnull final IBaseClient client) {
138+
final JsonObject content = getBatchRequestContentAsJson();
139+
return new MSBatchResponseContent(client.getServiceRoot() + "/",
140+
content,
141+
client.customRequest("/$batch")
142+
.buildRequest()
143+
.post(content)
144+
.getAsJsonObject());
144145
}
145146
/**
146147
* Executes the batch requests asynchronously and returns the response
@@ -152,11 +153,11 @@ public CompletableFuture<MSBatchResponseContent> executeAsync(@Nonnull final IBa
152153
if(client == null) {
153154
throw new IllegalArgumentException("client parameter cannot be null");
154155
}
155-
final String content = getBatchRequestContent();
156-
return client.customRequest(client.getServiceRoot() + "/$batch", String.class)
156+
final JsonObject content = getBatchRequestContentAsJson();
157+
return client.customRequest("/$batch")
157158
.buildRequest()
158159
.postAsync(content)
159-
.thenApply(resp -> new MSBatchResponseContent(client.getServiceRoot() + "/", content, resp));
160+
.thenApply(resp -> new MSBatchResponseContent(client.getServiceRoot() + "/", content, resp.getAsJsonObject()));
160161
}
161162

162163
private static final Pattern protocolAndHostReplacementPattern = Pattern.compile("(?i)^http[s]?:\\/\\/graph\\.microsoft\\.com\\/(?>v1\\.0|beta)\\/?"); // (?i) case insensitive
@@ -180,9 +181,9 @@ private JsonObject getBatchRequestObjectFromRequestStep(final MSBatchRequestStep
180181
contentmap.add("headers", headerMap);
181182
}
182183

183-
final List<String> arrayOfDependsOnIds = batchRequestStep.getArrayOfDependsOnIds();
184+
final HashSet<String> arrayOfDependsOnIds = batchRequestStep.getDependsOnIds();
184185
if (arrayOfDependsOnIds != null) {
185-
final JsonArray array = new JsonArray();
186+
final JsonArray array = new JsonArray(arrayOfDependsOnIds.size());
186187
for (final String dependsOnId : arrayOfDependsOnIds)
187188
array.add(dependsOnId);
188189
contentmap.add("dependsOn", array);

‎src/main/java/com/microsoft/graph/content/MSBatchRequestStep.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.microsoft.graph.content;
22

3-
import java.util.List;
3+
import java.util.Arrays;
4+
import java.util.HashSet;
45

56
import javax.annotation.Nullable;
67
import javax.annotation.Nonnull;
@@ -13,25 +14,25 @@
1314
public class MSBatchRequestStep {
1415
private String requestId;
1516
private Request request;
16-
private List<String> arrayOfDependsOnIds;
17+
private HashSet<String> dependsOnIds;
1718

1819
/**
1920
* Initializes a batch step from a raw HTTP request
2021
* @param requestId the id to assign to this step
2122
* @param request the request to send in the batch
2223
* @param arrayOfDependsOnIds the ids of steps this step depends on
2324
*/
24-
public MSBatchRequestStep(@Nonnull final String requestId, @Nonnull final Request request, @Nullable final List<String> arrayOfDependsOnIds) {
25+
public MSBatchRequestStep(@Nonnull final String requestId, @Nonnull final Request request, @Nullable final String... arrayOfDependsOnIds) {
2526
if(requestId == null)
2627
throw new IllegalArgumentException("Request Id cannot be null.");
2728
if(requestId.length() == 0)
2829
throw new IllegalArgumentException("Request Id cannot be empty.");
2930
if(request == null)
30-
new IllegalArgumentException("Request cannot be null.");
31+
throw new IllegalArgumentException("Request cannot be null.");
3132

3233
this.requestId = requestId;
3334
this.request = request;
34-
this.arrayOfDependsOnIds = arrayOfDependsOnIds;
35+
this.dependsOnIds = new HashSet<>(Arrays.asList(arrayOfDependsOnIds));
3536
}
3637

3738
/**
@@ -57,7 +58,7 @@ public Request getRequest() {
5758
* @return the list of steps this step depends on
5859
*/
5960
@Nullable
60-
public List<String> getArrayOfDependsOnIds(){
61-
return arrayOfDependsOnIds;
61+
public HashSet<String> getDependsOnIds(){
62+
return dependsOnIds;
6263
}
6364
}

‎src/main/java/com/microsoft/graph/content/MSBatchResponseContent.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ public MSBatchResponseContent(@Nullable final Response batchResponse) {
4848
* @param batchRequestData the batch request payload data as a JSON string
4949
* @param batchResponseData the batch response body as a JSON string
5050
*/
51-
protected MSBatchResponseContent(@Nonnull final String baseUrl, @Nonnull final String batchRequestData, @Nonnull final String batchResponseData) {
51+
protected MSBatchResponseContent(@Nonnull final String baseUrl, @Nonnull final JsonObject batchRequestData, @Nonnull final JsonObject batchResponseData) {
5252
this.protocol = Protocol.HTTP_1_1;
5353
this.message = "OK";
54-
final Map<String, Request> requestMap = createBatchRequestsHashMap(baseUrl, JsonParser.parseString(batchRequestData).getAsJsonObject());
54+
final Map<String, Request> requestMap = createBatchRequestsHashMap(baseUrl, batchRequestData);
5555
if (requestMap != null)
5656
batchRequestsHashMap.putAll(requestMap);
5757
updateFromResponseBody(batchResponseData);
@@ -168,17 +168,15 @@ public void update(@Nonnull final Response batchResponse) {
168168
try {
169169
final String batchResponseData = batchResponse.body().string();
170170
if (batchResponseData != null) {
171-
updateFromResponseBody(batchResponseData);
171+
updateFromResponseBody(stringToJSONObject(batchResponseData));
172172
}
173173
} catch (final IOException e) {
174174
e.printStackTrace();
175175
}
176176
}
177177
}
178-
private void updateFromResponseBody(@Nonnull final String batchResponseData) {
179-
final JsonObject batchResponseObj = stringToJSONObject(batchResponseData);
178+
private void updateFromResponseBody(@Nonnull final JsonObject batchResponseObj) {
180179
if (batchResponseObj != null) {
181-
182180
final JsonElement nextLinkElement = batchResponseObj.get("@odata.nextLink");
183181
if (nextLinkElement != null && nextLinkElement.isJsonPrimitive())
184182
nextLink = nextLinkElement.getAsString();

0 commit comments

Comments
 (0)