diff --git a/demo/src/main/java/com/google/android/fhir/demo/HomeFragment.kt b/demo/src/main/java/com/google/android/fhir/demo/HomeFragment.kt index f7b2a649c9..402a5e19ea 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/HomeFragment.kt +++ b/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(R.id.item_sync).setOnClickListener { - findNavController().navigate(HomeFragmentDirections.actionHomeFragmentToManualSyncFragment()) + findNavController().navigate(HomeFragmentDirections.actionHomeFragmentToSyncFragment()) } requireView().findViewById(R.id.item_periodic_sync).setOnClickListener { findNavController() diff --git a/demo/src/main/java/com/google/android/fhir/demo/PeriodicSyncFragment.kt b/demo/src/main/java/com/google/android/fhir/demo/PeriodicSyncFragment.kt index 3019cf136d..6e8e56de82 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/PeriodicSyncFragment.kt +++ b/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(R.id.last_sync_status).text = statusText - requireView().findViewById(R.id.last_sync_time).text = getString( - R.string.last_sync_timestamp, - syncFragmentViewModel.formatSyncTimestamp(lastStatus.timestamp) - ) + requireView().findViewById(R.id.last_sync_time).text = + getString( + R.string.last_sync_timestamp, + syncFragmentViewModel.formatSyncTimestamp(lastStatus.timestamp), + ) } } // Set current sync status val currentSyncStatusTextView = requireView().findViewById(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(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 + } } - } diff --git a/demo/src/main/res/navigation/reference_nav_graph.xml b/demo/src/main/res/navigation/reference_nav_graph.xml index 08a6e8d8e2..e4e4d22e63 100644 --- a/demo/src/main/res/navigation/reference_nav_graph.xml +++ b/demo/src/main/res/navigation/reference_nav_graph.xml @@ -22,8 +22,8 @@ app:destination="@id/patient_list" /> - diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt index 123c474cc5..ec85746b4a 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt +++ b/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( diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index bb4dfb0241..d03645c530 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/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) -> Flow), + upload: + (suspend (List, List) -> Flow< + UploadRequestResult, + >), ): Flow /** diff --git a/engine/src/main/java/com/google/android/fhir/db/Database.kt b/engine/src/main/java/com/google/android/fhir/db/Database.kt index 7f321ae712..24e5c300f0 100644 --- a/engine/src/main/java/com/google/android/fhir/db/Database.kt +++ b/engine/src/main/java/com/google/android/fhir/db/Database.kt @@ -215,7 +215,7 @@ internal data class ResourceWithUUID( val resource: R, ) -internal data class LocalChangeResourceReference( +data class LocalChangeResourceReference( val localChangeId: Long, val resourceReferenceValue: String, val resourceReferencePath: String?, diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index fe5331bd73..faef0c90ac 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/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) -> Flow), + upload: + (suspend (List, List) -> Flow< + UploadRequestResult, + >), ): Flow = 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 = diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt index afb43458be..45320ce810 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt +++ b/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), ), diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt index 83e8f0d345..4802efd75c 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt +++ b/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) = + suspend fun upload( + localChanges: List, + localChangesReferences: List, + ) = localChanges - .let { patchGenerator.generate(it) } + .let { patchGenerator.generate(it, localChangesReferences) } .let { requestGenerator.generateUploadRequests(it) } .asFlow() .transformWhile { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt index 6dfda6e191..fc13dedb3b 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt +++ b/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): List + suspend fun generate( + localChanges: List, + localChangesReferences: List, + ): List } 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 } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt index d0bfb5f364..e0d0dab3bb 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt +++ b/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.sccOrderByReferences( - database: Database, + fun List.sccOrderByReferences( + localChangeResourceReferences: List, ): List { 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> = - database - .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } }) - .groupBy { it.localChangeId } + val localChangeIdToResourceReferenceMap = + localChangeResourceReferences.groupBy { it.localChangeId } val adjacencyList = createAdjacencyListForCreateReferences(localChangeIdToResourceReferenceMap) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt index c208e8c6ea..8be3b21dc6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt +++ b/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, + localChangesReferences: List, ): List = localChanges .map { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index c524b32302..5579da3237 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/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, + localChangesReferences: List, ): List { - 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 - } - } } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index c99e17be73..0c5f80a1bf 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/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) -> Flow, + upload: + suspend (List, List) -> Flow, ): Flow = 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)) diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index 48be1a491b..f767675ed1 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -323,13 +323,13 @@ class FhirEngineImplTest { val emittedProgress = mutableListOf() fhirEngine - .syncUpload(UploadStrategy.AllChangesSquashedBundlePut) { - localChanges.addAll(it) + .syncUpload(UploadStrategy.AllChangesSquashedBundlePut) { lcs, _ -> + localChanges.addAll(lcs) flowOf( UploadRequestResult.Success( listOf( ResourceUploadResponseMapping( - it, + lcs, TEST_PATIENT_1, ), ), @@ -356,10 +356,10 @@ class FhirEngineImplTest { val emittedProgress = mutableListOf() val uploadError = ResourceSyncException(ResourceType.Patient, FHIRException("Did not work")) fhirEngine - .syncUpload(UploadStrategy.AllChangesSquashedBundlePut) { + .syncUpload(UploadStrategy.AllChangesSquashedBundlePut) { lcs, _ -> flowOf( UploadRequestResult.Failure( - it, + lcs, uploadError, ), ) @@ -767,12 +767,12 @@ class FhirEngineImplTest { fun `test local changes are consumed when using POST upload strategy`() = runBlocking { assertThat(services.database.getLocalChangesCount()).isEqualTo(1) fhirEngine - .syncUpload(UploadStrategy.SingleResourcePost) { + .syncUpload(UploadStrategy.SingleResourcePost) { lcs, _ -> flowOf( UploadRequestResult.Success( listOf( ResourceUploadResponseMapping( - it, + lcs, TEST_PATIENT_1, ), ), diff --git a/engine/src/test/java/com/google/android/fhir/sync/FhirSynchronizerTest.kt b/engine/src/test/java/com/google/android/fhir/sync/FhirSynchronizerTest.kt index 22c6fb6709..5967e05742 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/FhirSynchronizerTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/FhirSynchronizerTest.kt @@ -70,7 +70,7 @@ class FhirSynchronizerTest { fun `synchronize should return Success on successful download and upload`() = runTest(UnconfinedTestDispatcher()) { `when`(downloader.download()).thenReturn(flowOf(DownloadState.Success(listOf(), 10, 10))) - `when`(uploader.upload(any())) + `when`(uploader.upload(any(), any())) .thenReturn( flowOf(UploadRequestResult.Success(listOf())), ) @@ -97,7 +97,7 @@ class FhirSynchronizerTest { runTest(UnconfinedTestDispatcher()) { val error = ResourceSyncException(ResourceType.Patient, Exception("Download error")) `when`(downloader.download()).thenReturn(flowOf(DownloadState.Failure(error))) - `when`(uploader.upload(any())) + `when`(uploader.upload(any(), any())) .thenReturn( flowOf(UploadRequestResult.Success(listOf())), ) @@ -122,7 +122,7 @@ class FhirSynchronizerTest { runTest(UnconfinedTestDispatcher()) { `when`(downloader.download()).thenReturn(flowOf(DownloadState.Success(listOf(), 10, 10))) val error = ResourceSyncException(ResourceType.Patient, Exception("Upload error")) - `when`(uploader.upload(any())) + `when`(uploader.upload(any(), any())) .thenReturn(flowOf(UploadRequestResult.Failure(listOf(), error))) val emittedValues = mutableListOf() @@ -148,7 +148,7 @@ class FhirSynchronizerTest { fun `synchronize multiple invocations should execute in order`() = runTest(UnconfinedTestDispatcher()) { `when`(downloader.download()).thenReturn(flowOf(DownloadState.Success(listOf(), 0, 0))) - `when`(uploader.upload(any())) + `when`(uploader.upload(any(), any())) .thenReturn( flowOf( UploadRequestResult.Success( diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt index feffe91d7c..f71df803a1 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt @@ -65,9 +65,8 @@ class UploaderTest { MockitoAnnotations.openMocks(this) runTest { whenever(database.getLocalChangeResourceReferences(any())).thenReturn(emptyList()) } - perResourcePatchGenerator = - PatchGeneratorFactory.byMode(PatchGeneratorMode.PerResource, database) - perChangePatchGenerator = PatchGeneratorFactory.byMode(PatchGeneratorMode.PerChange, database) + perResourcePatchGenerator = PatchGeneratorFactory.byMode(PatchGeneratorMode.PerResource) + perChangePatchGenerator = PatchGeneratorFactory.byMode(PatchGeneratorMode.PerChange) } @Test @@ -98,7 +97,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestSuccess) + .upload(localChangesToTestSuccess, emptyList()) .toList() // With BundleUploadRequestGenerator, all patches will be squashed into 1 request (default @@ -156,7 +155,7 @@ class UploaderTest { perChangePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestSuccess) + .upload(localChangesToTestSuccess, emptyList()) .toList() // With BundleUploadRequestGenerator, all patches will be squashed into 1 request (default @@ -196,7 +195,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -221,7 +220,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -236,7 +235,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -252,7 +251,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -267,7 +266,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -299,7 +298,7 @@ class UploaderTest { perResourcePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestSuccess) + .upload(localChangesToTestSuccess, emptyList()) .toList() // With UrlUploadRequestGenerator, patch-per-resource is mapped to one url request. So total @@ -359,7 +358,7 @@ class UploaderTest { perChangePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestSuccess) + .upload(localChangesToTestSuccess, emptyList()) .toList() // With UrlUploadRequestGenerator, patch-per-resource is mapped to one url request. So total @@ -405,7 +404,7 @@ class UploaderTest { perResourcePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -430,7 +429,7 @@ class UploaderTest { perResourcePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -445,7 +444,7 @@ class UploaderTest { perResourcePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -460,7 +459,7 @@ class UploaderTest { perResourcePatchGenerator, urlUploadRequestGenerator, ) - .upload(localChangesToTestFail) + .upload(localChangesToTestFail, emptyList()) .toList() assertThat(result).hasSize(1) @@ -504,7 +503,7 @@ class UploaderTest { perResourcePatchGenerator, bundleUploadRequestGeneratorWithUnityBundleSize, ) - .upload(localChangesToTestSuccess) + .upload(localChangesToTestSuccess, emptyList()) .toList() // With BundleUploadRequestGenerator and bundleSize=1, each patch is mapped to a bundle diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt index f6bcae7883..2ccadfd464 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt @@ -18,7 +18,6 @@ package com.google.android.fhir.sync.upload.patch import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken -import com.google.android.fhir.db.Database import com.google.android.fhir.db.LocalChangeResourceReference import com.google.android.fhir.db.impl.dao.diff import com.google.android.fhir.logicalId @@ -36,26 +35,14 @@ import org.hl7.fhir.r4.model.Patient import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.RelatedPerson import org.hl7.fhir.r4.model.Resource -import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.Mock -import org.mockito.MockitoAnnotations -import org.mockito.kotlin.any -import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class PatchOrderingTest { - @Mock private lateinit var database: Database - private lateinit var patchGenerator: PerResourcePatchGenerator - - @Before - fun setUp() { - MockitoAnnotations.openMocks(this) - patchGenerator = PerResourcePatchGenerator.with(database) - } + private val patchGenerator = PerResourcePatchGenerator @Test fun `createReferenceAdjacencyList with local changes for only new resources should only have edges to inserted resources`() = @@ -77,8 +64,6 @@ class PatchOrderingTest { helper.createEncounter("encounter-3", 11, "Patient/patient-3") helper.createObservation("observation-3", 12, "Patient/patient-3", "Encounter/encounter-3") group = helper.updateGroup(group, 13, "Patient/patient-3") - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(helper.localChangeResourceReferences) val result = patchGenerator @@ -175,10 +160,7 @@ class PatchOrderingTest { helper.createEncounter("encounter-3", 12, "Patient/patient-3") helper.createPatient("patient-3", 13) - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(helper.localChangeResourceReferences) - - val result = patchGenerator.generate(helper.localChanges) + val result = patchGenerator.generate(helper.localChanges, helper.localChangeResourceReferences) // This order is based on the current implementation of the topological sort in [PatchOrdering], // it's entirely possible to generate different order here which is acceptable/correct, should @@ -221,10 +203,8 @@ class PatchOrderingTest { helper.createEncounter("encounter-2", 8, "Patient/patient-3") helper.createPatient("patient-3", 9) - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(helper.localChangeResourceReferences) - - val result = patchGenerator.generate(helper.localChanges) + val result = + patchGenerator.generate(helper.localChanges, helper.localChangeResourceReferences) assertThat( result.map { it.patchMappings.map { it.generatedPatch.resourceId } }, diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt index 38811c581d..11fb7b0979 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -20,7 +20,6 @@ import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken -import com.google.android.fhir.db.Database import com.google.android.fhir.db.impl.dao.diff import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.logicalId @@ -42,27 +41,14 @@ import org.hl7.fhir.r4.model.Patient import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.json.JSONArray -import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.Mock -import org.mockito.MockitoAnnotations -import org.mockito.kotlin.any -import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class PerResourcePatchGeneratorTest { - @Mock private lateinit var database: Database - private lateinit var patchGenerator: PerResourcePatchGenerator - - @Before - fun setUp() { - MockitoAnnotations.openMocks(this) - runTest { whenever(database.getLocalChangeResourceReferences(any())).thenReturn(emptyList()) } - patchGenerator = PerResourcePatchGenerator.with(database) - } + private val patchGenerator = PerResourcePatchGenerator @Test fun `should generate a single insert patch if the resource is inserted`() = runTest { @@ -70,7 +56,9 @@ class PerResourcePatchGeneratorTest { val insertionLocalChange = createInsertLocalChange(patient) val patches = - patchGenerator.generate(listOf(insertionLocalChange)).map { it.patchMappings.single() } + patchGenerator.generate(listOf(insertionLocalChange), emptyList()).map { + it.patchMappings.single() + } with(patches.single()) { with(generatedPatch) { @@ -102,7 +90,9 @@ class PerResourcePatchGeneratorTest { val updatePatch = readJsonArrayFromFile("/update_patch_1.json") val patches = - patchGenerator.generate(listOf(updateLocalChange1)).map { it.patchMappings.single() } + patchGenerator.generate(listOf(updateLocalChange1), emptyList()).map { + it.patchMappings.single() + } with(patches.single()) { with(generatedPatch) { @@ -132,7 +122,9 @@ class PerResourcePatchGeneratorTest { val deleteLocalChange = createDeleteLocalChange(remotePatient, 3L) val patches = - patchGenerator.generate(listOf(deleteLocalChange)).map { it.patchMappings.single() } + patchGenerator.generate(listOf(deleteLocalChange), emptyList()).map { + it.patchMappings.single() + } with(patches.single()) { with(generatedPatch) { @@ -159,7 +151,7 @@ class PerResourcePatchGeneratorTest { val patientString = jsonParser.encodeResourceToString(updatedPatient) val patches = - patchGenerator.generate(listOf(insertionLocalChange, updateLocalChange)).map { + patchGenerator.generate(listOf(insertionLocalChange, updateLocalChange), emptyList()).map { it.patchMappings.single() } @@ -218,7 +210,7 @@ class PerResourcePatchGeneratorTest { .toLocalChange() .apply { LocalChangeToken(listOf(2)) }, ) - val patchToUpload = patchGenerator.generate(changes) + val patchToUpload = patchGenerator.generate(changes, emptyList()) assertThat(patchToUpload).isEmpty() } @@ -296,7 +288,7 @@ class PerResourcePatchGeneratorTest { .toLocalChange() .apply { LocalChangeToken(listOf(3)) }, ) - val patchToUpload = patchGenerator.generate(changes) + val patchToUpload = patchGenerator.generate(changes, emptyList()) assertThat(patchToUpload).isEmpty() } @@ -319,7 +311,7 @@ class PerResourcePatchGeneratorTest { val updatePatch = readJsonArrayFromFile("/update_patch_2.json") val patches = - patchGenerator.generate(listOf(updateLocalChange1, updateLocalChange2)).map { + patchGenerator.generate(listOf(updateLocalChange1, updateLocalChange2), emptyList()).map { it.patchMappings.single() } @@ -367,7 +359,7 @@ class PerResourcePatchGeneratorTest { ) val patches = - patchGenerator.generate(listOf(updatedLocalChange1, updatedLocalChange2)).map { + patchGenerator.generate(listOf(updatedLocalChange1, updatedLocalChange2), emptyList()).map { it.patchMappings.single() } @@ -400,6 +392,7 @@ class PerResourcePatchGeneratorTest { patchGenerator .generate( listOf(updateLocalChange1, updateLocalChange2, deleteLocalChange), + emptyList(), ) .map { it.patchMappings.single() } @@ -448,7 +441,7 @@ class PerResourcePatchGeneratorTest { ) val errorMessage = - assertFailsWith { patchGenerator.generate(changes) } + assertFailsWith { patchGenerator.generate(changes, emptyList()) } .localizedMessage assertThat(errorMessage).isEqualTo("Changes after deletion of resource are not permitted") @@ -495,7 +488,7 @@ class PerResourcePatchGeneratorTest { .apply { LocalChangeToken(listOf(2)) }, ) val errorMessage = - assertFailsWith { patchGenerator.generate(changes) } + assertFailsWith { patchGenerator.generate(changes, emptyList()) } .localizedMessage assertThat(errorMessage).isEqualTo("Changes before creation of resource are not permitted")