Skip to content

Commit

Permalink
Make POST method accept dynamic URL (#2109)
Browse files Browse the repository at this point in the history
* post to any path

* have distinct uploader for bundle and indiviual

* rename bundle val to resource
  • Loading branch information
omarismail94 committed Aug 2, 2023
1 parent a26dcbe commit 32bfe22
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 35 deletions.
18 changes: 13 additions & 5 deletions engine/src/main/java/com/google/android/fhir/sync/Request.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package com.google.android.fhir.sync

import com.google.android.fhir.db.impl.dao.LocalChangeToken
import org.hl7.fhir.r4.model.Bundle
import org.hl7.fhir.r4.model.ResourceType
import org.hl7.fhir.r4.model.Resource

/**
* Structure represents a request that can be made to download resources from the FHIR server. The
Expand Down Expand Up @@ -90,17 +90,25 @@ internal constructor(val bundle: Bundle, override val headers: Map<String, Strin
* FHIR server.
*/
sealed class UploadRequest(
open val headers: Map<String, String>,
val url: String,
open val headers: Map<String, String> = emptyMap(),
open val resource: Resource,
open val localChangeToken: LocalChangeToken,
open val resourceType: ResourceType
)

/**
* A FHIR [Bundle] based request for uploads. Multiple resources/resource modifications can be
* uploaded as a single request using this.
*/
data class BundleUploadRequest(
val bundle: Bundle,
override val headers: Map<String, String> = emptyMap(),
override val resource: Bundle,
override val localChangeToken: LocalChangeToken,
) : UploadRequest(".", headers, resource, localChangeToken)

/** A [url] based FHIR request to upload resources to the server. */
data class UrlUploadRequest(
override val resource: Resource,
override val localChangeToken: LocalChangeToken,
override val headers: Map<String, String> = emptyMap()
) : UploadRequest(headers, localChangeToken, ResourceType.Bundle)
) : UploadRequest(resource.resourceType.name, headers, resource, localChangeToken)
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.google.android.fhir.sync.remote

import com.google.android.fhir.sync.BundleDownloadRequest
import com.google.android.fhir.sync.BundleUploadRequest
import com.google.android.fhir.sync.DataSource
import com.google.android.fhir.sync.DownloadRequest
import com.google.android.fhir.sync.UploadRequest
Expand All @@ -34,9 +33,9 @@ internal class FhirHttpDataSource(private val fhirHttpService: FhirHttpService)
when (downloadRequest) {
is UrlDownloadRequest -> fhirHttpService.get(downloadRequest.url, downloadRequest.headers)
is BundleDownloadRequest ->
fhirHttpService.post(downloadRequest.bundle, downloadRequest.headers)
fhirHttpService.post(".", downloadRequest.bundle, downloadRequest.headers)
}

