Introduce Patches: changes to be uploaded to the server (google#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
opensrp · Sep 12, 2023 · 87c3437 · 87c3437
1 parent 18d6980
commit 87c3437
Show file tree

Hide file tree

Showing 35 changed files with 1,187 additions and 1,133 deletions.
diff --git a/...datacapture/test/views/QuestionnaireItemDialogMultiSelectViewHolderFactoryEspressoTest.kt b/...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
@@ -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
@@ -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
@@ -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
@@ -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<Long>)
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
@@ -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
@@ -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
@@ -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
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
@@ -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
@@ -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