mongodb / stitch-android-sdk

MongoDB Stitch Android SDK
Apache License 2.0
58 stars 33 forks source link

STITCH-2715 add support for watching streams with compact change events #143

Closed adamchel closed 5 years ago

adamchel commented 5 years ago

This PR took a lot longer than expected due to the fact that the generics were tricky to get working correctly without causing breaking changes for the existing watch API. The end result for users is relatively clean and mostly follows what we had before, but could probably still be improved. I tried to take advantage of type erasure in more places so that user-facing types aren't so verbose.

In any case, I think this all will be worth revisiting when we rework Java’s “watch” into a more user-friendly listener approach.

The return types for watch and watchCompact in each of the user-facing SDKs are as follows

// watch for android (type parameters changed slightly, should now be simpler)

Task<ChangeStream<Task<ChangeEvent<DocumentT>>>>

// watch for Java server (type parameters changed slightly, should not be simpler)
Task<ChangeStream<ChangeEvent<DocumentT>>>

// watchCompact for android

Task<ChangeStream<Task<CompactChangeEvent<DocumentT>>>>

// watchCompact for Java server

Task<ChangeStream<CompactChangeEvent<DocumentT>>>


As a note, some of the tests in this PR are not actually written by me, they were just moved around as part of the restructuring of change events. For this reason, they may not necessarily follow the “test one thing per test” principle. I did try to follow the principle when writing new tests, except for the two watch integration tests (happy to elaborate offline about why I did this).

The only other unaddressed thing in this PR was a weird discrepancy I was seeing between the way Android hashes object IDs vs how the backend server was hashing them. I’m going to investigate this further. I left a TODO and commented code in the watch integration tests where this was an issue, and I plan on addressing it before merging this PR.