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

Provide ResourceSyncParams based DownloadManager for syncing data. #1259

Merged
merged 5 commits into from
Mar 30, 2022

Conversation

aditya-07
Copy link
Collaborator

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

Fixes #1258

Description
Implemented ResourceParamsBasedDownloadManager which takes ResourceSyncParams to generate the search urls for the resources and parses Bundle SearchSet to get extract resources and next urls.

Alternative(s) considered
No

Type
Choose one: Code health

Screenshots (if applicable)

Checklist

  • I have read and acknowledged the Code of conduct.
  • [ x 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).

Copy link
Contributor

@omarismail94 omarismail94 left a comment

Choose a reason for hiding this comment

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

Nice change! Just one minor comment

@codecov
Copy link

codecov bot commented Mar 24, 2022

Codecov Report

Merging #1259 (fbb7534) into master (a292e50) will decrease coverage by 0.22%.
The diff coverage is 90.90%.

@@             Coverage Diff              @@
##             master    #1259      +/-   ##
============================================
- Coverage     84.15%   83.92%   -0.23%     
- Complexity      666      678      +12     
============================================
  Files           146      147       +1     
  Lines         10609    10622      +13     
  Branches        807      844      +37     
============================================
- Hits           8928     8915      -13     
+ Misses         1278     1271       -7     
- Partials        403      436      +33     
Impacted Files Coverage Δ
...c/main/java/com/google/android/fhir/sync/Config.kt 94.73% <ø> (+29.51%) ⬆️
...src/main/java/com/google/android/fhir/sync/Sync.kt 23.80% <ø> (ø)
...n/java/com/google/android/fhir/sync/SyncJobImpl.kt 53.48% <ø> (-4.66%) ⬇️
...download/ResourceParamsBasedDownloadWorkManager.kt 85.71% <85.71%> (ø)
...ava/com/google/android/fhir/sync/FhirSyncWorker.kt 93.47% <100.00%> (ø)
...a/com/google/android/fhir/sync/FhirSynchronizer.kt 79.31% <100.00%> (-3.45%) ⬇️
...oogle/android/fhir/sync/download/DownloaderImpl.kt 100.00% <100.00%> (ø)
...alidation/PrimitiveTypeAnswerMinLengthValidator.kt 70.00% <0.00%> (-20.00%) ⬇️
...com/google/android/fhir/datacapture/DataCapture.kt 66.66% <0.00%> (-16.67%) ⬇️
...main/java/com/google/android/fhir/DatastoreUtil.kt 80.00% <0.00%> (-10.00%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ddd8e1...fbb7534. Read the comment docs.

* [DownloadManager] implementation based on the provided [ResourceSyncParams] to generate
* [Resource] search queries and parse [Bundle.BundleType.SEARCHSET] type [Bundle].
*/
class ResourceParamsBasedDownloadManager(syncParams: ResourceSyncParams) : DownloadManager {
Copy link
Collaborator

Choose a reason for hiding this comment

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

do the following

  1. remove SyncConfiguration in Config.kt and clean up ParamMap.concatParams
  2. move ResourceSyncParams to this file
  3. rename ResourceSyncParams to ResourceSearchParams as they are actually search parameters and have nothing to do with uploading.

…orkManagaer apis. Also, review comment changes
@aditya-07 aditya-07 enabled auto-merge (squash) March 30, 2022 07:44
@aditya-07 aditya-07 merged commit 251a522 into master Mar 30, 2022
@aditya-07 aditya-07 deleted the ak/resourcesyncparam-based-downloadmanager branch March 30, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Provide Resource Sync map based DownloadManager implementation in the ENgine
3 participants