Skip to content

Commit cf58d03

Browse files
grantlandgrantland
committed
Prevent JSONObject/Array + Map/List combinations and unnecessary conversions back and forth
We have a bunch of cases where we combine `JSONObject`/`JSONArray` and `Map`/`List` and a bunch of cases where unnecessarily convert `JSONObject` <-> `Map` and `JSONArray` <-> `List` back and forth. This PR simplifies things by removing `JSONObject` and `JSONArray` encoding so that we don't unnecessarily combine or convert back and forth. * Make encoding JSONObject/JSONArray illegal as we shouldn't be utilizing JSONObject/Array before encoding. * Encode JSONObject/Array on `ParseObject.put(key, value)` * Do not combine Map & JSONObject for ParsePush - `ParsePush#setData(data)` already accepts `JSONObject` parameter so we shouldn't use `Map<String ?>` to generate REST parameters that contain the `JSONObject` parameter just to convert it back to `JSONObject` * Do not combine JSONObject/Array with Map/List for ParseRESTObjectBatchCommand * Do not convert Map -> JSONObject -> Map -> JSONObject for ParseAnalytics * Simplify ParseConfig tests to utilize Maps/Lists - No reason to convert JSONObject/JSONArray <-> Map/List * Convert ParseConfig JSON constructor to factory method
1 parent e3f68fe commit cf58d03

16 files changed

+215
-290
lines changed

‎Parse/src/main/java/com/parse/ParseAnalytics.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import org.json.JSONException;
1414
import org.json.JSONObject;
1515

16+
import java.util.Collections;
17+
import java.util.HashMap;
1618
import java.util.LinkedHashMap;
1719
import java.util.Map;
1820

@@ -192,16 +194,15 @@ public static Task<Void> trackEventInBackground(final String name,
192194
if (name == null || name.trim().length() == 0) {
193195
throw new IllegalArgumentException("A name for the custom event must be provided.");
194196
}
195-
196-
final JSONObject jsonDimensions = dimensions != null
197-
? (JSONObject) NoObjectsEncoder.get().encode(dimensions)
197+
final Map<String, String> dimensionsCopy = dimensions != null
198+
? Collections.unmodifiableMap(new HashMap<>(dimensions))
198199
: null;
199200

200201
return ParseUser.getCurrentSessionTokenAsync().onSuccessTask(new Continuation<String, Task<Void>>() {
201202
@Override
202203
public Task<Void> then(Task<String> task) throws Exception {
203204
String sessionToken = task.getResult();
204-
return getAnalyticsController().trackEventInBackground(name, jsonDimensions, sessionToken);
205+
return getAnalyticsController().trackEventInBackground(name, dimensionsCopy, sessionToken);
205206
}
206207
});
207208
}

‎Parse/src/main/java/com/parse/ParseAnalyticsController.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
import org.json.JSONObject;
1212

13+
import java.util.Map;
14+
1315
import bolts.Task;
1416

