Changed column type of timestamp in LocalChangeEntity (#2101)

* Changed column type of timestamp in LocalChangeEntity * Fixed compilation issues * Updated kdoc * Update engine/src/main/java/com/google/android/fhir/LocalChange.kt Co-authored-by: Omar Ismail <[email protected]> --------- Co-authored-by: Omar Ismail <[email protected]>
google · Jul 31, 2023 · a26dcbe · a26dcbe
1 parent 65ddc7f
commit a26dcbe
Show file tree

Hide file tree

Showing 12 changed files with 1,066 additions and 36 deletions.
diff --git a/engine/schemas/com.google.android.fhir.db.impl.ResourceDatabase/6.json b/engine/schemas/com.google.android.fhir.db.impl.ResourceDatabase/6.json
diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/ResourceDatabaseMigrationTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/ResourceDatabaseMigrationTest.kt
@@ -21,6 +21,8 @@ import androidx.room.testing.MigrationTestHelper
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
 import ca.uhn.fhir.context.FhirContext
+import com.google.android.fhir.db.impl.entities.LocalChangeEntity
+import com.google.android.fhir.toTimeZoneString
 import com.google.common.truth.Truth.assertThat
 import java.io.IOException
 import java.util.Date
@@ -172,13 +174,64 @@ class ResourceDatabaseMigrationTest {
     assertThat(retrievedTask).isEqualTo(bedNetTask)
   }
 
+  @Test
+  fun migrate5To6_should_execute_with_no_exception(): Unit = runBlocking {
+    val taskId = "bed-net-001"
+    val bedNetTask: String =
+      Task()
+        .apply {
+          id = taskId
+          description = "Issue bed net"
+          meta.lastUpdated = Date()
+        }
+        .let { iParser.encodeResourceToString(it) }
+
+    // Since the migration here is to change the column type of LocalChangeEntity.timestamp from
+    // string to Instant (integer). We are making sure that the data is migrated properly.
+    helper.createDatabase(DB_NAME, 5).apply {
+      val date = Date()
+      execSQL(
+        "INSERT INTO ResourceEntity (resourceUuid, resourceType, resourceId, serializedResource, lastUpdatedLocal) VALUES ('bed-net-001', 'Task', 'bed-net-001', '$bedNetTask', '${DbTypeConverters.instantToLong(date.toInstant())}' );"
+      )
+
+      execSQL(
+        "INSERT INTO LocalChangeEntity (resourceType, resourceId, timestamp, type, payload) VALUES ('Task', 'bed-net-001', '${date.toTimeZoneString()}', '${DbTypeConverters.localChangeTypeToInt(LocalChangeEntity.Type.INSERT)}', '$bedNetTask'  );"
+      )
+      close()
+    }
+
+    helper.runMigrationsAndValidate(DB_NAME, 6, true, MIGRATION_5_6)
+
+    val retrievedTask: String?
+    val localChangeEntityTimeStamp: Long
+    val resourceEntityLastUpdatedLocal: Long
+    getMigratedRoomDatabase().apply {
+      retrievedTask = this.resourceDao().getResource(taskId, ResourceType.Task)
+      resourceEntityLastUpdatedLocal =
+        query("Select lastUpdatedLocal from ResourceEntity", null).let {
+          it.moveToFirst()
+          it.getLong(0)
+        }
+      localChangeEntityTimeStamp =
+        query("Select timestamp from LocalChangeEntity", null).let {
+          it.moveToFirst()
+          it.getLong(0)
+        }
+
+      openHelper.writableDatabase.close()
+    }
+
+    assertThat(retrievedTask).isEqualTo(bedNetTask)
+    assertThat(localChangeEntityTimeStamp).isEqualTo(resourceEntityLastUpdatedLocal)
+  }
+
   private fun getMigratedRoomDatabase(): ResourceDatabase =
     Room.databaseBuilder(
         InstrumentationRegistry.getInstrumentation().targetContext,
         ResourceDatabase::class.java,
         DB_NAME
       )
-      .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5)
+      .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6)
       .build()
 
   companion object {

diff --git a/engine/src/main/java/com/google/android/fhir/LocalChange.kt b/engine/src/main/java/com/google/android/fhir/LocalChange.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Google LLC
+ * Copyright 2022-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,6 +17,7 @@
 package com.google.android.fhir
 
 import com.google.android.fhir.db.impl.dao.LocalChangeToken
+import java.time.Instant
 import org.hl7.fhir.r4.model.Resource
 
 /** Data class for squashed local changes for resource */
@@ -27,8 +28,8 @@ data class LocalChange(
   val resourceId: String,
   /** This is the id of the version of the resource that this local change is based of */
   val versionId: String? = null,
-  /** last updated timestamp on server when this local changes are sync with server */
-  val timestamp: String = "",
+  /** 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 */

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
@@ -92,7 +92,7 @@ internal class DatabaseImpl(
             }
           }
 
-          addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5)
+          addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6)
         }
         .build()
   }

diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt b/engine/src/main/java/com/google/android/fhir/db/impl/ResourceDatabase.kt
@@ -50,7 +50,7 @@ import com.google.android.fhir.db.impl.entities.UriIndexEntity
       LocalChangeEntity::class,
       PositionIndexEntity::class
     ],
-  version = 5,
+  version = 6,
   exportSchema = true
 )
 @TypeConverters(DbTypeConverters::class)
@@ -108,3 +108,21 @@ val MIGRATION_4_5 =
       )
     }
   }
+
+/** Changes column type of [LocalChangeEntity.timestamp] from [String] to [java.time.Instant]. */
+val MIGRATION_5_6 =
+  object : Migration(/* startVersion = */ 5, /* endVersion = */ 6) {
+    override fun migrate(database: SupportSQLiteDatabase) {
+      database.execSQL(
+        "CREATE TABLE IF NOT EXISTS `_new_LocalChangeEntity` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `resourceType` TEXT NOT NULL, `resourceId` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, `type` INTEGER NOT NULL, `payload` TEXT NOT NULL, `versionId` TEXT)"
+      )
+      database.execSQL(
+        "INSERT INTO `_new_LocalChangeEntity` (`id`,`resourceType`,`resourceId`,`timestamp`,`type`,`payload`,`versionId`) SELECT `id`,`resourceType`,`resourceId`, strftime('%s', `timestamp`) ||  substr(strftime('%f', `timestamp`), 4),`type`,`payload`,`versionId` FROM `LocalChangeEntity`"
+      )
+      database.execSQL("DROP TABLE `LocalChangeEntity`")
+      database.execSQL("ALTER TABLE `_new_LocalChangeEntity` RENAME TO `LocalChangeEntity`")
+      database.execSQL(
+        "CREATE INDEX IF NOT EXISTS `index_LocalChangeEntity_resourceType_resourceId` ON `LocalChangeEntity` (`resourceType`, `resourceId`)"
+      )
+    }
+  }
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
@@ -25,7 +25,6 @@ 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
 import com.google.android.fhir.logicalId
-import com.google.android.fhir.toTimeZoneString
 import com.google.android.fhir.versionId
 import java.time.Instant
 import java.util.Date
@@ -50,15 +49,14 @@ internal abstract class LocalChangeDao {
   open suspend fun addInsert(resource: Resource, timeOfLocalChange: Instant) {
     val resourceId = resource.logicalId
     val resourceType = resource.resourceType
-    val timestamp = Date.from(timeOfLocalChange).toTimeZoneString()
     val resourceString = iParser.encodeResourceToString(resource)
 
     addLocalChange(
       LocalChangeEntity(
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
-        timestamp = timestamp,
+        timestamp = timeOfLocalChange,
         type = Type.INSERT,
         payload = resourceString,
         versionId = resource.versionId
@@ -69,7 +67,6 @@ internal abstract class LocalChangeDao {
   suspend fun addUpdate(oldEntity: ResourceEntity, resource: Resource, timeOfLocalChange: Instant) {
     val resourceId = resource.logicalId
     val resourceType = resource.resourceType
-    val timestamp = Date.from(timeOfLocalChange).toTimeZoneString()
 
     if (!localChangeIsEmpty(resourceId, resourceType) &&
         lastChangeType(resourceId, resourceType)!! == Type.DELETE
@@ -96,7 +93,7 @@ internal abstract class LocalChangeDao {
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
-        timestamp = timestamp,
+        timestamp = timeOfLocalChange,
         type = Type.UPDATE,
         payload = jsonDiff.toString(),
         versionId = oldEntity.versionId
@@ -105,13 +102,12 @@ internal abstract class LocalChangeDao {
   }
 
   suspend fun addDelete(resourceId: String, resourceType: ResourceType, remoteVersionId: String?) {
-    val timestamp = Date().toTimeZoneString()
     addLocalChange(
       LocalChangeEntity(
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
-        timestamp = timestamp,
+        timestamp = Date().toInstant(),
         type = Type.DELETE,
         payload = "",
         versionId = remoteVersionId

diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/entities/LocalChangeEntity.kt b/engine/src/main/java/com/google/android/fhir/db/impl/entities/LocalChangeEntity.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Google LLC
+ * Copyright 2022-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.
@@ -19,6 +19,7 @@ package com.google.android.fhir.db.impl.entities
 import androidx.room.Entity
 import androidx.room.Index
 import androidx.room.PrimaryKey
+import java.time.Instant
 
 /**
  * When a local change to a resource happens, the lastUpdated timestamp in [ResourceEntity] is
@@ -55,7 +56,7 @@ internal data class LocalChangeEntity(
   @PrimaryKey(autoGenerate = true) val id: Long,
   val resourceType: String,
   val resourceId: String,
-  val timestamp: String = "",
+  val timestamp: Instant,
   val type: Type,
   val payload: String,
   val versionId: String? = null

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
@@ -34,6 +34,7 @@ import com.google.android.fhir.sync.UploadRequest
 import com.google.android.fhir.sync.UrlDownloadRequest
 import com.google.common.truth.Truth.assertThat
 import java.net.SocketTimeoutException
+import java.time.Instant
 import java.time.OffsetDateTime
 import java.util.Date
 import java.util.LinkedList
@@ -173,7 +174,8 @@ object TestFhirEngineImpl : FhirEngine {
         resourceId = id,
         payload = "{}",
         token = LocalChangeToken(listOf()),
-        type = LocalChange.Type.INSERT
+        type = LocalChange.Type.INSERT,
+        timestamp = Instant.now()
       )
     )
   }

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
@@ -26,6 +26,7 @@ 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
@@ -62,7 +63,8 @@ class LocalChangeUtilsTest : TestCase() {
                 }
               )
             }
-          )
+          ),
+        timestamp = Instant.now()
       )
 
     val localChange = localChangeEntity.toLocalChange()
@@ -158,7 +160,8 @@ class LocalChangeUtilsTest : TestCase() {
       type = LocalChange.Type.UPDATE,
       payload = jsonDiff.toString(),
       versionId = oldEntity.versionId,
-      token = LocalChangeToken(listOf(currentChangeId + 1))
+      token = LocalChangeToken(listOf(currentChangeId + 1)),
+      timestamp = Instant.now()
     )
   }
 
@@ -169,7 +172,8 @@ class LocalChangeUtilsTest : TestCase() {
       type = LocalChange.Type.INSERT,
       payload = jsonParser.encodeResourceToString(entity),
       versionId = entity.versionId,
-      token = LocalChangeToken(listOf(1L))
+      token = LocalChangeToken(listOf(1L)),
+      timestamp = Instant.now()
     )
   }
 
@@ -183,7 +187,8 @@ class LocalChangeUtilsTest : TestCase() {
       type = LocalChange.Type.DELETE,
       payload = "",
       versionId = entity.versionId,
-      token = LocalChangeToken(listOf(currentChangeId + 1))
+      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
@@ -571,7 +571,7 @@ class FhirEngineImplTest {
         filter(
           LOCAL_LAST_UPDATED_PARAM,
           {
-            value = of(DateTimeType(localChangeTimestamp))
+            value = of(DateTimeType(Date.from(localChangeTimestamp)))
             prefix = ParamPrefixEnum.EQUAL
           }
         )
@@ -607,14 +607,14 @@ class FhirEngineImplTest {
           filter(
             LOCAL_LAST_UPDATED_PARAM,
             {
-              value = of(DateTimeType(localChangeTimestampWhenUpdated))
+              value = of(DateTimeType(Date.from(localChangeTimestampWhenUpdated)))
               prefix = ParamPrefixEnum.EQUAL
             }
           )
         }
 
-      assertThat(DateTimeType(localChangeTimestampWhenUpdated).value)
-        .isAtLeast(DateTimeType(localChangeTimestampWhenCreated).value)
+      assertThat(DateTimeType(Date.from(localChangeTimestampWhenUpdated)).value)
+        .isAtLeast(DateTimeType(Date.from(localChangeTimestampWhenCreated)).value)
       assertThat(result).isNotEmpty()
       assertThat(result.map { it.logicalId }).containsExactly("patient-id-update").inOrder()
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -92,7 +92,7 @@ internal class DatabaseImpl( @@
                 }
               }
-              addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5)
+              addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6)
             }
             .build()
       }
@@ Expand Down @@