Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed column type of timestamp in LocalChangeEntity #2101

Merged
merged 6 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions engine/src/main/java/com/google/android/fhir/LocalChange.kt
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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 */
Expand All @@ -28,7 +29,7 @@ data class LocalChange(
/** 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 = "",
val timestamp: Instant,
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
/** Type of local change like insert, delete, etc */
val type: Type,
/** json string with local changes */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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`"
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
)
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`)"
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.google.android.fhir.sync.Request
import com.google.android.fhir.sync.UrlRequest
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
Expand Down Expand Up @@ -168,7 +169,8 @@ object TestFhirEngineImpl : FhirEngine {
resourceId = id,
payload = "{}",
token = LocalChangeToken(listOf()),
type = LocalChange.Type.INSERT
type = LocalChange.Type.INSERT,
timestamp = Instant.now()
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -21,6 +21,7 @@ import ca.uhn.fhir.context.FhirVersionEnum
import com.google.android.fhir.LocalChange
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
Expand Down Expand Up @@ -53,7 +54,8 @@ class LocalChangeUtilsTest : TestCase() {
}
)
}
)
),
timestamp = Instant.now()
)

val localChange = localChangeEntity.toLocalChange()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ class FhirEngineImplTest {
filter(
LOCAL_LAST_UPDATED_PARAM,
{
value = of(DateTimeType(localChangeTimestamp))
value = of(DateTimeType(Date.from(localChangeTimestamp)))
prefix = ParamPrefixEnum.EQUAL
}
)
Expand Down Expand Up @@ -609,14 +609,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)
.isGreaterThan(DateTimeType(localChangeTimestampWhenCreated).value)
assertThat(DateTimeType(Date.from(localChangeTimestampWhenUpdated)).value)
.isGreaterThan(DateTimeType(Date.from(localChangeTimestampWhenCreated)).value)
assertThat(result).isNotEmpty()
assertThat(result.map { it.logicalId }).containsExactly("patient-id-update").inOrder()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -25,6 +25,7 @@ import com.google.android.fhir.sync.UploadResult
import com.google.android.fhir.testing.BundleDataSource
import com.google.common.truth.Truth.assertThat
import java.net.ConnectException
import java.time.Instant
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import org.hl7.fhir.r4.model.Bundle
Expand Down Expand Up @@ -133,7 +134,8 @@ class BundleUploaderTest {
}
)
}
)
),
timestamp = Instant.now()
)
.toLocalChange()
.apply { LocalChangeToken(listOf(1)) }
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -19,6 +19,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.common.truth.Truth.assertThat
import java.time.Instant
import org.hl7.fhir.r4.model.ResourceType
import org.junit.Test
import org.junit.runner.RunWith
Expand All @@ -36,35 +37,40 @@ class LocalChangesPaginatorTest {
"patient-001",
type = LocalChange.Type.INSERT,
payload = "{}",
token = LocalChangeToken(listOf(1L))
token = LocalChangeToken(listOf(1L)),
timestamp = Instant.now()
),
LocalChange(
ResourceType.Patient.name,
"patient-002",
type = LocalChange.Type.INSERT,
payload = "{}",
token = LocalChangeToken(listOf(2L))
token = LocalChangeToken(listOf(2L)),
timestamp = Instant.now()
),
LocalChange(
ResourceType.Patient.name,
"patient-003",
type = LocalChange.Type.INSERT,
payload = "{}",
token = LocalChangeToken(listOf(3L))
token = LocalChangeToken(listOf(3L)),
timestamp = Instant.now()
),
LocalChange(
ResourceType.Patient.name,
"patient-004",
type = LocalChange.Type.INSERT,
payload = "{}",
token = LocalChangeToken(listOf(4L))
token = LocalChangeToken(listOf(4L)),
timestamp = Instant.now()
),
LocalChange(
ResourceType.Patient.name,
"patient-005",
type = LocalChange.Type.INSERT,
payload = "{}",
token = LocalChangeToken(listOf(5L))
token = LocalChangeToken(listOf(5L)),
timestamp = Instant.now()
)
)

Expand Down
Loading
Loading