Skip to content

Prevent JSONObject/Array + Map/List combinations and unnecessary conversions back and forth #305

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

Jump to bottom
Merged
merged 1 commit into from
Dec 17, 2015
Merged

Prevent JSONObject/Array + Map/List combinations and unnecessary conversions back and forth #305

merged 1 commit into from
Dec 17, 2015

Conversation

grantland
Copy link
Contributor

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.

  • Convert ParseConfig JSON constructor to factory method
  • Simplify ParseConfig tests to utilize Maps/Lists
  • Do not convert Map -> JSONObject -> Map -> JSONObject for ParseAnalytics
  • Do not combine JSONObject/Array with Map/List for ParseRESTObjectBatchCommand
  • Do not combine Map & JSONObject for ParsePush
  • Do not allow encoding JSONObject/JSONArray

Note: This breaks compatibility with ParseCrashReporting

Depends on #303

@grantland grantland mentioned this pull request Dec 17, 2015
Merged
@grantland
Copy link
Contributor Author

/cc @hallucinogen @richardjrossiii

@hallucinogen
Copy link
Contributor

LGTM. Consistency + Performance ftw

@grantland grantland modified the milestone: 1.11.1 Dec 17, 2015
@grantland
Prevent JSONObject/Array + Map/List combinations and unnecessary conv…
cf58d03 
…ersions 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
grantland added a commit that referenced this pull request Dec 17, 2015
@grantland
Merge pull request #305 from ParsePlatform/grantland.json
d813767 
Prevent JSONObject/Array + Map/List combinations and unnecessary conversions back and forth
@grantland grantland merged commit d813767 into master Dec 17, 2015
@grantland grantland deleted the grantland.json branch December 17, 2015 20:27
@facebook-github-bot
Copy link

By analyzing the blame information on this pull request, we identified @grantland and @wangmengyan95 to be potential reviewers.

@yuzeh
Copy link

yuzeh commented Feb 16, 2016

This change broke backwards compatibility on CloudFunction calls when I was updating my app from 1.10.3 -- the values in the second argument of ParseCloud.callFunctionInBackground cannot be JSONObjects or JSONArrays.

What is the blessed way for encoding complex objects going forward? Is there a simple answer (e.g. "use java.util.Lists and java.util.Maps only", "use org.json objects only") to this question?

@yuzeh
Copy link

yuzeh commented Feb 19, 2016

@grantland curious if there is a good response to my question above. Thanks!

@grantland
Copy link
Contributor Author

@yuzeh I'd recommend using Java types such as List and Map.

@yuzeh
Copy link

yuzeh commented Feb 23, 2016

Great, thanks!

@facebook-github-bot
Copy link

@grantland updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@grantland grantland

Labels
None yet
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
5 participants
@grantland @hallucinogen @facebook-github-bot @yuzeh @wangmengyan95