From 87c343762f6071647123ff1fe51c5b59634e95be Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Tue, 12 Sep 2023 19:03:38 +0100 Subject: [PATCH] Introduce Patches: changes to be uploaded to the server (#2156) * Introduce the notion of Patch representing changes to be uploaded to server * Fix unit tests for patch generator * Fix imports in android test folder * Update PatchGenerator kdoc * Update PerChangePatchGenerator kdoc * Inline return value in PerChangePatchGenerator#generate * Update kdoc * Update kdoc for PerResourcePatchGenerator * Inline return value for LocalChange * Update kdoc --- ...ultiSelectViewHolderFactoryEspressoTest.kt | 2 +- .../android/fhir/demo/FhirApplication.kt | 2 +- .../android/fhir/db/impl/DatabaseImplTest.kt | 2 +- .../com/google/android/fhir/FhirEngine.kt | 1 - .../com/google/android/fhir/LocalChange.kt | 19 +- .../com/google/android/fhir/db/Database.kt | 2 +- .../android/fhir/db/impl/DatabaseImpl.kt | 4 +- .../fhir/db/impl/dao/LocalChangeDao.kt | 50 +- .../fhir/db/impl/dao/LocalChangeUtils.kt | 183 -------- .../android/fhir/impl/FhirEngineImpl.kt | 3 +- .../android/fhir/sync/FhirSyncWorker.kt | 2 +- .../com/google/android/fhir/sync/Request.kt | 8 +- .../SquashedChangesUploadWorkManager.kt | 28 +- .../fhir/sync/upload/UploadWorkManager.kt | 21 +- .../android/fhir/sync/upload/Uploader.kt | 2 +- .../android/fhir/sync/upload/UploaderImpl.kt | 24 +- .../android/fhir/sync/upload/patch/Patch.kt | 55 +++ .../fhir/sync/upload/patch/PatchGenerator.kt | 37 ++ .../upload/patch/PerChangePatchGenerator.kt | 39 ++ .../upload/patch/PerResourcePatchGenerator.kt | 118 +++++ .../request/BundleEntryComponentGenerator.kt | 34 +- ...eEntryComponentGeneratorImplementations.kt | 14 +- .../request/IndividualRequestGenerator.kt | 50 +- .../request/TransactionBundleGenerator.kt | 39 +- .../upload/request/UploadRequestGenerator.kt | 8 +- .../google/android/fhir/testing/Utilities.kt | 2 +- .../google/android/fhir/LocalChangeTest.kt | 71 +++ .../fhir/db/impl/dao/LocalChangeUtilsTest.kt | 194 -------- .../android/fhir/impl/FhirEngineImplTest.kt | 2 +- .../sync/remote/FhirHttpDataSourceTest.kt | 10 +- .../fhir/sync/upload/UploaderImplTest.kt | 5 +- .../patch/PerResourcePatchGeneratorTest.kt | 431 ++++++++++++++++++ .../SquashedChangesUploadWorkManagerTest.kt | 218 --------- .../upload/request/IndividualGeneratorTest.kt | 307 +++++-------- .../request/TransactionBundleGeneratorTest.kt | 333 ++++++-------- 35 files changed, 1187 insertions(+), 1133 deletions(-) delete mode 100644 engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeUtils.kt rename engine/src/main/java/com/google/android/fhir/sync/upload/{patch => }/SquashedChangesUploadWorkManager.kt (55%) create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/patch/Patch.kt create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt create mode 100644 engine/src/test/java/com/google/android/fhir/LocalChangeTest.kt delete mode 100644 engine/src/test/java/com/google/android/fhir/db/impl/dao/LocalChangeUtilsTest.kt create mode 100644 engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt delete mode 100644 engine/src/test/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManagerTest.kt diff --git a/datacapture/src/androidTest/java/com/google/android/fhir/datacapture/test/views/QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest.kt b/datacapture/src/androidTest/java/com/google/android/fhir/datacapture/test/views/QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest.kt index cb6b22c3c5..4d1bf1cb48 100644 --- a/datacapture/src/androidTest/java/com/google/android/fhir/datacapture/test/views/QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest.kt +++ b/datacapture/src/androidTest/java/com/google/android/fhir/datacapture/test/views/QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest.kt @@ -360,7 +360,7 @@ class QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest { } @Test - fun `shouldHideErrorTextviewInHeader`() { + fun shouldHideErrorTextviewInHeader() { val questionnaireItem = answerOptions(true, "Coding 1") questionnaireItem.addExtension(openChoiceType) val questionnaireViewItem = 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 a85da15082..35695330d6 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 @@ -63,7 +63,7 @@ class FhirApplication : Application(), DataCaptureConfig.Provider { dataCaptureConfig = DataCaptureConfig().apply { urlResolver = ReferenceUrlResolver(this@FhirApplication as Context) - xFhirQueryResolver = XFhirQueryResolver { fhirEngine.search(it).map { it.resource } } + xFhirQueryResolver = XFhirQueryResolver { it -> fhirEngine.search(it).map { it.resource } } } } diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt index bbec0d44c8..a5338bef1d 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt @@ -24,10 +24,10 @@ import ca.uhn.fhir.rest.param.ParamPrefixEnum import com.google.android.fhir.DateProvider import com.google.android.fhir.FhirServices import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.SearchResult import com.google.android.fhir.db.Database import com.google.android.fhir.db.ResourceNotFoundException -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.logicalId import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM import com.google.android.fhir.search.Operation 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 f0cc2f72f9..59a2a1322e 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -17,7 +17,6 @@ package com.google.android.fhir import com.google.android.fhir.db.ResourceNotFoundException -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver import java.time.OffsetDateTime diff --git a/engine/src/main/java/com/google/android/fhir/LocalChange.kt b/engine/src/main/java/com/google/android/fhir/LocalChange.kt index 144f95bb28..91b1b4fdd7 100644 --- a/engine/src/main/java/com/google/android/fhir/LocalChange.kt +++ b/engine/src/main/java/com/google/android/fhir/LocalChange.kt @@ -16,7 +16,7 @@ package com.google.android.fhir -import com.google.android.fhir.db.impl.dao.LocalChangeToken +import com.google.android.fhir.db.impl.entities.LocalChangeEntity import java.time.Instant import org.hl7.fhir.r4.model.Resource @@ -43,11 +43,24 @@ data class LocalChange( enum class Type(val value: Int) { INSERT(1), // create a new resource. payload is the entire resource json. UPDATE(2), // patch. payload is the json patch. - DELETE(3), // delete. payload is empty string. - NO_OP(4); // no-op. Discard + DELETE(3); // delete. payload is empty string. companion object { fun from(input: Int): Type = values().first { it.value == input } } } } + +/** Method to convert LocalChangeEntity to LocalChange instance. */ +internal fun LocalChangeEntity.toLocalChange(): LocalChange = + LocalChange( + resourceType, + resourceId, + versionId, + timestamp, + LocalChange.Type.from(type.value), + payload, + LocalChangeToken(listOf(id)) + ) + +data class LocalChangeToken(val ids: List) 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 a3a3fc3386..b2cb416fcb 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 @@ -17,8 +17,8 @@ package com.google.android.fhir.db import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.impl.dao.IndexedIdAndResource -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.db.impl.entities.ResourceEntity import com.google.android.fhir.search.SearchQuery 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 65094691a0..8da36f8bce 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 @@ -24,15 +24,15 @@ import androidx.sqlite.db.SimpleSQLiteQuery import ca.uhn.fhir.parser.IParser import com.google.android.fhir.DatabaseErrorStrategy import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.impl.DatabaseImpl.Companion.UNENCRYPTED_DATABASE_NAME import com.google.android.fhir.db.impl.dao.IndexedIdAndResource -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.ResourceEntity import com.google.android.fhir.index.ResourceIndexer import com.google.android.fhir.logicalId import com.google.android.fhir.search.SearchQuery +import com.google.android.fhir.toLocalChange import java.time.Instant import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt index 9132179845..f8c9524439 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt @@ -21,6 +21,10 @@ import androidx.room.Insert import androidx.room.Query import androidx.room.Transaction import ca.uhn.fhir.parser.IParser +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper +import com.github.fge.jsonpatch.diff.JsonDiff +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.db.impl.entities.LocalChangeEntity.Type import com.google.android.fhir.db.impl.entities.ResourceEntity @@ -30,6 +34,7 @@ import java.time.Instant import java.util.Date import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType +import org.json.JSONArray import timber.log.Timber /** @@ -76,11 +81,7 @@ internal abstract class LocalChangeDao { ) } val jsonDiff = - LocalChangeUtils.diff( - iParser, - iParser.parseResource(oldEntity.serializedResource) as Resource, - resource - ) + diff(iParser, iParser.parseResource(oldEntity.serializedResource) as Resource, resource) if (jsonDiff.length() == 0) { Timber.i( "New resource ${resource.resourceType}/${resource.id} is same as old resource. " + @@ -189,3 +190,42 @@ internal abstract class LocalChangeDao { class InvalidLocalChangeException(message: String?) : Exception(message) } + +/** Calculates the JSON patch between two [Resource] s. */ +internal fun diff(parser: IParser, source: Resource, target: Resource): JSONArray { + val objectMapper = ObjectMapper() + return getFilteredJSONArray( + JsonDiff.asJson( + objectMapper.readValue(parser.encodeResourceToString(source), JsonNode::class.java), + objectMapper.readValue(parser.encodeResourceToString(target), JsonNode::class.java) + ) + ) +} + +/** + * This function returns the json diff as a json array of operation objects. We remove the "/meta" + * and "/text" paths as they cause path not found issue when we update the resource. They are + * usually present in the downloaded resource object but are missing in the edited object as these + * aren't supposed to be edited. Thus, the Json diff creates a DELETE- OP for "/meta" and "/text" + * and causes the issue with server update. + * + * An unfiltered JSON Array for family name update looks like + * ``` + * [{"op":"remove","path":"/meta"}, {"op":"remove","path":"/text"}, + * {"op":"replace","path":"/name/0/family","value":"Nucleus"}] + * ``` + * + * A filtered JSON Array for family name update looks like + * ``` + * [{"op":"replace","path":"/name/0/family","value":"Nucleus"}] + * ``` + */ +private fun getFilteredJSONArray(jsonDiff: JsonNode) = + with(JSONArray(jsonDiff.toString())) { + val ignorePaths = setOf("/meta", "/text") + return@with JSONArray( + (0 until length()) + .map { optJSONObject(it) } + .filterNot { jsonObject -> ignorePaths.any { jsonObject.optString("path").startsWith(it) } } + ) + } diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeUtils.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeUtils.kt deleted file mode 100644 index c49635bffb..0000000000 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeUtils.kt +++ /dev/null @@ -1,183 +0,0 @@ -/* - * Copyright 2023 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 ca.uhn.fhir.parser.IParser -import com.fasterxml.jackson.databind.JsonNode -import com.fasterxml.jackson.databind.ObjectMapper -import com.github.fge.jsonpatch.JsonPatch -import com.github.fge.jsonpatch.diff.JsonDiff -import com.google.android.fhir.LocalChange -import com.google.android.fhir.LocalChange.Type -import com.google.android.fhir.db.impl.entities.LocalChangeEntity -import org.hl7.fhir.r4.model.Resource -import org.json.JSONArray -import org.json.JSONObject - -internal object LocalChangeUtils { - - /** Squash the changes by merging them two at a time. */ - fun squash(localChanges: List): LocalChange = - localChanges.reduce { first, second -> mergeLocalChanges(first, second) } - - private fun mergeLocalChanges(first: LocalChange, second: LocalChange): LocalChange { - // TODO (maybe this should throw exception when two entities don't have the same versionID) - val type: Type - val payload: String - when (second.type) { - Type.UPDATE -> - when (first.type) { - Type.UPDATE -> { - type = Type.UPDATE - payload = mergePatches(first.payload, second.payload) - } - Type.INSERT -> { - type = Type.INSERT - payload = applyPatch(first.payload, second.payload) - } - else -> { - throw IllegalArgumentException( - "Cannot merge local changes with type ${first.type} and ${second.type}." - ) - } - } - Type.DELETE -> - when (first.type) { - Type.INSERT -> { - // If an object is inserted and then deleted, return a special LocalChange that - // represents no-op - return LocalChange( - resourceId = second.resourceId, - resourceType = second.resourceType, - type = Type.NO_OP, - payload = "", - versionId = second.versionId, - token = LocalChangeToken(first.token.ids + second.token.ids), - timestamp = second.timestamp - ) - } - else -> { - type = Type.DELETE - payload = "" - } - } - Type.INSERT -> { - type = Type.INSERT - payload = second.payload - } - Type.NO_OP -> { - throw IllegalArgumentException( - "Cannot merge local changes with type ${first.type} and ${second.type}." - ) - } - } - return LocalChange( - resourceId = second.resourceId, - resourceType = second.resourceType, - type = type, - payload = payload, - versionId = second.versionId, - token = LocalChangeToken(first.token.ids + second.token.ids), - timestamp = second.timestamp - ) - } - - /** Update a JSON object with a JSON patch (RFC 6902). */ - private fun applyPatch(resourceString: String, patchString: String): String { - val objectMapper = ObjectMapper() - val resourceJson = objectMapper.readValue(resourceString, JsonNode::class.java) - val patchJson = objectMapper.readValue(patchString, JsonPatch::class.java) - return patchJson.apply(resourceJson).toString() - } - - /** Merge two JSON patch strings by concatenating their elements into a new JSON array. */ - private fun mergePatches(firstPatch: String, secondPatch: String): String { - // TODO: validate patches are RFC 6902 compliant JSON patches - val firstMap = JSONArray(firstPatch).patchMergeMap() - val secondMap = JSONArray(secondPatch).patchMergeMap() - firstMap.putAll(secondMap) - return JSONArray(firstMap.values).toString() - } - - /** Calculates the JSON patch between two [Resource] s. */ - internal fun diff(parser: IParser, source: Resource, target: Resource): JSONArray { - val objectMapper = ObjectMapper() - return getFilteredJSONArray( - JsonDiff.asJson( - objectMapper.readValue(parser.encodeResourceToString(source), JsonNode::class.java), - objectMapper.readValue(parser.encodeResourceToString(target), JsonNode::class.java) - ) - ) - } - - /** - * Creates a mutable map from operation type (e.g. add/remove) + property path to the entire - * operation containing the updated value. Two such maps can be merged using `Map.putAll()` to - * yield a minimal set of operations equivalent to individual patches. - */ - private fun JSONArray.patchMergeMap(): MutableMap, JSONObject> { - return (0 until this.length()) - .map { this.optJSONObject(it) } - .associateBy { it.optString("op") to it.optString("path") } - .toMutableMap() - } - - /** - * This function returns the json diff as a json array of operation objects. We remove the "/meta" - * and "/text" paths as they cause path not found issue when we update the resource. They are - * usually present in the downloaded resource object but are missing in the edited object as these - * aren't supposed to be edited. Thus, the Json diff creates a DELETE- OP for "/meta" and "/text" - * and causes the issue with server update. - * - * An unfiltered JSON Array for family name update looks like - * ``` - * [{"op":"remove","path":"/meta"}, {"op":"remove","path":"/text"}, - * {"op":"replace","path":"/name/0/family","value":"Nucleus"}] - * ``` - * - * A filtered JSON Array for family name update looks like - * ``` - * [{"op":"replace","path":"/name/0/family","value":"Nucleus"}] - * ``` - */ - private fun getFilteredJSONArray(jsonDiff: JsonNode) = - with(JSONArray(jsonDiff.toString())) { - val ignorePaths = setOf("/meta", "/text") - return@with JSONArray( - (0 until length()) - .map { optJSONObject(it) } - .filterNot { jsonObject -> - ignorePaths.any { jsonObject.optString("path").startsWith(it) } - } - ) - } -} - -/** Method to convert LocalChangeEntity to LocalChange instance. */ -internal fun LocalChangeEntity.toLocalChange(): LocalChange { - return LocalChange( - resourceType, - resourceId, - versionId, - timestamp, - LocalChange.Type.from(type.value), - payload, - LocalChangeToken(listOf(id)) - ) -} - -data class LocalChangeToken(val ids: List) 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 3b9911d66f..3d90ed3114 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,9 +20,9 @@ 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.LocalChangeToken import com.google.android.fhir.SearchResult import com.google.android.fhir.db.Database -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.logicalId import com.google.android.fhir.search.Search import com.google.android.fhir.search.count @@ -31,7 +31,6 @@ import com.google.android.fhir.sync.ConflictResolver import com.google.android.fhir.sync.Resolved import java.time.OffsetDateTime import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.collect import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt index b10d494144..07c3e644a1 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt @@ -25,9 +25,9 @@ import com.google.android.fhir.FhirEngine import com.google.android.fhir.FhirEngineProvider import com.google.android.fhir.OffsetDateTimeTypeAdapter import com.google.android.fhir.sync.download.DownloaderImpl +import com.google.android.fhir.sync.upload.SquashedChangesUploadWorkManager import com.google.android.fhir.sync.upload.UploadWorkManager import com.google.android.fhir.sync.upload.UploaderImpl -import com.google.android.fhir.sync.upload.patch.SquashedChangesUploadWorkManager import com.google.gson.ExclusionStrategy import com.google.gson.FieldAttributes import com.google.gson.GsonBuilder diff --git a/engine/src/main/java/com/google/android/fhir/sync/Request.kt b/engine/src/main/java/com/google/android/fhir/sync/Request.kt index 5aba5c0faa..7cc693ecd2 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Request.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Request.kt @@ -16,7 +16,6 @@ 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.Resource import org.hl7.fhir.r4.model.codesystems.HttpVerb @@ -94,7 +93,6 @@ sealed class UploadRequest( open val url: String, open val headers: Map = emptyMap(), open val resource: Resource, - open val localChangeToken: LocalChangeToken, ) /** @@ -104,14 +102,12 @@ sealed class UploadRequest( data class BundleUploadRequest( override val headers: Map = emptyMap(), override val resource: Bundle, - override val localChangeToken: LocalChangeToken, -) : UploadRequest(".", headers, resource, localChangeToken) +) : UploadRequest(".", headers, resource) /** A [url] based FHIR request to upload resources to the server. */ data class UrlUploadRequest( val httpVerb: HttpVerb, override val url: String, override val resource: Resource, - override val localChangeToken: LocalChangeToken, override val headers: Map = emptyMap() -) : UploadRequest(url, headers, resource, localChangeToken) +) : UploadRequest(url, headers, resource) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/SquashedChangesUploadWorkManager.kt similarity index 55% rename from engine/src/main/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManager.kt rename to engine/src/main/java/com/google/android/fhir/sync/upload/SquashedChangesUploadWorkManager.kt index 4d12554a93..9705c56a85 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManager.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/SquashedChangesUploadWorkManager.kt @@ -14,12 +14,12 @@ * limitations under the License. */ -package com.google.android.fhir.sync.upload.patch +package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.dao.LocalChangeUtils import com.google.android.fhir.sync.UploadRequest -import com.google.android.fhir.sync.upload.UploadWorkManager +import com.google.android.fhir.sync.upload.patch.Patch +import com.google.android.fhir.sync.upload.patch.PerResourcePatchGenerator import com.google.android.fhir.sync.upload.request.TransactionBundleGenerator /** @@ -32,24 +32,14 @@ class SquashedChangesUploadWorkManager : UploadWorkManager { /** * The implementation is to squash all the changes by resource type so that there is at most one - * local change to be uploaded per resource + * patch to be uploaded per resource. */ - override fun prepareChangesForUpload(localChanges: List): List { - return localChanges - .groupBy { it.resourceId to it.resourceType } - .values.map { localResourceChanges -> LocalChangeUtils.squash(localResourceChanges) } + override fun generatePatches(localChanges: List): List { + return PerResourcePatchGenerator.generate(localChanges) } - /** - * Use the [TransactionBundleGenerator] to bundle the [LocalChange]s into [BundleUploadRequest]s - */ - override fun createUploadRequestsFromLocalChanges( - localChanges: List - ): List { - return bundleUploadRequestGenerator.generateUploadRequests(localChanges) + /** Use the [TransactionBundleGenerator] to bundle the [Patch]es into [BundleUploadRequest]s. */ + override fun generateRequests(patches: List): List { + return bundleUploadRequestGenerator.generateUploadRequests(patches) } - - /** Simple progress indicator determined by the number of pending requests. */ - override fun getPendingUploadsIndicator(uploadRequests: List): Int = - uploadRequests.size } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploadWorkManager.kt index 6d7485d25d..88c1994a2f 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploadWorkManager.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploadWorkManager.kt @@ -18,26 +18,19 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange import com.google.android.fhir.sync.UploadRequest +import com.google.android.fhir.sync.upload.patch.Patch /** * Manager that pre-processes the local FHIR changes and handles how to upload them to the server. */ internal interface UploadWorkManager { /** - * Transforms the [LocalChange]s to the final set of changes that needs to be uploaded to the - * server. The transformations can be of various types like squashing the [LocalChange]s by - * [Resource] e.g. [SquashedChangesUploadWorkManager] or filtering out certain [LocalChange]s or - * grouping the changes. + * Transforms the [LocalChange]s to the final set of [Patch]es that need to be uploaded to the + * server. The transformation can be of various types like grouping the [LocalChange]s by resource + * or filtering out certain [LocalChange]s. */ - fun prepareChangesForUpload(localChanges: List): List + fun generatePatches(localChanges: List): List - /** Generates a list of [UploadRequest]s from the [LocalChange]s to be uploaded to the server */ - fun createUploadRequestsFromLocalChanges(localChanges: List): List - - /** - * Gets the [Int] to indicate the progress in terms of the pending uploads. The indicator could be - * determined at the resource level (by extracting resource information from the upload requests) - * etc. - */ - fun getPendingUploadsIndicator(uploadRequests: List): Int + /** Generates a list of [UploadRequest]s from the [Patch]es to be uploaded to the server */ + fun generateRequests(patches: List): List } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt index a656f70bae..9c76abccfd 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/Uploader.kt @@ -17,7 +17,7 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.dao.LocalChangeToken +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.sync.ResourceSyncException import kotlinx.coroutines.flow.Flow import org.hl7.fhir.r4.model.Resource diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt index a966e1a973..081fc8b430 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploaderImpl.kt @@ -17,7 +17,7 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.dao.LocalChangeToken +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.ResourceSyncException import kotlinx.coroutines.flow.Flow @@ -41,26 +41,16 @@ internal class UploaderImpl( ) : Uploader { override suspend fun upload(localChanges: List): Flow = flow { - val transformedChanges = uploadWorkManager.prepareChangesForUpload(localChanges) - val uploadRequests = uploadWorkManager.createUploadRequestsFromLocalChanges(transformedChanges) - val total = uploadWorkManager.getPendingUploadsIndicator(uploadRequests) - var completed = 0 + val patches = uploadWorkManager.generatePatches(localChanges) + val requests = uploadWorkManager.generateRequests(patches) + val token = LocalChangeToken(localChanges.flatMap { it.token.ids }) + val total = requests.size emit(UploadState.Started(total)) - val pendingRequests = uploadRequests.toMutableList() - while (pendingRequests.isNotEmpty()) { - val uploadRequest = pendingRequests.first() - pendingRequests.remove(uploadRequest) + requests.forEachIndexed { index, uploadRequest -> try { val response = dataSource.upload(uploadRequest) - completed = total - uploadWorkManager.getPendingUploadsIndicator(pendingRequests) emit( - getUploadResult( - uploadRequest.resource.resourceType, - response, - uploadRequest.localChangeToken, - total, - completed - ) + getUploadResult(uploadRequest.resource.resourceType, response, token, total, index + 1) ) } catch (e: Exception) { Timber.e(e) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/Patch.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/Patch.kt new file mode 100644 index 0000000000..b282f7e0a8 --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/Patch.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2023 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.upload.patch + +import com.google.android.fhir.LocalChange +import java.time.Instant +import org.hl7.fhir.r4.model.Resource + +/** Data class for squashed local changes for resource */ +data class Patch( + /** The [ResourceType] */ + val resourceType: String, + /** The resource id [Resource.id] */ + val resourceId: String, + /** This is the id of the version of the resource that this local change is based of */ + val versionId: String? = null, + /** The time instant the app user performed a CUD operation on the resource. */ + val timestamp: Instant, + /** Type of local change like insert, delete, etc */ + val type: Type, + /** json string with local changes */ + val payload: String, +) { + enum class Type(val value: Int) { + INSERT(1), // create a new resource. payload is the entire resource json. + UPDATE(2), // patch. payload is the json patch. + DELETE(3); // delete. payload is empty string. + + companion object { + fun from(input: Int): Type = values().first { it.value == input } + } + } +} + +internal fun LocalChange.Type.toPatchType(): Patch.Type { + return when (this) { + LocalChange.Type.INSERT -> Patch.Type.INSERT + LocalChange.Type.UPDATE -> Patch.Type.UPDATE + LocalChange.Type.DELETE -> Patch.Type.DELETE + } +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt new file mode 100644 index 0000000000..f0ff53f7ce --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt @@ -0,0 +1,37 @@ +/* + * Copyright 2023 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.upload.patch + +import com.google.android.fhir.LocalChange + +/** + * Generates [Patch]es from [LocalChange]s. + * + * INTERNAL ONLY. This interface should NEVER been exposed as an external API because it works + * together with other components in the upload package to fulfill a specific upload strategy. + * Application-specific implementations of this interface are unlikely to catch all the edge cases + * and work with other components in the upload package seamlessly. Should there be a genuine need + * to control the [Patch]es to be uploaded to the server, more granulated control mechanisms should + * be opened up to applications to guarantee correctness. + */ +internal interface PatchGenerator { + /** + * NOTE: different implementations may have requirements on the size of [localChanges] and output + * certain numbers of [Patch]es. + */ + fun generate(localChanges: List): List +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt new file mode 100644 index 0000000000..9d2840cd2f --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2023 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.upload.patch + +import com.google.android.fhir.LocalChange + +/** + * Generates a [Patch] for each [LocalChange]. + * + * Used when all client-side changes to FHIR resources need to be uploaded to the server in order to + * maintain an audit trail. + */ +internal object PerChangePatchGenerator : PatchGenerator { + override fun generate(localChanges: List): List = + localChanges.map { + Patch( + resourceType = it.resourceType, + resourceId = it.resourceId, + versionId = it.versionId, + timestamp = it.timestamp, + type = it.type.toPatchType(), + payload = it.payload, + ) + } +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt new file mode 100644 index 0000000000..83812a28e6 --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -0,0 +1,118 @@ +/* + * Copyright 2023 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.upload.patch + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper +import com.github.fge.jsonpatch.JsonPatch +import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChange.Type +import org.json.JSONArray +import org.json.JSONObject + +/** + * Generates a [Patch] for all [LocalChange]es made to a single FHIR resource. + * + * Used when individual client-side changes do not need to be uploaded to the server in order to + * maintain an audit trail, but instead, multiple changes made to the same FHIR resource on the + * client can be recorded as a single change on the server. + */ +internal object PerResourcePatchGenerator : PatchGenerator { + + override fun generate(localChanges: List): List { + return localChanges + .groupBy { it.resourceType to it.resourceId } + .values.mapNotNull { mergeLocalChangesForSingleResource(it) } + } + + private fun mergeLocalChangesForSingleResource(localChanges: List): Patch? { + // TODO (maybe this should throw exception when two entities don't have the same versionID) + val firstDeleteLocalChange = localChanges.indexOfFirst { it.type == Type.DELETE } + require(firstDeleteLocalChange == -1 || firstDeleteLocalChange == localChanges.size - 1) { + "Changes after deletion of resource are not permitted" + } + + val lastInsertLocalChange = localChanges.indexOfLast { it.type == Type.INSERT } + require(lastInsertLocalChange == -1 || lastInsertLocalChange == 0) { + "Changes before creation of resource are not permitted" + } + + return when { + localChanges.first().type == Type.INSERT && localChanges.last().type == Type.DELETE -> null + localChanges.first().type == Type.INSERT -> { + createPatch( + localChanges = localChanges, + type = Patch.Type.INSERT, + payload = localChanges.map { it.payload }.reduce(::applyPatch), + ) + } + localChanges.last().type == Type.DELETE -> { + createPatch( + localChanges = localChanges, + type = Patch.Type.DELETE, + payload = "", + ) + } + else -> { + createPatch( + localChanges = localChanges, + type = Patch.Type.UPDATE, + payload = localChanges.map { it.payload }.reduce(::mergePatches), + ) + } + } + } + + private fun createPatch(localChanges: List, type: Patch.Type, payload: String) = + Patch( + resourceId = localChanges.first().resourceId, + resourceType = localChanges.first().resourceType, + type = type, + payload = payload, + versionId = localChanges.first().versionId, + timestamp = localChanges.last().timestamp, + ) + + /** Update a JSON object with a JSON patch (RFC 6902). */ + private fun applyPatch(resourceString: String, patchString: String): String { + val objectMapper = ObjectMapper() + val resourceJson = objectMapper.readValue(resourceString, JsonNode::class.java) + val patchJson = objectMapper.readValue(patchString, JsonPatch::class.java) + return patchJson.apply(resourceJson).toString() + } + + /** Merge two JSON patch strings by concatenating their elements into a new JSON array. */ + private fun mergePatches(firstPatch: String, secondPatch: String): String { + // TODO: validate patches are RFC 6902 compliant JSON patches + val firstMap = JSONArray(firstPatch).patchMergeMap() + val secondMap = JSONArray(secondPatch).patchMergeMap() + firstMap.putAll(secondMap) + return JSONArray(firstMap.values).toString() + } + + /** + * Creates a mutable map from operation type (e.g. add/remove) + property path to the entire + * operation containing the updated value. Two such maps can be merged using `Map.putAll()` to + * yield a minimal set of operations equivalent to individual patches. + */ + private fun JSONArray.patchMergeMap(): MutableMap, JSONObject> { + return (0 until this.length()) + .map { this.optJSONObject(it) } + .associateBy { it.optString("op") to it.optString("path") } + .toMutableMap() + } +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGenerator.kt index bc00035d1a..4025bc4625 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGenerator.kt @@ -16,8 +16,7 @@ package com.google.android.fhir.sync.upload.request -import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.entities.LocalChangeEntity +import com.google.android.fhir.sync.upload.patch.Patch import org.hl7.fhir.instance.model.api.IBaseResource import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.Enumeration @@ -25,9 +24,9 @@ import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.UriType /** - * Abstract class for generating [Bundle.BundleEntryComponent] for a [LocalChange] to be added to - * the [Bundle] based on [Bundle.HTTPVerb] supported by the Fhir server. Concrete implementations of - * the class should provide implementation of [getEntryResource] to provide [Resource] for the + * Abstract class for generating [Bundle.BundleEntryComponent] for a [Patch] to be added to the + * [Bundle] based on [Bundle.HTTPVerb] supported by the Fhir server. Concrete implementations of the + * class should provide implementation of [getEntryResource] to provide [Resource] for the * [LocalChangeEntity]. See [https://www.hl7.org/fhir/http.html#transaction] for more info regarding * the supported [Bundle.HTTPVerb]. */ @@ -41,30 +40,29 @@ abstract class BundleEntryComponentGenerator( * [Resource] may not be required in the request like in the case of a [Bundle.HTTPVerb.DELETE] * request. */ - protected abstract fun getEntryResource(localChange: LocalChange): IBaseResource? + protected abstract fun getEntryResource(patch: Patch): IBaseResource? - /** Returns a [Bundle.BundleEntryComponent] for a [LocalChange] to be added to the [Bundle] . */ - fun getEntry(localChange: LocalChange): Bundle.BundleEntryComponent { + /** Returns a [Bundle.BundleEntryComponent] for a [Patch] to be added to the [Bundle] . */ + fun getEntry(patch: Patch): Bundle.BundleEntryComponent { return Bundle.BundleEntryComponent().apply { - resource = getEntryResource(localChange) as Resource? - request = getEntryRequest(localChange) + resource = getEntryResource(patch) as Resource? + request = getEntryRequest(patch) fullUrl = request?.url } } - private fun getEntryRequest(localChange: LocalChange) = + private fun getEntryRequest(patch: Patch) = Bundle.BundleEntryRequestComponent( Enumeration(Bundle.HTTPVerbEnumFactory()).apply { value = httpVerb }, - UriType("${localChange.resourceType}/${localChange.resourceId}") + UriType("${patch.resourceType}/${patch.resourceId}") ) .apply { - if (useETagForUpload && !localChange.versionId.isNullOrEmpty()) { + if (useETagForUpload && !patch.versionId.isNullOrEmpty()) { // FHIR supports weak Etag, See ETag section https://hl7.org/fhir/http.html#Http-Headers - when (localChange.type) { - LocalChange.Type.UPDATE, - LocalChange.Type.DELETE -> ifMatch = "W/\"${localChange.versionId}\"" - LocalChange.Type.INSERT -> {} - LocalChange.Type.NO_OP -> error("Cannot create a bundle from a NO_OP request") + when (patch.type) { + Patch.Type.UPDATE, + Patch.Type.DELETE -> ifMatch = "W/\"${patch.versionId}\"" + Patch.Type.INSERT -> {} } } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt index 984d066871..28486b559d 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt @@ -19,31 +19,29 @@ package com.google.android.fhir.sync.upload.request import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.ContentTypes -import com.google.android.fhir.LocalChange +import com.google.android.fhir.sync.upload.patch.Patch import org.hl7.fhir.instance.model.api.IBaseResource import org.hl7.fhir.r4.model.Binary import org.hl7.fhir.r4.model.Bundle internal class HttpPutForCreateEntryComponentGenerator(useETagForUpload: Boolean) : BundleEntryComponentGenerator(Bundle.HTTPVerb.PUT, useETagForUpload) { - override fun getEntryResource(localChange: LocalChange): IBaseResource { - return FhirContext.forCached(FhirVersionEnum.R4) - .newJsonParser() - .parseResource(localChange.payload) + override fun getEntryResource(patch: Patch): IBaseResource { + return FhirContext.forCached(FhirVersionEnum.R4).newJsonParser().parseResource(patch.payload) } } internal class HttpPatchForUpdateEntryComponentGenerator(useETagForUpload: Boolean) : BundleEntryComponentGenerator(Bundle.HTTPVerb.PATCH, useETagForUpload) { - override fun getEntryResource(localChange: LocalChange): IBaseResource { + override fun getEntryResource(patch: Patch): IBaseResource { return Binary().apply { contentType = ContentTypes.APPLICATION_JSON_PATCH - data = localChange.payload.toByteArray() + data = patch.payload.toByteArray() } } } internal class HttpDeleteEntryComponentGenerator(useETagForUpload: Boolean) : BundleEntryComponentGenerator(Bundle.HTTPVerb.DELETE, useETagForUpload) { - override fun getEntryResource(localChange: LocalChange): IBaseResource? = null + override fun getEntryResource(patch: Patch): IBaseResource? = null } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/IndividualRequestGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/IndividualRequestGenerator.kt index bcb1609dee..b0e4b22e24 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/IndividualRequestGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/IndividualRequestGenerator.kt @@ -19,22 +19,19 @@ package com.google.android.fhir.sync.upload.request import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.ContentTypes -import com.google.android.fhir.LocalChange -import com.google.android.fhir.LocalChange.Type -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.sync.UrlUploadRequest +import com.google.android.fhir.sync.upload.patch.Patch import org.hl7.fhir.r4.model.Binary import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.codesystems.HttpVerb -/** Generates list of [UrlUploadRequest]s with for each [LocalChange] given. */ +/** Generates list of [UrlUploadRequest]s with for each [Patch] given. */ class IndividualRequestGenerator( - private val getIndividualRequestForLocalChangeType: - (type: Type, localChange: LocalChange) -> UrlUploadRequest + private val getIndividualRequestForPatchType: (type: Patch.Type, patch: Patch) -> UrlUploadRequest ) : UploadRequestGenerator { - override fun generateUploadRequests(localChanges: List): List = - localChanges.map { getIndividualRequestForLocalChangeType(it.type, it) } + override fun generateUploadRequests(patches: List): List = + patches.map { getIndividualRequestForPatchType(it.type, it) } companion object Factory { @@ -75,46 +72,41 @@ class IndividualRequestGenerator( "Update using $httpVerbToUseForUpdate is not supported." ) - return IndividualRequestGenerator { type, localChange -> + return IndividualRequestGenerator { type, patch -> when (type) { - Type.INSERT -> createFunction(localChange) - Type.UPDATE -> updateFunction(localChange) - Type.DELETE -> deleteFunction(localChange) - Type.NO_OP -> error("Cannot create a request from a NO_OP type") + Patch.Type.INSERT -> createFunction(patch) + Patch.Type.UPDATE -> updateFunction(patch) + Patch.Type.DELETE -> deleteFunction(patch) } } } - private fun deleteFunction(localChange: LocalChange) = + private fun deleteFunction(patch: Patch) = UrlUploadRequest( httpVerb = HttpVerb.DELETE, - url = "${localChange.resourceType}/${localChange.resourceId}", - resource = parser.parseResource(localChange.payload) as Resource, - localChangeToken = LocalChangeToken(localChange.token.ids), + url = "${patch.resourceType}/${patch.resourceId}", + resource = parser.parseResource(patch.payload) as Resource, ) - private fun postForCreateResource(localChange: LocalChange) = + private fun postForCreateResource(patch: Patch) = UrlUploadRequest( httpVerb = HttpVerb.POST, - url = localChange.resourceType, - resource = parser.parseResource(localChange.payload) as Resource, - localChangeToken = LocalChangeToken(localChange.token.ids), + url = patch.resourceType, + resource = parser.parseResource(patch.payload) as Resource, ) - private fun putForCreateResource(localChange: LocalChange) = + private fun putForCreateResource(patch: Patch) = UrlUploadRequest( httpVerb = HttpVerb.PUT, - url = "${localChange.resourceType}/${localChange.resourceId}", - resource = parser.parseResource(localChange.payload) as Resource, - localChangeToken = LocalChangeToken(localChange.token.ids), + url = "${patch.resourceType}/${patch.resourceId}", + resource = parser.parseResource(patch.payload) as Resource, ) - private fun patchForUpdateResource(localChange: LocalChange) = + private fun patchForUpdateResource(patch: Patch) = UrlUploadRequest( httpVerb = HttpVerb.PATCH, - url = "${localChange.resourceType}/${localChange.resourceId}", - resource = Binary().apply { data = localChange.payload.toByteArray() }, - localChangeToken = LocalChangeToken(localChange.token.ids), + url = "${patch.resourceType}/${patch.resourceId}", + resource = Binary().apply { data = patch.payload.toByteArray() }, headers = mapOf("Content-Type" to ContentTypes.APPLICATION_JSON_PATCH) ) } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt index 845a31add9..2f4a698b5f 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt @@ -16,10 +16,9 @@ package com.google.android.fhir.sync.upload.request -import com.google.android.fhir.LocalChange -import com.google.android.fhir.LocalChange.Type -import com.google.android.fhir.db.impl.dao.LocalChangeToken +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.sync.BundleUploadRequest +import com.google.android.fhir.sync.upload.patch.Patch import org.hl7.fhir.r4.model.Bundle /** @@ -30,32 +29,26 @@ class TransactionBundleGenerator( private val generatedBundleSize: Int, private val useETagForUpload: Boolean, private val getBundleEntryComponentGeneratorForLocalChangeType: - (type: Type, useETagForUpload: Boolean) -> BundleEntryComponentGenerator + (type: Patch.Type, useETagForUpload: Boolean) -> BundleEntryComponentGenerator ) : UploadRequestGenerator { - override fun generateUploadRequests(localChanges: List): List { - return localChanges - .chunked(generatedBundleSize) - .filter { it.isNotEmpty() } - .map { generateBundleRequest(it) } + override fun generateUploadRequests(patches: List): List { + return patches.chunked(generatedBundleSize).map { generateBundleRequest(it) } } - private fun generateBundleRequest(localChanges: List): BundleUploadRequest { + private fun generateBundleRequest(patches: List): BundleUploadRequest { val bundleRequest = Bundle().apply { type = Bundle.BundleType.TRANSACTION - localChanges - .filterNot { it.type == Type.NO_OP } - .forEach { - this.addEntry( - getBundleEntryComponentGeneratorForLocalChangeType(it.type, useETagForUpload) - .getEntry(it) - ) - } + patches.forEach { + this.addEntry( + getBundleEntryComponentGeneratorForLocalChangeType(it.type, useETagForUpload) + .getEntry(it) + ) + } } return BundleUploadRequest( resource = bundleRequest, - localChangeToken = LocalChangeToken(localChanges.flatMap { it.token.ids }) ) } @@ -100,11 +93,9 @@ class TransactionBundleGenerator( return TransactionBundleGenerator(generatedBundleSize, useETagForUpload) { type, useETag -> when (type) { - Type.INSERT -> createFunction(useETag) - Type.UPDATE -> updateFunction(useETag) - Type.DELETE -> HttpDeleteEntryComponentGenerator(useETag) - Type.NO_OP -> - error("NO_OP type represents a no-operation and is not mapped to an HTTP operation.") + Patch.Type.INSERT -> createFunction(useETag) + Patch.Type.UPDATE -> updateFunction(useETag) + Patch.Type.DELETE -> HttpDeleteEntryComponentGenerator(useETag) } } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/UploadRequestGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/UploadRequestGenerator.kt index 7777835a9f..5cad87079e 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/UploadRequestGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/UploadRequestGenerator.kt @@ -16,11 +16,11 @@ package com.google.android.fhir.sync.upload.request -import com.google.android.fhir.LocalChange import com.google.android.fhir.sync.UploadRequest +import com.google.android.fhir.sync.upload.patch.Patch -/** Generator that generates [UploadRequest]s from the [LocalChange]s */ +/** Generator that generates [UploadRequest]s from the [Patch]es */ internal interface UploadRequestGenerator { - /** Generates a list of [UploadRequest] from the [localChanges] */ - fun generateUploadRequests(localChanges: List): List + /** Generates a list of [UploadRequest] from the [Patch]es */ + fun generateUploadRequests(patches: List): List } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index 59e035d41f..6b7a27ae57 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -22,8 +22,8 @@ import ca.uhn.fhir.context.FhirVersionEnum import ca.uhn.fhir.parser.IParser import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.SearchResult -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search import com.google.android.fhir.sync.BundleDownloadRequest import com.google.android.fhir.sync.BundleUploadRequest diff --git a/engine/src/test/java/com/google/android/fhir/LocalChangeTest.kt b/engine/src/test/java/com/google/android/fhir/LocalChangeTest.kt new file mode 100644 index 0000000000..47499d6a8c --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/LocalChangeTest.kt @@ -0,0 +1,71 @@ +/* + * Copyright 2023 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 + +import ca.uhn.fhir.context.FhirContext +import ca.uhn.fhir.context.FhirVersionEnum +import ca.uhn.fhir.parser.IParser +import com.google.android.fhir.db.impl.entities.LocalChangeEntity +import com.google.common.truth.Truth.assertThat +import java.time.Instant +import junit.framework.TestCase +import kotlinx.coroutines.runBlocking +import org.hl7.fhir.r4.model.HumanName +import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.ResourceType +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class LocalChangeTest : TestCase() { + + private val jsonParser: IParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser() + + @Test + fun `should convert the database entity class to a local change`() = runBlocking { + val localChangeEntity = + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ) + + val localChange = localChangeEntity.toLocalChange() + assertThat(localChangeEntity.id).isEqualTo(localChange.token.ids.first()) + assertThat(localChangeEntity.resourceType).isEqualTo(localChange.resourceType) + assertThat(localChangeEntity.resourceId).isEqualTo(localChange.resourceId) + assertThat(localChangeEntity.timestamp).isEqualTo(localChange.timestamp) + assertThat(LocalChange.Type.from(localChangeEntity.type.value)).isEqualTo(localChange.type) + assertThat(localChangeEntity.payload).isEqualTo(localChange.payload) + assertThat(localChangeEntity.versionId).isEqualTo(localChange.versionId) + } +} diff --git a/engine/src/test/java/com/google/android/fhir/db/impl/dao/LocalChangeUtilsTest.kt b/engine/src/test/java/com/google/android/fhir/db/impl/dao/LocalChangeUtilsTest.kt deleted file mode 100644 index 98b758c32d..0000000000 --- a/engine/src/test/java/com/google/android/fhir/db/impl/dao/LocalChangeUtilsTest.kt +++ /dev/null @@ -1,194 +0,0 @@ -/* - * Copyright 2023 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 ca.uhn.fhir.context.FhirContext -import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.entities.LocalChangeEntity -import com.google.android.fhir.logicalId -import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder -import com.google.android.fhir.testing.readFromFile -import com.google.android.fhir.testing.readJsonArrayFromFile -import com.google.android.fhir.versionId -import com.google.common.truth.Truth.assertThat -import java.time.Instant -import java.util.Date -import junit.framework.TestCase -import kotlinx.coroutines.runBlocking -import org.hl7.fhir.r4.model.HumanName -import org.hl7.fhir.r4.model.Meta -import org.hl7.fhir.r4.model.Patient -import org.hl7.fhir.r4.model.Resource -import org.hl7.fhir.r4.model.ResourceType -import org.json.JSONArray -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@RunWith(RobolectricTestRunner::class) -class LocalChangeUtilsTest : TestCase() { - - val jsonParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser() - @Test - fun `toLocalChange() should return LocalChange`() = runBlocking { - val localChangeEntity = - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.INSERT, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - - val localChange = localChangeEntity.toLocalChange() - assertThat(localChangeEntity.id).isEqualTo(localChange.token.ids.first()) - assertThat(localChangeEntity.resourceType).isEqualTo(localChange.resourceType) - assertThat(localChangeEntity.resourceId).isEqualTo(localChange.resourceId) - assertThat(localChangeEntity.timestamp).isEqualTo(localChange.timestamp) - assertThat(LocalChange.Type.from(localChangeEntity.type.value)).isEqualTo(localChange.type) - assertThat(localChangeEntity.payload).isEqualTo(localChange.payload) - assertThat(localChangeEntity.versionId).isEqualTo(localChange.versionId) - } - - @Test - fun insertAndUpdateLocalChanges_shouldReturnInsertChange() = runBlocking { - val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") - val insertionLocalChange = generateResourceAsLocalChange(patient) - val updatedPatient = readFromFile(Patient::class.java, "/update_test_patient_1.json") - val updateLocalChange = generateResourceUpdateAsLocalChange(patient, updatedPatient, 1L) - val patientString = jsonParser.encodeResourceToString(updatedPatient) - val squashedChange = LocalChangeUtils.squash(listOf(insertionLocalChange, updateLocalChange)) - with(squashedChange) { - assertThat(type).isEqualTo(LocalChange.Type.INSERT) - assertThat(resourceId).isEqualTo(patient.logicalId) - assertThat(resourceType).isEqualTo(patient.resourceType.name) - assertThat(payload).isEqualTo(patientString) - } - } - - @Test - fun updateTwice_shouldReturnMergedPatch() = runBlocking { - val remoteMeta = - Meta().apply { - versionId = "patient-version-1" - lastUpdated = Date() - } - val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") - remotePatient.meta = remoteMeta - val updatedPatient1 = readFromFile(Patient::class.java, "/update_test_patient_1.json") - updatedPatient1.meta = remoteMeta - val updatelocalChange1 = generateResourceUpdateAsLocalChange(remotePatient, updatedPatient1, 1L) - val updatedPatient2 = readFromFile(Patient::class.java, "/update_test_patient_2.json") - updatedPatient2.meta = remoteMeta - val updatelocalChange2 = - generateResourceUpdateAsLocalChange(updatedPatient1, updatedPatient2, 2L) - val updatePatch = readJsonArrayFromFile("/update_patch_2.json") - val squashedChange = LocalChangeUtils.squash(listOf(updatelocalChange1, updatelocalChange2)) - with(squashedChange) { - assertThat(type).isEqualTo(LocalChange.Type.UPDATE) - assertThat(resourceId).isEqualTo(remotePatient.logicalId) - assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) - assertThat(versionId).isEqualTo(remoteMeta.versionId) - assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), updatePatch) - } - } - - @Test - fun deleteAfterUpdates_shouldReturnDeleteLocalChange() = runBlocking { - val remoteMeta = - Meta().apply { - versionId = "patient-version-1" - lastUpdated = Date() - } - val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") - remotePatient.meta = remoteMeta - val updatedPatient1 = remotePatient.copy() - updatedPatient1.name = listOf(HumanName().addGiven("John").setFamily("Doe")) - val updatelocalChange1 = generateResourceUpdateAsLocalChange(remotePatient, updatedPatient1, 1L) - val updatedPatient2 = updatedPatient1.copy() - updatedPatient2.name = listOf(HumanName().addGiven("Jimmy").setFamily("Doe")) - val updatelocalChange2 = - generateResourceUpdateAsLocalChange(updatedPatient1, updatedPatient2, 2L) - val deleteLocalChange = generateDeleteResourceAsLocalChange(updatedPatient2, 3L) - val squashedChange = - LocalChangeUtils.squash(listOf(updatelocalChange1, updatelocalChange2, deleteLocalChange)) - with(squashedChange) { - assertThat(type).isEqualTo(LocalChange.Type.DELETE) - assertThat(resourceId).isEqualTo(remotePatient.logicalId) - assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) - assertThat(versionId).isEqualTo(remoteMeta.versionId) - assertThat(payload).isEmpty() - } - } - - private fun generateResourceUpdateAsLocalChange( - oldEntity: Resource, - updatedResource: Resource, - currentChangeId: Long - ): LocalChange { - val jsonDiff = LocalChangeUtils.diff(jsonParser, oldEntity, updatedResource) - return LocalChange( - resourceId = oldEntity.logicalId, - resourceType = oldEntity.resourceType.name, - type = LocalChange.Type.UPDATE, - payload = jsonDiff.toString(), - versionId = oldEntity.versionId, - token = LocalChangeToken(listOf(currentChangeId + 1)), - timestamp = Instant.now() - ) - } - - private fun generateResourceAsLocalChange(entity: Resource): LocalChange { - return LocalChange( - resourceId = entity.logicalId, - resourceType = entity.resourceType.name, - type = LocalChange.Type.INSERT, - payload = jsonParser.encodeResourceToString(entity), - versionId = entity.versionId, - token = LocalChangeToken(listOf(1L)), - timestamp = Instant.now() - ) - } - - private fun generateDeleteResourceAsLocalChange( - entity: Resource, - currentChangeId: Long - ): LocalChange { - return LocalChange( - resourceId = entity.logicalId, - resourceType = entity.resourceType.name, - type = LocalChange.Type.DELETE, - payload = "", - versionId = entity.versionId, - token = LocalChangeToken(listOf(currentChangeId + 1)), - timestamp = Instant.now() - ) - } -} diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index fd050bbcfc..d19d1b8dfc 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -21,8 +21,8 @@ import ca.uhn.fhir.rest.param.ParamPrefixEnum import com.google.android.fhir.FhirServices.Companion.builder import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChange.Type +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.ResourceNotFoundException -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.get import com.google.android.fhir.logicalId import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM diff --git a/engine/src/test/java/com/google/android/fhir/sync/remote/FhirHttpDataSourceTest.kt b/engine/src/test/java/com/google/android/fhir/sync/remote/FhirHttpDataSourceTest.kt index 9f508ed2d5..053da3c6df 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/remote/FhirHttpDataSourceTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/remote/FhirHttpDataSourceTest.kt @@ -17,7 +17,6 @@ import ca.uhn.fhir.context.FhirContext import com.google.android.fhir.ContentTypes import com.google.android.fhir.NetworkConfiguration -import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.sync.UrlUploadRequest import com.google.android.fhir.sync.remote.FhirHttpDataSource import com.google.android.fhir.sync.remote.RetrofitHttpService @@ -86,8 +85,7 @@ internal class FhirHttpDataSourceTest { } ) } - val request = - UrlUploadRequest(HttpVerb.POST, "Patient", patient, LocalChangeToken(listOf(1)), emptyMap()) + val request = UrlUploadRequest(HttpVerb.POST, "Patient", patient, emptyMap()) mockWebServer.enqueue(mockResponse) dataSource.upload(request) val recordedRequest: RecordedRequest = mockWebServer.takeRequest() @@ -114,8 +112,7 @@ internal class FhirHttpDataSourceTest { HttpVerb.PUT, "Patient/Patient-001", patient, - LocalChangeToken(listOf(1)), - emptyMap() + emptyMap(), ) mockWebServer.enqueue(mockResponse) dataSource.upload(request) @@ -139,8 +136,7 @@ internal class FhirHttpDataSourceTest { HttpVerb.PATCH, "Patient/Patient-001", patchToApply, - LocalChangeToken(listOf(1)), - mapOf("Content-Type" to ContentTypes.APPLICATION_JSON_PATCH) + mapOf("Content-Type" to ContentTypes.APPLICATION_JSON_PATCH), ) mockWebServer.enqueue(mockResponse) dataSource.upload(request) diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt index e77685dea7..49b75a8eb7 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderImplTest.kt @@ -18,11 +18,10 @@ package com.google.android.fhir.sync.upload import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.db.impl.dao.LocalChangeToken -import com.google.android.fhir.db.impl.dao.toLocalChange +import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.impl.entities.LocalChangeEntity -import com.google.android.fhir.sync.upload.patch.SquashedChangesUploadWorkManager import com.google.android.fhir.testing.BundleDataSource +import com.google.android.fhir.toLocalChange import com.google.common.truth.Truth.assertThat import java.net.ConnectException import java.time.Instant diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt new file mode 100644 index 0000000000..54eccc499b --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -0,0 +1,431 @@ +/* + * Copyright 2023 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.upload.patch + +import ca.uhn.fhir.context.FhirContext +import ca.uhn.fhir.context.FhirVersionEnum +import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken +import com.google.android.fhir.db.impl.dao.diff +import com.google.android.fhir.db.impl.entities.LocalChangeEntity +import com.google.android.fhir.logicalId +import com.google.android.fhir.sync.upload.SquashedChangesUploadWorkManager +import com.google.android.fhir.testing.assertJsonArrayEqualsIgnoringOrder +import com.google.android.fhir.testing.jsonParser +import com.google.android.fhir.testing.readFromFile +import com.google.android.fhir.testing.readJsonArrayFromFile +import com.google.android.fhir.toLocalChange +import com.google.android.fhir.versionId +import com.google.common.truth.Truth.assertThat +import java.time.Instant +import java.util.Date +import org.hl7.fhir.r4.model.HumanName +import org.hl7.fhir.r4.model.Meta +import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.Resource +import org.hl7.fhir.r4.model.ResourceType +import org.json.JSONArray +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class PerResourcePatchGeneratorTest { + + @Test + fun `should generate a single insert patch if the resource is inserted`() { + val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + val insertionLocalChange = createInsertLocalChange(patient) + + val patches = PerResourcePatchGenerator.generate(listOf(insertionLocalChange)) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.INSERT) + assertThat(resourceId).isEqualTo(patient.logicalId) + assertThat(resourceType).isEqualTo(patient.resourceType.name) + assertThat(payload).isEqualTo(jsonParser.encodeResourceToString(patient)) + } + } + + @Test + fun `should generate a single update patch if the resource is updated`() { + val remoteMeta = + Meta().apply { + versionId = "patient-version-1" + lastUpdated = Date() + } + val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + remotePatient.meta = remoteMeta + val updatedPatient1 = readFromFile(Patient::class.java, "/update_test_patient_1.json") + updatedPatient1.meta = remoteMeta + val updateLocalChange1 = createUpdateLocalChange(remotePatient, updatedPatient1, 1L) + val updatePatch = readJsonArrayFromFile("/update_patch_1.json") + + val patches = PerResourcePatchGenerator.generate(listOf(updateLocalChange1)) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.UPDATE) + assertThat(resourceId).isEqualTo(remotePatient.logicalId) + assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) + assertThat(versionId).isEqualTo(remoteMeta.versionId) + assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), updatePatch) + } + } + + @Test + fun `should generate a single delete patch if the resource is deleted`() { + val remoteMeta = + Meta().apply { + versionId = "patient-version-1" + lastUpdated = Date() + } + val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + remotePatient.meta = remoteMeta + val deleteLocalChange = createDeleteLocalChange(remotePatient, 3L) + + val patches = PerResourcePatchGenerator.generate(listOf(deleteLocalChange)) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.DELETE) + assertThat(resourceId).isEqualTo(remotePatient.logicalId) + assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) + assertThat(versionId).isEqualTo(remoteMeta.versionId) + assertThat(payload).isEmpty() + } + } + + @Test + fun `should generate a single insert patch if the resource is inserted and updated`() { + val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + val insertionLocalChange = createInsertLocalChange(patient) + val updatedPatient = readFromFile(Patient::class.java, "/update_test_patient_1.json") + val updateLocalChange = createUpdateLocalChange(patient, updatedPatient, 1L) + val patientString = jsonParser.encodeResourceToString(updatedPatient) + + val patches = + PerResourcePatchGenerator.generate(listOf(insertionLocalChange, updateLocalChange)) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.INSERT) + assertThat(resourceId).isEqualTo(patient.logicalId) + assertThat(resourceType).isEqualTo(patient.resourceType.name) + assertThat(payload).isEqualTo(patientString) + } + } + + @Test + fun `should generate no patch if the resource is inserted and deleted`() { + val changes = + listOf( + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.INSERT, + payload = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(1)) }, + LocalChangeEntity( + id = 2, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.DELETE, + payload = "", + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(2)) } + ) + val patchToUpload = PerResourcePatchGenerator.generate(changes) + + assertThat(patchToUpload).isEmpty() + } + + @Test + fun `should generate no patch if the resource is inserted, updated, and deleted`() { + val changes = + listOf( + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.INSERT, + payload = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(1)) }, + LocalChangeEntity( + id = 2, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.UPDATE, + payload = + diff( + jsonParser, + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("Jane") + family = "Doe" + } + ) + }, + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("Janet") + family = "Doe" + } + ) + } + ) + .toString(), + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(1)) }, + LocalChangeEntity( + id = 3, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.DELETE, + payload = "", + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(3)) }, + ) + val patchToUpload = SquashedChangesUploadWorkManager().generatePatches(changes) + + assertThat(patchToUpload).isEmpty() + } + + @Test + fun `should generate a single update patch if the resource is updated twice`() { + val remoteMeta = + Meta().apply { + versionId = "patient-version-1" + lastUpdated = Date() + } + val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + remotePatient.meta = remoteMeta + val updatedPatient1 = readFromFile(Patient::class.java, "/update_test_patient_1.json") + updatedPatient1.meta = remoteMeta + val updateLocalChange1 = createUpdateLocalChange(remotePatient, updatedPatient1, 1L) + val updatedPatient2 = readFromFile(Patient::class.java, "/update_test_patient_2.json") + updatedPatient2.meta = remoteMeta + val updateLocalChange2 = createUpdateLocalChange(updatedPatient1, updatedPatient2, 2L) + val updatePatch = readJsonArrayFromFile("/update_patch_2.json") + + val patches = PerResourcePatchGenerator.generate(listOf(updateLocalChange1, updateLocalChange2)) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.UPDATE) + assertThat(resourceId).isEqualTo(remotePatient.logicalId) + assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) + assertThat(versionId).isEqualTo(remoteMeta.versionId) + assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), updatePatch) + } + } + + @Test + fun `should generate a single delete patch if the resource is updated and deleted`() { + val remoteMeta = + Meta().apply { + versionId = "patient-version-1" + lastUpdated = Date() + } + val remotePatient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") + remotePatient.meta = remoteMeta + val updatedPatient1 = remotePatient.copy() + updatedPatient1.name = listOf(HumanName().addGiven("John").setFamily("Doe")) + val updateLocalChange1 = createUpdateLocalChange(remotePatient, updatedPatient1, 1L) + val updatedPatient2 = updatedPatient1.copy() + updatedPatient2.name = listOf(HumanName().addGiven("Jimmy").setFamily("Doe")) + val updateLocalChange2 = createUpdateLocalChange(updatedPatient1, updatedPatient2, 2L) + val deleteLocalChange = createDeleteLocalChange(updatedPatient2, 3L) + + val patches = + PerResourcePatchGenerator.generate( + listOf(updateLocalChange1, updateLocalChange2, deleteLocalChange) + ) + + with(patches.single()) { + assertThat(type).isEqualTo(Patch.Type.DELETE) + assertThat(resourceId).isEqualTo(remotePatient.logicalId) + assertThat(resourceType).isEqualTo(remotePatient.resourceType.name) + assertThat(versionId).isEqualTo(remoteMeta.versionId) + assertThat(payload).isEmpty() + } + } + + @Test + fun `should throw an error if a change is done after a resource is deleted locally`() { + val changes = + listOf( + LocalChangeEntity( + id = 2, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.DELETE, + payload = "", + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(2)) }, + LocalChangeEntity( + id = 3, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.UPDATE, + payload = "", + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(3)) }, + ) + + val errorMessage = + Assert.assertThrows(IllegalArgumentException::class.java) { + PerResourcePatchGenerator.generate(changes) + } + .localizedMessage + + assertThat(errorMessage).isEqualTo("Changes after deletion of resource are not permitted") + } + + @Test + fun `should throw an error if a change is done before a resource is created locally`() { + val changes = + listOf( + LocalChangeEntity( + id = 3, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.UPDATE, + payload = "", + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(1)) }, + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = LocalChangeEntity.Type.INSERT, + payload = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ) + .toLocalChange() + .apply { LocalChangeToken(listOf(2)) }, + ) + + val errorMessage = + Assert.assertThrows(IllegalArgumentException::class.java) { + PerResourcePatchGenerator.generate(changes) + } + .localizedMessage + + assertThat(errorMessage).isEqualTo("Changes before creation of resource are not permitted") + } + + private fun createUpdateLocalChange( + oldEntity: Resource, + updatedResource: Resource, + currentChangeId: Long + ): LocalChange { + val jsonDiff = diff(jsonParser, oldEntity, updatedResource) + return LocalChange( + resourceId = oldEntity.logicalId, + resourceType = oldEntity.resourceType.name, + type = LocalChange.Type.UPDATE, + payload = jsonDiff.toString(), + versionId = oldEntity.versionId, + token = LocalChangeToken(listOf(currentChangeId + 1)), + timestamp = Instant.now() + ) + } + + private fun createInsertLocalChange(entity: Resource): LocalChange { + return LocalChange( + resourceId = entity.logicalId, + resourceType = entity.resourceType.name, + type = LocalChange.Type.INSERT, + payload = jsonParser.encodeResourceToString(entity), + versionId = entity.versionId, + token = LocalChangeToken(listOf(1L)), + timestamp = Instant.now() + ) + } + + private fun createDeleteLocalChange(entity: Resource, currentChangeId: Long): LocalChange { + return LocalChange( + resourceId = entity.logicalId, + resourceType = entity.resourceType.name, + type = LocalChange.Type.DELETE, + payload = "", + versionId = entity.versionId, + token = LocalChangeToken(listOf(currentChangeId + 1)), + timestamp = Instant.now() + ) + } +} diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManagerTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManagerTest.kt deleted file mode 100644 index cba8d22594..0000000000 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/SquashedChangesUploadWorkManagerTest.kt +++ /dev/null @@ -1,218 +0,0 @@ -/* - * Copyright 2023 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.upload.patch - -import ca.uhn.fhir.context.FhirContext -import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.LocalChange -import com.google.android.fhir.db.impl.dao.LocalChangeToken -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.common.truth.Truth.assertThat -import java.time.Instant -import org.hl7.fhir.r4.model.HumanName -import org.hl7.fhir.r4.model.Patient -import org.hl7.fhir.r4.model.ResourceType -import org.junit.Assert.assertThrows -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@RunWith(RobolectricTestRunner::class) -class SquashedChangesUploadWorkManagerTest { - - private val jsonParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser() - - @Test - fun `prepareChangesForUpload should return no LocalChanges for same resource locally inserted and deleted`() { - val changes = - listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.INSERT, - payload = - FhirContext.forCached(FhirVersionEnum.R4) - .newJsonParser() - .encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.DELETE, - payload = "", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) } - ) - val patchToUpload = SquashedChangesUploadWorkManager().prepareChangesForUpload(changes) - - assertThat(patchToUpload).hasSize(1) - assertThat(patchToUpload.first().type).isEqualTo(LocalChange.Type.NO_OP) - } - - @Test - fun `prepareChangesForUpload should squash change to NO_OP if insert is followed by an update, then a delete`() { - val changes = - listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.INSERT, - payload = - FhirContext.forCached(FhirVersionEnum.R4) - .newJsonParser() - .encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.UPDATE, - payload = - LocalChangeUtils.diff( - jsonParser, - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("Jane") - family = "Doe" - } - ) - }, - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("Janet") - family = "Doe" - } - ) - } - ) - .toString(), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 3, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.DELETE, - payload = "", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(3)) }, - ) - val patchToUpload = SquashedChangesUploadWorkManager().prepareChangesForUpload(changes) - - assertThat(patchToUpload).hasSize(1) - assertThat(patchToUpload.first().type).isEqualTo(LocalChange.Type.NO_OP) - } - - @Test - fun `prepareChangesForUpload should throw an error if a change is done after a resource is deleted locally`() { - val changes = - listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.INSERT, - payload = - FhirContext.forCached(FhirVersionEnum.R4) - .newJsonParser() - .encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.DELETE, - payload = "", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) }, - LocalChangeEntity( - id = 3, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = LocalChangeEntity.Type.UPDATE, - payload = "", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(3)) } - ) - - val errorMessage = - assertThrows(IllegalArgumentException::class.java) { - SquashedChangesUploadWorkManager().prepareChangesForUpload(changes) - } - .localizedMessage - - assertThat(errorMessage).isEqualTo("Cannot merge local changes with type NO_OP and UPDATE.") - } -} diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/request/IndividualGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/request/IndividualGeneratorTest.kt index c93bca8039..211fdb4da0 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/request/IndividualGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/request/IndividualGeneratorTest.kt @@ -18,11 +18,7 @@ package com.google.android.fhir.sync.upload.request import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.db.impl.dao.LocalChangeToken -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.upload.patch.Patch import com.google.common.truth.Truth.assertThat import java.time.Instant import kotlinx.coroutines.test.runTest @@ -43,41 +39,38 @@ class IndividualGeneratorTest { @Test fun `should return empty list if there are no local changes`() = runTest { val generator = IndividualRequestGenerator.getDefault() - val result = generator.generateUploadRequests(listOf()) - assertThat(result).isEmpty() + val requests = generator.generateUploadRequests(listOf()) + assertThat(requests).isEmpty() } @Test fun `should create a POST request for insert`() = runTest { val generator = IndividualRequestGenerator.getGenerator(HttpVerb.POST, HttpVerb.PATCH) - val result = + val requests = generator.generateUploadRequests( listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.INSERT, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { token = LocalChangeToken(listOf(1)) } + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ), ) ) - with(result.single()) { + with(requests.single()) { assertThat(httpVerb).isEqualTo(HttpVerb.POST) assertThat(url).isEqualTo("Patient") } @@ -86,34 +79,31 @@ class IndividualGeneratorTest { @Test fun `should create a PUT request for insert`() = runTest { val generator = IndividualRequestGenerator.getDefault() - val result = + val requests = generator.generateUploadRequests( listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.INSERT, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { token = LocalChangeToken(listOf(1)) } + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ), ) ) - with(result.single()) { + with(requests.single()) { assertThat(httpVerb).isEqualTo(HttpVerb.PUT) assertThat(url).isEqualTo("Patient/Patient-001") } @@ -121,45 +111,21 @@ class IndividualGeneratorTest { @Test fun `should create a PATCH request for update`() = runTest { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = - LocalChangeUtils.diff( - jsonParser, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Jane") - family = "Doe" - } - ) - }, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Janet") - family = "Doe" - } - ) - } - ) - .toString(), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) }, + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = + "[{\"op\":\"replace\",\"path\":\"\\/name\\/0\\/given\\/0\",\"value\":\"Janet\"}]", + timestamp = Instant.now() + ), ) val generator = IndividualRequestGenerator.Factory.getDefault() - val result = generator.generateUploadRequests(changes) - with(result.single()) { - assertThat(result.size).isEqualTo(1) + val requests = generator.generateUploadRequests(patches) + with(requests.single()) { + assertThat(requests.size).isEqualTo(1) assertThat(httpVerb).isEqualTo(HttpVerb.PATCH) assertThat(url).isEqualTo("Patient/Patient-002") assertThat((resource as Binary).data.toString(Charsets.UTF_8)) @@ -171,33 +137,30 @@ class IndividualGeneratorTest { @Test fun `should create a DELETE request for delete`() = runTest { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.DELETE, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(1)) }, + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.DELETE, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ), ) val generator = IndividualRequestGenerator.Factory.getDefault() - val result = generator.generateUploadRequests(changes) - with(result.single()) { + val requests = generator.generateUploadRequests(patches) + with(requests.single()) { assertThat(httpVerb).isEqualTo(HttpVerb.DELETE) assertThat(url).isEqualTo("Patient/Patient-001") } @@ -205,85 +168,55 @@ class IndividualGeneratorTest { @Test fun `should return multiple requests in order`() = runTest { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.INSERT, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-001" - addName( - HumanName().apply { - addGiven("John") - family = "Doe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { token = LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = - LocalChangeUtils.diff( - jsonParser, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Jane") - family = "Doe" - } - ) - }, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Janet") - family = "Doe" - } - ) + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" } ) - .toString(), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) }, - LocalChangeEntity( - id = 3, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-003", - type = Type.DELETE, - payload = - jsonParser.encodeResourceToString( - Patient().apply { - id = "Patient-003" - addName( - HumanName().apply { - addGiven("John") - family = "Roe" - } - ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(3)) } + } + ), + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = + "[{\"op\":\"replace\",\"path\":\"\\/name\\/0\\/given\\/0\",\"value\":\"Janet\"}]", + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-003", + type = Patch.Type.DELETE, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-003" + addName( + HumanName().apply { + addGiven("John") + family = "Roe" + } + ) + } + ), + timestamp = Instant.now() + ), ) val generator = IndividualRequestGenerator.Factory.getDefault() - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) assertThat(result).hasSize(3) assertThat(result.map { it.httpVerb }) .containsExactly(HttpVerb.PUT, HttpVerb.PATCH, HttpVerb.DELETE) diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt index c9e0da065f..e9db8c43cf 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt @@ -18,11 +18,8 @@ package com.google.android.fhir.sync.upload.request import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum -import com.google.android.fhir.db.impl.dao.LocalChangeToken -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.db.impl.dao.diff +import com.google.android.fhir.sync.upload.patch.Patch import com.google.common.truth.Truth.assertThat import java.time.Instant import kotlinx.coroutines.runBlocking @@ -49,85 +46,76 @@ class TransactionBundleGeneratorTest { fun `generateUploadRequests() should return single Transaction Bundle with 3 entries`() = runBlocking { val jsonParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser() - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.INSERT, - payload = - jsonParser.encodeResourceToString( + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = + diff( + jsonParser, Patient().apply { - id = "Patient-001" + id = "Patient-002" addName( HumanName().apply { - addGiven("John") + addGiven("Jane") family = "Doe" } ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { token = LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = - LocalChangeUtils.diff( - jsonParser, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Jane") - family = "Doe" - } - ) - }, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Janet") - family = "Doe" - } - ) - } - ) - .toString(), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) }, - LocalChangeEntity( - id = 3, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-003", - type = Type.DELETE, - payload = - jsonParser.encodeResourceToString( + }, Patient().apply { - id = "Patient-003" + id = "Patient-002" addName( HumanName().apply { - addGiven("John") - family = "Roe" + addGiven("Janet") + family = "Doe" } ) } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(3)) } + ) + .toString(), + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-003", + type = Patch.Type.DELETE, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-003" + addName( + HumanName().apply { + addGiven("John") + family = "Roe" + } + ) + } + ), + timestamp = Instant.now() + ), ) val generator = TransactionBundleGenerator.Factory.getDefault() - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) assertThat(result).hasSize(1) val bundleUploadRequest = result[0] @@ -142,84 +130,75 @@ class TransactionBundleGeneratorTest { fun `generateUploadRequests() should return 3 Transaction Bundle with single entry each`() = runBlocking { val jsonParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser() - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-001", - type = Type.INSERT, - payload = - jsonParser.encodeResourceToString( + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-001", + type = Patch.Type.INSERT, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-001" + addName( + HumanName().apply { + addGiven("John") + family = "Doe" + } + ) + } + ), + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = + diff( + jsonParser, Patient().apply { - id = "Patient-001" + id = "Patient-002" addName( HumanName().apply { - addGiven("John") + addGiven("Jane") family = "Doe" } ) - } - ), - timestamp = Instant.now() - ) - .toLocalChange() - .apply { token = LocalChangeToken(listOf(1)) }, - LocalChangeEntity( - id = 2, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = - LocalChangeUtils.diff( - jsonParser, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Jane") - family = "Doe" - } - ) - }, - Patient().apply { - id = "Patient-002" - addName( - HumanName().apply { - addGiven("Janet") - family = "Doe" - } - ) - } - ) - .toString(), - versionId = "v-p002-01", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(2)) }, - LocalChangeEntity( - id = 3, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-003", - type = Type.DELETE, - payload = - jsonParser.encodeResourceToString( + }, Patient().apply { - id = "Patient-003" + id = "Patient-002" addName( HumanName().apply { - addGiven("John") - family = "Roe" + addGiven("Janet") + family = "Doe" } ) } - ), - versionId = "v-p003-01", - timestamp = Instant.now() - ) - .toLocalChange() - .apply { LocalChangeToken(listOf(3)) } + ) + .toString(), + versionId = "v-p002-01", + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-003", + type = Patch.Type.DELETE, + payload = + jsonParser.encodeResourceToString( + Patient().apply { + id = "Patient-003" + addName( + HumanName().apply { + addGiven("John") + family = "Roe" + } + ) + } + ), + versionId = "v-p003-01", + timestamp = Instant.now() + ), ) val generator = TransactionBundleGenerator.Factory.getGenerator( @@ -228,7 +207,7 @@ class TransactionBundleGeneratorTest { 1, true ) - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) // Exactly 3 Requests are generated assertThat(result).hasSize(3) @@ -247,21 +226,19 @@ class TransactionBundleGeneratorTest { @Test fun `generate() should return Bundle Entry without if-match when useETagForUpload is false`() = runBlocking { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = "[]", - versionId = "patient-002-version-1", - timestamp = Instant.now() - ) - .toLocalChange() + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = "[]", + versionId = "patient-002-version-1", + timestamp = Instant.now() + ) ) val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = false) - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) assertThat(result.first().resource.entry.first().request.ifMatch).isNull() } @@ -269,21 +246,19 @@ class TransactionBundleGeneratorTest { @Test fun `generate() should return Bundle Entry with if-match when useETagForUpload is true`() = runBlocking { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = "[]", - versionId = "patient-002-version-1", - timestamp = Instant.now() - ) - .toLocalChange() + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = "[]", + versionId = "patient-002-version-1", + timestamp = Instant.now() + ) ) val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true) - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) assertThat(result.first().resource.entry.first().request.ifMatch) .isEqualTo("W/\"patient-002-version-1\"") @@ -292,31 +267,27 @@ class TransactionBundleGeneratorTest { @Test fun `generate() should return Bundle Entry without if-match when the LocalChangeEntity has no versionId`() = runBlocking { - val changes = + val patches = listOf( - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-002", - type = Type.UPDATE, - payload = "[]", - versionId = "", - timestamp = Instant.now() - ) - .toLocalChange(), - LocalChangeEntity( - id = 1, - resourceType = ResourceType.Patient.name, - resourceId = "Patient-003", - type = Type.UPDATE, - payload = "[]", - versionId = null, - timestamp = Instant.now() - ) - .toLocalChange() + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Patch.Type.UPDATE, + payload = "[]", + versionId = "", + timestamp = Instant.now() + ), + Patch( + resourceType = ResourceType.Patient.name, + resourceId = "Patient-003", + type = Patch.Type.UPDATE, + payload = "[]", + versionId = null, + timestamp = Instant.now() + ), ) val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true) - val result = generator.generateUploadRequests(changes) + val result = generator.generateUploadRequests(patches) assertThat(result.first().resource.entry[0].request.ifMatch).isNull() assertThat(result.first().resource.entry[1].request.ifMatch).isNull()