From 5e5475e733ca4c3aac39dfdf6cf38d861d452194 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Wed, 1 Mar 2023 14:48:48 +0530 Subject: [PATCH 1/7] Added Body annotation for retrofit upload api --- .../com/google/android/fhir/sync/remote/RemoteFhirService.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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..3d2e5087d1 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 @@ -27,6 +27,7 @@ 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 +37,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, From 7c4efa32ee21192f4026bb270f1423fdc275bee1 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Wed, 1 Mar 2023 15:07:58 +0530 Subject: [PATCH 2/7] Logging exception when uploading --- .../java/com/google/android/fhir/sync/upload/BundleUploader.kt | 2 ++ 1 file changed, 2 insertions(+) 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))) } } From 8b27bce9f0efab27b5209aa4eb94c82d78319700 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Mon, 6 Mar 2023 18:55:13 +0530 Subject: [PATCH 3/7] Added tests --- buildSrc/src/main/kotlin/Dependencies.kt | 1 + engine/build.gradle.kts | 1 + .../fhir/sync/remote/RemoteFhirServiceTest.kt | 112 ++++++++++++++++++ 3 files changed, 114 insertions(+) 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..94b3117b7d 100644 --- a/buildSrc/src/main/kotlin/Dependencies.kt +++ b/buildSrc/src/main/kotlin/Dependencies.kt @@ -180,6 +180,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..ba81934388 100644 --- a/engine/build.gradle.kts +++ b/engine/build.gradle.kts @@ -130,6 +130,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/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..92e032a1aa --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt @@ -0,0 +1,112 @@ +/* + * 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.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 +import retrofit2.Retrofit +import retrofit2.converter.gson.GsonConverterFactory + +@RunWith(RobolectricTestRunner::class) +class RemoteFhirServiceTest { + private val mockWebServer = MockWebServer() + private val apiService by lazy { + Retrofit.Builder() + .baseUrl(mockWebServer.url("/")) + .addConverterFactory(FhirConverterFactory.create()) + .addConverterFactory(GsonConverterFactory.create()) + .build() + .create(RemoteFhirService::class.java) + } + private val parser = FhirContext.forR4Cached().newJsonParser() + + @Before + fun setUp() { + mockWebServer.start() + } + + @After + fun shutdown() { + mockWebServer.shutdown() + } + @Test + fun download() = runTest { + 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") + + assertThat(result).isInstanceOf(Patient::class.java) + assertThat((result as Patient).logicalId).isEqualTo("patient-001") + } + + @Test + fun upload() = 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") + } +} From 5c2828f72dddcbeed556398624bcd3c9301f8b12 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Wed, 8 Mar 2023 14:02:48 +0530 Subject: [PATCH 4/7] Updated tests and removed some dependency --- buildSrc/src/main/kotlin/Dependencies.kt | 1 - engine/build.gradle.kts | 1 - .../fhir/sync/remote/RemoteFhirService.kt | 2 - .../fhir/sync/remote/RemoteFhirServiceTest.kt | 55 ++++++++++++++----- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/buildSrc/src/main/kotlin/Dependencies.kt b/buildSrc/src/main/kotlin/Dependencies.kt index 94b3117b7d..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 { diff --git a/engine/build.gradle.kts b/engine/build.gradle.kts index ba81934388..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) 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 3d2e5087d1..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,6 @@ 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 @@ -82,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/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt b/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt index 92e032a1aa..444edcdb78 100644 --- 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 @@ -17,6 +17,7 @@ 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 @@ -31,20 +32,16 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import retrofit2.Retrofit -import retrofit2.converter.gson.GsonConverterFactory @RunWith(RobolectricTestRunner::class) class RemoteFhirServiceTest { + private val mockWebServer = MockWebServer() + private val apiService by lazy { - Retrofit.Builder() - .baseUrl(mockWebServer.url("/")) - .addConverterFactory(FhirConverterFactory.create()) - .addConverterFactory(GsonConverterFactory.create()) - .build() - .create(RemoteFhirService::class.java) + RemoteFhirService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build() } + private val parser = FhirContext.forR4Cached().newJsonParser() @Before @@ -56,8 +53,10 @@ class RemoteFhirServiceTest { fun shutdown() { mockWebServer.shutdown() } + @Test - fun download() = runTest { + 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) @@ -79,12 +78,13 @@ class RemoteFhirServiceTest { val result = apiService.download("Patient/patient-001") + // No exception should occur assertThat(result).isInstanceOf(Patient::class.java) - assertThat((result as Patient).logicalId).isEqualTo("patient-001") } @Test - fun upload() = runTest { + 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) @@ -103,10 +103,39 @@ class RemoteFhirServiceTest { id = "transaction-1" type = Bundle.BundleType.TRANSACTION } + val result = apiService.upload(request) + // No exception has occurred assertThat(result).isInstanceOf(Bundle::class.java) - assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE) - assertThat((result).logicalId).isEqualTo("transaction-response-1") } + + @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") + } } From e067b266b2ea0921dab387101026911f95b8365f Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 8 Mar 2023 17:54:10 +0000 Subject: [PATCH 5/7] Update engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt --- .../google/android/fhir/sync/remote/RemoteFhirServiceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 444edcdb78..b6ab53b864 100644 --- 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 @@ -54,7 +54,7 @@ class RemoteFhirServiceTest { mockWebServer.shutdown() } - @Test + @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 = From 471a81a767ca9764a0258c1b6341b0fe6513c970 Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 8 Mar 2023 17:54:17 +0000 Subject: [PATCH 6/7] Update engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt --- .../google/android/fhir/sync/remote/RemoteFhirServiceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index b6ab53b864..1f691b7843 100644 --- 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 @@ -82,7 +82,7 @@ class RemoteFhirServiceTest { assertThat(result).isInstanceOf(Patient::class.java) } - @Test + @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 = From 147488caf17516d27adbd97cd0785dd7db443aab Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Thu, 9 Mar 2023 12:15:44 +0530 Subject: [PATCH 7/7] Spotless --- .../google/android/fhir/sync/remote/RemoteFhirServiceTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 1f691b7843..b2d3ac27e7 100644 --- 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 @@ -54,7 +54,7 @@ class RemoteFhirServiceTest { mockWebServer.shutdown() } - @Test // https://github.com/google/android-fhir/issues/1892 + @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 = @@ -82,7 +82,7 @@ class RemoteFhirServiceTest { assertThat(result).isInstanceOf(Patient::class.java) } - @Test // https://github.com/google/android-fhir/issues/1892 + @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 =