From f87fa046c6e88b554bdc5b03eff87740ab3e007a Mon Sep 17 00:00:00 2001 From: aditya-07 Date: Thu, 9 Mar 2023 12:38:17 +0530 Subject: [PATCH] Fix engine upload sync (#1893) * Added Body annotation for retrofit upload api * Logging exception when uploading * Added tests * Updated tests and removed some dependency * Update engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt * Update engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt --------- Co-authored-by: PallaviGanorkar Co-authored-by: Jing Tang --- buildSrc/src/main/kotlin/Dependencies.kt | 2 +- engine/build.gradle.kts | 2 +- .../fhir/sync/remote/RemoteFhirService.kt | 5 +- .../fhir/sync/upload/BundleUploader.kt | 2 + .../fhir/sync/remote/RemoteFhirServiceTest.kt | 141 ++++++++++++++++++ 5 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt diff --git a/buildSrc/src/main/kotlin/Dependencies.kt b/buildSrc/src/main/kotlin/Dependencies.kt index 66b42e713c..302450eba7 100644 --- a/buildSrc/src/main/kotlin/Dependencies.kt +++ b/buildSrc/src/main/kotlin/Dependencies.kt @@ -149,7 +149,6 @@ object Dependencies { object Retrofit { const val coreRetrofit = "com.squareup.retrofit2:retrofit:${Versions.retrofit}" - const val gsonConverter = "com.squareup.retrofit2:converter-gson:${Versions.retrofit}" } object Room { @@ -180,6 +179,7 @@ object Dependencies { const val guava = "com.google.guava:guava:${Versions.guava}" const val httpInterceptor = "com.squareup.okhttp3:logging-interceptor:${Versions.http}" const val http = "com.squareup.okhttp3:okhttp:${Versions.http}" + const val mockWebServer = "com.squareup.okhttp3:mockwebserver:${Versions.http}" const val jsonToolsPatch = "com.github.java-json-tools:json-patch:${Versions.jsonToolsPatch}" const val kotlinPoet = "com.squareup:kotlinpoet:${Versions.kotlinPoet}" diff --git a/engine/build.gradle.kts b/engine/build.gradle.kts index 843be249af..933822a4a0 100644 --- a/engine/build.gradle.kts +++ b/engine/build.gradle.kts @@ -111,7 +111,6 @@ dependencies { implementation(Dependencies.Kotlin.stdlib) implementation(Dependencies.Lifecycle.liveDataKtx) implementation(Dependencies.Retrofit.coreRetrofit) - implementation(Dependencies.Retrofit.gsonConverter) implementation(Dependencies.Room.ktx) implementation(Dependencies.Room.runtime) implementation(Dependencies.androidFhirCommon) @@ -130,6 +129,7 @@ dependencies { testImplementation(Dependencies.junit) testImplementation(Dependencies.mockitoInline) testImplementation(Dependencies.mockitoKotlin) + testImplementation(Dependencies.mockWebServer) testImplementation(Dependencies.robolectric) testImplementation(Dependencies.truth) } diff --git a/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt b/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt index 5a554a3d3c..3bdfd0cbec 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt @@ -26,7 +26,7 @@ import okhttp3.logging.HttpLoggingInterceptor import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.Resource import retrofit2.Retrofit -import retrofit2.converter.gson.GsonConverterFactory +import retrofit2.http.Body import retrofit2.http.GET import retrofit2.http.POST import retrofit2.http.Url @@ -36,7 +36,7 @@ internal interface RemoteFhirService : DataSource { @GET override suspend fun download(@Url path: String): Resource - @POST(".") override suspend fun upload(bundle: Bundle): Resource + @POST(".") override suspend fun upload(@Body bundle: Bundle): Resource class Builder( private val baseUrl: String, @@ -81,7 +81,6 @@ internal interface RemoteFhirService : DataSource { .baseUrl(baseUrl) .client(client) .addConverterFactory(FhirConverterFactory.create()) - .addConverterFactory(GsonConverterFactory.create()) .build() .create(RemoteFhirService::class.java) } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt index 8b5af9a22d..43c021dca1 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt @@ -29,6 +29,7 @@ import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.OperationOutcome import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType +import timber.log.Timber /** [Uploader] implementation to work with Fhir [Bundle]. */ internal class BundleUploader( @@ -51,6 +52,7 @@ internal class BundleUploader( completed += bundle.entry.size emit(getUploadResult(response, localChangeTokens, total, completed)) } catch (e: Exception) { + Timber.e(e) emit(UploadResult.Failure(ResourceSyncException(ResourceType.Bundle, e))) } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt b/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt new file mode 100644 index 0000000000..b2d3ac27e7 --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt @@ -0,0 +1,141 @@ +/* + * 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.sync.remote + +import ca.uhn.fhir.context.FhirContext +import com.google.android.fhir.NetworkConfiguration +import com.google.android.fhir.logicalId +import com.google.common.truth.Truth.assertThat +import java.net.HttpURLConnection +import kotlinx.coroutines.test.runTest +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.hl7.fhir.r4.model.Bundle +import org.hl7.fhir.r4.model.HumanName +import org.hl7.fhir.r4.model.Patient +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class RemoteFhirServiceTest { + + private val mockWebServer = MockWebServer() + + private val apiService by lazy { + RemoteFhirService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build() + } + + private val parser = FhirContext.forR4Cached().newJsonParser() + + @Before + fun setUp() { + mockWebServer.start() + } + + @After + fun shutdown() { + mockWebServer.shutdown() + } + + @Test // https://github.com/google/android-fhir/issues/1892 + fun `should assemble download request correctly`() = runTest { + // checks that a download request can be made successfully with parameters without exception + val mockResponse = + MockResponse().apply { + setResponseCode(HttpURLConnection.HTTP_OK) + setBody( + parser.encodeResourceToString( + Patient().apply { + id = "patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ) + ) + } + mockWebServer.enqueue(mockResponse) + + val result = apiService.download("Patient/patient-001") + + // No exception should occur + assertThat(result).isInstanceOf(Patient::class.java) + } + + @Test // https://github.com/google/android-fhir/issues/1892 + fun `should assemble upload request correctly`() = runTest { + // checks that a upload request can be made successfully with parameters without exception + val mockResponse = + MockResponse().apply { + setResponseCode(HttpURLConnection.HTTP_OK) + setBody( + parser.encodeResourceToString( + Bundle().apply { + id = "transaction-response-1" + type = Bundle.BundleType.TRANSACTIONRESPONSE + } + ) + ) + } + mockWebServer.enqueue(mockResponse) + val request = + Bundle().apply { + id = "transaction-1" + type = Bundle.BundleType.TRANSACTION + } + + val result = apiService.upload(request) + + // No exception has occurred + assertThat(result).isInstanceOf(Bundle::class.java) + } + + @Test + fun `should use fhir converter to serialize and deserialize request and response for fhir resources`() = + runTest { + val mockResponse = + MockResponse().apply { + setResponseCode(HttpURLConnection.HTTP_OK) + setBody( + parser.encodeResourceToString( + Bundle().apply { + id = "transaction-response-1" + type = Bundle.BundleType.TRANSACTIONRESPONSE + } + ) + ) + } + mockWebServer.enqueue(mockResponse) + val request = + Bundle().apply { + id = "transaction-1" + type = Bundle.BundleType.TRANSACTION + } + + val result = apiService.upload(request) + + assertThat(result).isInstanceOf(Bundle::class.java) + assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE) + assertThat((result).logicalId).isEqualTo("transaction-response-1") + } +}