search

Azure / azure-cosmosdb-java

Java Async SDK for SQL API of Azure Cosmos DB
MIT License
54 stars 61 forks source link

Replace org.json usage with Jackson #38

Closed chetanmeh closed 6 years ago

chetanmeh commented 6 years ago

This PR is an initial attempt to fix #29 as thats the only issue blocking CosmosDB support in OpenWhisk

As changes are across multiple files this PR only touches those parts which are required for the switch. To keep the changes at minimum certain possible optimizations (due to Jackson features) have yet not been implemented and can be done in a later PR

  1. Use JsonNode.at(path) to get node at path in JsonSerializable#getObjectByPath
  2. Optimize construction of POJO instances - Currently JsonSerializable serializes existing json instance to string and then construct the pojo instance. Possibly this can be replaced with deepCopy

Behavior change

  1. Use of single quotes - Some tests were using single quotes in json strings. They have been modified to use double quotes
  2. JsonSerializable now uses ObjectNode instead of JSONObject from org.json

Test Results

On my fork the test mostly pass. Seeing intermittent failure in (such failures also seen on master)

  1. BackPressureCrossPartitionTest.query
  2. ChangeFeedTest.changesFromParitionKeyRangeId_FromBeginning

@moderakh Please have a look

moderakh commented 6 years ago

@chetanmeh thanks for the PR. As there are breaking changes in the PR, our team is discussing the way forward with the breaking changes. We will get back to you.

chetanmeh commented 6 years ago

@moderakh I understand change is bigger here. So take your time and let me know if any aspect needs to be improved

srinathnarayanan commented 6 years ago

I've changed the base branch to dev instead of master. There seem to be some merge conflicts. Once you resolve them, I'll merge this PR

chetanmeh commented 6 years ago

Thanks @srinathnarayanan for all the review feedback.

I have rebased this branch to dev (via git rebase --onto origin/dev origin/master) and also swithed to Jackson API for some of the newly added classes like UniqueKey etc

moderakh commented 6 years ago

Thanks @chetanmeh and @srinathnarayanan for completing this 👍