Refactor usages of Json Parser for thread safety (google#2749)

* Refactor usages of Json Parser

* Fix failing tests ✅

* Update JsonParser usages for Dao classes
Spotless clean

* Fix failing unit test ✅

engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt

@@ -118,6 +118,7 @@ class DatabaseImplTest {
   @JvmField @Parameterized.Parameter(0) var encrypted: Boolean = false
 
   private val context: Context = ApplicationProvider.getApplicationContext()
+  private val parser = FhirContext.forR4Cached().newJsonParser()
   private lateinit var services: FhirServices
   private lateinit var database: Database
 
@@ -202,7 +203,7 @@ class DatabaseImplTest {
   fun getLocalChanges_withSingleLocaleChange_shouldReturnSingleLocalChanges() = runBlocking {
     val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json")
     database.insert(patient)
-    val patientString = services.parser.encodeResourceToString(patient)
+    val patientString = parser.encodeResourceToString(patient)
     val resourceLocalChanges = database.getLocalChanges(patient.resourceType, patient.logicalId)
     assertThat(resourceLocalChanges.size).isEqualTo(1)
     with(resourceLocalChanges[0]) {
@@ -269,7 +270,7 @@ class DatabaseImplTest {
   fun clearDatabase_shouldClearAllTablesData() = runBlocking {
     val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json")
     database.insert(patient)
-    val patientString = services.parser.encodeResourceToString(patient)
+    val patientString = parser.encodeResourceToString(patient)
     val resourceLocalChanges = database.getLocalChanges(patient.resourceType, patient.logicalId)
     assertThat(resourceLocalChanges.size).isEqualTo(1)
     with(resourceLocalChanges[0]) {
@@ -393,7 +394,7 @@ class DatabaseImplTest {
 
   @Test
   fun insert_shouldAddInsertLocalChange() = runBlocking {
-    val testPatient2String = services.parser.encodeResourceToString(TEST_PATIENT_2)
+    val testPatient2String = parser.encodeResourceToString(TEST_PATIENT_2)
     database.insert(TEST_PATIENT_2)
     val resourceLocalChanges =
       database.getAllLocalChanges().filter { it.resourceId.equals(TEST_PATIENT_2_ID) }
@@ -481,7 +482,7 @@ class DatabaseImplTest {
     database.insert(patient)
     patient = readFromFile(Patient::class.java, "/update_test_patient_1.json")
     database.update(patient)
-    services.parser.encodeResourceToString(patient)
+    parser.encodeResourceToString(patient)
     val localChangeTokenIds =
       database
         .getAllLocalChanges()
@@ -4101,7 +4102,7 @@ class DatabaseImplTest {
       val observationLocalChange = updatedObservationLocalChanges[0]
       assertThat(observationLocalChange.type).isEqualTo(LocalChange.Type.INSERT)
       val observationLocalChangePayload =
-        services.parser.parseResource(observationLocalChange.payload) as Observation
+        parser.parseResource(observationLocalChange.payload) as Observation
       assertThat(observationLocalChangePayload.subject.reference)
         .isEqualTo("Patient/$remotelyCreatedPatientResourceId")
     }

engine/src/androidTest/java/com/google/android/fhir/db/impl/EncryptedDatabaseErrorTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023 Google LLC
+ * Copyright 2023-2024 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,7 +22,6 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.MediumTest
 import ca.uhn.fhir.context.FhirContext
-import ca.uhn.fhir.context.FhirVersionEnum
 import ca.uhn.fhir.util.FhirTerser
 import com.google.android.fhir.DatabaseErrorStrategy.RECREATE_AT_OPEN
 import com.google.android.fhir.DatabaseErrorStrategy.UNSPECIFIED
@@ -49,8 +48,7 @@ import org.junit.runner.RunWith
 @RunWith(AndroidJUnit4::class)
 class EncryptedDatabaseErrorTest {
   private val context: Context = ApplicationProvider.getApplicationContext()
-  private val parser = FhirContext.forR4().newJsonParser()
-  private val terser = FhirTerser(FhirContext.forCached(FhirVersionEnum.R4))
+  private val terser = FhirTerser(FhirContext.forR4Cached())
   private val resourceIndexer = ResourceIndexer(SearchParamDefinitionsProviderImpl())
 
   @After
@@ -66,7 +64,6 @@ class EncryptedDatabaseErrorTest {
         // GIVEN an unencrypted database.
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,
@@ -84,7 +81,6 @@ class EncryptedDatabaseErrorTest {
         // THEN it should throw SQLiteException
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,
@@ -115,7 +111,6 @@ class EncryptedDatabaseErrorTest {
         // GIVEN an unencrypted database.
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,
@@ -139,7 +134,6 @@ class EncryptedDatabaseErrorTest {
         // THEN it should throw SQLiteException
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,
@@ -169,7 +163,6 @@ class EncryptedDatabaseErrorTest {
       // GIVEN an unencrypted database.
       DatabaseImpl(
           context,
-          parser,
           terser,
           DatabaseConfig(
             inMemory = false,
@@ -193,7 +186,6 @@ class EncryptedDatabaseErrorTest {
       // THEN it should recreate the database
       DatabaseImpl(
           context,
-          parser,
           terser,
           DatabaseConfig(
             inMemory = false,
@@ -226,7 +218,6 @@ class EncryptedDatabaseErrorTest {
         // GIVEN an encrypted database.
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,
@@ -244,7 +235,6 @@ class EncryptedDatabaseErrorTest {
         // THEN it should recreate database.
         DatabaseImpl(
             context,
-            parser,
             terser,
             DatabaseConfig(
               inMemory = false,

engine/src/androidTest/java/com/google/android/fhir/db/impl/dao/LocalChangeDaoTest.kt

@@ -49,6 +49,7 @@ import org.junit.runner.RunWith
 class LocalChangeDaoTest {
   private lateinit var database: ResourceDatabase
   private lateinit var localChangeDao: LocalChangeDao
+  private val iParser = FhirContext.forR4Cached().newJsonParser()
 
   @Before
   fun setupDatabase() {
@@ -62,7 +63,6 @@ class LocalChangeDaoTest {
 
     localChangeDao =
       database.localChangeDao().also {
-        it.iParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser()
         it.fhirTerser = FhirTerser(FhirContext.forCached(FhirVersionEnum.R4))
       }
   }
@@ -97,8 +97,7 @@ class LocalChangeDaoTest {
     assertThat(carePlanLocalChange1.resourceUuid).isEqualTo(carePlanResourceUuid)
     assertThat(carePlanLocalChange1.resourceId).isEqualTo(carePlan.id)
     assertThat(carePlanLocalChange1.type).isEqualTo(LocalChangeEntity.Type.INSERT)
-    assertThat(carePlanLocalChange1.payload)
-      .isEqualTo(localChangeDao.iParser.encodeResourceToString(carePlan))
+    assertThat(carePlanLocalChange1.payload).isEqualTo(iParser.encodeResourceToString(carePlan))
     val carePlanLocalChange1Id = carePlanLocalChange1.id
 
     val localChangeResourceReferences =
@@ -150,7 +149,7 @@ class LocalChangeDaoTest {
           resourceId = originalCarePlan.logicalId,
           resourceType = originalCarePlan.resourceType,
           resourceUuid = carePlanResourceUuid,
-          serializedResource = localChangeDao.iParser.encodeResourceToString(originalCarePlan),
+          serializedResource = iParser.encodeResourceToString(originalCarePlan),
         ),
       updatedResource = modifiedCarePlan,
       timeOfLocalChange = carePlanUpdateTime,
@@ -163,7 +162,7 @@ class LocalChangeDaoTest {
     assertThat(carePlanLocalChange1.resourceId).isEqualTo(originalCarePlan.id)
     assertThat(carePlanLocalChange1.type).isEqualTo(LocalChangeEntity.Type.INSERT)
     assertThat(carePlanLocalChange1.payload)
-      .isEqualTo(localChangeDao.iParser.encodeResourceToString(originalCarePlan))
+      .isEqualTo(iParser.encodeResourceToString(originalCarePlan))
 
     val carePlanLocalChange2 = carePlanLocalChanges[1]
     assertThat(carePlanLocalChange2.resourceUuid).isEqualTo(carePlanResourceUuid)
@@ -224,8 +223,7 @@ class LocalChangeDaoTest {
     assertThat(carePlanLocalChange1.resourceUuid).isEqualTo(carePlanResourceUuid)
     assertThat(carePlanLocalChange1.resourceId).isEqualTo(carePlan.id)
     assertThat(carePlanLocalChange1.type).isEqualTo(LocalChangeEntity.Type.INSERT)
-    assertThat(carePlanLocalChange1.payload)
-      .isEqualTo(localChangeDao.iParser.encodeResourceToString(carePlan))
+    assertThat(carePlanLocalChange1.payload).isEqualTo(iParser.encodeResourceToString(carePlan))
 
     val carePlanLocalChange2 = carePlanLocalChanges[1]
     assertThat(carePlanLocalChange2.resourceUuid).isEqualTo(carePlanResourceUuid)
@@ -285,7 +283,7 @@ class LocalChangeDaoTest {
           resourceId = originalCarePlan.logicalId,
           resourceType = originalCarePlan.resourceType,
           resourceUuid = carePlanResourceUuid,
-          serializedResource = localChangeDao.iParser.encodeResourceToString(originalCarePlan),
+          serializedResource = iParser.encodeResourceToString(originalCarePlan),
         ),
       updatedResource = modifiedCarePlan,
       timeOfLocalChange = carePlanUpdateTime,
@@ -318,7 +316,7 @@ class LocalChangeDaoTest {
         activityFirstRep.detail.performer.add(Reference("Patient/$updatedPatientId"))
       }
     assertThat(carePlanLocalChange1.payload)
-      .isEqualTo(localChangeDao.iParser.encodeResourceToString(updatedReferencesCarePlan))
+      .isEqualTo(iParser.encodeResourceToString(updatedReferencesCarePlan))
     val carePlanLocalChange1Id = carePlanLocalChange1.id
     // assert that LocalChangeReferences are updated as well
     val localChange1ResourceReferences =

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

@@ -18,8 +18,6 @@ package com.google.android.fhir
 
 import android.content.Context
 import ca.uhn.fhir.context.FhirContext
-import ca.uhn.fhir.context.FhirVersionEnum
-import ca.uhn.fhir.parser.IParser
 import ca.uhn.fhir.util.FhirTerser
 import com.google.android.fhir.db.Database
 import com.google.android.fhir.db.impl.DatabaseConfig
@@ -38,7 +36,6 @@ import timber.log.Timber
 
 internal data class FhirServices(
   val fhirEngine: FhirEngine,
-  val parser: IParser,
   val database: Database,
   val remoteDataSource: DataSource? = null,
   val fhirDataStore: FhirDataStore,
@@ -74,15 +71,13 @@ internal data class FhirServices(
     }
 
     fun build(): FhirServices {
-      val parser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser()
-      val terser = FhirTerser(FhirContext.forCached(FhirVersionEnum.R4))
+      val terser = FhirTerser(FhirContext.forR4Cached())
       val searchParamMap =
         searchParameters?.asMapOfResourceTypeToSearchParamDefinitions() ?: emptyMap()
       val provider = SearchParamDefinitionsProviderImpl(searchParamMap)
       val db =
         DatabaseImpl(
           context = context,
-          iParser = parser,
           fhirTerser = terser,
           DatabaseConfig(inMemory, enableEncryption, databaseErrorStrategy),
           resourceIndexer = ResourceIndexer(provider),
@@ -100,7 +95,6 @@ internal data class FhirServices(
         }
       return FhirServices(
         fhirEngine = engine,
-        parser = parser,
         database = db,
         remoteDataSource = remoteDataSource,
         fhirDataStore = FhirDataStore(context),

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

@@ -22,7 +22,6 @@ import androidx.room.Room
 import androidx.room.withTransaction
 import androidx.sqlite.db.SimpleSQLiteQuery
 import ca.uhn.fhir.context.FhirContext
-import ca.uhn.fhir.parser.IParser
 import ca.uhn.fhir.util.FhirTerser
 import com.google.android.fhir.DatabaseErrorStrategy
 import com.google.android.fhir.LocalChange
@@ -56,7 +55,6 @@ import org.hl7.fhir.r4.model.ResourceType
 @Suppress("UNCHECKED_CAST")
 internal class DatabaseImpl(
   private val context: Context,
-  private val iParser: IParser,
   private val fhirTerser: FhirTerser,
   databaseConfig: DatabaseConfig,
   private val resourceIndexer: ResourceIndexer,
@@ -122,18 +120,9 @@ internal class DatabaseImpl(
         .build()
   }
 
-  private val resourceDao by lazy {
-    db.resourceDao().also {
-      it.iParser = iParser
-      it.resourceIndexer = resourceIndexer
-    }
-  }
+  private val resourceDao by lazy { db.resourceDao().also { it.resourceIndexer = resourceIndexer } }
 
-  private val localChangeDao =
-    db.localChangeDao().also {
-      it.iParser = iParser
-      it.fhirTerser = fhirTerser
-    }
+  private val localChangeDao = db.localChangeDao().also { it.fhirTerser = fhirTerser }
 
   override suspend fun <R : Resource> insert(vararg resource: R): List<String> {
     val logicalIds = mutableListOf<String>()
@@ -191,18 +180,21 @@ internal class DatabaseImpl(
     db.withTransaction {
       resourceDao.getResourceEntity(oldResourceId, resourceType)?.let { oldResourceEntity ->
         val updatedResource =
-          (iParser.parseResource(oldResourceEntity.serializedResource) as Resource).apply {
-            idElement = IdType(newResourceId)
-            updateMeta(versionId, lastUpdatedRemote)
-          }
+          (FhirContext.forR4Cached()
+              .newJsonParser()
+              .parseResource(oldResourceEntity.serializedResource) as Resource)
+            .apply {
+              idElement = IdType(newResourceId)
+              updateMeta(versionId, lastUpdatedRemote)
+            }
         updateResourceAndReferences(oldResourceId, updatedResource)
       }
     }
   }
 
   override suspend fun select(type: ResourceType, id: String): Resource {
     return resourceDao.getResource(resourceId = id, resourceType = type)?.let {
-      iParser.parseResource(it) as Resource
+      FhirContext.forR4Cached().newJsonParser().parseResource(it) as Resource
     }
       ?: throw ResourceNotFoundException(type.name, id)
   }
@@ -317,7 +309,10 @@ internal class DatabaseImpl(
   ) {
     db.withTransaction {
       val currentResourceEntity = selectEntity(updatedResource.resourceType, currentResourceId)
-      val oldResource = iParser.parseResource(currentResourceEntity.serializedResource) as Resource
+      val oldResource =
+        FhirContext.forR4Cached()
+          .newJsonParser()
+          .parseResource(currentResourceEntity.serializedResource) as Resource
       val resourceUuid = currentResourceEntity.resourceUuid
       updateResourceEntity(resourceUuid, updatedResource)
 
@@ -375,6 +370,7 @@ internal class DatabaseImpl(
     val updatedReferenceValue = "${updatedResource.resourceType.name}/${updatedResource.logicalId}"
     referringResourcesUuids.forEach { resourceUuid ->
       resourceDao.getResourceEntity(resourceUuid)?.let {
+        val iParser = FhirContext.forR4Cached().newJsonParser()
         val referringResource = iParser.parseResource(it.serializedResource) as Resource
         val updatedReferringResource =
           addUpdatedReferenceToResource(

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

@@ -21,6 +21,7 @@ import androidx.room.Insert
 import androidx.room.OnConflictStrategy
 import androidx.room.Query
 import androidx.room.Transaction
+import ca.uhn.fhir.context.FhirContext
 import ca.uhn.fhir.parser.IParser
 import ca.uhn.fhir.util.FhirTerser
 import ca.uhn.fhir.util.ResourceReferenceInfo
@@ -50,8 +51,6 @@ import timber.log.Timber
  */
 @Dao
 internal abstract class LocalChangeDao {
-
-  lateinit var iParser: IParser
   lateinit var fhirTerser: FhirTerser
 
   @Insert(onConflict = OnConflictStrategy.REPLACE)
@@ -70,7 +69,7 @@ internal abstract class LocalChangeDao {
   open suspend fun addInsert(resource: Resource, resourceUuid: UUID, timeOfLocalChange: Instant) {
     val resourceId = resource.logicalId
     val resourceType = resource.resourceType
-    val resourceString = iParser.encodeResourceToString(resource)
+    val resourceString = FhirContext.forR4Cached().newJsonParser().encodeResourceToString(resource)
 
     val localChangeEntity =
       LocalChangeEntity(
@@ -128,6 +127,7 @@ internal abstract class LocalChangeDao {
         "Unexpected DELETE when updating $resourceType/$resourceId. UPDATE failed.",
       )
     }
+    val iParser = FhirContext.forR4Cached().newJsonParser()
     val oldResource = iParser.parseResource(oldEntity.serializedResource) as Resource
     val jsonDiff = diff(iParser, oldResource, updatedResource)
     if (jsonDiff.length() == 0) {
@@ -475,6 +475,7 @@ internal abstract class LocalChangeDao {
     oldReference: String,
     updatedReference: String,
   ): LocalChangeEntity {
+    val iParser = FhirContext.forR4Cached().newJsonParser()
     return when (localChange.type) {
       LocalChangeEntity.Type.INSERT -> {
         val insertResourcePayload = iParser.parseResource(localChange.payload) as Resource