Refactoring UploadRequestGenerator for Upload Sync Redesign (google#2168

)

* refactoring for upload request generator

* spotless application

* spotless application

* added UploadRequestGeneratorFactory

* Spotless applications

* review comments

demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt

@@ -17,9 +17,9 @@
 package com.google.android.fhir.demo.data
 
 import com.google.android.fhir.demo.DemoDataStore
-import com.google.android.fhir.sync.DownloadRequest
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.SyncDataParams
+import com.google.android.fhir.sync.download.DownloadRequest
 import java.time.ZoneId
 import java.time.format.DateTimeFormatter
 import java.util.Date
@@ -98,14 +98,15 @@ class TimestampBasedDownloadWorkManagerImpl(private val dataStore: DemoDataStore
   }
 
   private suspend fun extractAndSaveLastUpdateTimestampToFetchFutureUpdates(
-    resources: List<Resource>
+    resources: List<Resource>,
   ) {
     resources
       .groupBy { it.resourceType }
-      .entries.map { map ->
+      .entries
+      .map { map ->
         dataStore.saveLastUpdatedTimestamp(
           map.key,
-          map.value.maxOfOrNull { it.meta.lastUpdated }?.toTimeZoneString() ?: ""
+          map.value.maxOfOrNull { it.meta.lastUpdated }?.toTimeZoneString() ?: "",
         )
       }
   }

engine/benchmark/src/androidTest/java/com/google/android/fhir/benchmark/FhirSyncWorkerBenchmark.kt

@@ -32,9 +32,9 @@ import com.google.android.fhir.FhirEngineConfiguration
 import com.google.android.fhir.FhirEngineProvider
 import com.google.android.fhir.ServerConfiguration
 import com.google.android.fhir.sync.AcceptRemoteConflictResolver
-import com.google.android.fhir.sync.DownloadRequest
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.FhirSyncWorker
+import com.google.android.fhir.sync.download.DownloadRequest
 import com.google.common.truth.Truth.assertThat
 import java.math.BigDecimal
 import java.util.LinkedList
@@ -77,13 +77,15 @@ class FhirSyncWorkerBenchmark {
 
   class BenchmarkTestOneTimeSyncWorker(
     private val appContext: Context,
-    workerParams: WorkerParameters
+    workerParams: WorkerParameters,
   ) : FhirSyncWorker(appContext, workerParams) {
 
     override fun getFhirEngine(): FhirEngine {
       return FhirEngineProvider.getInstance(appContext)
     }
+
     override fun getDownloadWorkManager(): DownloadWorkManager = BenchmarkTestDownloadManagerImpl()
+
     override fun getConflictResolver() = AcceptRemoteConflictResolver
   }
 
@@ -92,6 +94,7 @@ class FhirSyncWorkerBenchmark {
     private val urls = LinkedList(queries)
 
     override suspend fun getNextRequest() = urls.poll()?.let { DownloadRequest.of(it) }
+
     override suspend fun getSummaryRequestUrls(): Map<ResourceType, String> {
       return emptyMap()
     }
@@ -142,7 +145,7 @@ class FhirSyncWorkerBenchmark {
   private fun setupMockServerDispatcher(
     numberPatients: Int,
     numberObservations: Int,
-    numberEncounters: Int
+    numberEncounters: Int,
   ) {
     mockWebServer.dispatcher =
       object : Dispatcher() {
@@ -209,14 +212,17 @@ class FhirSyncWorkerBenchmark {
     return Patient().apply {
       id = patientId
       gender =
-        if (patientId.last().isDigit()) Enumerations.AdministrativeGender.FEMALE
-        else Enumerations.AdministrativeGender.MALE
+        if (patientId.last().isDigit()) {
+          Enumerations.AdministrativeGender.FEMALE
+        } else {
+          Enumerations.AdministrativeGender.MALE
+        }
       name =
         listOf(
           HumanName().apply {
             given = listOf(StringType("Test patient Name $patientId"))
             family = "Patient Family"
-          }
+          },
         )
       address =
         listOf(
@@ -227,7 +233,7 @@ class FhirSyncWorkerBenchmark {
             state = "Vic"
             postalCode = "postalCode"
             line = listOf(StringType("534 Erewhon St"))
-          }
+          },
         )
       contact =
         listOf(
@@ -242,11 +248,14 @@ class FhirSyncWorkerBenchmark {
                       family = "Patient Family"
                     }
                   gender =
-                    if (patientId.last().isDigit()) Enumerations.AdministrativeGender.MALE
-                    else Enumerations.AdministrativeGender.FEMALE
-                }
+                    if (patientId.last().isDigit()) {
+                      Enumerations.AdministrativeGender.MALE
+                    } else {
+                      Enumerations.AdministrativeGender.FEMALE
+                    }
+                },
               )
-          }
+          },
         )
       telecom =
         listOf(
@@ -259,7 +268,7 @@ class FhirSyncWorkerBenchmark {
             system = ContactPoint.ContactPointSystem.PHONE
             value = "(03) 3410 5613"
             use = ContactPoint.ContactPointUse.WORK
-          }
+          },
         )
     }
   }
