Merge branch 'master' into sp/periodic-sync-separate-screen

demo/src/main/java/com/google/android/fhir/demo/HomeFragment.kt

@@ -45,7 +45,7 @@ class HomeFragment : Fragment(R.layout.fragment_home) {
       findNavController().navigate(HomeFragmentDirections.actionHomeFragmentToPatientList())
     }
     requireView().findViewById<CardView>(R.id.item_sync).setOnClickListener {
-      findNavController().navigate(HomeFragmentDirections.actionHomeFragmentToManualSyncFragment())
+      findNavController().navigate(HomeFragmentDirections.actionHomeFragmentToSyncFragment())
     }
     requireView().findViewById<CardView>(R.id.item_periodic_sync).setOnClickListener {
       findNavController()

demo/src/main/java/com/google/android/fhir/demo/PeriodicSyncFragment.kt

@@ -75,41 +75,48 @@ class PeriodicSyncFragment : Fragment() {
   private fun periodicSyncJobStatus(periodicSyncJobStatus: PeriodicSyncJobStatus) {
     // Handle last sync job status and update UI
     periodicSyncJobStatus.lastSyncJobStatus?.let { lastStatus ->
-      val lastSyncStatusValue = when (lastStatus) {
-        is LastSyncJobStatus.Succeeded -> getString(R.string.last_sync_status, LastSyncJobStatus.Succeeded::class.java.simpleName)
-        is LastSyncJobStatus.Failed -> getString(R.string.last_sync_status, LastSyncJobStatus.Failed::class.java.simpleName)
-        else -> null
-      }
+      val lastSyncStatusValue =
+        when (lastStatus) {
+          is LastSyncJobStatus.Succeeded ->
+            getString(R.string.last_sync_status, LastSyncJobStatus.Succeeded::class.java.simpleName)
+          is LastSyncJobStatus.Failed ->
+            getString(R.string.last_sync_status, LastSyncJobStatus.Failed::class.java.simpleName)
+          else -> null
+        }
 
       lastSyncStatusValue?.let { statusText ->
         requireView().findViewById<TextView>(R.id.last_sync_status).text = statusText
-        requireView().findViewById<TextView>(R.id.last_sync_time).text = getString(
-          R.string.last_sync_timestamp,
-          syncFragmentViewModel.formatSyncTimestamp(lastStatus.timestamp)
-        )
+        requireView().findViewById<TextView>(R.id.last_sync_time).text =
+          getString(
+            R.string.last_sync_timestamp,
+            syncFragmentViewModel.formatSyncTimestamp(lastStatus.timestamp),
+          )
       }
     }
 
     // Set current sync status
     val currentSyncStatusTextView = requireView().findViewById<TextView>(R.id.current_sync_status)
-    currentSyncStatusTextView.text = getString(
-      R.string.current_status,
-      periodicSyncJobStatus.currentSyncJobStatus::class.java.simpleName
-    )
+    currentSyncStatusTextView.text =
+      getString(
+        R.string.current_status,
+        periodicSyncJobStatus.currentSyncJobStatus::class.java.simpleName,
+      )
 
     // Update progress indicator visibility based on current sync status
     val syncIndicator = requireView().findViewById<ProgressBar>(R.id.sync_indicator)
-    syncIndicator.visibility = if (periodicSyncJobStatus.currentSyncJobStatus is CurrentSyncJobStatus.Running) {
-      View.VISIBLE
-    } else {
-      View.GONE
-    }
+    syncIndicator.visibility =
+      if (periodicSyncJobStatus.currentSyncJobStatus is CurrentSyncJobStatus.Running) {
+        View.VISIBLE
+      } else {
+        View.GONE
+      }
 
     // Control the visibility of the current sync status text view
-    currentSyncStatusTextView.visibility = when (periodicSyncJobStatus.currentSyncJobStatus) {
-      is CurrentSyncJobStatus.Failed, is CurrentSyncJobStatus.Succeeded -> View.GONE
-      else -> View.VISIBLE
-    }
+    currentSyncStatusTextView.visibility =
+      when (periodicSyncJobStatus.currentSyncJobStatus) {
+        is CurrentSyncJobStatus.Failed,
+        is CurrentSyncJobStatus.Succeeded, -> View.GONE
+        else -> View.VISIBLE
+      }
   }
-
 }

demo/src/main/res/navigation/reference_nav_graph.xml

@@ -22,8 +22,8 @@
             app:destination="@id/patient_list"
         />
     <action
-            android:id="@+id/action_home_fragment_to_manual_sync_fragment"
-            app:destination="@id/manualSyncFragment"
+            android:id="@+id/action_home_fragment_to_sync_fragment"
+            app:destination="@id/syncFragment"
         />
     <action
             android:id="@+id/action_home_fragment_to_periodic_sync_fragment"
@@ -89,7 +89,7 @@
     </fragment>
 
   <fragment
-        android:id="@+id/manualSyncFragment"
+        android:id="@+id/syncFragment"
         android:name="com.google.android.fhir.demo.SyncFragment"
         android:label="fragment_manual_sync"
         tools:layout="@layout/sync"
@@ -101,5 +101,4 @@
         android:label="fragment_periodic_sync"
         tools:layout="@layout/periodic_sync"
     />
-
 </navigation>

engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt

@@ -559,8 +559,8 @@ class DatabaseImplTest {
     // Delete the patient created in setup as we only want to upload the patient in this test
     database.deleteUpdates(listOf(TEST_PATIENT_1))
     services.fhirEngine
-      .syncUpload(AllChangesSquashedBundlePut) {
-        it
+      .syncUpload(AllChangesSquashedBundlePut) { lcs, _ ->
+        lcs
           .first { it.resourceId == "remote-patient-3" }
           .let {
             flowOf(

engine/src/main/java/com/google/android/fhir/FhirEngine.kt

@@ -16,6 +16,7 @@
 
 package com.google.android.fhir
 
+import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.db.ResourceNotFoundException
 import com.google.android.fhir.search.Search
 import com.google.android.fhir.sync.ConflictResolver
@@ -121,7 +122,10 @@ interface FhirEngine {
   @Deprecated("To be deprecated.")
   suspend fun syncUpload(
     uploadStrategy: UploadStrategy,
-    upload: (suspend (List<LocalChange>) -> Flow<UploadRequestResult>),
+    upload:
+      (suspend (List<LocalChange>, List<LocalChangeResourceReference>) -> Flow<
+          UploadRequestResult,
+        >),
   ): Flow<SyncUploadProgress>
 
   /**

engine/src/main/java/com/google/android/fhir/db/Database.kt

@@ -215,7 +215,7 @@ internal data class ResourceWithUUID<R>(
   val resource: R,
 )
 
-internal data class LocalChangeResourceReference(
+data class LocalChangeResourceReference(
   val localChangeId: Long,
   val resourceReferenceValue: String,
   val resourceReferencePath: String?,

engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt

@@ -22,6 +22,7 @@ import com.google.android.fhir.FhirEngineProvider
 import com.google.android.fhir.LocalChange
 import com.google.android.fhir.SearchResult
 import com.google.android.fhir.db.Database
+import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.logicalId
 import com.google.android.fhir.search.Search
 import com.google.android.fhir.search.count
@@ -124,7 +125,10 @@ internal class FhirEngineImpl(private val database: Database, private val contex
 
   override suspend fun syncUpload(
     uploadStrategy: UploadStrategy,
-    upload: (suspend (List<LocalChange>) -> Flow<UploadRequestResult>),
+    upload:
+      (suspend (List<LocalChange>, List<LocalChangeResourceReference>) -> Flow<
+          UploadRequestResult,
+        >),
   ): Flow<SyncUploadProgress> = flow {
     val resourceConsolidator =
       ResourceConsolidatorFactory.byHttpVerb(uploadStrategy.requestGeneratorMode, database)
@@ -140,8 +144,10 @@ internal class FhirEngineImpl(private val database: Database, private val contex
 
     while (localChangeFetcher.hasNext()) {
       val localChanges = localChangeFetcher.next()
+      val localChangeReferences =
+        database.getLocalChangeResourceReferences(localChanges.flatMap { it.token.ids })
       val uploadRequestResult =
-        upload(localChanges)
+        upload(localChanges, localChangeReferences)
           .onEach { result ->
             resourceConsolidator.consolidate(result)
             val newProgress =

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

@@ -94,11 +94,7 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter
           uploader =
             Uploader(
               dataSource = dataSource,
-              patchGenerator =
-                PatchGeneratorFactory.byMode(
-                  getUploadStrategy().patchGeneratorMode,
-                  FhirEngineProvider.getFhirDatabase(applicationContext),
-                ),
+              patchGenerator = PatchGeneratorFactory.byMode(getUploadStrategy().patchGeneratorMode),
               requestGenerator =
                 UploadRequestGeneratorFactory.byMode(getUploadStrategy().requestGeneratorMode),
             ),

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

@@ -17,6 +17,7 @@
 package com.google.android.fhir.sync.upload
 
 import com.google.android.fhir.LocalChange
+import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.sync.DataSource
 import com.google.android.fhir.sync.ResourceSyncException
 import com.google.android.fhir.sync.upload.patch.PatchGenerator
@@ -49,9 +50,12 @@ internal class Uploader(
   private val patchGenerator: PatchGenerator,
   private val requestGenerator: UploadRequestGenerator,
 ) {
-  suspend fun upload(localChanges: List<LocalChange>) =
+  suspend fun upload(
+    localChanges: List<LocalChange>,
+    localChangesReferences: List<LocalChangeResourceReference>,
+  ) =
     localChanges
-      .let { patchGenerator.generate(it) }
+      .let { patchGenerator.generate(it, localChangesReferences) }
       .let { requestGenerator.generateUploadRequests(it) }
       .asFlow()
       .transformWhile {

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt

@@ -17,7 +17,7 @@
 package com.google.android.fhir.sync.upload.patch
 
 import com.google.android.fhir.LocalChange
-import com.google.android.fhir.db.Database
+import com.google.android.fhir.db.LocalChangeResourceReference
 
 /**
  * Generates [Patch]es from [LocalChange]s and output [List<[StronglyConnectedPatchMappings]>] to
@@ -35,17 +35,17 @@ internal interface PatchGenerator {
    * NOTE: different implementations may have requirements on the size of [localChanges] and output
    * certain numbers of [Patch]es.
    */
-  suspend fun generate(localChanges: List<LocalChange>): List<StronglyConnectedPatchMappings>
+  suspend fun generate(
+    localChanges: List<LocalChange>,
+    localChangesReferences: List<LocalChangeResourceReference>,
+  ): List<StronglyConnectedPatchMappings>
 }
 
 internal object PatchGeneratorFactory {
-  fun byMode(
-    mode: PatchGeneratorMode,
-    database: Database,
-  ): PatchGenerator =
+  fun byMode(mode: PatchGeneratorMode): PatchGenerator =
     when (mode) {
       is PatchGeneratorMode.PerChange -> PerChangePatchGenerator
-      is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator.with(database)
+      is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator
     }
 }
 

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt

@@ -17,7 +17,6 @@
 package com.google.android.fhir.sync.upload.patch
 
 import androidx.annotation.VisibleForTesting
-import com.google.android.fhir.db.Database
 import com.google.android.fhir.db.LocalChangeResourceReference
 
 /** Represents a resource e.g. 'Patient/123' , 'Encounter/123'. */
@@ -68,27 +67,27 @@ internal object PatchOrdering {
     get() = "${generatedPatch.resourceType}/${generatedPatch.resourceId}"
 
   /**
-   * Order the [PatchMapping] 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.
+   * Orders the list of [PatchMapping]s to maintain referential integrity.
    *
-   * @return A ordered list of the [StronglyConnectedPatchMappings] containing:
-   * - [StronglyConnectedPatchMappings] with single value for the [PatchMapping] based on the
-   *   references to other [PatchMapping] if the mappings are acyclic
-   * - [StronglyConnectedPatchMappings] with multiple values for [PatchMapping]s based on the
-   *   references to other [PatchMapping]s if the mappings are cyclic.
+   * This function ensures that if resource A has a CREATE reference to resources B and C, then B
+   * and C appear before A in the ordered list. UPDATE references are not considered as they do not
+   * impact referential integrity.
+   *
+   * The function uses Strongly Connected Components (SCC) to handle cyclic dependencies.
+   *
+   * @return A list of [StronglyConnectedPatchMappings]:
+   *     - Each [StronglyConnectedPatchMappings] object represents an SCC.
+   *     - If the graph of references is acyclic, each [StronglyConnectedPatchMappings] will contain
+   *       a single [PatchMapping].
+   *     - If the graph has cycles, a [StronglyConnectedPatchMappings] object will contain multiple
+   *       [PatchMapping]s involved in the cycle.
    */
-  suspend fun List<PatchMapping>.sccOrderByReferences(
-    database: Database,
+  fun List<PatchMapping>.sccOrderByReferences(
+    localChangeResourceReferences: List<LocalChangeResourceReference>,
   ): List<StronglyConnectedPatchMappings> {
     val resourceIdToPatchMapping = associateBy { patchMapping -> patchMapping.resourceTypeAndId }
-    /* Get LocalChangeResourceReferences for all the local changes. A single LocalChange may have
-    multiple LocalChangeResourceReference, one for each resource reference in the
-    LocalChange.payload.*/
-    val localChangeIdToResourceReferenceMap: Map<Long, List<LocalChangeResourceReference>> =
-      database
-        .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } })
-        .groupBy { it.localChangeId }
+    val localChangeIdToResourceReferenceMap =
+      localChangeResourceReferences.groupBy { it.localChangeId }
 
     val adjacencyList = createAdjacencyListForCreateReferences(localChangeIdToResourceReferenceMap)
 

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt

@@ -17,6 +17,7 @@
 package com.google.android.fhir.sync.upload.patch
 
 import com.google.android.fhir.LocalChange
+import com.google.android.fhir.db.LocalChangeResourceReference
 
 /**
  * Generates a [Patch] for each [LocalChange].
@@ -27,6 +28,7 @@ import com.google.android.fhir.LocalChange
 internal object PerChangePatchGenerator : PatchGenerator {
   override suspend fun generate(
     localChanges: List<LocalChange>,
+    localChangesReferences: List<LocalChangeResourceReference>,
   ): List<StronglyConnectedPatchMappings> =
     localChanges
       .map {

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt

@@ -22,7 +22,7 @@ import com.github.fge.jackson.JsonLoader
 import com.github.fge.jsonpatch.JsonPatch
 import com.google.android.fhir.LocalChange
 import com.google.android.fhir.LocalChange.Type
-import com.google.android.fhir.db.Database
+import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.sync.upload.patch.PatchOrdering.sccOrderByReferences
 
 /**
@@ -32,13 +32,13 @@ import com.google.android.fhir.sync.upload.patch.PatchOrdering.sccOrderByReferen
  * maintain an audit trail, but instead, multiple changes made to the same FHIR resource on the
  * client can be recorded as a single change on the server.
  */
-internal class PerResourcePatchGenerator private constructor(val database: Database) :
-  PatchGenerator {
+internal object PerResourcePatchGenerator : PatchGenerator {
 
   override suspend fun generate(
     localChanges: List<LocalChange>,
+    localChangesReferences: List<LocalChangeResourceReference>,
   ): List<StronglyConnectedPatchMappings> {
-    return generateSquashedChangesMapping(localChanges).sccOrderByReferences(database)
+    return generateSquashedChangesMapping(localChanges).sccOrderByReferences(localChangesReferences)
   }
 
   @androidx.annotation.VisibleForTesting
@@ -147,20 +147,4 @@ internal class PerResourcePatchGenerator private constructor(val database: Datab
     mergedOperations.values.flatten().forEach(mergedNode::add)
     return objectMapper.writeValueAsString(mergedNode)
   }
-
-  companion object {
-
-    @Volatile private lateinit var _instance: PerResourcePatchGenerator
-
-    @Synchronized
-    fun with(database: Database): PerResourcePatchGenerator {
-      if (!::_instance.isInitialized) {
-        _instance = PerResourcePatchGenerator(database)
-      } else if (_instance.database != database) {
-        _instance = PerResourcePatchGenerator(database)
-      }
-
-      return _instance
-    }
-  }
 }

engine/src/main/java/com/google/android/fhir/testing/Utilities.kt

@@ -24,6 +24,7 @@ import com.google.android.fhir.FhirEngine
 import com.google.android.fhir.LocalChange
 import com.google.android.fhir.LocalChangeToken
 import com.google.android.fhir.SearchResult
+import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.search.Search
 import com.google.android.fhir.sync.ConflictResolver
 import com.google.android.fhir.sync.DataSource
@@ -158,10 +159,11 @@ internal object TestFhirEngineImpl : FhirEngine {
 
   override suspend fun syncUpload(
     uploadStrategy: UploadStrategy,
-    upload: suspend (List<LocalChange>) -> Flow<UploadRequestResult>,
+    upload:
+      suspend (List<LocalChange>, List<LocalChangeResourceReference>) -> Flow<UploadRequestResult>,
   ): Flow<SyncUploadProgress> = flow {
     emit(SyncUploadProgress(1, 1))
-    upload(getLocalChanges(ResourceType.Patient, "123")).collect {
+    upload(getLocalChanges(ResourceType.Patient, "123"), emptyList()).collect {
       when (it) {
         is UploadRequestResult.Success -> emit(SyncUploadProgress(0, 1))
         is UploadRequestResult.Failure -> emit(SyncUploadProgress(1, 1, it.uploadError))