1517
/** package */ class ParseAnalyticsController {
@@ -21,8 +23,8 @@ public ParseAnalyticsController(ParseEventuallyQueue eventuallyQueue) {
2123
}
2224

2325
public Task<Void> trackEventInBackground(final String name,
24-
JSONObject jsonDimensions, String sessionToken) {
25-
ParseRESTCommand command = ParseRESTAnalyticsCommand.trackEventCommand(name, jsonDimensions,
26+
Map<String, String> dimensions, String sessionToken) {
27+
ParseRESTCommand command = ParseRESTAnalyticsCommand.trackEventCommand(name, dimensions,
2628
sessionToken);
2729

2830
Task<JSONObject> eventuallyTask = eventuallyQueue.enqueueEventuallyAsync(command, null);

‎Parse/src/main/java/com/parse/ParseConfig.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package com.parse;
1010

1111
import org.json.JSONArray;
12-
import org.json.JSONException;
1312
import org.json.JSONObject;
1413

1514
import java.util.Collections;
@@ -104,13 +103,18 @@ public Task<ParseConfig> then(Task<Void> task) throws Exception {
104103
});
105104
}
106105

107-
/* package */ ParseConfig(JSONObject object, ParseDecoder decoder) {
108-
Map<String, Object> decodedObject = (Map<String, Object>) decoder.decode(object);
106+
@SuppressWarnings("unchecked")
107+
/* package */ static ParseConfig decode(JSONObject json, ParseDecoder decoder) {
108+
Map<String, Object> decodedObject = (Map<String, Object>) decoder.decode(json);
109109
Map<String, Object> decodedParams = (Map<String, Object>) decodedObject.get("params");
110110
if (decodedParams == null) {
111111
throw new RuntimeException("Object did not contain the 'params' key.");
112112
}
113-
this.params = Collections.unmodifiableMap(decodedParams);
113+
return new ParseConfig(decodedParams);
114+
}
115+
116+
/* package */ ParseConfig(Map<String, Object> params) {
117+
this.params = Collections.unmodifiableMap(params);
114118
}
115119

116120
/* package */ ParseConfig() {

‎Parse/src/main/java/com/parse/ParseConfigController.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public Task<ParseConfig> getAsync(String sessionToken) {
3535
public Task<ParseConfig> then(Task<JSONObject> task) throws Exception {
3636
JSONObject result = task.getResult();
3737

38-
final ParseConfig config = new ParseConfig(result, ParseDecoder.get());
38+
final ParseConfig config = ParseConfig.decode(result, ParseDecoder.get());
3939
return currentConfigController.setCurrentConfigAsync(config).continueWith(new Continuation<Void, ParseConfig>() {
4040
@Override
4141
public ParseConfig then(Task<Void> task) throws Exception {

‎Parse/src/main/java/com/parse/ParseCurrentConfigController.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public ParseConfig call() throws Exception {
6868
} catch (IOException | JSONException e) {
6969
return null;
7070
}
71-
return new ParseConfig(json, ParseDecoder.get());
71+
return ParseConfig.decode(json, ParseDecoder.get());
7272
}
7373

7474
/* package */ void clearCurrentConfigForTesting() {

‎Parse/src/main/java/com/parse/ParseEncoder.java‎

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import java.util.Collection;
1818
import java.util.Date;
19-
import java.util.Iterator;
2019
import java.util.List;
2120
import java.util.Map;
2221

@@ -29,11 +28,19 @@
2928
/** package */ abstract class ParseEncoder {
3029

3130
/* package */ static boolean isValidType(Object value) {
32-
return value instanceof JSONObject || value instanceof JSONArray || value instanceof String
33-
|| value instanceof Number || value instanceof Boolean || value == JSONObject.NULL
34-
|| value instanceof ParseObject || value instanceof ParseACL || value instanceof ParseFile
35-
|| value instanceof ParseGeoPoint || value instanceof Date || value instanceof byte[]
36-
|| value instanceof List || value instanceof Map || value instanceof ParseRelation;
31+
return value instanceof String
32+
|| value instanceof Number
33+
|| value instanceof Boolean
34+
|| value instanceof Date
35+
|| value instanceof List
36+
|| value instanceof Map
37+
|| value instanceof byte[]
38+
|| value == JSONObject.NULL
39+
|| value instanceof ParseObject
40+
|| value instanceof ParseACL
41+
|| value instanceof ParseFile
42+
|| value instanceof ParseGeoPoint
43+
|| value instanceof ParseRelation;
3744
}
3845

3946
public Object encode(Object object) {
@@ -92,17 +99,6 @@ public Object encode(Object object) {
9299
return json;
93100
}
94101

95-
if (object instanceof JSONObject) {
96-
JSONObject map = (JSONObject) object;
97-
JSONObject json = new JSONObject();
98-
Iterator<?> keys = map.keys();
99-
while (keys.hasNext()) {
100-
String key = (String) keys.next();
101-
json.put(key, encode(map.opt(key)));
102-
}
103-
return json;
104-
}
105-
106102
if (object instanceof Collection) {
107103
JSONArray array = new JSONArray();
108104
for (Object item : (Collection<?>) object) {
@@ -111,15 +107,6 @@ public Object encode(Object object) {
111107
return array;
112108
}
113109

114-
if (object instanceof JSONArray) {
115-
JSONArray array = (JSONArray) object;
116-
JSONArray json = new JSONArray();
117-
for (int i = 0; i < array.length(); ++i) {
118-
json.put(encode(array.opt(i)));
119-
}
120-
return json;
121-
}
122-
123110
if (object instanceof ParseRelation) {
124111
ParseRelation<?> relation = (ParseRelation<?>) object;
125112
return relation.encodeToJSON(this);

‎Parse/src/main/java/com/parse/ParseObject.java‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,6 +2918,14 @@ public void put(String key, Object value) {
29182918
throw new IllegalArgumentException("value may not be null.");
29192919
}
29202920

2921+
if (value instanceof JSONObject) {
2922+
ParseDecoder decoder = ParseDecoder.get();
2923+
value = decoder.convertJSONObjectToMap((JSONObject) value);
2924+
} else if (value instanceof JSONArray){
2925+
ParseDecoder decoder = ParseDecoder.get();
2926+
value = decoder.convertJSONArrayToList((JSONArray) value);
2927+
}
2928+
29212929
if (!ParseEncoder.isValidType(value)) {
29222930
throw new IllegalArgumentException("invalid type for value: " + value.getClass().toString());
29232931
}
@@ -3166,12 +3174,6 @@ public JSONArray getJSONArray(String key) {
31663174
public <T> List<T> getList(String key) {
31673175
synchronized (mutex) {
31683176
Object value = estimatedData.get(key);
3169-
3170-
if (value instanceof JSONArray) {
3171-
ParseDecoder decoder = ParseDecoder.get();
3172-
value = decoder.convertJSONArrayToList((JSONArray) value);
3173-
}
3174-
31753177
if (!(value instanceof List)) {
31763178
return null;
31773179
}
@@ -3192,12 +3194,6 @@ public <T> List<T> getList(String key) {
31923194
public <V> Map<String, V> getMap(String key) {
31933195
synchronized (mutex) {
31943196
Object value = estimatedData.get(key);
3195-
3196-
if (value instanceof JSONObject) {
3197-
ParseDecoder decoder = ParseDecoder.get();
3198-
value = decoder.convertJSONObjectToMap((JSONObject) value);
3199-
}
3200-
32013197
if (!(value instanceof Map)) {
32023198
return null;
32033199
}

‎Parse/src/main/java/com/parse/ParseRESTAnalyticsCommand.java‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212

1313
import com.parse.http.ParseHttpRequest;
1414

15+
import org.json.JSONException;
1516
import org.json.JSONObject;
1617

1718
import java.util.Date;
18-
import java.util.HashMap;
1919
import java.util.Map;
2020

2121
/** package */ class ParseRESTAnalyticsCommand extends ParseRESTCommand {
@@ -26,43 +26,46 @@
2626
// Tracks the AppOpened event
2727
/* package for test */ static final String EVENT_APP_OPENED = "AppOpened";
2828

29-
public ParseRESTAnalyticsCommand(
30-
String httpPath,
31-
ParseHttpRequest.Method httpMethod,
32-
Map<String, ?> parameters,
33-
String sessionToken) {
34-
super(httpPath, httpMethod, parameters, sessionToken);
35-
}
29+
private static final String PATH = "events/%s";
3630

37-
public static ParseRESTAnalyticsCommand trackAppOpenedCommand(String pushHash,
38-
String sessionToken) {
39-
Map<String, String> parameters = null;
40-
if (pushHash != null) {
41-
parameters = new HashMap<>();
42-
parameters.put("push_hash", pushHash);
43-
}
44-
return trackEventCommand(EVENT_APP_OPENED, parameters, sessionToken);
31+
private static final String KEY_AT = "at";
32+
private static final String KEY_PUSH_HASH = "push_hash";
33+
private static final String KEY_DIMENSIONS = "dimensions";
34+
35+
public static ParseRESTAnalyticsCommand trackAppOpenedCommand(
36+
String pushHash, String sessionToken) {
37+
return trackEventCommand(EVENT_APP_OPENED, pushHash, null, sessionToken);
4538
}
4639

47-
public static ParseRESTAnalyticsCommand trackEventCommand(String eventName, JSONObject dimensions,
48-
String sessionToken) {
49-
Map<String, Object> parameters = null;
50-
if (dimensions != null) {
51-
parameters = new HashMap<>();
52-
parameters.put("dimensions", dimensions);
53-
}
54-
return trackEventCommand(eventName, parameters, sessionToken);
40+
public static ParseRESTAnalyticsCommand trackEventCommand(
41+
String eventName, Map<String, String> dimensions, String sessionToken) {
42+
return trackEventCommand(eventName, null, dimensions, sessionToken);
5543
}
5644

57-
/* package */ static ParseRESTAnalyticsCommand trackEventCommand(String eventName,
58-
Map<String, ?> parameters, String sessionToken) {
59-
String httpPath = String.format("events/%s", Uri.encode(eventName));
60-
Map<String, Object> commandParameters = new HashMap<>();
61-
if (parameters != null) {
62-
commandParameters.putAll(parameters);
45+
/* package */ static ParseRESTAnalyticsCommand trackEventCommand(
46+
String eventName, String pushHash, Map<String, String> dimensions, String sessionToken) {
47+
String httpPath = String.format(PATH, Uri.encode(eventName));
48+
JSONObject parameters = new JSONObject();
49+
try {
50+
parameters.put(KEY_AT, NoObjectsEncoder.get().encode(new Date()));
51+
if (pushHash != null) {
52+
parameters.put(KEY_PUSH_HASH, pushHash);
53+
}
54+
if (dimensions != null) {
55+
parameters.put(KEY_DIMENSIONS, NoObjectsEncoder.get().encode(dimensions));
56+
}
57+
} catch (JSONException e) {
58+
throw new RuntimeException(e);
6359
}
64-
commandParameters.put("at", NoObjectsEncoder.get().encode(new Date()));
6560
return new ParseRESTAnalyticsCommand(
66-
httpPath, ParseHttpRequest.Method.POST, commandParameters, sessionToken);
61+
httpPath, ParseHttpRequest.Method.POST, parameters, sessionToken);
62+
}
63+
64+
public ParseRESTAnalyticsCommand(
65+
String httpPath,
66+
ParseHttpRequest.Method httpMethod,
67+
JSONObject parameters,
68+
String sessionToken) {
69+
super(httpPath, httpMethod, parameters, sessionToken);
6770
}
6871
}

‎Parse/src/main/java/com/parse/ParseRESTCommand.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public ParseRESTCommand(
137137
sessionToken);
138138
}
139139

140-
protected ParseRESTCommand(
140+
public ParseRESTCommand(
141141
String httpPath,
142142
ParseHttpRequest.Method httpMethod,
143143
JSONObject jsonParameters,

‎Parse/src/main/java/com/parse/ParseRESTObjectBatchCommand.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import java.io.IOException;
1919
import java.io.InputStream;
2020
import java.util.ArrayList;
21-
import java.util.HashMap;
2221
import java.util.List;
23-
import java.util.Map;
2422

2523
import bolts.Continuation;
2624
import bolts.Task;
@@ -59,7 +57,8 @@ public static List<Task<JSONObject>> executeBatch(
5957
tasks.add(tcs.getTask());
6058
}
6159

62-
List<JSONObject> requests = new ArrayList<>(batchSize);
60+
JSONObject parameters = new JSONObject();
61+
JSONArray requests = new JSONArray();
6362
try {
6463
for (ParseRESTObjectCommand command : commands) {
6564
JSONObject requestParameters = new JSONObject();
@@ -69,14 +68,13 @@ public static List<Task<JSONObject>> executeBatch(
6968
if (body != null) {
7069
requestParameters.put("body", body);
7170
}
72-
requests.add(requestParameters);
71+
requests.put(requestParameters);
7372
}
73+
parameters.put("requests", requests);
7474
} catch (JSONException e) {
7575
throw new RuntimeException(e);
7676
}
7777

78-
Map<String, List<JSONObject>> parameters = new HashMap<>();
79-
parameters.put("requests", requests);
8078
ParseRESTCommand command = new ParseRESTObjectBatchCommand(
8179
"batch", ParseHttpRequest.Method.POST, parameters, sessionToken);
8280

@@ -132,7 +130,7 @@ public Void then(Task<JSONObject> task) throws Exception {
132130
private ParseRESTObjectBatchCommand(
133131
String httpPath,
134132
ParseHttpRequest.Method httpMethod,
135-
Map<String, ?> parameters,
133+
JSONObject parameters,
136134
String sessionToken) {
137135
super(httpPath, httpMethod, parameters, sessionToken);
138136
}

0 commit comments

Comments
 (0)