Odata opaque cursor

[sadiqkhoja]

Closes #665

What has been done to verify that this works as intended?

Integration tests

Why is this the best possible solution? Were any other approaches considered?

• Base64 of a json object is used for skipToken. Fields of the json object depends on the odata table. In case of Submission, it is instanceId, for Submission subtable (repeat) it is repeatId (hash of the data - existing approach) and for Entities it is UUID
• I didn't compress the token, base64 encoding fulfills the requirement
• We are still fetching all the Submissions when data for the subtable is requested because we need to count the total repeating rows and return nextlink conditionally i.e. only when there are more rows to be returned. So pagination for subtable is not much of a help, it only saves bandwidth between server and client. I wish we could get rid of those two requirements then odata feed for subtable would be efficient as well.

Caveat
Since I am using hash ID of the repeat row and if the data of that repeat item is changed then subsequent nextlink will return nothing

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

None

Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.

TODO

Before submitting this PR, please make sure you have:
[ ] run make test-full and confirmed all checks still pass OR confirm CircleCI build passes
[ ] verified that any code from external sources are properly credited in comments or that everything is internally sourced

[matthew-white]

btoa() wouldn't work if the instance ID contained some Unicode characters. But we don't see such instance IDs in practice, so I don't think we should worry about it.

[matthew-white]

What do you think about prefixing this string with a version number in case we ever want to change the implementation of the cursor? That would give us an easy way to identify new- vs. old-format cursors. I'm thinking we could just prefix it with 01 for "version 01". I can't think of why we would change the implementation, but it seems appealing to give ourselves the flexibility to do so more easily. No strong feelings!

[matthew-white]

The strategy of using instanceId + __id makes sense to me. 👍 How long will this token be? Should we try to compress the data like we do with app user QR codes? (See CollectQr in Frontend.)

[sadiqkhoja]

I am thinking to use just repeatId for subtable and remove instanceId

[matthew-white]

Since I am using hash ID of the repeat row and if the data of that repeat item is changed then subsequent nextlink will return nothing

Could you say more about this? What data contributes to the hash ID? Is it all fields of the repeat group, such that any edit to the repeat group could change its ID? Or is it just the path (including index) of the repeat group?

[sadiqkhoja]

Could you say more about this? What data contributes to the hash ID? Is it all fields of the repeat group, such that any edit to the repeat group could change its ID? Or is it just the path (including index) of the repeat group?

It's an existing function lib/data/odata.js:hashId. The hash is calculated based on instanceId of the Submission and the fullpath of the repeat item like Submissions#double%%children#%%child#1.

I was wrong when I said "data of the repeat item" can change the hash, it is the index that can affect the hash.

Here is an example where there will be a problem:

• Submission has 10 repeat items
• User requests $top=5
• User deletes 6 repeat items
• Next odata request will return nothing because hashId will not match

Similar problem can arise if user manually deletes Submission because skipToken is based on instanceId and if instanceId is not found then cursor is pointing nowhere. However, probability of this happening for Submissions is quite low because we don't provide any way to delete individual Submissions.

In both cases, I don't see this being a big problem, just wanted to make note of it.

[matthew-white]

I ended up prioritizing reviewing getodk/central-frontend#851, and I ran out of time to review this one. Sorry about that! I think the only thought I want to offer is that I think it's OK if $skiptoken isn't 100% resilient to concurrent modification. As long as it returns an error when there's a problem and doesn't return the wrong data or incorrectly return no data, I think it's OK.