Merge "Detecting all "complex" schema changes." into androidx-main
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 a07f3e2..d8818644 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,
@@ -101,18 +104,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(),
