From 2b047b66310816c3d236e72649ea595c31bb3c2a Mon Sep 17 00:00:00 2001 From: omarismail Date: Tue, 15 Aug 2023 16:06:36 +0100 Subject: [PATCH 01/11] DRAFT: add a localchangeselector --- .../android/fhir/db/impl/DatabaseImplTest.kt | 6 ++-- .../com/google/android/fhir/FhirEngine.kt | 5 ++- .../android/fhir/impl/FhirEngineImpl.kt | 27 ++++++++++------ .../android/fhir/sync/FhirSynchronizer.kt | 7 ++-- .../com/google/android/fhir/sync/Uploader.kt | 3 +- .../fhir/sync/upload/LocalChangeSelector.kt | 32 +++++++++++++++++++ .../android/fhir/sync/upload/UploaderImpl.kt | 6 ++-- .../google/android/fhir/testing/Utilities.kt | 8 +++-- .../android/fhir/impl/FhirEngineImplTest.kt | 10 +++--- .../fhir/sync/upload/UploaderImplTest.kt | 11 ++++--- 10 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt 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 62c85008a6..a0096e7c6a 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 @@ -35,6 +35,7 @@ import com.google.android.fhir.search.Search import com.google.android.fhir.search.StringFilterModifier import com.google.android.fhir.search.getQuery import com.google.android.fhir.search.has +import com.google.android.fhir.sync.upload.FetchStrategyType import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.readFromFile @@ -504,8 +505,9 @@ class DatabaseImplTest { lastUpdated = Date() } database.insert(patient) - services.fhirEngine.syncUpload { it -> - it + services.fhirEngine.syncUpload(FetchStrategyType.ALL_CHANGES) { it -> + val localChanges = it.next() + localChanges .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 69389e4366..e09dcdc7a9 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -20,6 +20,8 @@ import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver +import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.LocalChangeSelector import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -54,7 +56,8 @@ interface FhirEngine { * api caller should [Flow.collect] it. */ suspend fun syncUpload( - upload: (suspend (List) -> Flow>) + selectorStrategyType: FetchStrategyType, + upload: suspend (LocalChangeSelector) -> Flow>, ) /** 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 e228a4a434..31d04ed486 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 @@ -28,9 +28,10 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved +import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.LocalChangeSelector import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.collect import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -123,20 +124,26 @@ internal class FhirEngineImpl(private val database: Database, private val contex .intersect(database.getAllLocalChanges().map { it.resourceId }.toSet()) override suspend fun syncUpload( - upload: suspend (List) -> Flow> + selectorStrategyType: FetchStrategyType, + upload: suspend (LocalChangeSelector) -> Flow> ) { - val localChanges = database.getAllLocalChanges() - if (localChanges.isNotEmpty()) { - upload(localChanges).collect { - database.deleteUpdates(it.first) - when (it.second) { - is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) - else -> updateVersionIdAndLastUpdated(it.second) - } + val localChangeSelector = + when (selectorStrategyType) { + FetchStrategyType.ALL_CHANGES -> LocalChangeSelector(::getAllLocalChanges) + else -> error("${selectorStrategyType.name} does not have a fetching strategy yet.") + } + + upload(localChangeSelector).collect { + database.deleteUpdates(it.first) + when (it.second) { + is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) + else -> updateVersionIdAndLastUpdated(it.second) } } } + private suspend fun getAllLocalChanges(): List = database.getAllLocalChanges() + private suspend fun updateVersionIdAndLastUpdated(bundle: Bundle) { when (bundle.type) { Bundle.BundleType.TRANSACTIONRESPONSE -> { diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt index 8205f1cba4..518aecb791 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt @@ -19,9 +19,9 @@ package com.google.android.fhir.sync import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine +import com.google.android.fhir.sync.upload.FetchStrategyType import java.time.OffsetDateTime import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.flow import org.hl7.fhir.r4.model.ResourceType @@ -125,9 +125,10 @@ internal class FhirSynchronizer( private suspend fun upload(): SyncResult { val exceptions = mutableListOf() - fhirEngine.syncUpload { list -> + val strategyType = FetchStrategyType.ALL_CHANGES + fhirEngine.syncUpload(strategyType) { localChangeSelector -> flow { - uploader.upload(list).collect { result -> + uploader.upload(localChangeSelector).collect { result -> when (result) { is UploadState.Started -> setSyncState(SyncJobStatus.InProgress(SyncOperation.UPLOAD, result.total)) diff --git a/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt b/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt index 73d2ffd72a..dae9a3791e 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt @@ -18,6 +18,7 @@ package com.google.android.fhir.sync import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken +import com.google.android.fhir.sync.upload.LocalChangeSelector import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -29,7 +30,7 @@ internal interface Uploader { * transforming the [LocalChange]s to particular network operations. If [ProgressCallback] is * provided it also reports the intermediate progress */ - suspend fun upload(localChanges: List): Flow + suspend fun upload(localChangeSelector: LocalChangeSelector): Flow } internal sealed class UploadState { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt new file mode 100644 index 0000000000..912caae399 --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.fhir.sync.upload + +import com.google.android.fhir.LocalChange + +typealias LocalChangeFetcher = suspend () -> List + +class LocalChangeSelector(private val fetcher: LocalChangeFetcher) { + suspend fun hasNext(): Boolean = fetcher().isNotEmpty() + suspend fun next(): List = fetcher() +} + +enum class FetchStrategyType { + ALL_CHANGES, + SINGLE_CHANGE, + PER_RESOURCE_CHANGE +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt index e4d8fe74a6..c46f893517 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt @@ -43,11 +43,11 @@ internal class UploaderImpl( private val uploadWorkManager: UploadWorkManager, ) : Uploader { - override suspend fun upload(localChanges: List): Flow = flow { - val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChanges) + override suspend fun upload(localChangeSelector: LocalChangeSelector): Flow = flow { + val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChangeSelector.next()) val uploadRequests = uploadWorkManager.createUploadRequestsFromLocalChanges(transformedChanges) val total = uploadWorkManager.getPendingUploadsIndicator(uploadRequests) - var completed = 0 + var completed: Int emit(UploadState.Started(total)) val pendingRequests = uploadRequests.toMutableList() while (pendingRequests.isNotEmpty()) { 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 5b5fbc75ea..fd2c5fa16b 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 @@ -32,6 +32,8 @@ import com.google.android.fhir.sync.DownloadRequest import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.UploadRequest import com.google.android.fhir.sync.UrlDownloadRequest +import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.LocalChangeSelector import com.google.common.truth.Truth.assertThat import java.net.SocketTimeoutException import java.time.Instant @@ -146,9 +148,11 @@ object TestFhirEngineImpl : FhirEngine { } override suspend fun syncUpload( - upload: suspend (List) -> Flow> + selectorStrategyType: FetchStrategyType, + upload: suspend (LocalChangeSelector) -> Flow> ) { - upload(getLocalChanges(ResourceType.Patient, "123")).collect() + val localChangeSelector = LocalChangeSelector { getLocalChanges(ResourceType.Patient, "123") } + upload(localChangeSelector).collect() } override suspend fun syncDownload( 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 bae224b5ff..f7760c626d 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 @@ -29,6 +29,7 @@ import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM import com.google.android.fhir.search.search import com.google.android.fhir.sync.AcceptLocalConflictResolver import com.google.android.fhir.sync.AcceptRemoteConflictResolver +import com.google.android.fhir.sync.upload.FetchStrategyType import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.assertResourceNotEquals import com.google.android.fhir.testing.readFromFile @@ -311,15 +312,14 @@ class FhirEngineImplTest { @Test fun syncUpload_uploadLocalChange() = runBlocking { val localChanges = mutableListOf() - fhirEngine.syncUpload { + fhirEngine.syncUpload(FetchStrategyType.ALL_CHANGES) { flow { - localChanges.addAll(it) - emit(LocalChangeToken(it.flatMap { it.token.ids }) to TEST_PATIENT_1) + localChanges.addAll(it.next()) + emit(LocalChangeToken(localChanges.flatMap { it.token.ids }) to TEST_PATIENT_1) } } assertThat(localChanges).hasSize(1) - // val localChange = localChanges[0].localChange with(localChanges[0]) { assertThat(this.resourceType).isEqualTo(ResourceType.Patient.toString()) assertThat(this.resourceId).isEqualTo(TEST_PATIENT_1.id) @@ -412,7 +412,7 @@ class FhirEngineImplTest { assertThat(get(0).payload).isEqualTo(patientString) } assertResourceEquals(patient, fhirEngine.get(ResourceType.Patient, patient.logicalId)) - // clear databse + // clear database runBlocking(Dispatchers.IO) { fhirEngine.clearDatabase() } // assert that previously present resource not available after clearing database assertThat(fhirEngine.getLocalChanges(patient.resourceType, patient.logicalId)).isEmpty() diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt index 0cc964a4ea..3c651049b9 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt @@ -47,7 +47,7 @@ class UploaderImplTest { BundleDataSource { Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE } }, SquashedChangesUploadWorkManager() ) - .upload(localChanges) + .upload(localChangeSelector) .toList() assertThat(result).hasSize(2) @@ -63,7 +63,7 @@ class UploaderImplTest { fun `upload Bundle transaction should emit Started state`() = runBlocking { val result = UploaderImpl(BundleDataSource { Bundle() }, SquashedChangesUploadWorkManager()) - .upload(localChanges) + .upload(localChangeSelector) .toList() assertThat(result.first()).isInstanceOf(UploadState.Started::class.java) @@ -86,7 +86,7 @@ class UploaderImplTest { }, SquashedChangesUploadWorkManager() ) - .upload(localChanges) + .upload(localChangeSelector) .toList() assertThat(result).hasSize(2) @@ -100,14 +100,14 @@ class UploaderImplTest { BundleDataSource { throw ConnectException("Failed to connect to server.") }, SquashedChangesUploadWorkManager() ) - .upload(localChanges) + .upload(localChangeSelector) .toList() assertThat(result).hasSize(2) assertThat(result.last()).isInstanceOf(UploadState.Failure::class.java) } companion object { - val localChanges = + val localChangeSelector = LocalChangeSelector { listOf( LocalChangeEntity( id = 1, @@ -133,5 +133,6 @@ class UploaderImplTest { .toLocalChange() .apply { LocalChangeToken(listOf(1)) } ) + } } } From d3619208f991013049be8dd4139fa54f19128ca6 Mon Sep 17 00:00:00 2001 From: omarismail Date: Tue, 15 Aug 2023 17:26:40 +0100 Subject: [PATCH 02/11] just have one class --- .../android/fhir/db/impl/DatabaseImplTest.kt | 4 +- .../com/google/android/fhir/FhirEngine.kt | 8 ++-- .../android/fhir/impl/FhirEngineImpl.kt | 22 +++++----- .../android/fhir/sync/FhirSynchronizer.kt | 8 ++-- .../com/google/android/fhir/sync/Uploader.kt | 4 +- .../fhir/sync/upload/LocalChangeFetcher.kt | 41 +++++++++++++++++++ .../fhir/sync/upload/LocalChangeSelector.kt | 32 --------------- .../android/fhir/sync/upload/UploaderImpl.kt | 4 +- .../google/android/fhir/testing/Utilities.kt | 19 ++++++--- .../android/fhir/impl/FhirEngineImplTest.kt | 4 +- .../fhir/sync/upload/UploaderImplTest.kt | 20 ++++++--- 11 files changed, 94 insertions(+), 72 deletions(-) create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt delete mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt 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 a0096e7c6a..4d19ec70b3 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 @@ -35,7 +35,7 @@ import com.google.android.fhir.search.Search import com.google.android.fhir.search.StringFilterModifier import com.google.android.fhir.search.getQuery import com.google.android.fhir.search.has -import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.FetchStrategy import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.readFromFile @@ -505,7 +505,7 @@ class DatabaseImplTest { lastUpdated = Date() } database.insert(patient) - services.fhirEngine.syncUpload(FetchStrategyType.ALL_CHANGES) { it -> + services.fhirEngine.syncUpload(FetchStrategy.AllChanges(100)) { it -> val localChanges = it.next() localChanges .first { it.resourceId == "remote-patient-3" } 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 e09dcdc7a9..9fc3e0ee79 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -20,8 +20,8 @@ import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver -import com.google.android.fhir.sync.upload.FetchStrategyType -import com.google.android.fhir.sync.upload.LocalChangeSelector +import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.LocalChangeFetcher import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -56,8 +56,8 @@ interface FhirEngine { * api caller should [Flow.collect] it. */ suspend fun syncUpload( - selectorStrategyType: FetchStrategyType, - upload: suspend (LocalChangeSelector) -> Flow>, + strategyType: FetchStrategy, + upload: suspend (fetcher: LocalChangeFetcher) -> Flow> ) /** 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 31d04ed486..4a69e3f759 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 @@ -28,8 +28,9 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved -import com.google.android.fhir.sync.upload.FetchStrategyType -import com.google.android.fhir.sync.upload.LocalChangeSelector +import com.google.android.fhir.sync.upload.AllChangesLocalChangeFetcher +import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.LocalChangeFetcher import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Bundle @@ -124,16 +125,15 @@ internal class FhirEngineImpl(private val database: Database, private val contex .intersect(database.getAllLocalChanges().map { it.resourceId }.toSet()) override suspend fun syncUpload( - selectorStrategyType: FetchStrategyType, - upload: suspend (LocalChangeSelector) -> Flow> + strategyType: FetchStrategy, + upload: suspend (fetcher: LocalChangeFetcher) -> Flow> ) { - val localChangeSelector = - when (selectorStrategyType) { - FetchStrategyType.ALL_CHANGES -> LocalChangeSelector(::getAllLocalChanges) - else -> error("${selectorStrategyType.name} does not have a fetching strategy yet.") + val localChangeFetcher = + when (strategyType) { + is FetchStrategy.AllChanges -> AllChangesLocalChangeFetcher(database) + else -> error("$strategyType does not have a fetching strategy yet.") } - - upload(localChangeSelector).collect { + upload(localChangeFetcher).collect { database.deleteUpdates(it.first) when (it.second) { is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) @@ -142,8 +142,6 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } - private suspend fun getAllLocalChanges(): List = database.getAllLocalChanges() - private suspend fun updateVersionIdAndLastUpdated(bundle: Bundle) { when (bundle.type) { Bundle.BundleType.TRANSACTIONRESPONSE -> { diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt index 518aecb791..847f8022d9 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt @@ -19,7 +19,7 @@ package com.google.android.fhir.sync import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine -import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.FetchStrategy import java.time.OffsetDateTime import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.flow @@ -125,10 +125,10 @@ internal class FhirSynchronizer( private suspend fun upload(): SyncResult { val exceptions = mutableListOf() - val strategyType = FetchStrategyType.ALL_CHANGES - fhirEngine.syncUpload(strategyType) { localChangeSelector -> + val strategyType = FetchStrategy.AllChanges(100) + fhirEngine.syncUpload(strategyType) { localChangeFetcher -> flow { - uploader.upload(localChangeSelector).collect { result -> + uploader.upload(localChangeFetcher).collect { result -> when (result) { is UploadState.Started -> setSyncState(SyncJobStatus.InProgress(SyncOperation.UPLOAD, result.total)) diff --git a/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt b/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt index dae9a3791e..03a3ee65a2 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Uploader.kt @@ -18,7 +18,7 @@ package com.google.android.fhir.sync import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken -import com.google.android.fhir.sync.upload.LocalChangeSelector +import com.google.android.fhir.sync.upload.LocalChangeFetcher import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -30,7 +30,7 @@ internal interface Uploader { * transforming the [LocalChange]s to particular network operations. If [ProgressCallback] is * provided it also reports the intermediate progress */ - suspend fun upload(localChangeSelector: LocalChangeSelector): Flow + suspend fun upload(localChangeFetcher: LocalChangeFetcher): Flow } internal sealed class UploadState { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt new file mode 100644 index 0000000000..963996b2a2 --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -0,0 +1,41 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.fhir.sync.upload + +import com.google.android.fhir.LocalChange +import com.google.android.fhir.db.Database + +interface LocalChangeFetcher { + suspend fun hasNext(): Boolean + suspend fun next(): List + suspend fun getProgress(): Double +} + +internal class AllChangesLocalChangeFetcher(val database: Database) : LocalChangeFetcher { + override suspend fun hasNext(): Boolean = database.getAllLocalChanges().isNotEmpty() + + override suspend fun next(): List = database.getAllLocalChanges() + + override suspend fun getProgress(): Double = database.getAllLocalChanges().size.toDouble() +} + +sealed class FetchStrategy { + + class AllChanges(val pageSize: Int) : FetchStrategy() + object PerResource : FetchStrategy() + object EarliestChange : FetchStrategy() +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt deleted file mode 100644 index 912caae399..0000000000 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeSelector.kt +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.android.fhir.sync.upload - -import com.google.android.fhir.LocalChange - -typealias LocalChangeFetcher = suspend () -> List - -class LocalChangeSelector(private val fetcher: LocalChangeFetcher) { - suspend fun hasNext(): Boolean = fetcher().isNotEmpty() - suspend fun next(): List = fetcher() -} - -enum class FetchStrategyType { - ALL_CHANGES, - SINGLE_CHANGE, - PER_RESOURCE_CHANGE -} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt index c46f893517..563ca2c0e0 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt @@ -43,8 +43,8 @@ internal class UploaderImpl( private val uploadWorkManager: UploadWorkManager, ) : Uploader { - override suspend fun upload(localChangeSelector: LocalChangeSelector): Flow = flow { - val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChangeSelector.next()) + override suspend fun upload(localChangeFetcher: LocalChangeFetcher): Flow = flow { + val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChangeFetcher.next()) val uploadRequests = uploadWorkManager.createUploadRequestsFromLocalChanges(transformedChanges) val total = uploadWorkManager.getPendingUploadsIndicator(uploadRequests) var completed: Int 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 fd2c5fa16b..ad261961a6 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 @@ -32,8 +32,8 @@ import com.google.android.fhir.sync.DownloadRequest import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.UploadRequest import com.google.android.fhir.sync.UrlDownloadRequest -import com.google.android.fhir.sync.upload.FetchStrategyType -import com.google.android.fhir.sync.upload.LocalChangeSelector +import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.LocalChangeFetcher import com.google.common.truth.Truth.assertThat import java.net.SocketTimeoutException import java.time.Instant @@ -148,11 +148,18 @@ object TestFhirEngineImpl : FhirEngine { } override suspend fun syncUpload( - selectorStrategyType: FetchStrategyType, - upload: suspend (LocalChangeSelector) -> Flow> + strategyType: FetchStrategy, + upload: suspend (LocalChangeFetcher) -> Flow> ) { - val localChangeSelector = LocalChangeSelector { getLocalChanges(ResourceType.Patient, "123") } - upload(localChangeSelector).collect() + val localChanges = getLocalChanges(ResourceType.Patient, "123") + val localChangeFetcher = + object : LocalChangeFetcher { + override suspend fun hasNext(): Boolean = localChanges.isNotEmpty() + override suspend fun next(): List = localChanges + override suspend fun getProgress(): Double = localChanges.size.toDouble() + } + + upload(localChangeFetcher).collect() } override suspend fun syncDownload( 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 f7760c626d..e4758f5cb9 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 @@ -29,7 +29,7 @@ import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM import com.google.android.fhir.search.search import com.google.android.fhir.sync.AcceptLocalConflictResolver import com.google.android.fhir.sync.AcceptRemoteConflictResolver -import com.google.android.fhir.sync.upload.FetchStrategyType +import com.google.android.fhir.sync.upload.FetchStrategy import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.assertResourceNotEquals import com.google.android.fhir.testing.readFromFile @@ -312,7 +312,7 @@ class FhirEngineImplTest { @Test fun syncUpload_uploadLocalChange() = runBlocking { val localChanges = mutableListOf() - fhirEngine.syncUpload(FetchStrategyType.ALL_CHANGES) { + fhirEngine.syncUpload(FetchStrategy.AllChanges(100)) { flow { localChanges.addAll(it.next()) emit(LocalChangeToken(localChanges.flatMap { it.token.ids }) to TEST_PATIENT_1) diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt index 3c651049b9..a595b40563 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt @@ -18,6 +18,7 @@ package com.google.android.fhir.sync.upload import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum +import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.db.impl.dao.toLocalChange import com.google.android.fhir.db.impl.entities.LocalChangeEntity @@ -47,7 +48,7 @@ class UploaderImplTest { BundleDataSource { Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE } }, SquashedChangesUploadWorkManager() ) - .upload(localChangeSelector) + .upload(localChangeFetcher) .toList() assertThat(result).hasSize(2) @@ -63,7 +64,7 @@ class UploaderImplTest { fun `upload Bundle transaction should emit Started state`() = runBlocking { val result = UploaderImpl(BundleDataSource { Bundle() }, SquashedChangesUploadWorkManager()) - .upload(localChangeSelector) + .upload(localChangeFetcher) .toList() assertThat(result.first()).isInstanceOf(UploadState.Started::class.java) @@ -86,7 +87,7 @@ class UploaderImplTest { }, SquashedChangesUploadWorkManager() ) - .upload(localChangeSelector) + .upload(localChangeFetcher) .toList() assertThat(result).hasSize(2) @@ -100,14 +101,14 @@ class UploaderImplTest { BundleDataSource { throw ConnectException("Failed to connect to server.") }, SquashedChangesUploadWorkManager() ) - .upload(localChangeSelector) + .upload(localChangeFetcher) .toList() assertThat(result).hasSize(2) assertThat(result.last()).isInstanceOf(UploadState.Failure::class.java) } companion object { - val localChangeSelector = LocalChangeSelector { + val localChanges = listOf( LocalChangeEntity( id = 1, @@ -133,6 +134,13 @@ class UploaderImplTest { .toLocalChange() .apply { LocalChangeToken(listOf(1)) } ) - } + val localChangeFetcher = + object : LocalChangeFetcher { + override suspend fun hasNext(): Boolean = localChanges.isNotEmpty() + + override suspend fun next(): List = localChanges + + override suspend fun getProgress(): Double = localChanges.size.toDouble() + } } } From ea98e1e8ad2528f654e87f3135cd0d3b91057919 Mon Sep 17 00:00:00 2001 From: omarismail Date: Thu, 7 Sep 2023 13:09:42 +0100 Subject: [PATCH 03/11] move iterations to syncUpload --- .../android/fhir/db/impl/DatabaseImplTest.kt | 10 ++++---- .../com/google/android/fhir/FhirEngine.kt | 7 +++--- .../android/fhir/impl/FhirEngineImpl.kt | 15 ++++------- .../android/fhir/sync/FhirSynchronizer.kt | 8 +++--- .../fhir/sync/upload/LocalChangeFetcher.kt | 25 ++++++++++++++++--- .../android/fhir/sync/upload/UploaderImpl.kt | 4 +-- .../google/android/fhir/testing/Utilities.kt | 19 +++----------- .../android/fhir/impl/FhirEngineImplTest.kt | 8 +++--- .../fhir/sync/upload/UploaderImplTest.kt | 17 +++---------- 9 files changed, 52 insertions(+), 61 deletions(-) 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 e038af9ce0..43433e3d5e 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 @@ -39,7 +39,7 @@ import com.google.android.fhir.search.getQuery import com.google.android.fhir.search.has import com.google.android.fhir.search.include import com.google.android.fhir.search.revInclude -import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.FetchMode import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.readFromFile @@ -513,9 +513,9 @@ class DatabaseImplTest { lastUpdated = Date() } database.insert(patient) - services.fhirEngine.syncUpload(FetchStrategy.AllChanges(100)) { it -> - val localChanges = it.next() - localChanges + services.fhirEngine.syncUpload(FetchMode.AllChanges(100)) { + println(it.first()) + it .first { it.resourceId == "remote-patient-3" } .let { flowOf( @@ -2372,7 +2372,7 @@ class DatabaseImplTest { @Test fun search_nameGivenDuplicate_deduplicatePatient() = runBlocking { - var patient: Patient = readFromFile(Patient::class.java, "/patient_name_given_duplicate.json") + val patient: Patient = readFromFile(Patient::class.java, "/patient_name_given_duplicate.json") database.insertRemote(patient) val result = database.search( 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 e7790b3c41..d3dc5a94ef 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -20,8 +20,7 @@ import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver -import com.google.android.fhir.sync.upload.FetchStrategy -import com.google.android.fhir.sync.upload.LocalChangeFetcher +import com.google.android.fhir.sync.upload.FetchMode import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -56,8 +55,8 @@ interface FhirEngine { * api caller should [Flow.collect] it. */ suspend fun syncUpload( - strategyType: FetchStrategy, - upload: suspend (fetcher: LocalChangeFetcher) -> Flow> + fetchMode: FetchMode, + upload: (suspend (List) -> Flow>) ) /** 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 25c35fcec0..92a9b85ee6 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 @@ -29,8 +29,7 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved -import com.google.android.fhir.sync.upload.AllChangesLocalChangeFetcher -import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.FetchMode import com.google.android.fhir.sync.upload.LocalChangeFetcher import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow @@ -126,15 +125,11 @@ internal class FhirEngineImpl(private val database: Database, private val contex .intersect(database.getAllLocalChanges().map { it.resourceId }.toSet()) override suspend fun syncUpload( - strategyType: FetchStrategy, - upload: suspend (fetcher: LocalChangeFetcher) -> Flow> + fetchMode: FetchMode, + upload: suspend (List) -> Flow> ) { - val localChangeFetcher = - when (strategyType) { - is FetchStrategy.AllChanges -> AllChangesLocalChangeFetcher(database) - else -> error("$strategyType does not have a fetching strategy yet.") - } - upload(localChangeFetcher).collect { + val localChangeFetcher = LocalChangeFetcher.byMode(fetchMode, database) + upload(localChangeFetcher.next()).collect { database.deleteUpdates(it.first) when (it.second) { is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt index 60199cbb82..d949238876 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt @@ -19,9 +19,9 @@ package com.google.android.fhir.sync import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine +import com.google.android.fhir.sync.upload.FetchMode import com.google.android.fhir.sync.upload.UploadState import com.google.android.fhir.sync.upload.Uploader -import com.google.android.fhir.sync.upload.FetchStrategy import java.time.OffsetDateTime import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.flow @@ -127,10 +127,10 @@ internal class FhirSynchronizer( private suspend fun upload(): SyncResult { val exceptions = mutableListOf() - val strategyType = FetchStrategy.AllChanges(100) - fhirEngine.syncUpload(strategyType) { localChangeFetcher -> + val fetchMode = FetchMode.AllChanges(100) + fhirEngine.syncUpload(fetchMode) { list -> flow { - uploader.upload(localChangeFetcher).collect { result -> + uploader.upload(list).collect { result -> when (result) { is UploadState.Started -> setSyncState(SyncJobStatus.InProgress(SyncOperation.UPLOAD, result.total)) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 963996b2a2..65d661a874 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -19,10 +19,26 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange import com.google.android.fhir.db.Database +/** + * Defines the contract for fetching local changes. + * + * This interface provides methods to check for the existence of further changes, retrieve the next + * batch of changes, and get the progress of fetched changes. + * + * It is marked as internal to keep [Database] unexposed to clients + */ interface LocalChangeFetcher { suspend fun hasNext(): Boolean suspend fun next(): List suspend fun getProgress(): Double + + companion object { + internal fun byMode(mode: FetchMode, database: Database): LocalChangeFetcher = + when (mode) { + is FetchMode.AllChanges -> AllChangesLocalChangeFetcher(database) + else -> error("$mode does not have an implementation yet.") + } + } } internal class AllChangesLocalChangeFetcher(val database: Database) : LocalChangeFetcher { @@ -33,9 +49,10 @@ internal class AllChangesLocalChangeFetcher(val database: Database) : LocalChang override suspend fun getProgress(): Double = database.getAllLocalChanges().size.toDouble() } -sealed class FetchStrategy { +/** Represents the mode in which local changes should be fetched. */ +sealed class FetchMode { - class AllChanges(val pageSize: Int) : FetchStrategy() - object PerResource : FetchStrategy() - object EarliestChange : FetchStrategy() + class AllChanges(val pageSize: Int) : FetchMode() + object PerResource : FetchMode() + object EarliestChange : FetchMode() } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt index 10c8d4fba0..c46be4d3e2 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt @@ -40,8 +40,8 @@ internal class UploaderImpl( private val uploadWorkManager: UploadWorkManager, ) : Uploader { - override suspend fun upload(localChangeFetcher: LocalChangeFetcher): Flow = flow { - val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChangeFetcher.next()) + override suspend fun upload(localChanges: List): Flow = flow { + val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChanges) val uploadRequests = uploadWorkManager.createUploadRequestsFromLocalChanges(transformedChanges) val total = uploadWorkManager.getPendingUploadsIndicator(uploadRequests) var completed: Int 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 d00717abf1..f606fda29d 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 @@ -33,8 +33,7 @@ import com.google.android.fhir.sync.DownloadRequest import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.UploadRequest import com.google.android.fhir.sync.UrlDownloadRequest -import com.google.android.fhir.sync.upload.FetchStrategy -import com.google.android.fhir.sync.upload.LocalChangeFetcher +import com.google.android.fhir.sync.upload.FetchMode import com.google.common.truth.Truth.assertThat import java.net.SocketTimeoutException import java.time.Instant @@ -149,19 +148,9 @@ object TestFhirEngineImpl : FhirEngine { } override suspend fun syncUpload( - strategyType: FetchStrategy, - upload: suspend (LocalChangeFetcher) -> Flow> - ) { - val localChanges = getLocalChanges(ResourceType.Patient, "123") - val localChangeFetcher = - object : LocalChangeFetcher { - override suspend fun hasNext(): Boolean = localChanges.isNotEmpty() - override suspend fun next(): List = localChanges - override suspend fun getProgress(): Double = localChanges.size.toDouble() - } - - upload(localChangeFetcher).collect() - } + fetchMode: FetchMode, + upload: suspend (List) -> Flow> + ) = upload(getLocalChanges(ResourceType.Patient, "123")).collect() override suspend fun syncDownload( conflictResolver: ConflictResolver, 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 245e467554..a823ccfad1 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 @@ -29,7 +29,7 @@ import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM import com.google.android.fhir.search.search import com.google.android.fhir.sync.AcceptLocalConflictResolver import com.google.android.fhir.sync.AcceptRemoteConflictResolver -import com.google.android.fhir.sync.upload.FetchStrategy +import com.google.android.fhir.sync.upload.FetchMode import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.assertResourceNotEquals import com.google.android.fhir.testing.readFromFile @@ -314,10 +314,10 @@ class FhirEngineImplTest { @Test fun syncUpload_uploadLocalChange() = runBlocking { val localChanges = mutableListOf() - fhirEngine.syncUpload(FetchStrategy.AllChanges(100)) { + fhirEngine.syncUpload(FetchMode.AllChanges(100)) { flow { - localChanges.addAll(it.next()) - emit(LocalChangeToken(localChanges.flatMap { it.token.ids }) to TEST_PATIENT_1) + localChanges.addAll(it) + emit(LocalChangeToken(it.flatMap { it.token.ids }) to TEST_PATIENT_1) } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt index c9d9e2d0f1..e77685dea7 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt @@ -18,7 +18,6 @@ package com.google.android.fhir.sync.upload import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.db.impl.dao.toLocalChange import com.google.android.fhir.db.impl.entities.LocalChangeEntity @@ -48,7 +47,7 @@ class UploaderImplTest { BundleDataSource { Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE } }, SquashedChangesUploadWorkManager() ) - .upload(localChangeFetcher) + .upload(localChanges) .toList() assertThat(result).hasSize(2) @@ -64,7 +63,7 @@ class UploaderImplTest { fun `upload Bundle transaction should emit Started state`() = runBlocking { val result = UploaderImpl(BundleDataSource { Bundle() }, SquashedChangesUploadWorkManager()) - .upload(localChangeFetcher) + .upload(localChanges) .toList() assertThat(result.first()).isInstanceOf(UploadState.Started::class.java) @@ -87,7 +86,7 @@ class UploaderImplTest { }, SquashedChangesUploadWorkManager() ) - .upload(localChangeFetcher) + .upload(localChanges) .toList() assertThat(result).hasSize(2) @@ -101,7 +100,7 @@ class UploaderImplTest { BundleDataSource { throw ConnectException("Failed to connect to server.") }, SquashedChangesUploadWorkManager() ) - .upload(localChangeFetcher) + .upload(localChanges) .toList() assertThat(result).hasSize(2) @@ -134,13 +133,5 @@ class UploaderImplTest { .toLocalChange() .apply { LocalChangeToken(listOf(1)) } ) - val localChangeFetcher = - object : LocalChangeFetcher { - override suspend fun hasNext(): Boolean = localChanges.isNotEmpty() - - override suspend fun next(): List = localChanges - - override suspend fun getProgress(): Double = localChanges.size.toDouble() - } } } From bdad438854f8714597e5c5f9f13bd31f5b7d81a2 Mon Sep 17 00:00:00 2001 From: omarismail Date: Fri, 8 Sep 2023 11:44:03 +0100 Subject: [PATCH 04/11] address comments --- .../android/fhir/db/impl/DatabaseImplTest.kt | 4 ++-- .../java/com/google/android/fhir/FhirEngine.kt | 4 ++-- .../com/google/android/fhir/impl/FhirEngineImpl.kt | 6 +++--- .../google/android/fhir/sync/FhirSynchronizer.kt | 6 +++--- .../android/fhir/sync/upload/LocalChangeFetcher.kt | 14 +++++++------- .../android/fhir/sync/upload/UploaderImpl.kt | 2 +- .../com/google/android/fhir/testing/Utilities.kt | 4 ++-- .../google/android/fhir/impl/FhirEngineImplTest.kt | 4 ++-- 8 files changed, 22 insertions(+), 22 deletions(-) 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 43433e3d5e..9b507b8137 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 @@ -39,7 +39,7 @@ import com.google.android.fhir.search.getQuery import com.google.android.fhir.search.has import com.google.android.fhir.search.include import com.google.android.fhir.search.revInclude -import com.google.android.fhir.sync.upload.FetchMode +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.readFromFile @@ -513,7 +513,7 @@ class DatabaseImplTest { lastUpdated = Date() } database.insert(patient) - services.fhirEngine.syncUpload(FetchMode.AllChanges(100)) { + services.fhirEngine.syncUpload(LocalChangesFetchMode.AllChanges) { println(it.first()) it .first { it.resourceId == "remote-patient-3" } 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 d3dc5a94ef..d99290a1e2 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -20,7 +20,7 @@ import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver -import com.google.android.fhir.sync.upload.FetchMode +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource @@ -55,7 +55,7 @@ interface FhirEngine { * api caller should [Flow.collect] it. */ suspend fun syncUpload( - fetchMode: FetchMode, + localChangesFetchMode: LocalChangesFetchMode, upload: (suspend (List) -> Flow>) ) 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 92a9b85ee6..cd0d2936f5 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 @@ -29,8 +29,8 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved -import com.google.android.fhir.sync.upload.FetchMode import com.google.android.fhir.sync.upload.LocalChangeFetcher +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Bundle @@ -125,10 +125,10 @@ internal class FhirEngineImpl(private val database: Database, private val contex .intersect(database.getAllLocalChanges().map { it.resourceId }.toSet()) override suspend fun syncUpload( - fetchMode: FetchMode, + localChangesFetchMode: LocalChangesFetchMode, upload: suspend (List) -> Flow> ) { - val localChangeFetcher = LocalChangeFetcher.byMode(fetchMode, database) + val localChangeFetcher = LocalChangeFetcher.byMode(localChangesFetchMode, database) upload(localChangeFetcher.next()).collect { database.deleteUpdates(it.first) when (it.second) { diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt index d949238876..ccc103e956 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt @@ -19,7 +19,7 @@ package com.google.android.fhir.sync import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine -import com.google.android.fhir.sync.upload.FetchMode +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.android.fhir.sync.upload.UploadState import com.google.android.fhir.sync.upload.Uploader import java.time.OffsetDateTime @@ -127,8 +127,8 @@ internal class FhirSynchronizer( private suspend fun upload(): SyncResult { val exceptions = mutableListOf() - val fetchMode = FetchMode.AllChanges(100) - fhirEngine.syncUpload(fetchMode) { list -> + val localChangesFetchMode = LocalChangesFetchMode.AllChanges + fhirEngine.syncUpload(localChangesFetchMode) { list -> flow { uploader.upload(list).collect { result -> when (result) { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 65d661a874..daf73117e1 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -27,15 +27,15 @@ import com.google.android.fhir.db.Database * * It is marked as internal to keep [Database] unexposed to clients */ -interface LocalChangeFetcher { +internal interface LocalChangeFetcher { suspend fun hasNext(): Boolean suspend fun next(): List suspend fun getProgress(): Double companion object { - internal fun byMode(mode: FetchMode, database: Database): LocalChangeFetcher = + internal fun byMode(mode: LocalChangesFetchMode, database: Database): LocalChangeFetcher = when (mode) { - is FetchMode.AllChanges -> AllChangesLocalChangeFetcher(database) + is LocalChangesFetchMode.AllChanges -> AllChangesLocalChangeFetcher(database) else -> error("$mode does not have an implementation yet.") } } @@ -50,9 +50,9 @@ internal class AllChangesLocalChangeFetcher(val database: Database) : LocalChang } /** Represents the mode in which local changes should be fetched. */ -sealed class FetchMode { +sealed class LocalChangesFetchMode { - class AllChanges(val pageSize: Int) : FetchMode() - object PerResource : FetchMode() - object EarliestChange : FetchMode() + object AllChanges : LocalChangesFetchMode() + object PerResource : LocalChangesFetchMode() + object EarliestChange : LocalChangesFetchMode() } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt index c46be4d3e2..a966e1a973 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt @@ -44,7 +44,7 @@ internal class UploaderImpl( val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChanges) val uploadRequests = uploadWorkManager.createUploadRequestsFromLocalChanges(transformedChanges) val total = uploadWorkManager.getPendingUploadsIndicator(uploadRequests) - var completed: Int + var completed = 0 emit(UploadState.Started(total)) val pendingRequests = uploadRequests.toMutableList() while (pendingRequests.isNotEmpty()) { 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 f606fda29d..d1b42d9cb1 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 @@ -33,7 +33,7 @@ import com.google.android.fhir.sync.DownloadRequest import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.UploadRequest import com.google.android.fhir.sync.UrlDownloadRequest -import com.google.android.fhir.sync.upload.FetchMode +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.common.truth.Truth.assertThat import java.net.SocketTimeoutException import java.time.Instant @@ -148,7 +148,7 @@ object TestFhirEngineImpl : FhirEngine { } override suspend fun syncUpload( - fetchMode: FetchMode, + localChangesFetchMode: LocalChangesFetchMode, upload: suspend (List) -> Flow> ) = upload(getLocalChanges(ResourceType.Patient, "123")).collect() 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 a823ccfad1..8fbfca8371 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 @@ -29,7 +29,7 @@ import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM import com.google.android.fhir.search.search import com.google.android.fhir.sync.AcceptLocalConflictResolver import com.google.android.fhir.sync.AcceptRemoteConflictResolver -import com.google.android.fhir.sync.upload.FetchMode +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.android.fhir.testing.assertResourceEquals import com.google.android.fhir.testing.assertResourceNotEquals import com.google.android.fhir.testing.readFromFile @@ -314,7 +314,7 @@ class FhirEngineImplTest { @Test fun syncUpload_uploadLocalChange() = runBlocking { val localChanges = mutableListOf() - fhirEngine.syncUpload(FetchMode.AllChanges(100)) { + fhirEngine.syncUpload(LocalChangesFetchMode.AllChanges) { flow { localChanges.addAll(it) emit(LocalChangeToken(it.flatMap { it.token.ids }) to TEST_PATIENT_1) From 63296e2940bd3895219ee9ffb5ea6a82eb2c366e Mon Sep 17 00:00:00 2001 From: omarismail Date: Mon, 11 Sep 2023 11:10:16 +0100 Subject: [PATCH 05/11] add tests for allchangeslocalchangefetcher --- .../android/fhir/db/impl/DatabaseImplTest.kt | 1 - .../fhir/sync/upload/LocalChangeFetcher.kt | 28 ++++-- .../AllChangesLocalChangeFetcherTest.kt | 96 +++++++++++++++++++ 3 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt 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 9b507b8137..5e17a87da0 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 @@ -514,7 +514,6 @@ class DatabaseImplTest { } database.insert(patient) services.fhirEngine.syncUpload(LocalChangesFetchMode.AllChanges) { - println(it.first()) it .first { it.resourceId == "remote-patient-3" } .let { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index daf73117e1..5193b1495c 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -20,7 +20,7 @@ import com.google.android.fhir.LocalChange import com.google.android.fhir.db.Database /** - * Defines the contract for fetching local changes. + * Fetches local changes. * * This interface provides methods to check for the existence of further changes, retrieve the next * batch of changes, and get the progress of fetched changes. @@ -28,25 +28,41 @@ import com.google.android.fhir.db.Database * It is marked as internal to keep [Database] unexposed to clients */ internal interface LocalChangeFetcher { + /** Checks if there are more local changes to be fetched. */ suspend fun hasNext(): Boolean + + /** Retrieves the next batch of local changes. */ suspend fun next(): List + + /** Returns the completion percentage of the local changes fetched. */ suspend fun getProgress(): Double companion object { - internal fun byMode(mode: LocalChangesFetchMode, database: Database): LocalChangeFetcher = - when (mode) { - is LocalChangesFetchMode.AllChanges -> AllChangesLocalChangeFetcher(database) + internal suspend fun byMode( + mode: LocalChangesFetchMode, + database: Database + ): LocalChangeFetcher { + val totalLocalChangeCount = database.getAllLocalChanges().size + return when (mode) { + is LocalChangesFetchMode.AllChanges -> + AllChangesLocalChangeFetcher(database, totalLocalChangeCount) else -> error("$mode does not have an implementation yet.") } + } } } -internal class AllChangesLocalChangeFetcher(val database: Database) : LocalChangeFetcher { +internal class AllChangesLocalChangeFetcher( + private val database: Database, + private val total: Int +) : LocalChangeFetcher { + override suspend fun hasNext(): Boolean = database.getAllLocalChanges().isNotEmpty() override suspend fun next(): List = database.getAllLocalChanges() - override suspend fun getProgress(): Double = database.getAllLocalChanges().size.toDouble() + override suspend fun getProgress(): Double = + 1.0 - database.getAllLocalChanges().size.div(total.toDouble()) } /** Represents the mode in which local changes should be fetched. */ diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt new file mode 100644 index 0000000000..cc7a715e53 --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt @@ -0,0 +1,96 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.fhir.sync.upload + +import androidx.test.core.app.ApplicationProvider +import com.google.android.fhir.FhirServices +import com.google.common.truth.Truth.assertThat +import java.util.Date +import kotlinx.coroutines.test.runTest +import org.hl7.fhir.r4.model.Enumerations +import org.hl7.fhir.r4.model.Meta +import org.hl7.fhir.r4.model.Patient +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class AllChangesLocalChangeFetcherTest { + private val services = + FhirServices.builder(ApplicationProvider.getApplicationContext()).inMemory().build() + private val database = services.database + private lateinit var fetcher: AllChangesLocalChangeFetcher + + @Before + fun setup() = runTest { + database.insert(TEST_PATIENT_1, TEST_PATIENT_2) + fetcher = AllChangesLocalChangeFetcher(database, 2) + } + + @Test + fun `next returns all the localChanges`() = runTest { + val localChanges = fetcher.next() + assertThat(fetcher.next().size).isEqualTo(2) + assertThat(localChanges.map { it.resourceId }) + .containsExactly(TEST_PATIENT_1_ID, TEST_PATIENT_2_ID) + } + + @Test + fun `hasNext returns true when there are local changes`() = runTest { + assertThat(fetcher.hasNext()).isTrue() + } + + @Test + fun `hasNext returns false when there are no local changes`() = runTest { + database.deleteUpdates(listOf(TEST_PATIENT_1, TEST_PATIENT_2)) + assertThat(fetcher.hasNext()).isFalse() + } + + @Test + fun `getProgress returns 1,0 when all local changes are removed`() = runTest { + database.deleteUpdates(listOf(TEST_PATIENT_1, TEST_PATIENT_2)) + assertThat(fetcher.getProgress()).isEqualTo(1.0) + } + + @Test + fun `getProgress returns 0,5 when half the local changes are removed`() = runTest { + database.deleteUpdates(listOf(TEST_PATIENT_1)) + assertThat(fetcher.getProgress()).isEqualTo(0.5) + } + + @Test + fun `getProgress returns 0,0 when none of the local changes are removed`() = runTest { + assertThat(fetcher.getProgress()).isEqualTo(0.0) + } + companion object { + private const val TEST_PATIENT_1_ID = "test_patient_1" + private var TEST_PATIENT_1 = + Patient().apply { + id = TEST_PATIENT_1_ID + gender = Enumerations.AdministrativeGender.MALE + } + + private const val TEST_PATIENT_2_ID = "test_patient_2" + private var TEST_PATIENT_2 = + Patient().apply { + id = TEST_PATIENT_2_ID + gender = Enumerations.AdministrativeGender.MALE + meta = Meta().apply { lastUpdated = Date() } + } + } +} From e1e14a4d40395f39fb5334520580a7b904ac372c Mon Sep 17 00:00:00 2001 From: omarismail Date: Wed, 13 Sep 2023 16:37:19 +0100 Subject: [PATCH 06/11] add progress state --- .../fhir/sync/upload/LocalChangeFetcher.kt | 16 ++++++++++++---- .../upload/AllChangesLocalChangeFetcherTest.kt | 12 ++++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 5193b1495c..79f134dfd0 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -34,8 +34,11 @@ internal interface LocalChangeFetcher { /** Retrieves the next batch of local changes. */ suspend fun next(): List - /** Returns the completion percentage of the local changes fetched. */ - suspend fun getProgress(): Double + /** + * Returns [ProgressState], which contains the remaining changes left to upload and the initial + * total to upload + */ + suspend fun getProgress(): ProgressState companion object { internal suspend fun byMode( @@ -52,6 +55,11 @@ internal interface LocalChangeFetcher { } } +data class ProgressState( + val remaining: Int, + val initialTotal: Int, +) + internal class AllChangesLocalChangeFetcher( private val database: Database, private val total: Int @@ -61,8 +69,8 @@ internal class AllChangesLocalChangeFetcher( override suspend fun next(): List = database.getAllLocalChanges() - override suspend fun getProgress(): Double = - 1.0 - database.getAllLocalChanges().size.div(total.toDouble()) + override suspend fun getProgress(): ProgressState = + ProgressState(database.getAllLocalChanges().size, total) } /** Represents the mode in which local changes should be fetched. */ diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt index cc7a715e53..eb093b6bfd 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt @@ -62,20 +62,20 @@ class AllChangesLocalChangeFetcherTest { } @Test - fun `getProgress returns 1,0 when all local changes are removed`() = runTest { + fun `getProgress when all local changes are removed`() = runTest { database.deleteUpdates(listOf(TEST_PATIENT_1, TEST_PATIENT_2)) - assertThat(fetcher.getProgress()).isEqualTo(1.0) + assertThat(fetcher.getProgress()).isEqualTo(ProgressState(0, 2)) } @Test - fun `getProgress returns 0,5 when half the local changes are removed`() = runTest { + fun `getProgress when half the local changes are removed`() = runTest { database.deleteUpdates(listOf(TEST_PATIENT_1)) - assertThat(fetcher.getProgress()).isEqualTo(0.5) + assertThat(fetcher.getProgress()).isEqualTo(ProgressState(1, 2)) } @Test - fun `getProgress returns 0,0 when none of the local changes are removed`() = runTest { - assertThat(fetcher.getProgress()).isEqualTo(0.0) + fun `getProgress when none of the local changes are removed`() = runTest { + assertThat(fetcher.getProgress()).isEqualTo(ProgressState(2, 2)) } companion object { private const val TEST_PATIENT_1_ID = "test_patient_1" From a8254fc2c097b8d0f4b607952fbf8415f7a91e14 Mon Sep 17 00:00:00 2001 From: omarismail Date: Thu, 14 Sep 2023 10:09:59 +0100 Subject: [PATCH 07/11] spotless --- .../android/fhir/db/impl/DatabaseImplTest.kt | 550 +++++++++--------- .../com/google/android/fhir/FhirEngine.kt | 24 +- .../android/fhir/impl/FhirEngineImpl.kt | 14 +- .../android/fhir/sync/FhirSynchronizer.kt | 2 +- .../fhir/sync/upload/LocalChangeFetcher.kt | 6 +- .../google/android/fhir/testing/Utilities.kt | 11 +- .../android/fhir/impl/FhirEngineImplTest.kt | 53 +- .../AllChangesLocalChangeFetcherTest.kt | 1 + 8 files changed, 339 insertions(+), 322 deletions(-) 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 821a3cabc5..508a92cd8b 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 @@ -91,9 +91,9 @@ import org.junit.runners.Parameterized.Parameters * Integration tests for [DatabaseImpl]. There are written as integration tests as officially * recommend because: * * Different versions of android are shipped with different versions of SQLite. Integration tests - * allow for better coverage on them. + * allow for better coverage on them. * * Robolectric's SQLite implementation does not match Android, e.g.: - * https://github.com/robolectric/robolectric/blob/master/shadows/framework/src/main/java/org/robolectric/shadows/ShadowSQLiteConnection.java#L97 + * https://github.com/robolectric/robolectric/blob/master/shadows/framework/src/main/java/org/robolectric/shadows/ShadowSQLiteConnection.java#L97 */ @MediumTest @RunWith(Parameterized::class) @@ -268,7 +268,7 @@ class DatabaseImplTest { } assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID!" + "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID!", ) assertThat(database.getLocalChanges(ResourceType.Patient, TEST_PATIENT_1_ID)).isEmpty() } @@ -281,7 +281,7 @@ class DatabaseImplTest { } assertThat(resourceIllegalStateException.message) .isEqualTo( - "Resource with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID has local changes, either sync with server or FORCE_PURGE required" + "Resource with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID has local changes, either sync with server or FORCE_PURGE required", ) } @@ -327,7 +327,7 @@ class DatabaseImplTest { } assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_2_ID!" + "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_2_ID!", ) } @@ -337,12 +337,10 @@ class DatabaseImplTest { assertThrows(ResourceNotFoundException::class.java) { runBlocking { database.update(TEST_PATIENT_2) } } - /* ktlint-disable max-line-length */ assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_2.resourceType.name} and id $TEST_PATIENT_2_ID!" - /* ktlint-enable max-line-length */ - ) + "Resource not found with type ${TEST_PATIENT_2.resourceType.name} and id $TEST_PATIENT_2_ID!", + ) } @Test @@ -384,7 +382,7 @@ class DatabaseImplTest { HumanName().apply { family = "FamilyName" addGiven("FirstName") - } + }, ) meta = Meta().apply { @@ -402,7 +400,7 @@ class DatabaseImplTest { HumanName().apply { family = "UpdatedFamilyName" addGiven("UpdatedFirstName") - } + }, ) } database.update(updatedPatient) @@ -439,7 +437,7 @@ class DatabaseImplTest { database .getAllLocalChanges() .map { it } - .none { it.type == LocalChange.Type.DELETE && it.resourceId == "nonexistent_patient" } + .none { it.type == LocalChange.Type.DELETE && it.resourceId == "nonexistent_patient" }, ) .isTrue() } @@ -522,7 +520,7 @@ class DatabaseImplTest { Patient().apply { id = it.resourceId meta = remoteMeta - } + }, ) } } @@ -539,7 +537,7 @@ class DatabaseImplTest { database .getAllLocalChanges() .map { it } - .none { it.resourceId in listOf(patient.logicalId, TEST_PATIENT_2_ID) } + .none { it.resourceId in listOf(patient.logicalId, TEST_PATIENT_2_ID) }, ) .isTrue() } @@ -553,14 +551,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("Jane") family = "Doe" - } + }, ) } database.insert(patient) val result = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(result.size).isEqualTo(1) @@ -571,14 +569,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("John") family = "Doe" - } + }, ) } database.insert(updatedPatient) val updatedResult = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(updatedResult.size).isEqualTo(0) } @@ -592,14 +590,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("Jane") family = "Doe" - } + }, ) } database.insertRemote(patient) val result = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(result.size).isEqualTo(1) @@ -610,14 +608,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("John") family = "Doe" - } + }, ) } database.insertRemote(updatedPatient) val updatedResult = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(updatedResult.size).isEqualTo(0) } @@ -649,14 +647,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("Jane") family = "Doe" - } + }, ) } database.insertRemote(patient) val result = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(result.size).isEqualTo(1) @@ -667,14 +665,14 @@ class DatabaseImplTest { HumanName().apply { addGiven("John") family = "Doe" - } + }, ) } database.update(updatedPatient) val updatedResult = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery(), ) assertThat(updatedResult.size).isEqualTo(0) } @@ -701,14 +699,14 @@ class DatabaseImplTest { val largerId = "risk_assessment_2" database.insert( riskAssessment(id = smallerId, probability = BigDecimal("0.3")), - riskAssessment(id = largerId, probability = BigDecimal("0.30000000001")) + riskAssessment(id = largerId, probability = BigDecimal("0.30000000001")), ) val results = database.search( Search(ResourceType.RiskAssessment) .apply { sort(RiskAssessment.PROBABILITY, Order.DESCENDING) } - .getQuery() + .getQuery(), ) val ids = results.map { it.id } @@ -724,7 +722,7 @@ class DatabaseImplTest { listOf( RiskAssessment.RiskAssessmentPredictionComponent().apply { setProbability(DecimalType(probability)) - } + }, ) } @@ -738,7 +736,7 @@ class DatabaseImplTest { database.insert(patient) val result = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "eve" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "eve" }) }.getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/${patient.id}") @@ -754,7 +752,7 @@ class DatabaseImplTest { database.insert(patient) val result = database.search( - Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "eve" }) }.getQuery() + Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "eve" }) }.getQuery(), ) assertThat(result).isEmpty() @@ -777,10 +775,10 @@ class DatabaseImplTest { { value = "Eve" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/${patient.id}") @@ -803,10 +801,10 @@ class DatabaseImplTest { { value = "Eve" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -830,10 +828,10 @@ class DatabaseImplTest { { value = "Eve" modifier = StringFilterModifier.CONTAINS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/${patient.id}") @@ -856,10 +854,10 @@ class DatabaseImplTest { { value = "eve" modifier = StringFilterModifier.CONTAINS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -871,7 +869,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -885,10 +883,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.EQUAL value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -900,7 +898,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100.5)), ) } @@ -914,10 +912,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.EQUAL value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -929,7 +927,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)), ) } @@ -943,10 +941,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.NOT_EQUAL value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") } @@ -957,7 +955,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -971,10 +969,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.NOT_EQUAL value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -986,7 +984,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)), ) } @@ -1000,10 +998,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.GREATERTHAN value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1015,7 +1013,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1029,10 +1027,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.GREATERTHAN value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1044,7 +1042,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1058,10 +1056,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1073,7 +1071,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)), ) } @@ -1087,10 +1085,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1102,7 +1100,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)), ) } @@ -1116,10 +1114,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.LESSTHAN value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1131,7 +1129,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1145,10 +1143,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.LESSTHAN value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1160,7 +1158,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1174,10 +1172,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.LESSTHAN_OR_EQUALS value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") } @@ -1188,7 +1186,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)), ) } @@ -1202,10 +1200,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.LESSTHAN_OR_EQUALS value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1217,7 +1215,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.0)), ) } @@ -1231,10 +1229,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.ENDS_BEFORE value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1246,7 +1244,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1260,10 +1258,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.ENDS_BEFORE value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1275,7 +1273,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(100)), ) } @@ -1289,10 +1287,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.STARTS_AFTER value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1304,7 +1302,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(99.5)), ) } @@ -1318,10 +1316,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.STARTS_AFTER value = BigDecimal("99.5") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1333,7 +1331,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(93)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(93)), ) } @@ -1347,10 +1345,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.APPROXIMATE value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("RiskAssessment/${riskAssessment.id}") @@ -1362,7 +1360,7 @@ class DatabaseImplTest { RiskAssessment().apply { id = "1" addPrediction( - RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(120)) + RiskAssessment.RiskAssessmentPredictionComponent().setProbability(DecimalType(120)), ) } @@ -1376,10 +1374,10 @@ class DatabaseImplTest { { prefix = ParamPrefixEnum.APPROXIMATE value = BigDecimal("100") - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() @@ -1403,10 +1401,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.APPROXIMATE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1429,10 +1427,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2020-03-14")) prefix = ParamPrefixEnum.APPROXIMATE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1455,10 +1453,10 @@ class DatabaseImplTest { { value = of(DateType("2013-03-14")) prefix = ParamPrefixEnum.APPROXIMATE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1481,10 +1479,10 @@ class DatabaseImplTest { { value = of(DateType("2020-03-14")) prefix = ParamPrefixEnum.APPROXIMATE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1506,10 +1504,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.STARTS_AFTER - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1531,10 +1529,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.STARTS_AFTER - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1556,10 +1554,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.ENDS_BEFORE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1581,10 +1579,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.ENDS_BEFORE - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1606,10 +1604,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.NOT_EQUAL - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1631,10 +1629,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.NOT_EQUAL - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1656,10 +1654,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.EQUAL - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1681,10 +1679,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.EQUAL - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1706,10 +1704,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.GREATERTHAN - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1731,10 +1729,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.GREATERTHAN - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1756,10 +1754,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1781,10 +1779,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1806,10 +1804,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.LESSTHAN - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1831,10 +1829,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.LESSTHAN - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1856,10 +1854,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14")) prefix = ParamPrefixEnum.LESSTHAN_OR_EQUALS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Patient/1") } @@ -1881,10 +1879,10 @@ class DatabaseImplTest { { value = of(DateTimeType("2013-03-14T00:00:00-00:00")) prefix = ParamPrefixEnum.LESSTHAN_OR_EQUALS - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1913,10 +1911,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -1945,10 +1943,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -1977,10 +1975,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2009,10 +2007,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2041,10 +2039,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2073,10 +2071,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2105,10 +2103,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2137,10 +2135,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2169,10 +2167,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2201,10 +2199,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2233,10 +2231,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2265,10 +2263,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2297,10 +2295,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2329,10 +2327,10 @@ class DatabaseImplTest { value = BigDecimal("5.403") system = "http://unitsofmeasure.org" unit = "g" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result).isEmpty() } @@ -2361,10 +2359,10 @@ class DatabaseImplTest { value = BigDecimal("5403") system = "http://unitsofmeasure.org" unit = "mg" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.single().id).isEqualTo("Observation/1") } @@ -2381,7 +2379,7 @@ class DatabaseImplTest { count = 100 from = 0 } - .getQuery() + .getQuery(), ) assertThat(result.filter { it.id == patient.id }).hasSize(1) } @@ -2402,8 +2400,8 @@ class DatabaseImplTest { Coding( "http://hl7.org/fhir/sid/cvx", "140", - "Influenza, seasonal, injectable, preservative free" - ) + "Influenza, seasonal, injectable, preservative free", + ), ) status = Immunization.ImmunizationStatus.COMPLETED } @@ -2421,10 +2419,10 @@ class DatabaseImplTest { Coding( "http://hl7.org/fhir/sid/cvx", "140", - "Influenza, seasonal, injectable, preservative free" - ) + "Influenza, seasonal, injectable, preservative free", + ), ) - } + }, ) // Follow Immunization.ImmunizationStatus @@ -2432,7 +2430,7 @@ class DatabaseImplTest { Immunization.STATUS, { value = of(Coding("http://hl7.org/fhir/event-status", "completed", "Body Weight")) - } + }, ) } @@ -2441,10 +2439,10 @@ class DatabaseImplTest { { modifier = StringFilterModifier.MATCHES_EXACTLY value = "IN" - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.map { it.logicalId }).containsExactly("100").inOrder() } @@ -2465,8 +2463,8 @@ class DatabaseImplTest { category = listOf( CodeableConcept( - Coding("http://snomed.info/sct", "698360004", "Diabetes self management plan") - ) + Coding("http://snomed.info/sct", "698360004", "Diabetes self management plan"), + ), ) } database.insert(patient, TEST_PATIENT_1, carePlan) @@ -2480,13 +2478,17 @@ class DatabaseImplTest { { value = of( - Coding("http://snomed.info/sct", "698360004", "Diabetes self management plan") + Coding( + "http://snomed.info/sct", + "698360004", + "Diabetes self management plan", + ), ) - } + }, ) } } - .getQuery() + .getQuery(), ) assertThat(result.map { it.logicalId }).containsExactly("100").inOrder() } @@ -2497,14 +2499,14 @@ class DatabaseImplTest { Patient().apply { id = "older-patient" birthDateElement = DateType("2020-12-12") - } + }, ) database.insert( Patient().apply { id = "younger-patient" birthDateElement = DateType("2020-12-13") - } + }, ) assertThat( @@ -2512,9 +2514,9 @@ class DatabaseImplTest { .search( Search(ResourceType.Patient) .apply { sort(Patient.BIRTHDATE, Order.DESCENDING) } - .getQuery() + .getQuery(), ) - .map { it.id } + .map { it.id }, ) .containsExactly("Patient/younger-patient", "Patient/older-patient", "Patient/test_patient_1") } @@ -2525,14 +2527,14 @@ class DatabaseImplTest { Patient().apply { id = "older-patient" birthDateElement = DateType("2020-12-12") - } + }, ) database.insert( Patient().apply { id = "younger-patient" birthDateElement = DateType("2020-12-13") - } + }, ) assertThat( @@ -2540,9 +2542,9 @@ class DatabaseImplTest { .search( Search(ResourceType.Patient) .apply { sort(Patient.BIRTHDATE, Order.ASCENDING) } - .getQuery() + .getQuery(), ) - .map { it.id } + .map { it.id }, ) .containsExactly("Patient/test_patient_1", "Patient/older-patient", "Patient/younger-patient") } @@ -2555,7 +2557,11 @@ class DatabaseImplTest { id = "immunization-1" vaccineCode = CodeableConcept( - Coding("http://id.who.int/icd11/mms", "XM1NL1", "COVID-19 vaccine, inactivated virus") + Coding( + "http://id.who.int/icd11/mms", + "XM1NL1", + "COVID-19 vaccine, inactivated virus", + ), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2566,8 +2572,8 @@ class DatabaseImplTest { Coding( "http://id.who.int/icd11/mms", "XM5DF6", - "COVID-19 vaccine, live attenuated virus" - ) + "COVID-19 vaccine, live attenuated virus", + ), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2575,7 +2581,7 @@ class DatabaseImplTest { id = "immunization-3" vaccineCode = CodeableConcept( - Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based") + Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2586,11 +2592,11 @@ class DatabaseImplTest { Coding( "http://hl7.org/fhir/sid/cvx", "140", - "Influenza, seasonal, injectable, preservative free" - ) + "Influenza, seasonal, injectable, preservative free", + ), ) status = Immunization.ImmunizationStatus.COMPLETED - } + }, ) database.insert(*resources.toTypedArray()) @@ -2607,8 +2613,8 @@ class DatabaseImplTest { Coding( "http://id.who.int/icd11/mms", "XM1NL1", - "COVID-19 vaccine, inactivated virus" - ) + "COVID-19 vaccine, inactivated virus", + ), ) }, { @@ -2617,14 +2623,14 @@ class DatabaseImplTest { Coding( "http://id.who.int/icd11/mms", "XM5DF6", - "COVID-19 vaccine, inactivated virus" - ) + "COVID-19 vaccine, inactivated virus", + ), ) }, - operation = Operation.OR + operation = Operation.OR, ) } - .getQuery() + .getQuery(), ) assertThat(result.map { it.vaccineCode.codingFirstRep.code }) @@ -2640,7 +2646,11 @@ class DatabaseImplTest { id = "immunization-1" vaccineCode = CodeableConcept( - Coding("http://id.who.int/icd11/mms", "XM1NL1", "COVID-19 vaccine, inactivated virus") + Coding( + "http://id.who.int/icd11/mms", + "XM1NL1", + "COVID-19 vaccine, inactivated virus", + ), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2651,8 +2661,8 @@ class DatabaseImplTest { Coding( "http://id.who.int/icd11/mms", "XM5DF6", - "COVID-19 vaccine, live attenuated virus" - ) + "COVID-19 vaccine, live attenuated virus", + ), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2660,7 +2670,7 @@ class DatabaseImplTest { id = "immunization-3" vaccineCode = CodeableConcept( - Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based") + Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"), ) status = Immunization.ImmunizationStatus.COMPLETED }, @@ -2671,11 +2681,11 @@ class DatabaseImplTest { Coding( "http://hl7.org/fhir/sid/cvx", "140", - "Influenza, seasonal, injectable, preservative free" - ) + "Influenza, seasonal, injectable, preservative free", + ), ) status = Immunization.ImmunizationStatus.COMPLETED - } + }, ) database.insert(*resources.toTypedArray()) @@ -2686,16 +2696,16 @@ class DatabaseImplTest { .apply { filter( Immunization.VACCINE_CODE, - { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) } + { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) }, ) filter( Immunization.VACCINE_CODE, - { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) } + { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) }, ) operation = Operation.OR } - .getQuery() + .getQuery(), ) assertThat(result.map { it.vaccineCode.codingFirstRep.code }) @@ -2713,7 +2723,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("John") family = "Doe" - } + }, ) }, Patient().apply { @@ -2722,7 +2732,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("Jane") family = "Doe" - } + }, ) }, Patient().apply { @@ -2731,7 +2741,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("John") family = "Roe" - } + }, ) }, Patient().apply { @@ -2740,7 +2750,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("Jane") family = "Roe" - } + }, ) }, Patient().apply { @@ -2749,9 +2759,9 @@ class DatabaseImplTest { HumanName().apply { addGiven("Rocky") family = "Balboa" - } + }, ) - } + }, ) database.insert(*resources.toTypedArray()) @@ -2769,7 +2779,7 @@ class DatabaseImplTest { value = "Jane" modifier = StringFilterModifier.MATCHES_EXACTLY }, - operation = Operation.OR + operation = Operation.OR, ) filter( @@ -2782,12 +2792,12 @@ class DatabaseImplTest { value = "Roe" modifier = StringFilterModifier.MATCHES_EXACTLY }, - operation = Operation.OR + operation = Operation.OR, ) operation = Operation.AND } - .getQuery() + .getQuery(), ) assertThat(result.map { it.nameFirstRep.nameAsSingleString }) @@ -2814,7 +2824,7 @@ class DatabaseImplTest { Identifier().apply { system = "https://custom-identifier-namespace" value = "OfficialIdentifier_DarcySmith_0001" - } + }, ) addName( @@ -2824,14 +2834,14 @@ class DatabaseImplTest { addGiven("Darcy") gender = Enumerations.AdministrativeGender.FEMALE birthDateElement = DateType("1970-01-01") - } + }, ) addExtension( Extension().apply { url = "http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName" setValue(StringType("Marca")) - } + }, ) } // Get rid of the default service and create one with search params @@ -2848,10 +2858,10 @@ class DatabaseImplTest { { value = "Marca" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.map { it.nameFirstRep.nameAsSingleString }).contains("Darcy Smith") @@ -2865,7 +2875,7 @@ class DatabaseImplTest { Identifier().apply { system = "https://custom-identifier-namespace" value = "OfficialIdentifier_DarcySmith_0001" - } + }, ) addName( @@ -2875,14 +2885,14 @@ class DatabaseImplTest { addGiven("Darcy") gender = Enumerations.AdministrativeGender.FEMALE birthDateElement = DateType("1970-01-01") - } + }, ) addExtension( Extension().apply { url = "http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName" setValue(StringType("Marca")) - } + }, ) } val identifierPartialSearchParameter = @@ -2909,10 +2919,10 @@ class DatabaseImplTest { { value = "OfficialIdentifier_" modifier = StringFilterModifier.STARTS_WITH - } + }, ) } - .getQuery() + .getQuery(), ) assertThat(result.map { it.nameFirstRep.nameAsSingleString }).contains("Darcy Smith") @@ -2923,21 +2933,21 @@ class DatabaseImplTest { database.insert( Patient().apply { id = "patient-test-001" }, Patient().apply { id = "patient-test-002" }, - Patient().apply { id = "patient-test-003" } + Patient().apply { id = "patient-test-003" }, ) database.update( Patient().apply { id = "patient-test-002" gender = Enumerations.AdministrativeGender.FEMALE - } + }, ) val result = database.search( Search(ResourceType.Patient) .apply { sort(LOCAL_LAST_UPDATED_PARAM, Order.DESCENDING) } - .getQuery() + .getQuery(), ) assertThat(result.map { it.logicalId }) @@ -2954,7 +2964,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Gorden" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-01")) addGeneralPractitioner(Reference("Practitioner/gp-02")) @@ -2967,7 +2977,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Bond" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-02")) addGeneralPractitioner(Reference("Practitioner/gp-03")) @@ -2981,7 +2991,7 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-01" addGiven("General-01") - } + }, ) active = true } @@ -2992,7 +3002,7 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-02" addGiven("General-02") - } + }, ) active = false } @@ -3003,7 +3013,7 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-03" addGiven("General-03") - } + }, ) active = true } @@ -3020,7 +3030,7 @@ class DatabaseImplTest { { value = "James" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) include(Patient.GENERAL_PRACTITIONER) { @@ -3035,14 +3045,14 @@ class DatabaseImplTest { SearchResult( patient01, included = mapOf(Patient.GENERAL_PRACTITIONER.paramName to listOf(gp01)), - revIncluded = null + revIncluded = null, ), SearchResult( patient02, included = mapOf(Patient.GENERAL_PRACTITIONER.paramName to listOf(gp03)), - revIncluded = null - ) - ) + revIncluded = null, + ), + ), ) } @@ -3055,7 +3065,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Gorden" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-01")) } @@ -3067,7 +3077,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Bond" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-02")) } @@ -3109,7 +3119,7 @@ class DatabaseImplTest { { value = "James" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) revInclude(Condition.SUBJECT) { filter(Condition.CODE, { value = of(diabetesCodeableConcept) }) @@ -3126,15 +3136,15 @@ class DatabaseImplTest { patient01, included = null, revIncluded = - mapOf((ResourceType.Condition to Condition.SUBJECT.paramName) to listOf(con1)) + mapOf((ResourceType.Condition to Condition.SUBJECT.paramName) to listOf(con1)), ), SearchResult( patient02, included = null, revIncluded = - mapOf((ResourceType.Condition to Condition.SUBJECT.paramName) to listOf(con3)) - ) - ) + mapOf((ResourceType.Condition to Condition.SUBJECT.paramName) to listOf(con3)), + ), + ), ) } @@ -3155,7 +3165,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Gorden" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-01")) addGeneralPractitioner(Reference("Practitioner/gp-02")) @@ -3168,7 +3178,7 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Bond" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-01")) addGeneralPractitioner(Reference("Practitioner/gp-02")) @@ -3181,13 +3191,13 @@ class DatabaseImplTest { HumanName().apply { addGiven("James") family = "Doe" - } + }, ) addGeneralPractitioner(Reference("Practitioner/gp-01")) addGeneralPractitioner(Reference("Practitioner/gp-02")) addGeneralPractitioner(Reference("Practitioner/gp-03")) managingOrganization = Reference("Organization/org-03") - } + }, ) val practitioners = @@ -3198,7 +3208,7 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-01" addGiven("General-01") - } + }, ) active = true }, @@ -3208,7 +3218,7 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-02" addGiven("General-02") - } + }, ) active = true }, @@ -3218,10 +3228,10 @@ class DatabaseImplTest { HumanName().apply { family = "Practitioner-03" addGiven("General-03") - } + }, ) active = false - } + }, ) val organizations = @@ -3240,7 +3250,7 @@ class DatabaseImplTest { id = "org-03" name = "Organization-03" active = false - } + }, ) val conditions = @@ -3374,7 +3384,7 @@ class DatabaseImplTest { start = DateType(2022, 2, 1).value end = DateType(2022, 11, 1).value } - } + }, ) // 3 Patients. // Each has 3 conditions, only 2 should match @@ -3395,7 +3405,7 @@ class DatabaseImplTest { { value = "James" modifier = StringFilterModifier.MATCHES_EXACTLY - } + }, ) include(Patient.GENERAL_PRACTITIONER) { @@ -3405,7 +3415,7 @@ class DatabaseImplTest { { value = "Practitioner" modifier = StringFilterModifier.STARTS_WITH - } + }, ) operation = Operation.AND } @@ -3415,7 +3425,7 @@ class DatabaseImplTest { { value = "Organization" modifier = StringFilterModifier.STARTS_WITH - } + }, ) filter(Practitioner.ACTIVE, { value = of(true) }) operation = Operation.AND @@ -3432,7 +3442,7 @@ class DatabaseImplTest { { value = of(DateTimeType("2023-01-01")) prefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS - } + }, ) } } @@ -3451,21 +3461,21 @@ class DatabaseImplTest { Pair(ResourceType.Condition, "subject") to listOf(resources["con-01-pa-01"]!!, resources["con-03-pa-01"]!!), Pair(ResourceType.Encounter, "subject") to - listOf(resources["en-01-pa-01"]!!, resources["en-02-pa-01"]!!) - ) + listOf(resources["en-01-pa-01"]!!, resources["en-02-pa-01"]!!), + ), ), SearchResult( resources["pa-02"]!!, mapOf( "general-practitioner" to listOf(resources["gp-01"]!!, resources["gp-02"]!!), - "organization" to listOf(resources["org-02"]!!) + "organization" to listOf(resources["org-02"]!!), ), mapOf( Pair(ResourceType.Condition, "subject") to listOf(resources["con-01-pa-02"]!!, resources["con-03-pa-02"]!!), Pair(ResourceType.Encounter, "subject") to - listOf(resources["en-01-pa-02"]!!, resources["en-02-pa-02"]!!) - ) + listOf(resources["en-01-pa-02"]!!, resources["en-02-pa-02"]!!), + ), ), SearchResult( resources["pa-03"]!!, @@ -3476,10 +3486,10 @@ class DatabaseImplTest { Pair(ResourceType.Condition, "subject") to listOf(resources["con-01-pa-03"]!!, resources["con-03-pa-03"]!!), Pair(ResourceType.Encounter, "subject") to - listOf(resources["en-01-pa-03"]!!, resources["en-02-pa-03"]!!) - ) - ) - ) + listOf(resources["en-01-pa-03"]!!, resources["en-02-pa-03"]!!), + ), + ), + ), ) } 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 7e7695c7db..0b9a31b4fb 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -55,7 +55,7 @@ interface FhirEngine { */ suspend fun syncUpload( localChangesFetchMode: LocalChangesFetchMode, - upload: (suspend (List) -> Flow>) + upload: (suspend (List) -> Flow>), ) /** @@ -64,7 +64,7 @@ interface FhirEngine { */ suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend () -> Flow> + download: suspend () -> Flow>, ) /** @@ -89,29 +89,32 @@ interface FhirEngine { * Retrieves a list of [LocalChange]s for [Resource] with given type and id, which can be used to * purge resource from database. If there is no local change for given [resourceType] and * [Resource.id], return an empty list. + * * @param type The [ResourceType] * @param id The resource id [Resource.id] * @return [List]<[LocalChange]> A list of local changes for given [resourceType] and - * [Resource.id] . If there is no local change for given [resourceType] and [Resource.id], return - * an empty list. + * [Resource.id] . If there is no local change for given [resourceType] and [Resource.id], + * return an empty list. */ suspend fun getLocalChanges(type: ResourceType, id: String): List /** * Purges a resource from the database based on resource type and id without any deletion of data * from the server. + * * @param type The [ResourceType] * @param id The resource id [Resource.id] * @param isLocalPurge default value is false here resource will not be deleted from - * LocalChangeEntity table but it will throw IllegalStateException("Resource has local changes - * either sync with server or FORCE_PURGE required") if local change exists. If true this API will - * delete resource entry from LocalChangeEntity table. + * LocalChangeEntity table but it will throw IllegalStateException("Resource has local changes + * either sync with server or FORCE_PURGE required") if local change exists. If true this API + * will delete resource entry from LocalChangeEntity table. */ suspend fun purge(type: ResourceType, id: String, forcePurge: Boolean = false) } /** * Returns a FHIR resource of type [R] with [id] from the local storage. + * * @param The resource type which should be a subtype of [Resource]. * @throws ResourceNotFoundException if the resource is not found */ @@ -122,6 +125,7 @@ suspend inline fun FhirEngine.get(id: String): R { /** * Deletes a FHIR resource of type [R] with [id] from the local storage. + * * @param The resource type which should be a subtype of [Resource]. */ suspend inline fun FhirEngine.delete(id: String) { @@ -140,7 +144,7 @@ data class SearchResult( /** Matching referenced resources as per the [Search.include] criteria in the query. */ val included: Map>?, /** Matching referenced resources as per the [Search.revInclude] criteria in the query. */ - val revIncluded: Map, List>? + val revIncluded: Map, List>?, ) { override fun equals(other: Any?) = other is SearchResult<*> && @@ -157,7 +161,7 @@ data class SearchResult( private fun equalsShallow( first: Map>?, - second: Map>? + second: Map>?, ) = if (first != null && second != null && first.size == second.size) { first.entries.asSequence().zip(second.entries.asSequence()).all { (x, y) -> @@ -170,7 +174,7 @@ data class SearchResult( @JvmName("equalsShallowRevInclude") private fun equalsShallow( first: Map, List>?, - second: Map, List>? + second: Map, List>?, ) = if (first != null && second != null && first.size == second.size) { first.entries.asSequence().zip(second.entries.asSequence()).all { (x, y) -> 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 9476133b8c..ea8198a6aa 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 @@ -83,7 +83,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex override suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend () -> Flow> + download: suspend () -> Flow>, ) { download().collect { resources -> database.withTransaction { @@ -91,7 +91,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex resolveConflictingResources( resources, getConflictingResourceIds(resources), - conflictResolver + conflictResolver, ) database.insertSyncedResources(resources) saveResolvedResourcesToDatabase(resolved) @@ -109,7 +109,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex private suspend fun resolveConflictingResources( resources: List, conflictingResourceIds: Set, - conflictResolver: ConflictResolver + conflictResolver: ConflictResolver, ) = resources .filter { conflictingResourceIds.contains(it.logicalId) } @@ -126,7 +126,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex override suspend fun syncUpload( localChangesFetchMode: LocalChangesFetchMode, - upload: suspend (List) -> Flow> + upload: suspend (List) -> Flow>, ) { val localChangeFetcher = LocalChangeFetcher.byMode(localChangesFetchMode, database) upload(localChangeFetcher.next()).collect { @@ -162,7 +162,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex id, type, getVersionFromETag(response.etag), - response.lastModified.toInstant() + response.lastModified.toInstant(), ) } } @@ -174,7 +174,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex resource.id, resource.resourceType, resource.meta.versionId, - resource.meta.lastUpdated.toInstant() + resource.meta.lastUpdated.toInstant(), ) } } @@ -198,9 +198,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex * [Bundle.BundleEntryResponseComponent.location]. * * [Bundle.BundleEntryResponseComponent.location] may be: - * * 1. absolute path: `///_history/` - * * 2. relative path: `//_history/` */ private val Bundle.BundleEntryResponseComponent.resourceIdAndType: Pair? diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt index f1a43b480b..f2f6de79b0 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt @@ -19,9 +19,9 @@ package com.google.android.fhir.sync import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine -import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.android.fhir.sync.download.DownloadState import com.google.android.fhir.sync.download.Downloader +import com.google.android.fhir.sync.upload.LocalChangesFetchMode import com.google.android.fhir.sync.upload.UploadState import com.google.android.fhir.sync.upload.Uploader import java.time.OffsetDateTime diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 79f134dfd0..78b67733b0 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -43,7 +43,7 @@ internal interface LocalChangeFetcher { companion object { internal suspend fun byMode( mode: LocalChangesFetchMode, - database: Database + database: Database, ): LocalChangeFetcher { val totalLocalChangeCount = database.getAllLocalChanges().size return when (mode) { @@ -62,7 +62,7 @@ data class ProgressState( internal class AllChangesLocalChangeFetcher( private val database: Database, - private val total: Int + private val total: Int, ) : LocalChangeFetcher { override suspend fun hasNext(): Boolean = database.getAllLocalChanges().isNotEmpty() @@ -77,6 +77,8 @@ internal class AllChangesLocalChangeFetcher( sealed class LocalChangesFetchMode { object AllChanges : LocalChangesFetchMode() + object PerResource : LocalChangesFetchMode() + object EarliestChange : LocalChangesFetchMode() } 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 b80f6aec68..ebd90ab6dc 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 @@ -112,7 +112,7 @@ object TestDataSourceImpl : DataSource { } open class TestDownloadManagerImpl( - private val queries: List = listOf("Patient?address-city=NAIROBI") + private val queries: List = listOf("Patient?address-city=NAIROBI"), ) : DownloadWorkManager { private val urls = LinkedList(queries) @@ -149,15 +149,16 @@ object TestFhirEngineImpl : FhirEngine { override suspend fun syncUpload( localChangesFetchMode: LocalChangesFetchMode, - upload: suspend (List) -> Flow> + upload: suspend (List) -> Flow>, ) = upload(getLocalChanges(ResourceType.Patient, "123")).collect() override suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend () -> Flow> + download: suspend () -> Flow>, ) { download().collect() } + override suspend fun count(search: Search): Long { return 0 } @@ -176,8 +177,8 @@ object TestFhirEngineImpl : FhirEngine { payload = "{ 'resourceType' : 'Patient', 'id' : '123' }", token = LocalChangeToken(listOf()), type = LocalChange.Type.INSERT, - timestamp = Instant.now() - ) + timestamp = Instant.now(), + ), ) } 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 99826e3362..fbf400007e 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 @@ -86,7 +86,7 @@ class FhirEngineImplTest { HumanName().apply { family = "FamilyName" addGiven("GivenName") - } + }, ) } val ids = fhirEngine.create(patient.copy()) @@ -102,7 +102,7 @@ class FhirEngineImplTest { } assertThat(exception.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_2.resourceType.name} and id $TEST_PATIENT_2_ID!" + "Resource not found with type ${TEST_PATIENT_2.resourceType.name} and id $TEST_PATIENT_2_ID!", ) } @@ -161,7 +161,7 @@ class FhirEngineImplTest { } assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${ResourceType.Patient.name} and id nonexistent_patient!" + "Resource not found with type ${ResourceType.Patient.name} and id nonexistent_patient!", ) } @@ -176,7 +176,7 @@ class FhirEngineImplTest { listOf( buildPatient("3", "C", Enumerations.AdministrativeGender.FEMALE), buildPatient("2", "B", Enumerations.AdministrativeGender.FEMALE), - buildPatient("1", "A", Enumerations.AdministrativeGender.MALE) + buildPatient("1", "A", Enumerations.AdministrativeGender.MALE), ) fhirEngine.create(*patients.toTypedArray()) @@ -185,7 +185,7 @@ class FhirEngineImplTest { assertThat(result.size).isEqualTo(2) assertThat( - result.all { (it.resource as Patient).gender == Enumerations.AdministrativeGender.FEMALE } + result.all { (it.resource as Patient).gender == Enumerations.AdministrativeGender.FEMALE }, ) .isTrue() } @@ -196,7 +196,7 @@ class FhirEngineImplTest { listOf( buildPatient("3", "C", Enumerations.AdministrativeGender.FEMALE), buildPatient("2", "B", Enumerations.AdministrativeGender.FEMALE), - buildPatient("1", "A", Enumerations.AdministrativeGender.MALE) + buildPatient("1", "A", Enumerations.AdministrativeGender.MALE), ) fhirEngine.create(*patients.toTypedArray()) @@ -213,7 +213,7 @@ class FhirEngineImplTest { listOf( buildPatient("3", "C", Enumerations.AdministrativeGender.FEMALE), buildPatient("2", "B", Enumerations.AdministrativeGender.FEMALE), - buildPatient("1", "A", Enumerations.AdministrativeGender.MALE) + buildPatient("1", "A", Enumerations.AdministrativeGender.MALE), ) fhirEngine.create(*patients.toTypedArray()) @@ -259,7 +259,7 @@ class FhirEngineImplTest { }, buildPatient("2", "Patient2", Enumerations.AdministrativeGender.FEMALE).apply { meta = Meta().setTag(mutableListOf(Coding("http://d-tree.org/", "Tag2", "Tag 2"))) - } + }, ) fhirEngine.create(*patients.toTypedArray()) @@ -280,15 +280,15 @@ class FhirEngineImplTest { .setProfile( mutableListOf( CanonicalType( - "http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1" - ) - ) + "http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1", + ), + ), ) }, buildPatient("4", "C", Enumerations.AdministrativeGender.FEMALE).apply { meta = Meta().setProfile(mutableListOf(CanonicalType("http://d-tree.org/Diabetes-Patient"))) - } + }, ) fhirEngine.create(*patients.toTypedArray()) @@ -296,7 +296,7 @@ class FhirEngineImplTest { val result = fhirEngine .search( - "Patient?_profile=http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1" + "Patient?_profile=http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1", ) .map { it.resource as Patient } @@ -306,7 +306,7 @@ class FhirEngineImplTest { patient.meta.profile.all { it.value.equals("http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1") } - } + }, ) .isTrue() } @@ -340,7 +340,7 @@ class FhirEngineImplTest { private fun buildPatient( patientId: String, name: String, - patientGender: Enumerations.AdministrativeGender + patientGender: Enumerations.AdministrativeGender, ) = Patient().apply { id = patientId @@ -436,7 +436,7 @@ class FhirEngineImplTest { } assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID!" + "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID!", ) assertThat(fhirEngine.getLocalChanges(ResourceType.Patient, TEST_PATIENT_1_ID)).isEmpty() } @@ -450,7 +450,7 @@ class FhirEngineImplTest { } assertThat(resourceIllegalStateException.message) .isEqualTo( - "Resource with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID has local changes, either sync with server or FORCE_PURGE required" + "Resource with type ${TEST_PATIENT_1.resourceType.name} and id $TEST_PATIENT_1_ID has local changes, either sync with server or FORCE_PURGE required", ) } @@ -462,9 +462,10 @@ class FhirEngineImplTest { } assertThat(resourceNotFoundException.message) .isEqualTo( - "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id nonexistent_patient!" + "Resource not found with type ${TEST_PATIENT_1.resourceType.name} and id nonexistent_patient!", ) } + fun syncDownload_conflictResolution_acceptRemote_shouldHaveNoLocalChangeAnymore() = runBlocking { val originalPatient = Patient().apply { @@ -478,7 +479,7 @@ class FhirEngineImplTest { HumanName().apply { family = "Stark" addGiven("Tony") - } + }, ) } fhirEngine.syncDownload(AcceptRemoteConflictResolver) { flowOf((listOf((originalPatient)))) } @@ -500,7 +501,7 @@ class FhirEngineImplTest { fhirEngine.syncDownload(AcceptRemoteConflictResolver) { flowOf((listOf(remoteChange))) } assertThat( - services.database.getAllLocalChanges().filter { it.resourceId == "Patient/original-001" } + services.database.getAllLocalChanges().filter { it.resourceId == "Patient/original-001" }, ) .isEmpty() assertResourceEquals(fhirEngine.get("original-001"), remoteChange) @@ -521,7 +522,7 @@ class FhirEngineImplTest { HumanName().apply { family = "Stark" addGiven("Tony") - } + }, ) } fhirEngine.syncDownload(AcceptLocalConflictResolver) { flowOf((listOf((originalPatient)))) } @@ -535,7 +536,7 @@ class FhirEngineImplTest { Address().apply { city = "Malibu" state = "California" - } + }, ) } fhirEngine.update(localChange) @@ -555,7 +556,7 @@ class FhirEngineImplTest { val localChangeDiff = """[{"op":"remove","path":"\/address\/0\/country"},{"op":"add","path":"\/address\/0\/city","value":"Malibu"},{"op":"add","path":"\/address\/-","value":{"city":"Malibu","state":"California"}}]""" assertThat( - services.database.getAllLocalChanges().first { it.resourceId == "original-002" }.payload + services.database.getAllLocalChanges().first { it.resourceId == "original-002" }.payload, ) .isEqualTo(localChangeDiff) assertResourceEquals(fhirEngine.get("original-002"), localChange) @@ -575,7 +576,7 @@ class FhirEngineImplTest { { value = of(DateTimeType(Date.from(localChangeTimestamp))) prefix = ParamPrefixEnum.EQUAL - } + }, ) } @@ -597,7 +598,7 @@ class FhirEngineImplTest { HumanName().apply { addGiven("John") family = "Doe" - } + }, ) } fhirEngine.update(patientUpdate) @@ -611,7 +612,7 @@ class FhirEngineImplTest { { value = of(DateTimeType(Date.from(localChangeTimestampWhenUpdated))) prefix = ParamPrefixEnum.EQUAL - } + }, ) } diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt index eb093b6bfd..b31b28b10f 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt @@ -77,6 +77,7 @@ class AllChangesLocalChangeFetcherTest { fun `getProgress when none of the local changes are removed`() = runTest { assertThat(fetcher.getProgress()).isEqualTo(ProgressState(2, 2)) } + companion object { private const val TEST_PATIENT_1_ID = "test_patient_1" private var TEST_PATIENT_1 = From 48eaf32f0416e5519067d0c5b463cfb9e5b30021 Mon Sep 17 00:00:00 2001 From: omarismail Date: Thu, 14 Sep 2023 14:00:52 +0100 Subject: [PATCH 08/11] add count api to db and refactor mode init --- .../android/fhir/db/impl/DatabaseImplTest.kt | 19 +++++++ .../com/google/android/fhir/db/Database.kt | 17 ++++--- .../android/fhir/db/impl/DatabaseImpl.kt | 31 ++++++----- .../fhir/db/impl/dao/LocalChangeDao.kt | 51 ++++++++++++------- .../android/fhir/impl/FhirEngineImpl.kt | 12 +++-- .../fhir/sync/upload/LocalChangeFetcher.kt | 30 ++++++----- .../AllChangesLocalChangeFetcherTest.kt | 8 +-- 7 files changed, 110 insertions(+), 58 deletions(-) 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 508a92cd8b..82bf1237dc 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 @@ -511,6 +511,8 @@ class DatabaseImplTest { lastUpdated = Date() } database.insert(patient) + // 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(LocalChangesFetchMode.AllChanges) { it .first { it.resourceId == "remote-patient-3" } @@ -693,6 +695,23 @@ class DatabaseImplTest { } } + @Test + fun one_localChange_should_return_one_for_count() = runBlocking { + assertThat(database.getLocalChangesCount()).isEqualTo(1) + } + + @Test + fun add_resource_should_count_two_for_localChanges() = runBlocking { + database.insert(TEST_PATIENT_2) + assertThat(database.getLocalChangesCount()).isEqualTo(2) + } + + @Test + fun no_localChanges_should_return_zero_for_count() = runBlocking { + database.deleteUpdates(listOf(TEST_PATIENT_1)) + assertThat(database.getLocalChangesCount()).isEqualTo(0) + } + @Test fun search_sortDescending_twoVeryCloseFloatingPointNumbers_orderedCorrectly() = runBlocking { val smallerId = "risk_assessment_1" 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 b2cb416fcb..cbbc840b64 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 @@ -58,7 +58,7 @@ internal interface Database { resourceId: String, resourceType: ResourceType, versionId: String, - lastUpdated: Instant + lastUpdated: Instant, ) /** @@ -105,6 +105,9 @@ internal interface Database { */ suspend fun getAllLocalChanges(): List + /** Retrieves the count of [LocalChange]s stored in the database. */ + suspend fun getLocalChangesCount(): Int + /** Remove the [LocalChangeEntity] s with given ids. Call this after a successful sync. */ suspend fun deleteUpdates(token: LocalChangeToken) @@ -127,23 +130,25 @@ internal interface Database { * Retrieve a list of [LocalChange] for [Resource] with given type and id, which can be used to * purge resource from database. If there is no local change for given [resourceType] and * [Resource.id], return an empty list. + * * @param type The [ResourceType] * @param id The resource id [Resource.id] * @return [List]<[LocalChange]> A list of local changes for given [resourceType] and - * [Resource.id] . If there is no local change for given [resourceType] and [Resource.id], return - * empty list. + * [Resource.id] . If there is no local change for given [resourceType] and [Resource.id], + * return empty list. */ suspend fun getLocalChanges(type: ResourceType, id: String): List /** * Purge resource from database based on resource type and id without any deletion of data from * the server. + * * @param type The [ResourceType] * @param id The resource id [Resource.id] * @param isLocalPurge default value is false here resource will not be deleted from - * LocalChangeEntity table but it will throw IllegalStateException("Resource has local changes - * either sync with server or FORCE_PURGE required") if local change exists. If true this API will - * delete resource entry from LocalChangeEntity table. + * LocalChangeEntity table but it will throw IllegalStateException("Resource has local changes + * either sync with server or FORCE_PURGE required") if local change exists. If true this API + * will delete resource entry from LocalChangeEntity table. */ suspend fun purge(type: ResourceType, id: String, forcePurge: Boolean = false) } diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt index 8da36f8bce..8ca9fff23c 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt @@ -46,7 +46,7 @@ internal class DatabaseImpl( private val context: Context, private val iParser: IParser, databaseConfig: DatabaseConfig, - private val resourceIndexer: ResourceIndexer + private val resourceIndexer: ResourceIndexer, ) : com.google.android.fhir.db.Database { val db: ResourceDatabase @@ -88,8 +88,10 @@ internal class DatabaseImpl( openHelperFactory { SQLCipherSupportHelper( it, - databaseErrorStrategy = databaseConfig.databaseErrorStrategy - ) { DatabaseEncryptionKeyProvider.getOrCreatePassphrase(DATABASE_PASSPHRASE_NAME) } + databaseErrorStrategy = databaseConfig.databaseErrorStrategy, + ) { + DatabaseEncryptionKeyProvider.getOrCreatePassphrase(DATABASE_PASSPHRASE_NAME) + } } } @@ -115,7 +117,7 @@ internal class DatabaseImpl( val timeOfLocalChange = Instant.now() localChangeDao.addInsert(it, timeOfLocalChange) resourceDao.insertLocalResource(it, timeOfLocalChange) - } + }, ) } return logicalIds @@ -140,14 +142,14 @@ internal class DatabaseImpl( resourceId: String, resourceType: ResourceType, versionId: String, - lastUpdated: Instant + lastUpdated: Instant, ) { db.withTransaction { resourceDao.updateAndIndexRemoteVersionIdAndLastUpdate( resourceId, resourceType, versionId, - lastUpdated + lastUpdated, ) } } @@ -174,12 +176,13 @@ internal class DatabaseImpl( null } val rowsDeleted = resourceDao.deleteResource(resourceId = id, resourceType = type) - if (rowsDeleted > 0) + if (rowsDeleted > 0) { localChangeDao.addDelete( resourceId = id, resourceType = type, - remoteVersionId = remoteVersionId + remoteVersionId = remoteVersionId, ) + } } } @@ -200,7 +203,7 @@ internal class DatabaseImpl( IndexedIdAndResource( it.matchingIndex, it.idOfBaseResourceOnWhichThisMatchedInc ?: it.idOfBaseResourceOnWhichThisMatchedRev!!, - iParser.parseResource(it.serializedResource) as Resource + iParser.parseResource(it.serializedResource) as Resource, ) } } @@ -216,6 +219,10 @@ internal class DatabaseImpl( return db.withTransaction { localChangeDao.getAllLocalChanges().map { it.toLocalChange() } } } + override suspend fun getLocalChangesCount(): Int { + return db.withTransaction { localChangeDao.getLocalChangesCount() } + } + override suspend fun deleteUpdates(token: LocalChangeToken) { db.withTransaction { localChangeDao.discardLocalChanges(token) } } @@ -265,12 +272,12 @@ internal class DatabaseImpl( if (forcePurge) { resourceDao.deleteResource(resourceId = id, resourceType = type) localChangeDao.discardLocalChanges( - token = LocalChangeToken(localChangeEntityList.map { it.id }) + token = LocalChangeToken(localChangeEntityList.map { it.id }), ) } else { // local change is available but FORCE_PURGE = false then throw exception throw IllegalStateException( - "Resource with type $type and id $id has local changes, either sync with server or FORCE_PURGE required" + "Resource with type $type and id $id has local changes, either sync with server or FORCE_PURGE required", ) } } @@ -301,5 +308,5 @@ internal class DatabaseImpl( data class DatabaseConfig( val inMemory: Boolean, val enableEncryption: Boolean, - val databaseErrorStrategy: DatabaseErrorStrategy + val databaseErrorStrategy: DatabaseErrorStrategy, ) diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt index f8c9524439..d69684a9bd 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt @@ -64,8 +64,8 @@ internal abstract class LocalChangeDao { timestamp = timeOfLocalChange, type = Type.INSERT, payload = resourceString, - versionId = resource.versionId - ) + versionId = resource.versionId, + ), ) } @@ -73,11 +73,12 @@ internal abstract class LocalChangeDao { val resourceId = resource.logicalId val resourceType = resource.resourceType - if (!localChangeIsEmpty(resourceId, resourceType) && + if ( + !localChangeIsEmpty(resourceId, resourceType) && lastChangeType(resourceId, resourceType)!! == Type.DELETE ) { throw InvalidLocalChangeException( - "Unexpected DELETE when updating $resourceType/$resourceId. UPDATE failed." + "Unexpected DELETE when updating $resourceType/$resourceId. UPDATE failed.", ) } val jsonDiff = @@ -85,7 +86,7 @@ internal abstract class LocalChangeDao { if (jsonDiff.length() == 0) { Timber.i( "New resource ${resource.resourceType}/${resource.id} is same as old resource. " + - "Not inserting UPDATE LocalChange." + "Not inserting UPDATE LocalChange.", ) return } @@ -97,8 +98,8 @@ internal abstract class LocalChangeDao { timestamp = timeOfLocalChange, type = Type.UPDATE, payload = jsonDiff.toString(), - versionId = oldEntity.versionId - ) + versionId = oldEntity.versionId, + ), ) } @@ -111,8 +112,8 @@ internal abstract class LocalChangeDao { timestamp = Date().toInstant(), type = Type.DELETE, payload = "", - versionId = remoteVersionId - ) + versionId = remoteVersionId, + ), ) } @@ -124,7 +125,7 @@ internal abstract class LocalChangeDao { AND resourceType = :resourceType ORDER BY id ASC LIMIT 1 - """ + """, ) abstract suspend fun lastChangeType(resourceId: String, resourceType: ResourceType): Type? @@ -135,7 +136,7 @@ internal abstract class LocalChangeDao { WHERE resourceId = :resourceId AND resourceType = :resourceType LIMIT 1 - """ + """, ) abstract suspend fun countLastChange(resourceId: String, resourceType: ResourceType): Int @@ -146,15 +147,23 @@ internal abstract class LocalChangeDao { """ SELECT * FROM LocalChangeEntity - ORDER BY LocalChangeEntity.id ASC""" + ORDER BY LocalChangeEntity.id ASC""", ) abstract suspend fun getAllLocalChanges(): List + @Query( + """ + SELECT COUNT(*) + FROM LocalChangeEntity + """, + ) + abstract suspend fun getLocalChangesCount(): Int + @Query( """ DELETE FROM LocalChangeEntity WHERE LocalChangeEntity.id = (:id) - """ + """, ) abstract suspend fun discardLocalChanges(id: Long) @@ -168,7 +177,7 @@ internal abstract class LocalChangeDao { DELETE FROM LocalChangeEntity WHERE resourceId = (:resourceId) AND resourceType = :resourceType - """ + """, ) abstract suspend fun discardLocalChanges(resourceId: String, resourceType: ResourceType) @@ -181,11 +190,11 @@ internal abstract class LocalChangeDao { SELECT * FROM LocalChangeEntity WHERE resourceId = :resourceId AND resourceType = :resourceType - """ + """, ) abstract suspend fun getLocalChanges( resourceType: ResourceType, - resourceId: String + resourceId: String, ): List class InvalidLocalChangeException(message: String?) : Exception(message) @@ -197,8 +206,8 @@ internal fun diff(parser: IParser, source: Resource, target: Resource): JSONArra return getFilteredJSONArray( JsonDiff.asJson( objectMapper.readValue(parser.encodeResourceToString(source), JsonNode::class.java), - objectMapper.readValue(parser.encodeResourceToString(target), JsonNode::class.java) - ) + objectMapper.readValue(parser.encodeResourceToString(target), JsonNode::class.java), + ), ) } @@ -210,12 +219,14 @@ internal fun diff(parser: IParser, source: Resource, target: Resource): JSONArra * and causes the issue with server update. * * An unfiltered JSON Array for family name update looks like + * * ``` * [{"op":"remove","path":"/meta"}, {"op":"remove","path":"/text"}, * {"op":"replace","path":"/name/0/family","value":"Nucleus"}] * ``` * * A filtered JSON Array for family name update looks like + * * ``` * [{"op":"replace","path":"/name/0/family","value":"Nucleus"}] * ``` @@ -226,6 +237,8 @@ private fun getFilteredJSONArray(jsonDiff: JsonNode) = return@with JSONArray( (0 until length()) .map { optJSONObject(it) } - .filterNot { jsonObject -> ignorePaths.any { jsonObject.optString("path").startsWith(it) } } + .filterNot { jsonObject -> + ignorePaths.any { jsonObject.optString("path").startsWith(it) } + }, ) } 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 ea8198a6aa..e2d101c65b 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 @@ -129,11 +129,13 @@ internal class FhirEngineImpl(private val database: Database, private val contex upload: suspend (List) -> Flow>, ) { val localChangeFetcher = LocalChangeFetcher.byMode(localChangesFetchMode, database) - upload(localChangeFetcher.next()).collect { - database.deleteUpdates(it.first) - when (it.second) { - is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) - else -> updateVersionIdAndLastUpdated(it.second) + while (localChangeFetcher.hasNext()) { + upload(localChangeFetcher.next()).collect { + database.deleteUpdates(it.first) + when (it.second) { + is Bundle -> updateVersionIdAndLastUpdated(it.second as Bundle) + else -> updateVersionIdAndLastUpdated(it.second) + } } } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 78b67733b0..b3b3dae319 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -18,6 +18,7 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange import com.google.android.fhir.db.Database +import kotlin.properties.Delegates /** * Fetches local changes. @@ -35,42 +36,45 @@ internal interface LocalChangeFetcher { suspend fun next(): List /** - * Returns [ProgressState], which contains the remaining changes left to upload and the initial + * Returns [FetchProgress], which contains the remaining changes left to upload and the initial * total to upload */ - suspend fun getProgress(): ProgressState + suspend fun getProgress(): FetchProgress companion object { internal suspend fun byMode( mode: LocalChangesFetchMode, database: Database, - ): LocalChangeFetcher { - val totalLocalChangeCount = database.getAllLocalChanges().size - return when (mode) { + ): LocalChangeFetcher = + when (mode) { is LocalChangesFetchMode.AllChanges -> - AllChangesLocalChangeFetcher(database, totalLocalChangeCount) + AllChangesLocalChangeFetcher(database).apply { initTotalCount() } else -> error("$mode does not have an implementation yet.") } - } } } -data class ProgressState( +data class FetchProgress( val remaining: Int, val initialTotal: Int, ) internal class AllChangesLocalChangeFetcher( private val database: Database, - private val total: Int, ) : LocalChangeFetcher { - override suspend fun hasNext(): Boolean = database.getAllLocalChanges().isNotEmpty() + private var total by Delegates.notNull() + + suspend fun initTotalCount() { + total = database.getLocalChangesCount() + } + + override suspend fun hasNext(): Boolean = database.getLocalChangesCount().isNotZero() override suspend fun next(): List = database.getAllLocalChanges() - override suspend fun getProgress(): ProgressState = - ProgressState(database.getAllLocalChanges().size, total) + override suspend fun getProgress(): FetchProgress = + FetchProgress(database.getLocalChangesCount(), total) } /** Represents the mode in which local changes should be fetched. */ @@ -82,3 +86,5 @@ sealed class LocalChangesFetchMode { object EarliestChange : LocalChangesFetchMode() } + +private fun Int.isNotZero() = this != 0 diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt index b31b28b10f..b14637f446 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/AllChangesLocalChangeFetcherTest.kt @@ -39,7 +39,7 @@ class AllChangesLocalChangeFetcherTest { @Before fun setup() = runTest { database.insert(TEST_PATIENT_1, TEST_PATIENT_2) - fetcher = AllChangesLocalChangeFetcher(database, 2) + fetcher = AllChangesLocalChangeFetcher(database).apply { initTotalCount() } } @Test @@ -64,18 +64,18 @@ class AllChangesLocalChangeFetcherTest { @Test fun `getProgress when all local changes are removed`() = runTest { database.deleteUpdates(listOf(TEST_PATIENT_1, TEST_PATIENT_2)) - assertThat(fetcher.getProgress()).isEqualTo(ProgressState(0, 2)) + assertThat(fetcher.getProgress()).isEqualTo(FetchProgress(0, 2)) } @Test fun `getProgress when half the local changes are removed`() = runTest { database.deleteUpdates(listOf(TEST_PATIENT_1)) - assertThat(fetcher.getProgress()).isEqualTo(ProgressState(1, 2)) + assertThat(fetcher.getProgress()).isEqualTo(FetchProgress(1, 2)) } @Test fun `getProgress when none of the local changes are removed`() = runTest { - assertThat(fetcher.getProgress()).isEqualTo(ProgressState(2, 2)) + assertThat(fetcher.getProgress()).isEqualTo(FetchProgress(2, 2)) } companion object { From 1be186b4d00d648e17ac28a83baec1f993deffdb Mon Sep 17 00:00:00 2001 From: omarismail Date: Thu, 14 Sep 2023 15:01:41 +0100 Subject: [PATCH 09/11] try change --- .../android/fhir/db/impl/DatabaseImplTest.kt | 16 +++++----- .../android/fhir/impl/FhirEngineImpl.kt | 4 +-- .../fhir/sync/upload/LocalChangeFetcher.kt | 31 ++++++++++--------- 3 files changed, 27 insertions(+), 24 deletions(-) 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 82bf1237dc..2e6055fd82 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 @@ -696,20 +696,20 @@ class DatabaseImplTest { } @Test - fun one_localChange_should_return_one_for_count() = runBlocking { - assertThat(database.getLocalChangesCount()).isEqualTo(1) + fun getLocalChangesCount_noLocalChange_returnsZero() = runBlocking { + database.deleteUpdates(listOf(TEST_PATIENT_1)) + assertThat(database.getLocalChangesCount()).isEqualTo(0) } @Test - fun add_resource_should_count_two_for_localChanges() = runBlocking { - database.insert(TEST_PATIENT_2) - assertThat(database.getLocalChangesCount()).isEqualTo(2) + fun getLocalChangesCount_oneLocalChange_returnsOne() = runBlocking { + assertThat(database.getLocalChangesCount()).isEqualTo(1) } @Test - fun no_localChanges_should_return_zero_for_count() = runBlocking { - database.deleteUpdates(listOf(TEST_PATIENT_1)) - assertThat(database.getLocalChangesCount()).isEqualTo(0) + fun getLocalChangesCount_twoLocalChange_returnsTwo() = runBlocking { + database.insert(TEST_PATIENT_2) + assertThat(database.getLocalChangesCount()).isEqualTo(2) } @Test 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 e2d101c65b..d45b407720 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 @@ -29,7 +29,7 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved -import com.google.android.fhir.sync.upload.LocalChangeFetcher +import com.google.android.fhir.sync.upload.LocalChangeFetcherFactory import com.google.android.fhir.sync.upload.LocalChangesFetchMode import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow @@ -128,7 +128,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex localChangesFetchMode: LocalChangesFetchMode, upload: suspend (List) -> Flow>, ) { - val localChangeFetcher = LocalChangeFetcher.byMode(localChangesFetchMode, database) + val localChangeFetcher = LocalChangeFetcherFactory.byMode(localChangesFetchMode, database) while (localChangeFetcher.hasNext()) { upload(localChangeFetcher.next()).collect { database.deleteUpdates(it.first) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index b3b3dae319..31d1bc2e28 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -29,6 +29,10 @@ import kotlin.properties.Delegates * It is marked as internal to keep [Database] unexposed to clients */ internal interface LocalChangeFetcher { + + /** Represents the initial total number of local changes to upload. */ + val total: Int + /** Checks if there are more local changes to be fetched. */ suspend fun hasNext(): Boolean @@ -40,18 +44,6 @@ internal interface LocalChangeFetcher { * total to upload */ suspend fun getProgress(): FetchProgress - - companion object { - internal suspend fun byMode( - mode: LocalChangesFetchMode, - database: Database, - ): LocalChangeFetcher = - when (mode) { - is LocalChangesFetchMode.AllChanges -> - AllChangesLocalChangeFetcher(database).apply { initTotalCount() } - else -> error("$mode does not have an implementation yet.") - } - } } data class FetchProgress( @@ -63,7 +55,7 @@ internal class AllChangesLocalChangeFetcher( private val database: Database, ) : LocalChangeFetcher { - private var total by Delegates.notNull() + override var total by Delegates.notNull() suspend fun initTotalCount() { total = database.getLocalChangesCount() @@ -79,7 +71,6 @@ internal class AllChangesLocalChangeFetcher( /** Represents the mode in which local changes should be fetched. */ sealed class LocalChangesFetchMode { - object AllChanges : LocalChangesFetchMode() object PerResource : LocalChangesFetchMode() @@ -87,4 +78,16 @@ sealed class LocalChangesFetchMode { object EarliestChange : LocalChangesFetchMode() } +internal object LocalChangeFetcherFactory { + suspend fun byMode( + mode: LocalChangesFetchMode, + database: Database, + ): LocalChangeFetcher = + when (mode) { + is LocalChangesFetchMode.AllChanges -> + AllChangesLocalChangeFetcher(database).apply { initTotalCount() } + else -> throw NotImplementedError("$mode is not implemented yet.") + } +} + private fun Int.isNotZero() = this != 0 From 5871b4445ca2c022c1e0ac52fc79699ec6ad06b5 Mon Sep 17 00:00:00 2001 From: omarismail Date: Thu, 14 Sep 2023 15:37:01 +0100 Subject: [PATCH 10/11] spotless --- .../main/java/com/google/android/fhir/impl/FhirEngineImpl.kt | 2 +- .../com/google/android/fhir/sync/upload/LocalChangeFetcher.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 82ffbe6b26..adf2611487 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 @@ -29,9 +29,9 @@ import com.google.android.fhir.search.count import com.google.android.fhir.search.execute import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved +import com.google.android.fhir.sync.upload.DefaultResourceConsolidator import com.google.android.fhir.sync.upload.LocalChangeFetcherFactory import com.google.android.fhir.sync.upload.LocalChangesFetchMode -import com.google.android.fhir.sync.upload.DefaultResourceConsolidator import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 31d1bc2e28..9b0e66d8c6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -30,7 +30,7 @@ import kotlin.properties.Delegates */ internal interface LocalChangeFetcher { - /** Represents the initial total number of local changes to upload. */ + /** Represents the initial total number of local changes to upload. */ val total: Int /** Checks if there are more local changes to be fetched. */ From 2b9ea35851b6f03811ed417ba9399304d6cebd58 Mon Sep 17 00:00:00 2001 From: Omar Ismail <44980219+omarismail94@users.noreply.github.com> Date: Thu, 14 Sep 2023 16:12:19 +0100 Subject: [PATCH 11/11] Update engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt Co-authored-by: Jing Tang --- .../com/google/android/fhir/sync/upload/LocalChangeFetcher.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt index 9b0e66d8c6..34f8eee4d4 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/LocalChangeFetcher.kt @@ -41,7 +41,7 @@ internal interface LocalChangeFetcher { /** * Returns [FetchProgress], which contains the remaining changes left to upload and the initial - * total to upload + * total to upload. */ suspend fun getProgress(): FetchProgress }