Revert "Updated PerResourcePatchGenerator to return ordered PatchMapp…

…ing to avoid referential integrity issues. (google#2442)"

This reverts commit 9697a8a.

engine/build.gradle.kts

@@ -150,7 +150,6 @@ dependencies {
  testImplementation(Dependencies.AndroidxTest.core)
  testImplementation(Dependencies.AndroidxTest.workTestingRuntimeKtx)
  testImplementation(Dependencies.Kotlin.kotlinCoroutinesTest)
- testImplementation(Dependencies.Kotlin.kotlinTestJunit)
  testImplementation(Dependencies.junit)
  testImplementation(Dependencies.jsonAssert)
  testImplementation(Dependencies.mockitoInline)

engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt

@@ -18,7 +18,6 @@ package com.google.android.fhir
 
 import android.content.Context
 import com.google.android.fhir.DatabaseErrorStrategy.UNSPECIFIED
-import com.google.android.fhir.db.Database
 import com.google.android.fhir.sync.DataSource
 import com.google.android.fhir.sync.FhirDataStore
 import com.google.android.fhir.sync.HttpAuthenticator
@@ -74,11 +73,6 @@ object FhirEngineProvider {
  return getOrCreateFhirService(context).fhirDataStore
  }
 
- @Synchronized
- internal fun getFhirDatabase(context: Context): Database {
- return getOrCreateFhirService(context).database
- }
-
  @Synchronized
  private fun getOrCreateFhirService(context: Context): FhirServices {
  if (fhirServices == null) {

engine/src/main/java/com/google/android/fhir/db/Database.kt

@@ -194,24 +194,9 @@ internal interface Database {
  * will delete resource entry from LocalChangeEntity table.
  */
  suspend fun purge(type: ResourceType, id: String, forcePurge: Boolean = false)
-
- /**
- * @return List of [LocalChangeResourceReference] associated with the [LocalChangeEntity.id]s. A
- * single [LocalChangeEntity] may have one or more [LocalChangeResourceReference] associated
- * with it.
- */
- suspend fun getLocalChangeResourceReferences(
- localChangeIds: List<Long>,
- ): List<LocalChangeResourceReference>
 }
 
 data class ResourceWithUUID<R>(
  val uuid: UUID,
  val resource: R,
 )
-
-data class LocalChangeResourceReference(
- val localChangeId: Long,
- val resourceReferenceValue: String,
- val resourceReferencePath: String?,
-)

engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt

@@ -26,7 +26,6 @@ import ca.uhn.fhir.util.FhirTerser
 import com.google.android.fhir.DatabaseErrorStrategy
 import com.google.android.fhir.LocalChange
 import com.google.android.fhir.LocalChangeToken
-import com.google.android.fhir.db.LocalChangeResourceReference
 import com.google.android.fhir.db.ResourceNotFoundException
 import com.google.android.fhir.db.ResourceWithUUID
 import com.google.android.fhir.db.impl.DatabaseImpl.Companion.UNENCRYPTED_DATABASE_NAME
@@ -414,18 +413,6 @@ internal class DatabaseImpl(
  }
  }
 
- override suspend fun getLocalChangeResourceReferences(
- localChangeIds: List<Long>,
- ): List<LocalChangeResourceReference> {
- return localChangeDao.getReferencesForLocalChanges(localChangeIds).map {
- LocalChangeResourceReference(
- it.localChangeId,
- it.resourceReferenceValue,
- it.resourceReferencePath,
- )
- }
- }
-
  companion object {
  /**
  * The name for unencrypted database.

engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023-2024 Google LLC
+ * 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.
@@ -348,17 +348,6 @@ internal abstract class LocalChangeDao {
  localChangeId: Long,
  ): List<LocalChangeResourceReferenceEntity>
 
- @Query(
- """
- SELECT *
- FROM LocalChangeResourceReferenceEntity
- WHERE localChangeId = (:localChangeId)
- """,
- )
- abstract suspend fun getReferencesForLocalChanges(
- localChangeId: List<Long>,
- ): List<LocalChangeResourceReferenceEntity>
-
  @Insert(onConflict = OnConflictStrategy.REPLACE)
  abstract suspend fun insertLocalChangeResourceReferences(
  resourceReferences: List<LocalChangeResourceReferenceEntity>,

engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt

@@ -98,11 +98,7 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter
  UploadConfiguration(
  Uploader(
  dataSource = dataSource,
- patchGenerator =
- PatchGeneratorFactory.byMode(
- getUploadStrategy().patchGeneratorMode,
- FhirEngineProvider.getFhirDatabase(applicationContext),
- ),
+ patchGenerator = PatchGeneratorFactory.byMode(getUploadStrategy().patchGeneratorMode),
  requestGenerator =
  UploadRequestGeneratorFactory.byMode(getUploadStrategy().requestGeneratorMode),
  ),

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023-2024 Google LLC
+ * 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.
@@ -17,7 +17,6 @@
 package com.google.android.fhir.sync.upload.patch
 
 import com.google.android.fhir.LocalChange
-import com.google.android.fhir.db.Database
 
 /**
  * Generates [Patch]es from [LocalChange]s and output [List<[PatchMapping]>] to keep a mapping of
@@ -35,17 +34,16 @@ internal interface PatchGenerator {
  * NOTE: different implementations may have requirements on the size of [localChanges] and output
  * certain numbers of [Patch]es.
  */
- suspend fun generate(localChanges: List<LocalChange>): List<PatchMapping>
+ fun generate(localChanges: List<LocalChange>): List<PatchMapping>
 }
 
 internal object PatchGeneratorFactory {
  fun byMode(
  mode: PatchGeneratorMode,
- database: Database,
  ): PatchGenerator =
  when (mode) {
  is PatchGeneratorMode.PerChange -> PerChangePatchGenerator
- is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator.with(database)
+ is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator
  }
 }
 

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023-2024 Google LLC
+ * 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.
@@ -25,7 +25,7 @@ import com.google.android.fhir.LocalChange
  * maintain an audit trail.
  */
 internal object PerChangePatchGenerator : PatchGenerator {
- override suspend fun generate(localChanges: List<LocalChange>): List<PatchMapping> =
+ override fun generate(localChanges: List<LocalChange>): List<PatchMapping> =
  localChanges.map {
  PatchMapping(
  localChanges = listOf(it),

engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt

@@ -22,8 +22,6 @@ import com.github.fge.jackson.JsonLoader
 import com.github.fge.jsonpatch.JsonPatch
 import com.google.android.fhir.LocalChange
 import com.google.android.fhir.LocalChange.Type
-import com.google.android.fhir.db.Database
-import com.google.android.fhir.sync.upload.patch.PatchOrdering.orderByReferences
 
 /**
  * Generates a [Patch] for all [LocalChange]es made to a single FHIR resource.
@@ -32,16 +30,10 @@ import com.google.android.fhir.sync.upload.patch.PatchOrdering.orderByReferences
  * 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 class PerResourcePatchGenerator private constructor(val database: Database) :
- PatchGenerator {
+internal object PerResourcePatchGenerator : PatchGenerator {
 
- override suspend fun generate(localChanges: List<LocalChange>): List<PatchMapping> {
- return generateSquashedChangesMapping(localChanges).orderByReferences(database)
- }
-
- @androidx.annotation.VisibleForTesting
- internal fun generateSquashedChangesMapping(localChanges: List<LocalChange>) =
- localChanges
+ override fun generate(localChanges: List<LocalChange>): List<PatchMapping> {
+ return localChanges
  .groupBy { it.resourceType to it.resourceId }
  .values
  .mapNotNull { resourceLocalChanges ->
@@ -52,6 +44,7 @@ internal class PerResourcePatchGenerator private constructor(val database: Datab
  )
  }
  }
+ }
 
  private fun mergeLocalChangesForSingleResource(localChanges: List<LocalChange>): Patch? {
  // TODO (maybe this should throw exception when two entities don't have the same versionID)
@@ -145,20 +138,4 @@ internal class PerResourcePatchGenerator private constructor(val database: Datab
  mergedOperations.values.flatten().forEach(mergedNode::add)
  return objectMapper.writeValueAsString(mergedNode)
  }
-
- companion object {
-
- @Volatile private lateinit var _instance: PerResourcePatchGenerator
-
- @Synchronized
- fun with(database: Database): PerResourcePatchGenerator {
- if (!::_instance.isInitialized) {
- _instance = PerResourcePatchGenerator(database)
- } else if (_instance.database != database) {
- _instance = PerResourcePatchGenerator(database)
- }
-
- return _instance
- }
- }
 }

engine/src/main/java/com/google/android/fhir/testing/Utilities.kt

@@ -191,8 +191,8 @@ object TestFhirEngineImpl : FhirEngine {
  LocalChange(
  resourceType = type.name,
  resourceId = id,
- payload = """{ "resourceType" : "$type", "id" : "$id" }""",
- token = LocalChangeToken(listOf(1)),
+ payload = "{ 'resourceType' : '$type', 'id' : '$id' }",
+ token = LocalChangeToken(listOf()),
  type = LocalChange.Type.INSERT,
  timestamp = Instant.now(),
  ),