Updated PerResourcePatchGenerator to return ordered PatchMapping to avoid referential integrity issues.

[aditya-07]

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #[issue number]

Description

1. Create a Directed Graph based on all the outgoing references for each Patch. Since the referential integrity comes to picture if the resource is not created yet, the edges of the graph are only the outgoing references to resources that are getting created during this upload. This may helps avoid some cycles as well.
2. The references are taken from LocalChangeResourceReferenceEntity which stores each outgoing references associated with a particular LocalChange as new entry.
3. Order the Patches so that if the resource A has outgoing references {B,C} (CREATE) and {D} (UPDATE), then B,C needs to go before the resource A so that referential integrity is retained. Order of D shouldn't matter for the purpose of referential integrity.

Alternative(s) considered

1. Splitting changes to CREATE first and subsequent UPDATE, but this would also require ordering the the create list incase it can overflow to multiple Bundles. This would be similar to the current approach but, would require multiple PUSH for the same resource incase the resource was Created and then Updated on the device before being pushed to the server.

Type Choose one: Bug fix

Screenshots (if applicable)

Checklist

• [ ] I have read and acknowledged the Code of conduct.
• [ ] I have read the Contributing page.
• [ ] I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
• [ ] I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
• [ ] I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
• [ ] I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
• [ ] I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

[MJ1998]

I feel this approach cannot solve for SingResource uploading strategy. Issue #2454.

But I also believe we should not discard this strategy as some fhir servers may not support bundle transaction and relying on SingleChange strategies can become expensive.

Currently for SingleResource upload strategy we fetch relevant LocalChanges from db, generate a patch and upload it. What if we fetch all LocalChanges irrespective of the strategy, generate patches, order the patches according to this PR and then based on the upload strategy create upload requests ?!

@jingtang10 @aditya-07

[jingtang10]

I think we can have a better design:- Crux: Instead of fetching LocalChanges we will fetch LocalChange.id as we don't need full LocalChange objects to create a toposort of Patches.

Steps:-

1. fetchLocalChangeIds(): List<LocalChangeId>
2. orderByResourceTypeAndId(): Map<ResourceTypeWithId, List<LocalChangeId>> set alias RTWI = ResourceTypeWithId set alias IDPatch = List<LocalChangeId> So orderByResourceTypeAndId(): Map<RTWI, IDPatch>
3. buildTopoSort(): List<Pair<RTWI, IDPatch>> (use database.getLocalChangeResourceReferenceIds)
4. Process this list based on UploadStrategy. For "single-resource" strategy process one by one, for "all-local-changes" process all of them at once. process = fetch whole of local changes: Flow<List<LocalChange>> Now our old approach
5. patchGenerator(List<LocalChange>>): List<Patch>
6. uploadRequestGenerator(List<Patch>): List<UploadRequest>

Pros:- This will significantly reduce the memory consumed by delaying the fetching of full LocalChange objects. Supports both upload strategies.

Con:- One caveat could be double database fetching. But fetching just the LocalChangeIds should not take much time as its LocalChangeEntity's PrimaryKey.

I think given we want to support offline, memory is a more concerning issue than time complexity to sync here. But without profiling the two designs agains a db it's difficult to say.

Also, maybe we can take this up as a new issue and a new PR.

thanks for this suggestion jajoo! i'd like to understand a bit better the exact savings of this approach. but yes we should do this in a separate issue.

[jingtang10]

looks good - if jajoo is ok with this you can merge this aditya!

thanks!