Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Sync API #1578

Merged
merged 19 commits into from
Nov 9, 2022
Merged

Refactor Sync API #1578

merged 19 commits into from
Nov 9, 2022

Conversation

jingtang10
Copy link
Collaborator

@jingtang10 jingtang10 commented Sep 4, 2022

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

Description

  1. Rename FhirPeriodicSyncWorker to FhirSyncWorker since it is now used by both periodic sync and one time sync
  2. Remove the SyncJob API and its implementation SyncJobImp since it is not the right level of abstraction we should be providing to the application.
  3. Use StateFlow to return the worker status for both one time and periodic work.
  4. Use the worker name to uniquely identify a job to allow multiple jobs to be scheduled.
  5. Stop leaking the internal result to external APIs and rename State to SyncJobStatus

Future work:

  1. Still need to emit the last sync timestamp for the application to listen to.

Alternative(s) considered
@aditya-07 and I have discussed the current approach and concluded the current API isn't easy to use by the client

Type
Code health

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).

@jingtang10
Copy link
Collaborator Author

@maimoonak FYI - the current sync APIs need some simplification. please see this pr. i think the main issue is that the sync job shouldn't be exposed to the client and information should be exposed as flow. we can discuss in more detail if you like.

@joiskash and @PallaviGanorkar fyi too

discussed this with @aditya-07

@jingtang10 jingtang10 changed the title Remove the SyncJob interface Refactor Sync API Oct 3, 2022
@jingtang10 jingtang10 mentioned this pull request Oct 10, 2022
7 tasks
@aditya-07 aditya-07 assigned jingtang10 and unassigned aditya-07 Oct 14, 2022
@jingtang10 jingtang10 assigned aditya-07 and unassigned jingtang10 Nov 3, 2022
@aditya-07 aditya-07 assigned jingtang10 and unassigned aditya-07 Nov 4, 2022
Copy link
Contributor

@aditya-07 aditya-07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jingtang10 jingtang10 merged commit c4ba736 into google:master Nov 9, 2022
@jingtang10 jingtang10 deleted the sync-api branch November 9, 2022 08:24
ktarasenko pushed a commit to ktarasenko/android-fhir that referenced this pull request Nov 19, 2022
* Remove the `SyncJob` interface

* Remove the `SyncJob` interface

* Run spotless

* Use worker's class name as the unique identifier for the job

* Refactor result API

* Run spotless apply

* Add API to retrieve last updated timestamp

* Uncomment accidentally commented code

* Add documentation to sync job status

* Use KEEP policy for periodic sync

* Wrap one time sync in a function

* Fix application context

* Run spotless

Co-authored-by: aditya-07 <[email protected]>
ktarasenko pushed a commit to ktarasenko/android-fhir that referenced this pull request Nov 19, 2022
* Remove the `SyncJob` interface

* Remove the `SyncJob` interface

* Run spotless

* Use worker's class name as the unique identifier for the job

* Refactor result API

* Run spotless apply

* Add API to retrieve last updated timestamp

* Uncomment accidentally commented code

* Add documentation to sync job status

* Use KEEP policy for periodic sync

* Wrap one time sync in a function

* Fix application context

* Run spotless

Co-authored-by: aditya-07 <[email protected]>
@jingtang10
Copy link
Collaborator Author

#1307

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

5 participants