override suspend fun upload(request: UploadRequest): Resource =
fhirHttpService.post((request as BundleUploadRequest).bundle, request.headers)
fhirHttpService.post(request.url, request.resource, request.headers)
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ internal interface FhirHttpService {
* Makes a HTTP-POST method request to the server with the [Bundle] as request-body.
* @return The server may return [Bundle] or [OperationOutcome] based on the request processing.
*/
suspend fun post(bundle: Bundle, headers: Map<String, String>): Resource
suspend fun post(path: String, resource: Resource, headers: Map<String, String>): Resource
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import java.util.concurrent.TimeUnit
import okhttp3.Interceptor
import okhttp3.OkHttpClient
import okhttp3.logging.HttpLoggingInterceptor
import org.hl7.fhir.r4.model.Bundle
import org.hl7.fhir.r4.model.Resource
import retrofit2.Retrofit
import retrofit2.http.Body
Expand All @@ -37,8 +36,12 @@ internal interface RetrofitHttpService : FhirHttpService {
@GET
override suspend fun get(@Url path: String, @HeaderMap headers: Map<String, String>): Resource

@POST(".")
override suspend fun post(@Body bundle: Bundle, @HeaderMap headers: Map<String, String>): Resource
@POST
override suspend fun post(
@Url path: String,
@Body resource: Resource,
@HeaderMap headers: Map<String, String>
): Resource

class Builder(
private val baseUrl: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ open class TransactionBundleGenerator(
}
}
return BundleUploadRequest(
bundleRequest,
LocalChangeToken(localChanges.flatMap { it.token.ids })
resource = bundleRequest,
localChangeToken = LocalChangeToken(localChanges.flatMap { it.token.ids })
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal class UploaderImpl(
completed = total - uploadWorkManager.getPendingUploadsIndicator(pendingRequests)
emit(
getUploadResult(
uploadRequest.resourceType,
uploadRequest.resource.resourceType,
response,
uploadRequest.localChangeToken,
total,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,5 +209,5 @@ class BundleDataSource(val onPostBundle: suspend (Bundle) -> Resource) : DataSou
}

override suspend fun upload(request: UploadRequest) =
onPostBundle((request as BundleUploadRequest).bundle)
onPostBundle((request as BundleUploadRequest).resource)
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ class RetrofitHttpServiceTest {
type = Bundle.BundleType.TRANSACTION
}

val result = retrofitHttpService.post(request, mapOf("If-Match" to "randomResourceVersionID"))
val result =
retrofitHttpService.post(".", request, mapOf("If-Match" to "randomResourceVersionID"))
val serverRequest = mockWebServer.takeRequest()
assertThat(serverRequest.headers["If-Match"]).isEqualTo("randomResourceVersionID")
// No exception has occurred
Expand Down Expand Up @@ -135,7 +136,7 @@ class RetrofitHttpServiceTest {
type = Bundle.BundleType.TRANSACTION
}

val result = retrofitHttpService.post(request, emptyMap())
val result = retrofitHttpService.post(".", request, emptyMap())

assertThat(result).isInstanceOf(Bundle::class.java)
assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import com.google.android.fhir.db.impl.dao.LocalChangeUtils
import com.google.android.fhir.db.impl.dao.toLocalChange
import com.google.android.fhir.db.impl.entities.LocalChangeEntity
import com.google.android.fhir.db.impl.entities.LocalChangeEntity.Type
import com.google.android.fhir.sync.BundleUploadRequest
import com.google.common.truth.Truth.assertThat
import java.time.Instant
import kotlinx.coroutines.runBlocking
Expand Down Expand Up @@ -131,10 +130,10 @@ class TransactionBundleGeneratorTest {
val result = generator.generateUploadRequests(changes)

assertThat(result).hasSize(1)
val bundleUploadRequest = result.get(0) as BundleUploadRequest
assertThat(bundleUploadRequest.bundle.type).isEqualTo(Bundle.BundleType.TRANSACTION)
assertThat(bundleUploadRequest.bundle.entry).hasSize(3)
assertThat(bundleUploadRequest.bundle.entry.map { it.request.method })
val bundleUploadRequest = result[0]
assertThat(bundleUploadRequest.resource.type).isEqualTo(Bundle.BundleType.TRANSACTION)
assertThat(bundleUploadRequest.resource.entry).hasSize(3)
assertThat(bundleUploadRequest.resource.entry.map { it.request.method })
.containsExactly(Bundle.HTTPVerb.PUT, Bundle.HTTPVerb.PATCH, Bundle.HTTPVerb.DELETE)
.inOrder()
}
Expand Down Expand Up @@ -234,17 +233,13 @@ class TransactionBundleGeneratorTest {
// Exactly 3 Requests are generated
assertThat(result).hasSize(3)
// Each Request is of type Bundle
assertThat(result.all { it is BundleUploadRequest }).isTrue()
assertThat(
result.all { (it as BundleUploadRequest).bundle.type == Bundle.BundleType.TRANSACTION }
)
.isTrue()
assertThat(result.all { it.resource.type == Bundle.BundleType.TRANSACTION }).isTrue()
// Each Bundle has exactly 1 entry
assertThat(result.all { (it as BundleUploadRequest).bundle.entry.size == 1 }).isTrue()
assertThat(result.map { (it as BundleUploadRequest).bundle.entry.first().request.method })
assertThat(result.all { it.resource.entry.size == 1 }).isTrue()
assertThat(result.map { it.resource.entry.first().request.method })
.containsExactly(Bundle.HTTPVerb.PUT, Bundle.HTTPVerb.PATCH, Bundle.HTTPVerb.DELETE)
.inOrder()
assertThat(result.map { it.bundle.entry.first().request.ifMatch })
assertThat(result.map { it.resource.entry.first().request.ifMatch })
.containsExactly(null, "W/\"v-p002-01\"", "W/\"v-p003-01\"")
.inOrder()
}
Expand All @@ -268,7 +263,7 @@ class TransactionBundleGeneratorTest {
val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = false)
val result = generator.generateUploadRequests(changes)

assertThat(result.first().bundle.entry.first().request.ifMatch).isNull()
assertThat(result.first().resource.entry.first().request.ifMatch).isNull()
}

@Test
Expand All @@ -290,7 +285,7 @@ class TransactionBundleGeneratorTest {
val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true)
val result = generator.generateUploadRequests(changes)

assertThat(result.first().bundle.entry.first().request.ifMatch)
assertThat(result.first().resource.entry.first().request.ifMatch)
.isEqualTo("W/\"patient-002-version-1\"")
}

Expand Down Expand Up @@ -323,7 +318,7 @@ class TransactionBundleGeneratorTest {
val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true)
val result = generator.generateUploadRequests(changes)

assertThat(result.first().bundle.entry[0].request.ifMatch).isNull()
assertThat(result.first().bundle.entry[1].request.ifMatch).isNull()
assertThat(result.first().resource.entry[0].request.ifMatch).isNull()
assertThat(result.first().resource.entry[1].request.ifMatch).isNull()
}
}

0 comments on commit 32bfe22

Please sign in to comment.