cloudant / java-cloudant

A Java client for Cloudant
Apache License 2.0
79 stars 68 forks source link

Type of `sinceSeq` can be also a String besides an Integer #527

Closed mojito317 closed 3 years ago

mojito317 commented 3 years ago

Checklist

Description

Fixes #526

Approach

seqs could be Integers in CouchDB 1.x and Strings in 2.x, so I changed all the places where they remained Integers to JsonElement. With this, both Integer and String can be used.

Security and Privacy

Testing

Monitoring and Logging

mojito317 commented 3 years ago

Nearly there, as discussed offline I think the seq things should preferably be in their own tests.

Done in 79abd33. Lmk if you would like to see more.

don't change the existing API in the ReplicatorDocument model class

Done in 103eb3b and 0c4e3c8.

  • changes entry

Done in 64770f4.

mojito317 commented 3 years ago

+1 just a couple nits

Thanks for the thorough review! I fixed the nits in f3018dd.

mojito317 commented 3 years ago

We should consider adding a negative test to assert that an error message is thrown when passing in an unexpected type.

An unexpected sinceSeq (e.g. a "true" or a "asd") will cause a crashing state in the replication and the replication will fail and timeout eventually in the waitForReplicatorToReachStatus method: https://github.com/cloudant/java-cloudant/blob/2c53b88a84906a3a47f844ce7c25078e11baee46/cloudant-client/src/test/java/com/cloudant/tests/util/Utils.java#L142

Meanwhile, the replication document will disappear from the _replicator database.

Other types cannot be added, because compilation would fail.

mojito317 commented 3 years ago

Thanks for the reviews @ricellis and @emlaver! I'm about to merge this!