@@ -298,17 +307,17 @@ class FhirSyncWorkerBenchmark {
                 code = "kPa"
                 system = "http://unitsofmeasure.org"
               }
-          }
+          },
         )
       interpretation =
         listOf(
           CodeableConcept(
             Coding(
               "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation",
               "H",
-              "high"
-            )
-          )
+              "high",
+            ),
+          ),
         )
     }
   }
@@ -328,7 +337,7 @@ class FhirSyncWorkerBenchmark {
           CodeableConcept().apply {
             coding =
               listOf(Coding("http://snomed.info/sct", "183807002", "Inpatient stay for nine days"))
-          }
+          },
         )
       subject = Reference("Patient/$patientId")
       episodeOfCare = listOf(Reference("Episode/${UUID.randomUUID()}"))
@@ -345,12 +354,12 @@ class FhirSyncWorkerBenchmark {
                       Coding(
                         "http://terminology.hl7.org/CodeSystem/v3-ParticipationType",
                         "PART",
-                        "Participant"
-                      )
+                        "Participant",
+                      ),
                     )
-                }
+                },
               )
-          }
+          },
         )
       diagnosis =
         listOf(
@@ -363,8 +372,8 @@ class FhirSyncWorkerBenchmark {
                     Coding(
                       "http://terminology.hl7.org/CodeSystem/diagnosis-role",
                       "AD",
-                      "Admission diagnosis"
-                    )
+                      "Admission diagnosis",
+                    ),
                   )
               }
           },
@@ -377,11 +386,11 @@ class FhirSyncWorkerBenchmark {
                     Coding(
                       "http://terminology.hl7.org/CodeSystem/diagnosis-role",
                       "DD",
-                      "Discharge diagnosis"
-                    )
+                      "Discharge diagnosis",
+                    ),
                   )
               }
-          }
+          },
         )
     }
   }
@@ -397,8 +406,8 @@ class FhirSyncWorkerBenchmark {
       FhirEngineProvider.init(
         FhirEngineConfiguration(
           serverConfiguration = ServerConfiguration("http://127.0.0.1:$mockServerPort/fhir/"),
-          testMode = true
-        )
+          testMode = true,
+        ),
       )
       mockWebServer.start(mockServerPort)
     }

engine/build.gradle.kts

@@ -145,14 +145,14 @@ tasks.dokkaHtml.configure {
       sourceLink {
         localDirectory.set(file("src/main/java"))
         remoteUrl.set(
-          URL("https://github.com/google/android-fhir/tree/master/engine/src/main/java")
+          URL("https://github.com/google/android-fhir/tree/master/engine/src/main/java"),
         )
         remoteLineSuffix.set("#L")
       }
       externalDocumentationLink {
         url.set(URL("https://hapifhir.io/hapi-fhir/apidocs/hapi-fhir-structures-r4/"))
         packageListUrl.set(
-          URL("https://hapifhir.io/hapi-fhir/apidocs/hapi-fhir-structures-r4/element-list")
+          URL("https://hapifhir.io/hapi-fhir/apidocs/hapi-fhir-structures-r4/element-list"),
         )
       }
     }

engine/src/main/java/com/google/android/fhir/sync/DataSource.kt

@@ -16,6 +16,8 @@
 
 package com.google.android.fhir.sync
 
+import com.google.android.fhir.sync.download.DownloadRequest
+import com.google.android.fhir.sync.upload.request.UploadRequest
 import org.hl7.fhir.r4.model.Bundle
 import org.hl7.fhir.r4.model.Bundle.BundleType
 import org.hl7.fhir.r4.model.OperationOutcome
@@ -28,8 +30,8 @@ internal interface DataSource {
 
   /**
    * @return [Bundle] of type [BundleType.TRANSACTIONRESPONSE] for a successful operation,
-   * [OperationOutcome] otherwise. Call this api with the [Bundle] that needs to be uploaded to the
-   * server.
+   *   [OperationOutcome] otherwise. Call this api with the [Bundle] that needs to be uploaded to
+   *   the server.
    */
   suspend fun upload(request: UploadRequest): Resource
 }

engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt

@@ -16,14 +16,15 @@
 
 package com.google.android.fhir.sync
 
