diff --git a/demo/build.gradle.kts b/demo/build.gradle.kts index 0625bdb87a..285458b1f6 100644 --- a/demo/build.gradle.kts +++ b/demo/build.gradle.kts @@ -47,6 +47,7 @@ dependencies { implementation(Dependencies.Androidx.activity) implementation(Dependencies.Androidx.appCompat) implementation(Dependencies.Androidx.constraintLayout) + implementation(Dependencies.Androidx.datastorePref) implementation(Dependencies.Androidx.fragmentKtx) implementation(Dependencies.Androidx.recyclerView) implementation(Dependencies.Androidx.workRuntimeKtx) diff --git a/demo/src/main/java/com/google/android/fhir/demo/DemoDataStore.kt b/demo/src/main/java/com/google/android/fhir/demo/DemoDataStore.kt new file mode 100644 index 0000000000..53a260b3ca --- /dev/null +++ b/demo/src/main/java/com/google/android/fhir/demo/DemoDataStore.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2022 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.demo + +import android.content.Context +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import androidx.datastore.preferences.preferencesDataStore +import com.google.android.fhir.sync.DownloadWorkManager +import kotlinx.coroutines.flow.first +import org.hl7.fhir.r4.model.ResourceType + +private val Context.dataStorage: DataStore by + preferencesDataStore(name = "demo_app_storage") + +/** + * Stores the lastUpdated timestamp per resource to be used by [DownloadWorkManager]'s + * implementation for optimal sync. See + * [_lastUpdated](https://build.fhir.org/search.html#_lastUpdated). + */ +class DemoDataStore(private val context: Context) { + + suspend fun saveLastUpdatedTimestamp(resourceType: ResourceType, timestamp: String) { + context.dataStorage.edit { pref -> pref[stringPreferencesKey(resourceType.name)] = timestamp } + } + + suspend fun getLasUpdateTimestamp(resourceType: ResourceType): String? { + return context.dataStorage.data.first()[stringPreferencesKey(resourceType.name)] + } +} diff --git a/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt b/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt index 224b18abfe..38f4e27394 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt +++ b/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt @@ -35,6 +35,8 @@ class FhirApplication : Application(), DataCaptureConfig.Provider { private var dataCaptureConfig: DataCaptureConfig? = null + private val dataStore by lazy { DemoDataStore(this) } + override fun onCreate() { super.onCreate() if (BuildConfig.DEBUG) { @@ -69,6 +71,8 @@ class FhirApplication : Application(), DataCaptureConfig.Provider { companion object { fun fhirEngine(context: Context) = (context.applicationContext as FhirApplication).fhirEngine + + fun dataStore(context: Context) = (context.applicationContext as FhirApplication).dataStore } override fun getDataCaptureConfig(): DataCaptureConfig = dataCaptureConfig ?: DataCaptureConfig() diff --git a/demo/src/main/java/com/google/android/fhir/demo/data/FhirSyncWorker.kt b/demo/src/main/java/com/google/android/fhir/demo/data/FhirSyncWorker.kt index 413036c16b..df92512e20 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/data/FhirSyncWorker.kt +++ b/demo/src/main/java/com/google/android/fhir/demo/data/FhirSyncWorker.kt @@ -27,7 +27,7 @@ class FhirSyncWorker(appContext: Context, workerParams: WorkerParameters) : FhirSyncWorker(appContext, workerParams) { override fun getDownloadWorkManager(): DownloadWorkManager { - return DownloadWorkManagerImpl() + return TimestampBasedDownloadWorkManagerImpl(FhirApplication.dataStore(applicationContext)) } override fun getConflictResolver() = AcceptLocalConflictResolver diff --git a/demo/src/main/java/com/google/android/fhir/demo/data/DownloadWorkManagerImpl.kt b/demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt similarity index 74% rename from demo/src/main/java/com/google/android/fhir/demo/data/DownloadWorkManagerImpl.kt rename to demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt index 0490557c7e..ba7ab25867 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/data/DownloadWorkManagerImpl.kt +++ b/demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt @@ -16,10 +16,14 @@ package com.google.android.fhir.demo.data -import com.google.android.fhir.SyncDownloadContext +import com.google.android.fhir.demo.DemoDataStore import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.SyncDataParams +import java.time.ZoneId +import java.time.format.DateTimeFormatter +import java.util.Date import java.util.LinkedList +import java.util.Locale import org.hl7.fhir.exceptions.FHIRException import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.ListResource @@ -28,25 +32,29 @@ import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType -class DownloadWorkManagerImpl : DownloadWorkManager { +class TimestampBasedDownloadWorkManagerImpl(private val dataStore: DemoDataStore) : + DownloadWorkManager { private val resourceTypeList = ResourceType.values().map { it.name } private val urls = - LinkedList(listOf("Patient?address-city=NAIROBI", "Binary?_id=android-fhir-thermometer-image")) + LinkedList( + listOf( + "Patient?address-city=NAIROBI&_sort=_lastUpdated", + "Binary?_id=android-fhir-thermometer-image" + ) + ) - override suspend fun getNextRequestUrl(context: SyncDownloadContext): String? { + override suspend fun getNextRequestUrl(): String? { var url = urls.poll() ?: return null val resourceTypeToDownload = ResourceType.fromCode(url.findAnyOf(resourceTypeList, ignoreCase = true)!!.second) - context.getLatestTimestampFor(resourceTypeToDownload)?.let { - url = affixLastUpdatedTimestamp(url!!, it) + dataStore.getLasUpdateTimestamp(resourceTypeToDownload)?.let { + url = affixLastUpdatedTimestamp(url, it) } return url } - override suspend fun getSummaryRequestUrls( - context: SyncDownloadContext - ): Map { + override suspend fun getSummaryRequestUrls(): Map { return urls.associate { ResourceType.fromCode(it.substringBefore("?")) to it.plus("&${SyncDataParams.SUMMARY_KEY}=${SyncDataParams.SUMMARY_COUNT_VALUE}") @@ -86,10 +94,26 @@ class DownloadWorkManagerImpl : DownloadWorkManager { // Finally, extract the downloaded resources from the bundle. var bundleCollection: Collection = mutableListOf() if (response is Bundle && response.type == Bundle.BundleType.SEARCHSET) { - bundleCollection = response.entry.map { it.resource } + bundleCollection = + response.entry + .map { it.resource } + .also { extractAndSaveLastUpdateTimestampToFetchFutureUpdates(it) } } return bundleCollection } + + private suspend fun extractAndSaveLastUpdateTimestampToFetchFutureUpdates( + resources: List + ) { + resources + .groupBy { it.resourceType } + .entries.map { map -> + dataStore.saveLastUpdatedTimestamp( + map.key, + map.value.maxOfOrNull { it.meta.lastUpdated }?.toTimeZoneString() ?: "" + ) + } + } } /** @@ -121,3 +145,10 @@ private fun affixLastUpdatedTimestamp(url: String, lastUpdated: String): String return downloadUrl } + +private fun Date.toTimeZoneString(): String { + val simpleDateFormat = + DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.getDefault()) + .withZone(ZoneId.systemDefault()) + return simpleDateFormat.format(this.toInstant()) +} diff --git a/engine/src/main/java/com/google/android/fhir/DatastoreUtil.kt b/engine/src/main/java/com/google/android/fhir/DatastoreUtil.kt index 0e8161e772..c3d972c7c0 100644 --- a/engine/src/main/java/com/google/android/fhir/DatastoreUtil.kt +++ b/engine/src/main/java/com/google/android/fhir/DatastoreUtil.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 Google LLC + * Copyright 2022 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,11 +26,10 @@ import java.time.OffsetDateTime import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking -val Context.dataStore: DataStore by preferencesDataStore( - name = "FHIR_ENGINE_PREF_DATASTORE" -) +private val Context.dataStore: DataStore by + preferencesDataStore(name = "FHIR_ENGINE_PREF_DATASTORE") -class DatastoreUtil(private val context: Context) { +internal class DatastoreUtil(private val context: Context) { private val lastSyncTimestampKey by lazy { stringPreferencesKey("LAST_SYNC_TIMESTAMP") } fun readLastSyncTimestamp(): OffsetDateTime? { 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 5ef7e11514..7d96f52c6e 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -63,7 +63,7 @@ interface FhirEngine { */ suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend (SyncDownloadContext) -> Flow> + download: suspend () -> Flow> ) /** @@ -127,7 +127,3 @@ suspend inline fun FhirEngine.get(id: String): R { suspend inline fun FhirEngine.delete(id: String) { delete(getResourceType(R::class.java), id) } - -interface SyncDownloadContext { - suspend fun getLatestTimestampFor(type: ResourceType): String? -} 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 3663b48bca..4d5f7d8f1b 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 @@ -20,7 +20,6 @@ import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.db.impl.dao.SquashedLocalChange import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.db.impl.entities.ResourceEntity -import com.google.android.fhir.db.impl.entities.SyncedResourceEntity import com.google.android.fhir.search.SearchQuery import java.time.Instant import org.hl7.fhir.r4.model.Resource @@ -79,22 +78,12 @@ internal interface Database { @Throws(ResourceNotFoundException::class) suspend fun selectEntity(type: ResourceType, id: String): ResourceEntity - /** - * Return the last update data of a resource based on the resource type. If no resource of - * [resourceType] is inserted, return `null`. - * @param resourceType The resource type - */ - suspend fun lastUpdate(resourceType: ResourceType): String? - /** * Insert resources that were synchronised. * * @param syncedResources The synced resource */ - suspend fun insertSyncedResources( - syncedResources: List, - resources: List - ) + suspend fun insertSyncedResources(resources: List) /** * Deletes the FHIR resource of type `clazz` with `id`. 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 b075ff5104..32258fbc55 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 @@ -30,7 +30,6 @@ import com.google.android.fhir.db.impl.dao.LocalChangeUtils import com.google.android.fhir.db.impl.dao.SquashedLocalChange import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.db.impl.entities.ResourceEntity -import com.google.android.fhir.db.impl.entities.SyncedResourceEntity import com.google.android.fhir.index.ResourceIndexer import com.google.android.fhir.logicalId import com.google.android.fhir.search.SearchQuery @@ -103,7 +102,7 @@ internal class DatabaseImpl( it.resourceIndexer = resourceIndexer } } - private val syncedResourceDao = db.syncedResourceDao() + private val localChangeDao = db.localChangeDao().also { it.iParser = iParser } override suspend fun insert(vararg resource: R): List { @@ -154,18 +153,8 @@ internal class DatabaseImpl( } as Resource } - override suspend fun lastUpdate(resourceType: ResourceType): String? { - return db.withTransaction { syncedResourceDao.getLastUpdate(resourceType) } - } - - override suspend fun insertSyncedResources( - syncedResources: List, - resources: List - ) { - db.withTransaction { - syncedResourceDao.insertAll(syncedResources) - insertRemote(*resources.toTypedArray()) - } + override suspend fun insertSyncedResources(resources: List) { + db.withTransaction { insertRemote(*resources.toTypedArray()) } } override suspend fun delete(type: ResourceType, id: String) { diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt b/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt index 4155a7224e..bc5e84d02f 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 Google LLC + * Copyright 2022 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,7 +21,6 @@ import androidx.room.RoomDatabase import androidx.room.TypeConverters import com.google.android.fhir.db.impl.dao.LocalChangeDao import com.google.android.fhir.db.impl.dao.ResourceDao -import com.google.android.fhir.db.impl.dao.SyncedResourceDao import com.google.android.fhir.db.impl.entities.DateIndexEntity import com.google.android.fhir.db.impl.entities.DateTimeIndexEntity import com.google.android.fhir.db.impl.entities.LocalChangeEntity @@ -31,7 +30,6 @@ import com.google.android.fhir.db.impl.entities.QuantityIndexEntity import com.google.android.fhir.db.impl.entities.ReferenceIndexEntity import com.google.android.fhir.db.impl.entities.ResourceEntity import com.google.android.fhir.db.impl.entities.StringIndexEntity -import com.google.android.fhir.db.impl.entities.SyncedResourceEntity import com.google.android.fhir.db.impl.entities.TokenIndexEntity import com.google.android.fhir.db.impl.entities.UriIndexEntity @@ -47,15 +45,14 @@ import com.google.android.fhir.db.impl.entities.UriIndexEntity DateIndexEntity::class, DateTimeIndexEntity::class, NumberIndexEntity::class, - SyncedResourceEntity::class, LocalChangeEntity::class, - PositionIndexEntity::class], + PositionIndexEntity::class + ], version = 1, exportSchema = false ) @TypeConverters(DbTypeConverters::class) internal abstract class ResourceDatabase : RoomDatabase() { abstract fun resourceDao(): ResourceDao - abstract fun syncedResourceDao(): SyncedResourceDao abstract fun localChangeDao(): LocalChangeDao } diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/SyncedResourceDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/SyncedResourceDao.kt deleted file mode 100644 index cbf8829cb0..0000000000 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/SyncedResourceDao.kt +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2022 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.db.impl.dao - -import androidx.room.Dao -import androidx.room.Insert -import androidx.room.OnConflictStrategy -import androidx.room.Query -import com.google.android.fhir.db.impl.entities.SyncedResourceEntity -import org.hl7.fhir.r4.model.ResourceType - -@Dao -internal interface SyncedResourceDao { - - @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun insert(entity: SyncedResourceEntity) - - @Insert(onConflict = OnConflictStrategy.REPLACE) - suspend fun insertAll(resources: List) - - /** - * We will always have 1 entry for each [ResourceType] as it's the primary key, so we can limit - * the result to 1. If there is no entry for that [ResourceType] then `null` will be returned. - */ - @Query( - """SELECT lastUpdate FROM SyncedResourceEntity - WHERE resourceType = :resourceType LIMIT 1""" - ) - suspend fun getLastUpdate(resourceType: ResourceType): String? -} diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/entities/SyncedResourceEntity.kt b/engine/src/main/java/com/google/android/fhir/db/impl/entities/SyncedResourceEntity.kt deleted file mode 100644 index 21135325e5..0000000000 --- a/engine/src/main/java/com/google/android/fhir/db/impl/entities/SyncedResourceEntity.kt +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2022 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.db.impl.entities - -import androidx.room.Entity -import androidx.room.PrimaryKey -import org.hl7.fhir.r4.model.ResourceType - -/** - * Class that models a table that holds all resource types that were synced and the highest - * `_lastUpdate` value of each resource type. - */ -@Entity -internal data class SyncedResourceEntity( - /** Resource synced */ - @PrimaryKey val resourceType: ResourceType, - /** The highest `_lastUpdate` value of the resources synced of a specific type */ - val lastUpdate: String -) diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index 3bb878c0b9..93c9c800cd 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 @@ -20,18 +20,15 @@ import android.content.Context import com.google.android.fhir.DatastoreUtil import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.db.Database 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.SyncedResourceEntity import com.google.android.fhir.logicalId import com.google.android.fhir.search.Search 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.toTimeZoneString import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.collect @@ -85,25 +82,20 @@ internal class FhirEngineImpl(private val database: Database, private val contex override suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend (SyncDownloadContext) -> Flow> + download: suspend () -> Flow> ) { - download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = database.lastUpdate(type) - } - ) - .collect { resources -> - database.withTransaction { - val resolved = - resolveConflictingResources( - resources, - getConflictingResourceIds(resources), - conflictResolver - ) - saveRemoteResourcesToDatabase(resources) - saveResolvedResourcesToDatabase(resolved) - } + download().collect { resources -> + database.withTransaction { + val resolved = + resolveConflictingResources( + resources, + getConflictingResourceIds(resources), + conflictResolver + ) + database.insertSyncedResources(resources) + saveResolvedResourcesToDatabase(resolved) } + } } private suspend fun saveResolvedResourcesToDatabase(resolved: List?) { @@ -113,14 +105,6 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } - private suspend fun saveRemoteResourcesToDatabase(resources: List) { - val timeStamps = - resources.groupBy { it.resourceType }.entries.map { - SyncedResourceEntity(it.key, it.value.maxOf { it.meta.lastUpdated }.toTimeZoneString()) - } - database.insertSyncedResources(timeStamps, resources) - } - private suspend fun resolveConflictingResources( resources: List, conflictingResourceIds: Set, @@ -207,7 +191,8 @@ internal class FhirEngineImpl(private val database: Database, private val contex */ private val Bundle.BundleEntryResponseComponent.resourceIdAndType: Pair? get() = - location?.split("/")?.takeIf { it.size > 3 }?.let { - it[it.size - 3] to ResourceType.fromCode(it[it.size - 4]) - } + location + ?.split("/") + ?.takeIf { it.size > 3 } + ?.let { it[it.size - 3] to ResourceType.fromCode(it[it.size - 4]) } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt index 202193d055..adbb253735 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt @@ -16,7 +16,6 @@ package com.google.android.fhir.sync -import com.google.android.fhir.SyncDownloadContext import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -31,13 +30,13 @@ interface DownloadWorkManager { * Returns the URL for the next download request, or `null` if there is no more download request * to be issued. */ - suspend fun getNextRequestUrl(context: SyncDownloadContext): String? + suspend fun getNextRequestUrl(): String? /* TODO: Generalize the DownloadWorkManager API to not sequentially download resource by type (https://github.com/google/android-fhir/issues/1884) */ /** * Returns the map of resourceType and URL for summary of total count for each download request */ - suspend fun getSummaryRequestUrls(context: SyncDownloadContext): Map + suspend fun getSummaryRequestUrls(): Map /** * Processes the download response and returns the resources to be saved to the local database. diff --git a/engine/src/main/java/com/google/android/fhir/sync/Downloader.kt b/engine/src/main/java/com/google/android/fhir/sync/Downloader.kt index 6aaa172066..9be709402c 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Downloader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Downloader.kt @@ -16,7 +16,6 @@ package com.google.android.fhir.sync -import com.google.android.fhir.SyncDownloadContext import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -27,7 +26,7 @@ internal interface Downloader { * @return Flow of the [DownloadState] which keeps emitting [Resource]s or Error based on the * response of each page download request. It also updates progress if [ProgressCallback] exists */ - suspend fun download(context: SyncDownloadContext): Flow + suspend fun download(): Flow } /* TODO: Generalize the Downloader API to not sequentially download resource by type (https://github.com/google/android-fhir/issues/1884) */ 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 094b698ef3..978e37cd51 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 @@ -99,7 +99,7 @@ internal class FhirSynchronizer( val exceptions = mutableListOf() fhirEngine.syncDownload(conflictResolver) { flow { - downloader.download(it).collect { + downloader.download().collect { when (it) { is DownloadState.Started -> { setSyncState(SyncJobStatus.InProgress(SyncOperation.DOWNLOAD, it.total)) diff --git a/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt index a6b8ea9358..d8692c0a44 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt @@ -16,7 +16,6 @@ package com.google.android.fhir.sync.download -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.DownloadState import com.google.android.fhir.sync.DownloadWorkManager @@ -40,13 +39,13 @@ internal class DownloaderImpl( ) : Downloader { private val resourceTypeList = ResourceType.values().map { it.name } - override suspend fun download(context: SyncDownloadContext): Flow = flow { + override suspend fun download(): Flow = flow { var resourceTypeToDownload: ResourceType = ResourceType.Bundle // download count summary of all resources for progress i.e. val progressSummary = downloadWorkManager - .getSummaryRequestUrls(context) + .getSummaryRequestUrls() .map { summary -> summary.key to runCatching { dataSource.download(summary.value) } @@ -63,7 +62,7 @@ internal class DownloaderImpl( emit(DownloadState.Started(resourceTypeToDownload, total)) - var url = downloadWorkManager.getNextRequestUrl(context) + var url = downloadWorkManager.getNextRequestUrl() while (url != null) { try { resourceTypeToDownload = @@ -80,7 +79,7 @@ internal class DownloaderImpl( emit(DownloadState.Failure(ResourceSyncException(resourceTypeToDownload, exception))) } - url = downloadWorkManager.getNextRequestUrl(context) + url = downloadWorkManager.getNextRequestUrl() } } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt index e0841b45dd..091439d686 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt @@ -16,12 +16,12 @@ package com.google.android.fhir.sync.download -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.GREATER_THAN_PREFIX import com.google.android.fhir.sync.ParamMap import com.google.android.fhir.sync.SyncDataParams import com.google.android.fhir.sync.concatParams +import com.google.android.fhir.toTimeZoneString import java.util.LinkedList import org.hl7.fhir.exceptions.FHIRException import org.hl7.fhir.r4.model.Bundle @@ -36,12 +36,14 @@ typealias ResourceSearchParams = Map * implementation takes a DFS approach and downloads all available resources for a particular * [ResourceType] before moving on to the next [ResourceType]. */ -class ResourceParamsBasedDownloadWorkManager(syncParams: ResourceSearchParams) : - DownloadWorkManager { +class ResourceParamsBasedDownloadWorkManager( + syncParams: ResourceSearchParams, + val context: TimestampContext +) : DownloadWorkManager { private val resourcesToDownloadWithSearchParams = LinkedList(syncParams.entries) private val urlOfTheNextPagesToDownloadForAResource = LinkedList() - override suspend fun getNextRequestUrl(context: SyncDownloadContext): String? { + override suspend fun getNextRequestUrl(): String? { if (urlOfTheNextPagesToDownloadForAResource.isNotEmpty()) return urlOfTheNextPagesToDownloadForAResource.poll() @@ -56,9 +58,7 @@ class ResourceParamsBasedDownloadWorkManager(syncParams: ResourceSearchParams) : /** * Returns the map of resourceType and URL for summary of total count for each download request */ - override suspend fun getSummaryRequestUrls( - context: SyncDownloadContext - ): Map { + override suspend fun getSummaryRequestUrls(): Map { return resourcesToDownloadWithSearchParams.associate { (resourceType, params) -> val newParams = params.toMutableMap().apply { @@ -73,14 +73,14 @@ class ResourceParamsBasedDownloadWorkManager(syncParams: ResourceSearchParams) : private suspend fun getLastUpdatedParam( resourceType: ResourceType, params: ParamMap, - context: SyncDownloadContext + context: TimestampContext ): MutableMap { val newParams = mutableMapOf() if (!params.containsKey(SyncDataParams.SORT_KEY)) { newParams[SyncDataParams.SORT_KEY] = SyncDataParams.LAST_UPDATED_KEY } if (!params.containsKey(SyncDataParams.LAST_UPDATED_KEY)) { - val lastUpdate = context.getLatestTimestampFor(resourceType) + val lastUpdate = context.getLasUpdateTimestamp(resourceType) if (!lastUpdate.isNullOrEmpty()) { newParams[SyncDataParams.LAST_UPDATED_KEY] = "$GREATER_THAN_PREFIX$lastUpdate" } @@ -106,9 +106,29 @@ class ResourceParamsBasedDownloadWorkManager(syncParams: ResourceSearchParams) : .firstOrNull { component -> component.relation == "next" } ?.url?.let { next -> urlOfTheNextPagesToDownloadForAResource.add(next) } - response.entry.map { it.resource } + response.entry + .map { it.resource } + .also { resources -> + resources + .groupBy { it.resourceType } + .entries.map { map -> + map.value + .filter { it.meta.lastUpdated != null } + .let { + context.saveLastUpdatedTimestamp( + map.key, + it.maxOfOrNull { it.meta.lastUpdated }?.toTimeZoneString() + ) + } + } + } } else { emptyList() } } + + interface TimestampContext { + suspend fun saveLastUpdatedTimestamp(resourceType: ResourceType, timestamp: String?) + suspend fun getLasUpdateTimestamp(resourceType: ResourceType): String? + } } diff --git a/engine/src/test-common/java/com/google/android/fhir/resource/TestingUtils.kt b/engine/src/test-common/java/com/google/android/fhir/resource/TestingUtils.kt index d9b8e2d340..51cafb48e4 100644 --- a/engine/src/test-common/java/com/google/android/fhir/resource/TestingUtils.kt +++ b/engine/src/test-common/java/com/google/android/fhir/resource/TestingUtils.kt @@ -20,7 +20,6 @@ import androidx.work.Data import ca.uhn.fhir.parser.IParser import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver @@ -105,16 +104,13 @@ class TestingUtils constructor(private val iParser: IParser) { ) : DownloadWorkManager { private val urls = LinkedList(queries) - override suspend fun getNextRequestUrl(context: SyncDownloadContext): String? = urls.poll() - override suspend fun getSummaryRequestUrls( - context: SyncDownloadContext - ): Map { - return queries + override suspend fun getNextRequestUrl(): String? = urls.poll() + override suspend fun getSummaryRequestUrls() = + queries .stream() .map { ResourceType.fromCode(it.substringBefore("?")) to it.plus("?_summary=count") } .toList() .toMap() - } override suspend fun processResponse(response: Resource): Collection { val patient = Patient().setMeta(Meta().setLastUpdated(Date())) @@ -149,16 +145,9 @@ class TestingUtils constructor(private val iParser: IParser) { override suspend fun syncDownload( conflictResolver: ConflictResolver, - download: suspend (SyncDownloadContext) -> Flow> + download: suspend () -> Flow> ) { - download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType): String { - return "123456788" - } - } - ) - .collect {} + download().collect() } override suspend fun count(search: Search): Long { return 0 diff --git a/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt index 2a1eecfdc2..5ef2d833c8 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt @@ -16,12 +16,10 @@ package com.google.android.fhir.sync.download -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.DownloadState import com.google.common.truth.Truth.assertThat import java.net.UnknownHostException -import kotlinx.coroutines.flow.collectIndexed import kotlinx.coroutines.runBlocking import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.Observation @@ -104,18 +102,13 @@ class DownloaderImplTest { mapOf( ResourceType.Patient to mapOf("param" to "patient-page1"), ResourceType.Observation to mapOf("param" to "observation-page1") - ) + ), + NoOpResourceParamsBasedDownloadWorkManagerContext ) ) val result = mutableListOf() - downloader - .download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType): String? = null - } - ) - .collect { result.add(it) } + downloader.download().collect { result.add(it) } assertThat(result.filterIsInstance()) .containsExactly( @@ -166,18 +159,13 @@ class DownloaderImplTest { mapOf( ResourceType.Patient to mapOf("param" to "patient-page1"), ResourceType.Observation to mapOf("param" to "observation-page1") - ) + ), + NoOpResourceParamsBasedDownloadWorkManagerContext ) ) val result = mutableListOf() - downloader - .download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = null - } - ) - .collect { result.add(it) } + downloader.download().collect { result.add(it) } assertThat(result.filterIsInstance()) .containsExactly( @@ -232,18 +220,13 @@ class DownloaderImplTest { mapOf( ResourceType.Patient to mapOf("param" to "patient-page1"), ResourceType.Observation to mapOf("param" to "observation-page1") - ) + ), + NoOpResourceParamsBasedDownloadWorkManagerContext ) ) val result = mutableListOf() - downloader - .download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = null - } - ) - .collect { result.add(it) } + downloader.download().collect { result.add(it) } assertThat(result.filterIsInstance()) .containsExactly( @@ -280,18 +263,13 @@ class DownloaderImplTest { } }, ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")) + mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")), + NoOpResourceParamsBasedDownloadWorkManagerContext ) ) val result = mutableListOf() - downloader - .download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType): String? = null - } - ) - .collectIndexed { index, value -> result.add(value) } + downloader.download().collect { value -> result.add(value) } assertThat(result.first()).isInstanceOf(DownloadState.Started::class.java) } @@ -314,18 +292,13 @@ class DownloaderImplTest { } }, ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")) + mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")), + NoOpResourceParamsBasedDownloadWorkManagerContext ) ) val result = mutableListOf() - downloader - .download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType): String? = null - } - ) - .collectIndexed { index, value -> result.add(value) } + downloader.download().collect { value -> result.add(value) } assertThat(result.first()).isInstanceOf(DownloadState.Started::class.java) assertThat(result.elementAt(1)).isInstanceOf(DownloadState.Success::class.java) diff --git a/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt b/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt index 0d2351d0e5..d0c814810d 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt @@ -16,7 +16,6 @@ package com.google.android.fhir.sync.download -import com.google.android.fhir.SyncDownloadContext import com.google.android.fhir.logicalId import com.google.android.fhir.sync.SyncDataParams import com.google.common.truth.Truth.assertThat @@ -42,17 +41,13 @@ class ResourceParamsBasedDownloadWorkManagerTest { ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI"), ResourceType.Immunization to emptyMap(), ResourceType.Observation to emptyMap(), - ) + ), + TestResourceParamsBasedDownloadWorkManagerContext("2022-03-20") ) val urlsToDownload = mutableListOf() do { - val url = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-03-20" - } - ) + val url = downloadManager.getNextRequestUrl() if (url != null) { urlsToDownload.add(url) } @@ -70,18 +65,13 @@ class ResourceParamsBasedDownloadWorkManagerTest { fun getNextRequestUrl_shouldReturnResourceAndPageUrlsAsNextUrls() = runBlockingTest { val downloadManager = ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to emptyMap(), ResourceType.Observation to emptyMap()) + mapOf(ResourceType.Patient to emptyMap(), ResourceType.Observation to emptyMap()), + TestResourceParamsBasedDownloadWorkManagerContext("2022-03-20") ) val urlsToDownload = mutableListOf() do { - val url = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-03-20" - } - ) - + val url = downloadManager.getNextRequestUrl() if (url != null) { urlsToDownload.add(url) } @@ -117,13 +107,11 @@ class ResourceParamsBasedDownloadWorkManagerTest { fun getNextRequestUrl_withLastUpdatedTimeProvidedInContext_ShouldAppendGtPrefixToLastUpdatedSearchParam() = runBlockingTest { val downloadManager = - ResourceParamsBasedDownloadWorkManager(mapOf(ResourceType.Patient to emptyMap())) - val url = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-06-28" - } + ResourceParamsBasedDownloadWorkManager( + mapOf(ResourceType.Patient to emptyMap()), + TestResourceParamsBasedDownloadWorkManagerContext("2022-06-28") ) + val url = downloadManager.getNextRequestUrl() assertThat(url).isEqualTo("Patient?_sort=_lastUpdated&_lastUpdated=gt2022-06-28") } @@ -138,14 +126,10 @@ class ResourceParamsBasedDownloadWorkManagerTest { SyncDataParams.LAST_UPDATED_KEY to "2022-06-28", SyncDataParams.SORT_KEY to "status" ) - ) - ) - val url = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-07-07" - } + ), + TestResourceParamsBasedDownloadWorkManagerContext("2022-07-07") ) + val url = downloadManager.getNextRequestUrl() assertThat(url).isEqualTo("Patient?_lastUpdated=2022-06-28&_sort=status") } @@ -154,14 +138,10 @@ class ResourceParamsBasedDownloadWorkManagerTest { runBlockingTest { val downloadManager = ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf(SyncDataParams.LAST_UPDATED_KEY to "gt2022-06-28")) - ) - val url = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-07-07" - } + mapOf(ResourceType.Patient to mapOf(SyncDataParams.LAST_UPDATED_KEY to "gt2022-06-28")), + TestResourceParamsBasedDownloadWorkManagerContext("2022-07-07") ) + val url = downloadManager.getNextRequestUrl() assertThat(url).isEqualTo("Patient?_lastUpdated=gt2022-06-28&_sort=_lastUpdated") } @@ -170,14 +150,10 @@ class ResourceParamsBasedDownloadWorkManagerTest { runBlockingTest { val downloadManager = ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")) - ) - val actual = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = null - } + mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")), + NoOpResourceParamsBasedDownloadWorkManagerContext ) + val actual = downloadManager.getNextRequestUrl() assertThat(actual).isEqualTo("Patient?address-city=NAIROBI&_sort=_lastUpdated") } @@ -186,14 +162,10 @@ class ResourceParamsBasedDownloadWorkManagerTest { runBlockingTest { val downloadManager = ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")) - ) - val actual = - downloadManager.getNextRequestUrl( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "" - } + mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")), + TestResourceParamsBasedDownloadWorkManagerContext("") ) + val actual = downloadManager.getNextRequestUrl() assertThat(actual).isEqualTo("Patient?address-city=NAIROBI&_sort=_lastUpdated") } @@ -205,15 +177,11 @@ class ResourceParamsBasedDownloadWorkManagerTest { ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI"), ResourceType.Immunization to emptyMap(), ResourceType.Observation to emptyMap(), - ) + ), + TestResourceParamsBasedDownloadWorkManagerContext("2022-03-20") ) - val urls = - downloadManager.getSummaryRequestUrls( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = "2022-03-20" - } - ) + val urls = downloadManager.getSummaryRequestUrls() assertThat(urls.map { it.key }) .containsExactly(ResourceType.Patient, ResourceType.Immunization, ResourceType.Observation) @@ -227,7 +195,11 @@ class ResourceParamsBasedDownloadWorkManagerTest { @Test fun processResponse_withBundleTypeSearchSet_shouldReturnPatient() = runBlockingTest { - val downloadManager = ResourceParamsBasedDownloadWorkManager(emptyMap()) + val downloadManager = + ResourceParamsBasedDownloadWorkManager( + emptyMap(), + NoOpResourceParamsBasedDownloadWorkManagerContext + ) val response = Bundle().apply { type = Bundle.BundleType.SEARCHSET @@ -248,7 +220,11 @@ class ResourceParamsBasedDownloadWorkManagerTest { @Test fun processResponse_withTransactionResponseBundle_shouldReturnEmptyList() = runBlockingTest { - val downloadManager = ResourceParamsBasedDownloadWorkManager(emptyMap()) + val downloadManager = + ResourceParamsBasedDownloadWorkManager( + emptyMap(), + NoOpResourceParamsBasedDownloadWorkManagerContext + ) val response = Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE @@ -270,7 +246,11 @@ class ResourceParamsBasedDownloadWorkManagerTest { @Test fun processResponse_withOperationOutcome_shouldThrowException() { - val downloadManager = ResourceParamsBasedDownloadWorkManager(emptyMap()) + val downloadManager = + ResourceParamsBasedDownloadWorkManager( + emptyMap(), + NoOpResourceParamsBasedDownloadWorkManagerContext + ) val response = OperationOutcome().apply { addIssue( @@ -287,3 +267,13 @@ class ResourceParamsBasedDownloadWorkManagerTest { assertThat(exception.localizedMessage).isEqualTo("Server couldn't fulfil the request.") } } + +val NoOpResourceParamsBasedDownloadWorkManagerContext = + TestResourceParamsBasedDownloadWorkManagerContext(null) + +class TestResourceParamsBasedDownloadWorkManagerContext(private val lastUpdatedTimeStamp: String?) : + ResourceParamsBasedDownloadWorkManager.TimestampContext { + override suspend fun saveLastUpdatedTimestamp(resourceType: ResourceType, timestamp: String?) {} + override suspend fun getLasUpdateTimestamp(resourceType: ResourceType): String? = + lastUpdatedTimeStamp +}