Detecting all "complex" schema changes.
Complex changes are arbitrary changes on the database schema that will require going through the 12-step generalized ALTER TABLE procedure in order to migrate the database. The current implementation does not handle FTS tables.
In addition, a "callback" field has been added to the AutoMigration annotation in preparation for switching to declaring AutoMigrations only by annotation (with an option to implement a callback for the user in case of column/table renames/deletes and/or usage of the onPostMigrate() function).
For testing, JUnit tests have been added to trigger processor errors to be thrown for "complex" schema changes at the Entity level (instead of Column level).
The follow-up CL will replace the processor errors thrown when complex schema changes are encountered at the Entity level, and implement the 12-step migration for these changes.
Test: SchemaDifferTest.kt
Bug: 74441348
Change-Id: I0b3a982b396a7541ad6279d3921b5372de6d9518
diff --git a/room/compiler/src/main/kotlin/androidx/room/processor/AutoMigrationProcessor.kt b/room/compiler/src/main/kotlin/androidx/room/processor/AutoMigrationProcessor.kt
index 00268f2..cc7781c 100644
--- a/room/compiler/src/main/kotlin/androidx/room/processor/AutoMigrationProcessor.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/processor/AutoMigrationProcessor.kt
@@ -26,6 +26,9 @@
import androidx.room.vo.AutoMigrationResult
import java.io.File
+// TODO: (b/183435544) Support downgrades in AutoMigrations.
+// TODO: (b/183007590) Use the callback in the AutoMigration annotation while end-to-end
+// testing, when column/table rename/deletes are supported
class AutoMigrationProcessor(
val context: Context,
val element: XTypeElement,
@@ -100,18 +103,18 @@
return null
}
+ // TODO: (b/183434667) Update the automigration result data object to handle complex
+ // schema changes' presence when writer code is introduced
return AutoMigrationResult(
element = element,
from = fromSchemaBundle.version,
to = toSchemaBundle.version,
- addedColumns = schemaDiff.addedColumn,
- addedTables = schemaDiff.addedTable
+ addedColumns = schemaDiff.addedColumns,
+ addedTables = schemaDiff.addedTables
)
}
- // TODO: File bug for not supporting downgrades.
- // TODO: (b/180389433) If the files don't exist the getSchemaFile() method should return
- // null and before calling process
+ // TODO: (b/180389433) Verify automigration schemas before calling the AutoMigrationProcessor
private fun getValidatedSchemaFile(version: Int): File? {
val schemaFile = File(
context.schemaOutFolder,
diff --git a/room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt b/room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
index 05a3b6a..a62aac8 100644
--- a/room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
@@ -841,8 +841,9 @@
}
fun columnWithChangedSchemaFound(columnName: String): String {
- return "Encountered column '$columnName' with a changed FieldBundle schema. This change " +
- "is not currently supported by AutoMigration."
+ return "Encountered column '$columnName' with an unsupported schema change at the column " +
+ "level (e.g. affinity change). These changes are not yet " +
+ "supported by AutoMigration."
}
fun removedOrRenamedColumnFound(columnName: String): String {
@@ -850,6 +851,12 @@
"renamed. This change is not currently supported by AutoMigration."
}
+ fun tableWithComplexChangedSchemaFound(tableName: String): String {
+ return "Encountered table '$tableName' with an unsupported schema change at the table " +
+ "level (e.g. primary key, foreign key or index change). These changes are not yet " +
+ "supported by AutoMigration."
+ }
+
fun removedOrRenamedTableFound(tableName: String): String {
return "Table '$tableName' has been either removed or " +
"renamed. This change is not currently supported by AutoMigration."
diff --git a/room/compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt b/room/compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
index a0a327d..53f1f231 100644
--- a/room/compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
@@ -17,6 +17,9 @@
package androidx.room.util
import androidx.room.migration.bundle.DatabaseBundle
+import androidx.room.migration.bundle.EntityBundle
+import androidx.room.migration.bundle.ForeignKeyBundle
+import androidx.room.migration.bundle.IndexBundle
import androidx.room.processor.ProcessorErrors
import androidx.room.vo.AutoMigrationResult
@@ -29,11 +32,12 @@
* Contains the added, changed and removed columns detected.
*/
data class SchemaDiffResult(
- val addedColumn: MutableList<AutoMigrationResult.AddedColumn>,
- val changedColumn: List<AutoMigrationResult.ChangedColumn>,
- val removedColumn: List<AutoMigrationResult.RemovedColumn>,
- val addedTable: List<AutoMigrationResult.AddedTable>,
- val removedTable: List<AutoMigrationResult.RemovedTable>
+ val addedColumns: List<AutoMigrationResult.AddedColumn>,
+ val changedColumns: List<AutoMigrationResult.ChangedColumn>,
+ val removedColumns: List<AutoMigrationResult.RemovedColumn>,
+ val addedTables: List<AutoMigrationResult.AddedTable>,
+ val complexChangedTables: List<AutoMigrationResult.ComplexChangedTable>,
+ val removedTables: List<AutoMigrationResult.RemovedTable>
)
/**
@@ -55,6 +59,7 @@
*/
fun diffSchemas(): SchemaDiffResult {
val addedTables = mutableListOf<AutoMigrationResult.AddedTable>()
+ val complexChangedTables = mutableListOf<AutoMigrationResult.ComplexChangedTable>()
val removedTables = mutableListOf<AutoMigrationResult.RemovedTable>()
val addedColumns = mutableListOf<AutoMigrationResult.AddedColumn>()
@@ -63,28 +68,32 @@
// Check going from the original version of the schema to the new version for changed and
// removed columns/tables
- fromSchemaBundle.entitiesByTableName.forEach { v1Table ->
- val v2Table = toSchemaBundle.entitiesByTableName[v1Table.key]
- if (v2Table == null) {
- removedTables.add(AutoMigrationResult.RemovedTable(v1Table.value))
+ fromSchemaBundle.entitiesByTableName.forEach { fromTable ->
+ val toTable = toSchemaBundle.entitiesByTableName[fromTable.key]
+ if (toTable == null) {
+ removedTables.add(AutoMigrationResult.RemovedTable(fromTable.value))
} else {
- val v1Columns = v1Table.value.fieldsByColumnName
- val v2Columns = v2Table.fieldsByColumnName
- v1Columns.entries.forEach { v1Column ->
- val match = v2Columns[v1Column.key]
- if (match != null && !match.isSchemaEqual(v1Column.value)) {
+ val complexChangedTable = tableContainsComplexChanges(fromTable.value, toTable)
+ if (complexChangedTable != null) {
+ complexChangedTables.add(complexChangedTable)
+ }
+ val fromColumns = fromTable.value.fieldsByColumnName
+ val toColumns = toTable.fieldsByColumnName
+ fromColumns.entries.forEach { fromColumn ->
+ val match = toColumns[fromColumn.key]
+ if (match != null && !match.isSchemaEqual(fromColumn.value)) {
+ // Any change in the field bundle schema of a column will be complex
changedColumns.add(
AutoMigrationResult.ChangedColumn(
- v1Table.key,
- v1Column.value,
- match
+ fromTable.key,
+ fromColumn.value
)
)
} else if (match == null) {
removedColumns.add(
AutoMigrationResult.RemovedColumn(
- v1Table.key,
- v1Column.value
+ fromTable.key,
+ fromColumn.value
)
)
}
@@ -94,25 +103,25 @@
// Check going from the new version of the schema to the original version for added
// tables/columns. Skip the columns with the same name as the previous loop would have
// processed them already.
- toSchemaBundle.entitiesByTableName.forEach { v2Table ->
- val v1Table = fromSchemaBundle.entitiesByTableName[v2Table.key]
- if (v1Table == null) {
- addedTables.add(AutoMigrationResult.AddedTable(v2Table.value))
+ toSchemaBundle.entitiesByTableName.forEach { toTable ->
+ val fromTable = fromSchemaBundle.entitiesByTableName[toTable.key]
+ if (fromTable == null) {
+ addedTables.add(AutoMigrationResult.AddedTable(toTable.value))
} else {
- val v2Columns = v2Table.value.fieldsByColumnName
- val v1Columns = v1Table.fieldsByColumnName
- v2Columns.entries.forEach { v2Column ->
- val match = v1Columns[v2Column.key]
+ val fromColumns = fromTable.fieldsByColumnName
+ val toColumns = toTable.value.fieldsByColumnName
+ toColumns.entries.forEach { toColumn ->
+ val match = fromColumns[toColumn.key]
if (match == null) {
- if (v2Column.value.isNonNull && v2Column.value.defaultValue == null) {
+ if (toColumn.value.isNonNull && toColumn.value.defaultValue == null) {
diffError(
- ProcessorErrors.newNotNullColumnMustHaveDefaultValue(v2Column.key)
+ ProcessorErrors.newNotNullColumnMustHaveDefaultValue(toColumn.key)
)
}
addedColumns.add(
AutoMigrationResult.AddedColumn(
- v2Table.key,
- v2Column.value
+ toTable.key,
+ toColumn.value
)
)
}
@@ -120,11 +129,13 @@
}
}
+ // TODO: (b/183007590) Remove the Processor Errors thrown when a complex change is
+ // encountered after AutoMigrationWriter supports generating the necessary migrations.
if (changedColumns.isNotEmpty()) {
changedColumns.forEach { changedColumn ->
diffError(
ProcessorErrors.columnWithChangedSchemaFound(
- changedColumn.originalFieldBundle.columnName
+ changedColumn.fieldBundle.columnName
)
)
}
@@ -140,6 +151,16 @@
}
}
+ if (complexChangedTables.isNotEmpty()) {
+ complexChangedTables.forEach { changedTable ->
+ diffError(
+ ProcessorErrors.tableWithComplexChangedSchemaFound(
+ changedTable.tableName
+ )
+ )
+ }
+ }
+
if (removedTables.isNotEmpty()) {
removedTables.forEach { removedTable ->
diffError(
@@ -151,15 +172,98 @@
}
return SchemaDiffResult(
- addedColumn = addedColumns,
- changedColumn = changedColumns,
- removedColumn = removedColumns,
- addedTable = addedTables,
- removedTable = removedTables
+ addedColumns = addedColumns,
+ changedColumns = changedColumns,
+ removedColumns = removedColumns,
+ addedTables = addedTables,
+ complexChangedTables = complexChangedTables,
+ removedTables = removedTables
)
}
+ /**
+ * Check for complex schema changes at a Table level and returns a ComplexTableChange
+ * including information on which table changes were found on, and whether foreign key or
+ * index related changes have occurred.
+ *
+ * @return null if complex schema change has not been found
+ */
+ // TODO: (b/181777611) Handle FTS tables
+ private fun tableContainsComplexChanges(
+ fromTable: EntityBundle,
+ toTable: EntityBundle
+ ): AutoMigrationResult.ComplexChangedTable? {
+ val foreignKeyChanged = !isForeignKeyBundlesListEqual(
+ fromTable.foreignKeys,
+ toTable.foreignKeys
+ )
+ val indexChanged = !isIndexBundlesListEqual(fromTable.indices, toTable.indices)
+ val primaryKeyChanged = !fromTable.primaryKey.isSchemaEqual(toTable.primaryKey)
+
+ if (primaryKeyChanged || foreignKeyChanged || indexChanged) {
+ return AutoMigrationResult.ComplexChangedTable(
+ tableName = toTable.tableName,
+ foreignKeyChanged = foreignKeyChanged,
+ indexChanged = indexChanged
+ )
+ }
+ return null
+ }
+
private fun diffError(errorMsg: String) {
throw DiffException(errorMsg)
}
+
+ /**
+ * Takes in two ForeignKeyBundle lists, attempts to find potential matches based on the columns
+ * of the Foreign Keys. Processes these potential matches by checking for schema equality.
+ *
+ * @return true if the two lists of foreign keys are equal
+ */
+ private fun isForeignKeyBundlesListEqual(
+ fromBundle: List<ForeignKeyBundle>,
+ toBundle: List<ForeignKeyBundle>
+ ): Boolean {
+ val set = fromBundle + toBundle
+ val matches = set.groupBy { it.columns }.entries
+
+ matches.forEach { (_, bundles) ->
+ if (bundles.size < 2) {
+ // A bundle was not matched at all, there must be a change between two versions
+ return false
+ }
+ val fromForeignKeyBundle = bundles[0]
+ val toForeignKeyBundle = bundles[1]
+ if (!fromForeignKeyBundle.isSchemaEqual(toForeignKeyBundle)) {
+ // A potential match for a bundle was found, but schemas did not match
+ return false
+ }
+ }
+ return true
+ }
+
+ /**
+ * Takes in two IndexBundle lists, attempts to find potential matches based on the names
+ * of the indexes. Processes these potential matches by checking for schema equality.
+ *
+ * @return true if the two lists of indexes are equal
+ */
+ private fun isIndexBundlesListEqual(
+ fromBundle: List<IndexBundle>,
+ toBundle: List<IndexBundle>
+ ): Boolean {
+ val set = fromBundle + toBundle
+ val matches = set.groupBy { it.name }.entries
+
+ matches.forEach { bundlesWithSameName ->
+ if (bundlesWithSameName.value.size < 2) {
+ // A bundle was not matched at all, there must be a change between two versions
+ return false
+ } else if (!bundlesWithSameName.value[0].isSchemaEqual(bundlesWithSameName.value[1])) {
+ // A potential match for a bundle was found, but schemas did not match
+ return false
+ }
+ }
+ return true
+ }
}
\ No newline at end of file
diff --git a/room/compiler/src/main/kotlin/androidx/room/vo/AutoMigrationResult.kt b/room/compiler/src/main/kotlin/androidx/room/vo/AutoMigrationResult.kt
index 91d4bd6..f6fcc1a 100644
--- a/room/compiler/src/main/kotlin/androidx/room/vo/AutoMigrationResult.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/vo/AutoMigrationResult.kt
@@ -49,8 +49,7 @@
*/
data class ChangedColumn(
val tableName: String,
- val originalFieldBundle: FieldBundle,
- val newFieldBundle: FieldBundle
+ val fieldBundle: FieldBundle
)
/**
@@ -69,6 +68,25 @@
data class AddedTable(val entityBundle: EntityBundle)
/**
+ * Stores the table name that contains a change in the primary key, foreign key(s) or index(es)
+ * in a newer version. Explicitly provides information on whether a foreign key change and/or
+ * an index change has occurred.
+ *
+ * As it is possible to have a table with only simple (non-complex) changes, which will be
+ * categorized as "AddedColumn" or "RemovedColumn" changes, all other
+ * changes at the table level are categorized as "complex" changes, using the category
+ * "ComplexChangedTable".
+ *
+ * At the column level, any change that is not a column add or a
+ * removal will be categorized as "ChangedColumn".
+ */
+ data class ComplexChangedTable(
+ val tableName: String,
+ val foreignKeyChanged: Boolean,
+ val indexChanged: Boolean
+ )
+
+ /**
* Stores the table that was present in the old version of a database but is not present in a
* new version of the same database, either because it was removed or renamed.
*
diff --git a/room/compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt b/room/compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
index 0dd90dc..94ed63a 100644
--- a/room/compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
+++ b/room/compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
@@ -19,6 +19,8 @@
import androidx.room.migration.bundle.DatabaseBundle
import androidx.room.migration.bundle.EntityBundle
import androidx.room.migration.bundle.FieldBundle
+import androidx.room.migration.bundle.ForeignKeyBundle
+import androidx.room.migration.bundle.IndexBundle
import androidx.room.migration.bundle.PrimaryKeyBundle
import androidx.room.migration.bundle.SchemaBundle
import androidx.room.processor.ProcessorErrors
@@ -29,12 +31,57 @@
class SchemaDifferTest {
@Test
+ fun testPrimaryKeyChanged() {
+ try {
+ SchemaDiffer(
+ fromSchemaBundle = from.database,
+ toSchemaBundle = toChangeInPrimaryKey.database
+ ).diffSchemas()
+ fail("DiffException should have been thrown.")
+ } catch (ex: DiffException) {
+ assertThat(ex.errorMessage).isEqualTo(
+ ProcessorErrors.tableWithComplexChangedSchemaFound("Song")
+ )
+ }
+ }
+
+ @Test
+ fun testForeignKeyFieldChanged() {
+ try {
+ SchemaDiffer(
+ fromSchemaBundle = from.database,
+ toSchemaBundle = toForeignKeyAdded.database
+ ).diffSchemas()
+ fail("DiffException should have been thrown.")
+ } catch (ex: DiffException) {
+ assertThat(ex.errorMessage).isEqualTo(
+ ProcessorErrors.tableWithComplexChangedSchemaFound("Song")
+ )
+ }
+ }
+
+ @Test
+ fun testComplexChangeInvolvingIndex() {
+ try {
+ SchemaDiffer(
+ fromSchemaBundle = from.database,
+ toSchemaBundle = toIndexAdded.database
+ ).diffSchemas()
+ fail("DiffException should have been thrown.")
+ } catch (ex: DiffException) {
+ assertThat(ex.errorMessage).isEqualTo(
+ ProcessorErrors.tableWithComplexChangedSchemaFound("Song")
+ )
+ }
+ }
+
+ @Test
fun testColumnAddedWithColumnInfoDefaultValue() {
val schemaDiffResult = SchemaDiffer(
fromSchemaBundle = from.database,
toSchemaBundle = toColumnAddedWithColumnInfoDefaultValue.database
).diffSchemas()
- assertThat(schemaDiffResult.addedColumn[0].fieldBundle.columnName).isEqualTo("artistId")
+ assertThat(schemaDiffResult.addedColumns[0].fieldBundle.columnName).isEqualTo("artistId")
}
@Test
@@ -58,8 +105,8 @@
fromSchemaBundle = from.database,
toSchemaBundle = toTableAddedWithColumnInfoDefaultValue.database
).diffSchemas()
- assertThat(schemaDiffResult.addedTable[0].entityBundle.tableName).isEqualTo("Artist")
- assertThat(schemaDiffResult.addedTable[1].entityBundle.tableName).isEqualTo("Album")
+ assertThat(schemaDiffResult.addedTables[0].entityBundle.tableName).isEqualTo("Artist")
+ assertThat(schemaDiffResult.addedTables[1].entityBundle.tableName).isEqualTo("Album")
}
@Test
@@ -78,7 +125,7 @@
}
@Test
- fun testColumnAffinityChanged() {
+ fun testColumnFieldBundleChanged() {
try {
SchemaDiffer(
fromSchemaBundle = from.database,
@@ -496,10 +543,74 @@
)
)
- val toTableAddedWithNoDefaultValue = SchemaBundle(
- 1,
+ private val toForeignKeyAdded = SchemaBundle(
+ 2,
DatabaseBundle(
- 1,
+ 2,
+ "",
+ listOf(
+ EntityBundle(
+ "Song",
+ "CREATE TABLE IF NOT EXISTS `Song` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, `artistId` " +
+ "INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`id`), FOREIGN KEY(`title`) " +
+ "REFERENCES `Song`(`artistId`) ON UPDATE NO ACTION ON DELETE NO " +
+ "ACTION DEFERRABLE INITIALLY DEFERRED))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "artistId",
+ "artistId",
+ "INTEGER",
+ true,
+ "0"
+ )
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ emptyList(),
+ listOf(
+ ForeignKeyBundle(
+ "Song",
+ "onDelete",
+ "onUpdate",
+ listOf("title"),
+ listOf("artistId")
+ )
+ )
+ )
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ )
+
+ val toIndexAdded = SchemaBundle(
+ 2,
+ DatabaseBundle(
+ 2,
"",
mutableListOf(
EntityBundle(
@@ -533,25 +644,62 @@
false,
mutableListOf("id")
),
- mutableListOf(),
- mutableListOf()
- ),
- EntityBundle(
- "Album",
- "CREATE TABLE IF NOT EXISTS `Album` (`id` INTEGER NOT NULL, `name` TEXT NOT " +
- "NULL, PRIMARY KEY(`id`))",
listOf(
- FieldBundle(
- "albumId",
- "albumId",
- "INTEGER",
+ IndexBundle(
+ "index1",
true,
- null
+ listOf("title"),
+ "CREATE UNIQUE INDEX IF NOT EXISTS `index1` ON `Song`" +
+ "(`title`)"
)
),
- PrimaryKeyBundle(true, listOf("id")),
- listOf(),
- listOf()
+ mutableListOf()
+ )
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ )
+
+ val toChangeInPrimaryKey = SchemaBundle(
+ 2,
+ DatabaseBundle(
+ 2,
+ "",
+ mutableListOf(
+ EntityBundle(
+ "Song",
+ "CREATE TABLE IF NOT EXISTS `Song` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`title`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ )
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("title")
+ ),
+ mutableListOf(),
+ mutableListOf()
)
),
mutableListOf(),