+import com.google.android.fhir.sync.download.DownloadRequest
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.ResourceType
 
 /**
  * Manager that generates the FHIR requests and handles the FHIR responses of a download job.
  *
  * TODO(jingtang10): What happens after the end of a download job. Should a new download work
- * manager be created or should there be an API to restart a new download job.
+ *   manager be created or should there be an API to restart a new download job.
  */
 interface DownloadWorkManager {
   /**

engine/src/main/java/com/google/android/fhir/sync/Request.kt → engine/src/main/java/com/google/android/fhir/sync/download/DownloadRequest.kt

@@ -14,11 +14,9 @@
  * limitations under the License.
  */
 
-package com.google.android.fhir.sync
+package com.google.android.fhir.sync.download
 
 import org.hl7.fhir.r4.model.Bundle
-import org.hl7.fhir.r4.model.Resource
-import org.hl7.fhir.r4.model.codesystems.HttpVerb
 
 /**
  * Structure represents a request that can be made to download resources from the FHIR server. The
@@ -32,9 +30,11 @@ import org.hl7.fhir.r4.model.codesystems.HttpVerb
  *
  * The application developer may use a request like below to get an update on Patient/123 since it
  * was last downloaded.
+ *
  * ```
  *  Request.of("/Patient/123", mapOf("If-Modified-Since" to "knownLastUpdatedOfPatient123"))
  * ```
+ *
  * **BundleRequest**
  *
  * The application developer may use a request like below to download multiple resources in a single
@@ -84,30 +84,3 @@ internal constructor(val url: String, override val headers: Map<String, String>
 data class BundleDownloadRequest
 internal constructor(val bundle: Bundle, override val headers: Map<String, String> = emptyMap()) :
   DownloadRequest(headers)
-
-/**
- * Structure represents a request that can be made to upload resources/resource modifications to the
- * FHIR server.
- */
-sealed class UploadRequest(
-  open val url: String,
-  open val headers: Map<String, String> = emptyMap(),
-  open val resource: Resource,
-)
-
-/**
- * A FHIR [Bundle] based request for uploads. Multiple resources/resource modifications can be
- * uploaded as a single request using this.
- */
-data class BundleUploadRequest(
-  override val headers: Map<String, String> = emptyMap(),
-  override val resource: Bundle,
-) : UploadRequest(".", headers, resource)
-
-/** A [url] based FHIR request to upload resources to the server. */
-data class UrlUploadRequest(
-  val httpVerb: HttpVerb,
-  override val url: String,
-  override val resource: Resource,
-  override val headers: Map<String, String> = emptyMap()
-) : UploadRequest(url, headers, resource)

engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt

@@ -16,12 +16,9 @@
 
 package com.google.android.fhir.sync.download
 
-import com.google.android.fhir.sync.BundleDownloadRequest
 import com.google.android.fhir.sync.DataSource
-import com.google.android.fhir.sync.DownloadRequest
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.ResourceSyncException
-import com.google.android.fhir.sync.UrlDownloadRequest
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.flow
 import org.hl7.fhir.r4.model.Bundle

engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt

@@ -16,7 +16,6 @@
 
 package com.google.android.fhir.sync.download
 
-import com.google.android.fhir.sync.DownloadRequest
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.GREATER_THAN_PREFIX
 import com.google.android.fhir.sync.ParamMap

engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpDataSource.kt

@@ -18,19 +18,20 @@ package com.google.android.fhir.sync.remote
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.github.fge.jsonpatch.JsonPatch
-import com.google.android.fhir.sync.BundleDownloadRequest
-import com.google.android.fhir.sync.BundleUploadRequest
 import com.google.android.fhir.sync.DataSource
-import com.google.android.fhir.sync.DownloadRequest
-import com.google.android.fhir.sync.UploadRequest
-import com.google.android.fhir.sync.UrlDownloadRequest
-import com.google.android.fhir.sync.UrlUploadRequest
+import com.google.android.fhir.sync.download.BundleDownloadRequest
+import com.google.android.fhir.sync.download.DownloadRequest
+import com.google.android.fhir.sync.download.UrlDownloadRequest
+import com.google.android.fhir.sync.upload.request.BundleUploadRequest
+import com.google.android.fhir.sync.upload.request.UploadRequest
+import com.google.android.fhir.sync.upload.request.UrlUploadRequest
 import org.hl7.fhir.r4.model.Binary
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.codesystems.HttpVerb
 
 /**
  * Implementation of [DataSource] to sync data with the FHIR server using HTTP method calls.
+ *
  * @param fhirHttpService Http service to make requests to the server.
  */
 internal class FhirHttpDataSource(private val fhirHttpService: FhirHttpService) : DataSource {