From 52ceb2d83477cac667c24b317f74ffb9e8b2f260 Mon Sep 17 00:00:00 2001 From: Elly Kitoto Date: Mon, 17 Oct 2022 16:16:56 +0300 Subject: [PATCH 1/5] Fix IllegalStateException Remove state emission from catch section of try-catch block. This violates flow exception handling. Signed-off-by: Elly Kitoto --- .../com/google/android/fhir/sync/download/DownloaderImpl.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 d66b0dc73b..5121195d1e 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 @@ -25,6 +25,7 @@ import com.google.android.fhir.sync.ResourceSyncException import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import org.hl7.fhir.r4.model.ResourceType +import timber.log.Timber /** * Implementation of the [Downloader]. It orchestrates the pre & post processing of resources via @@ -53,7 +54,7 @@ internal class DownloaderImpl( ) ) } catch (exception: Exception) { - emit(DownloadState.Failure(ResourceSyncException(resourceTypeToDownload, exception))) + Timber.e(exception, "Resource type $resourceTypeToDownload failed to sync") } url = downloadWorkManager.getNextRequestUrl(context) From 71ca6227b6f6b6b15c33ce1874ca2db822ea6e0d Mon Sep 17 00:00:00 2001 From: Elly Kitoto Date: Mon, 17 Oct 2022 16:22:44 +0300 Subject: [PATCH 2/5] Handle exceptions emitted by sync download When an exception occurs while attempting to save a resource, the rest of the process should not be affected. Signed-off-by: Elly Kitoto --- .../android/fhir/impl/FhirEngineImpl.kt | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 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 3bb878c0b9..dbb157423a 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 @@ -87,23 +87,27 @@ internal class FhirEngineImpl(private val database: Database, private val contex conflictResolver: ConflictResolver, download: suspend (SyncDownloadContext) -> 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) + try { + 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) + } + } + } catch (exception: Exception){ + Timber.e(exception, "Sync download failed") + } } private suspend fun saveResolvedResourcesToDatabase(resolved: List?) { From 65f8e44a7b9fefd90cb913189dae29b6eb760613 Mon Sep 17 00:00:00 2001 From: Elly Kitoto Date: Mon, 17 Oct 2022 16:30:31 +0300 Subject: [PATCH 3/5] Run spotlessApply Signed-off-by: Elly Kitoto --- .../android/fhir/impl/FhirEngineImpl.kt | 26 +++++++++++-------- .../fhir/sync/download/DownloaderImpl.kt | 3 +-- 2 files changed, 16 insertions(+), 13 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 dbb157423a..53330b33e2 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 @@ -89,10 +89,11 @@ internal class FhirEngineImpl(private val database: Database, private val contex ) { try { download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = database.lastUpdate(type) - } - ) + object : SyncDownloadContext { + override suspend fun getLatestTimestampFor(type: ResourceType) = + database.lastUpdate(type) + } + ) .collect { resources -> database.withTransaction { val resolved = @@ -105,7 +106,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex saveResolvedResourcesToDatabase(resolved) } } - } catch (exception: Exception){ + } catch (exception: Exception) { Timber.e(exception, "Sync download failed") } } @@ -119,9 +120,11 @@ 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()) - } + resources + .groupBy { it.resourceType } + .entries.map { + SyncedResourceEntity(it.key, it.value.maxOf { it.meta.lastUpdated }.toTimeZoneString()) + } database.insertSyncedResources(timeStamps, resources) } @@ -211,7 +214,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/download/DownloaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt index 5121195d1e..cc80fc6b79 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 @@ -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 com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.DownloadState import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.Downloader -import com.google.android.fhir.sync.ResourceSyncException import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import org.hl7.fhir.r4.model.ResourceType From 5a6f8b781ca192d778304cdde293e773f9793e38 Mon Sep 17 00:00:00 2001 From: Elly Kitoto Date: Mon, 17 Oct 2022 16:53:25 +0300 Subject: [PATCH 4/5] Move try-catch block inside flow collect operator Signed-off-by: Elly Kitoto --- .../android/fhir/impl/FhirEngineImpl.kt | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 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 53330b33e2..a29c90c85c 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 @@ -87,14 +87,13 @@ internal class FhirEngineImpl(private val database: Database, private val contex conflictResolver: ConflictResolver, download: suspend (SyncDownloadContext) -> Flow> ) { - try { - download( - object : SyncDownloadContext { - override suspend fun getLatestTimestampFor(type: ResourceType) = - database.lastUpdate(type) - } - ) - .collect { resources -> + download( + object : SyncDownloadContext { + override suspend fun getLatestTimestampFor(type: ResourceType) = database.lastUpdate(type) + } + ) + .collect { resources -> + try { database.withTransaction { val resolved = resolveConflictingResources( @@ -105,10 +104,10 @@ internal class FhirEngineImpl(private val database: Database, private val contex saveRemoteResourcesToDatabase(resources) saveResolvedResourcesToDatabase(resolved) } + } catch (exception: Exception) { + Timber.e(exception, "Save remote and resolved conflicting resources to database failed") } - } catch (exception: Exception) { - Timber.e(exception, "Sync download failed") - } + } } private suspend fun saveResolvedResourcesToDatabase(resolved: List?) { From 091313744dcf91c5b6b6d20deb02b14411e2459a Mon Sep 17 00:00:00 2001 From: Elly Kitoto Date: Mon, 17 Oct 2022 18:04:17 +0300 Subject: [PATCH 5/5] Revert emit ResourceSyncException Signed-off-by: Elly Kitoto --- .../com/google/android/fhir/sync/download/DownloaderImpl.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 cc80fc6b79..f795d640b7 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 @@ -21,10 +21,10 @@ import com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.DownloadState import com.google.android.fhir.sync.DownloadWorkManager import com.google.android.fhir.sync.Downloader +import com.google.android.fhir.sync.ResourceSyncException import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import org.hl7.fhir.r4.model.ResourceType -import timber.log.Timber /** * Implementation of the [Downloader]. It orchestrates the pre & post processing of resources via @@ -53,7 +53,7 @@ internal class DownloaderImpl( ) ) } catch (exception: Exception) { - Timber.e(exception, "Resource type $resourceTypeToDownload failed to sync") + emit(DownloadState.Failure(ResourceSyncException(resourceTypeToDownload, exception))) } url = downloadWorkManager.getNextRequestUrl(context)