Skip to content
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.

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

Replace Jackson parser with a JSONObject #351

Closed
rotemmiz opened this issue Oct 19, 2017 · 1 comment
Closed

Replace Jackson parser with a JSONObject #351

rotemmiz opened this issue Oct 19, 2017 · 1 comment

Comments

@rotemmiz
Copy link
Member

rotemmiz commented Oct 19, 2017

Our invocation protocol uses JSON to pass instructions from the node process (tester) to the device (testee).
The Android implementation of the invocation mechanism uses Jackson ObjectMapper to parse the JSON input automatically by mapping the JSON string into a POJO (meaning there’s no need to write custom JSON parsers). This was a very fast and easy implementation but using Jackson in a library project was a mistake, as mentioned in #334 , it may easily cause collisions with the users applications if they use Jackson in their projects (it's a pretty popular library).

We need to replace Jackson's parser, instead we can use the built in org.json.JSONObject and create manual parsers to parse the following objects.

Parsing the invocation objects is fully tested and should be pretty easy to and safe to replace.
The main part of the implementation is in JsonParser.

@DanielMSchmidt
Copy link
Contributor

DanielMSchmidt commented Nov 12, 2017

@rotemmiz I tried to solve this issue, and I think I was on a good track, and "finished" a part of the integration. When I wanted to try it out I got a strange result, JSONObject seems to always return null. I could reproduce is in two tests in a nice and small diff: 88f884d

I think I am missing a small point, but I can't find it 😭 Could anyone lend me an eye on this?

DanielMSchmidt added a commit that referenced this issue Nov 18, 2017
This allows us to eliminate this often painful dependency

Closes #351
@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants