Merge "Don't key auto-migration added columns by name." into androidx-main
diff --git a/room/room-compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt b/room/room-compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
index 234af6d..867bca9 100644
--- a/room/room-compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
+++ b/room/room-compiler/src/main/kotlin/androidx/room/util/SchemaDiffer.kt
@@ -44,7 +44,7 @@
* Contains the changes detected between the two schema versions provided.
*/
data class SchemaDiffResult(
- val addedColumns: LinkedHashMap<String, AutoMigration.AddedColumn>,
+ val addedColumns: List<AutoMigration.AddedColumn>,
val deletedColumns: List<AutoMigration.DeletedColumn>,
val addedTables: Set<AutoMigration.AddedTable>,
val renamedTables: Map<String, String>,
@@ -93,9 +93,9 @@
mutableMapOf<String, AutoMigration.ComplexChangedTable>()
private val deletedTables = deleteTableEntries.map { it.deletedTableName }.toSet()
- // Map of columns that have been added in the database, keyed by the column name, note that
- // the table these columns have been added to will not contain any complex schema changes.
- private val addedColumns = linkedMapOf<String, AutoMigration.AddedColumn>()
+ // Map of columns that have been added in the database, note that the table these columns
+ // have been added to will not contain any complex schema changes.
+ private val addedColumns = mutableListOf<AutoMigration.AddedColumn>()
private val deletedColumns = deleteColumnEntries
/**
@@ -375,8 +375,8 @@
return true
}
// Check if the to table or the from table is an FTS table while the other is not.
- if (fromTable is FtsEntityBundle && !(toTable is FtsEntityBundle) ||
- toTable is FtsEntityBundle && !(fromTable is FtsEntityBundle)
+ if (fromTable is FtsEntityBundle && toTable !is FtsEntityBundle ||
+ toTable is FtsEntityBundle && fromTable !is FtsEntityBundle
) {
return true
}
@@ -557,11 +557,12 @@
// need to account for it as the table will be recreated already with the new
// table.
if (!complexChangedTables.containsKey(fromTable.tableName)) {
- addedColumns[toColumn.columnName] =
+ addedColumns.add(
AutoMigration.AddedColumn(
toTable.tableName,
toColumn
)
+ )
}
}
}
diff --git a/room/room-compiler/src/main/kotlin/androidx/room/writer/AutoMigrationWriter.kt b/room/room-compiler/src/main/kotlin/androidx/room/writer/AutoMigrationWriter.kt
index f30bcb2..290606e 100644
--- a/room/room-compiler/src/main/kotlin/androidx/room/writer/AutoMigrationWriter.kt
+++ b/room/room-compiler/src/main/kotlin/androidx/room/writer/AutoMigrationWriter.kt
@@ -427,11 +427,11 @@
addedColumns.forEach {
val addNewColumnSql = buildString {
append(
- "ALTER TABLE `${it.value.tableName}` ADD COLUMN `${it.key}` " +
- "${it.value.fieldBundle.affinity} "
+ "ALTER TABLE `${it.tableName}` ADD COLUMN `${it.fieldBundle.columnName}` " +
+ "${it.fieldBundle.affinity} "
)
- if (it.value.fieldBundle.isNonNull) {
- append("NOT NULL DEFAULT ${it.value.fieldBundle.defaultValue}")
+ if (it.fieldBundle.isNonNull) {
+ append("NOT NULL DEFAULT ${it.fieldBundle.defaultValue}")
} else {
append("DEFAULT NULL")
}
diff --git a/room/room-compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt b/room/room-compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
index b5be2a8..bd6a7a3 100644
--- a/room/room-compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
+++ b/room/room-compiler/src/test/kotlin/androidx/room/util/SchemaDifferTest.kt
@@ -86,7 +86,7 @@
renameTableEntries = listOf(),
deleteTableEntries = listOf()
).diffSchemas()
- assertThat(schemaDiffResult.addedColumns["artistId"]?.fieldBundle?.columnName)
+ assertThat(schemaDiffResult.addedColumns.single().fieldBundle.columnName)
.isEqualTo("artistId")
}
@@ -101,9 +101,10 @@
renameTableEntries = listOf(),
deleteTableEntries = listOf()
).diffSchemas()
- assertThat(schemaDiffResult.addedColumns["recordLabelId"]?.fieldBundle?.columnName)
+ assertThat(schemaDiffResult.addedColumns).hasSize(2)
+ assertThat(schemaDiffResult.addedColumns[0].fieldBundle.columnName)
.isEqualTo("recordLabelId")
- assertThat(schemaDiffResult.addedColumns["artistId"]?.fieldBundle?.columnName)
+ assertThat(schemaDiffResult.addedColumns[1].fieldBundle.columnName)
.isEqualTo("artistId")
}
@@ -143,6 +144,28 @@
}
@Test
+ fun testColumnsAddedWithSameName() {
+ val schemaDiffResult = SchemaDiffer(
+ fromSchemaBundle = from.database,
+ toSchemaBundle = toColumnsAddedWithSameName.database,
+ className = "MyAutoMigration",
+ renameColumnEntries = listOf(),
+ deleteColumnEntries = listOf(),
+ renameTableEntries = listOf(),
+ deleteTableEntries = listOf()
+ ).diffSchemas()
+ assertThat(schemaDiffResult.addedColumns).hasSize(2)
+ schemaDiffResult.addedColumns[0].let { addedColumn ->
+ assertThat(addedColumn.tableName).isEqualTo("Artist")
+ assertThat(addedColumn.fieldBundle.columnName).isEqualTo("newColumn")
+ }
+ schemaDiffResult.addedColumns[1].let { addedColumn ->
+ assertThat(addedColumn.tableName).isEqualTo("Song")
+ assertThat(addedColumn.fieldBundle.columnName).isEqualTo("newColumn")
+ }
+ }
+
+ @Test
fun testColumnRenamed() {
try {
SchemaDiffer(
@@ -222,7 +245,7 @@
}
}
- val from = SchemaBundle(
+ private val from = SchemaBundle(
1,
DatabaseBundle(
1,
@@ -302,8 +325,9 @@
)
)
- /** Valid "to" Schemas */
- val toTableRenamed = SchemaBundle(
+ //region Valid "to" Schemas
+
+ private val toTableRenamed = SchemaBundle(
2,
DatabaseBundle(
2,
@@ -383,7 +407,7 @@
)
)
- val toTableDeleted = SchemaBundle(
+ private val toTableDeleted = SchemaBundle(
2,
DatabaseBundle(
2,
@@ -429,7 +453,7 @@
)
)
- val toColumnAddedWithColumnInfoDefaultValue = SchemaBundle(
+ private val toColumnAddedWithColumnInfoDefaultValue = SchemaBundle(
2,
DatabaseBundle(
2,
@@ -517,182 +541,9 @@
)
)
- /** Invalid "to" Schemas (These are expected to throw an error.) */
-
- /**
- * The length column is removed from the first version. No other changes made.
- *
- */
- private val toColumnRemoved = SchemaBundle(
- 2,
- DatabaseBundle(
- 2,
- "",
- listOf(
- EntityBundle(
- "Song",
- "CREATE TABLE IF NOT EXISTS `Song` (`id` INTEGER NOT NULL, " +
- "`title` TEXT NOT NULL, PRIMARY KEY(`id`))",
- listOf(
- FieldBundle(
- "id",
- "id",
- "INTEGER",
- true,
- "1"
- ),
- FieldBundle(
- "title",
- "title",
- "TEXT",
- true,
- ""
- ),
- ),
- PrimaryKeyBundle(
- false,
- mutableListOf("id")
- ),
- emptyList(),
- emptyList()
- ),
- EntityBundle(
- "Artist",
- "CREATE TABLE IF NOT EXISTS `Artist` (`id` INTEGER NOT NULL, " +
- "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`id`))",
- listOf(
- FieldBundle(
- "id",
- "id",
- "INTEGER",
- true,
- "1"
- ),
- FieldBundle(
- "title",
- "title",
- "TEXT",
- true,
- ""
- ),
- FieldBundle(
- "length",
- "length",
- "INTEGER",
- true,
- "1"
- )
- ),
- PrimaryKeyBundle(
- false,
- mutableListOf("id")
- ),
- mutableListOf(),
- mutableListOf()
- )
- ),
- mutableListOf(),
- mutableListOf()
- )
- )
-
- /**
- * If the user declared the default value in the SQL statement and not used a @ColumnInfo,
- * Room will put null for that default value in the exported schema. In this case we
- * can't migrate.
- */
- private val toColumnAddedWithNoDefaultValue = SchemaBundle(
- 2,
- DatabaseBundle(
- 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, PRIMARY KEY(`id`))",
- listOf(
- FieldBundle(
- "id",
- "id",
- "INTEGER",
- true,
- "1"
- ),
- FieldBundle(
- "title",
- "title",
- "TEXT",
- true,
- ""
- ),
- FieldBundle(
- "length",
- "length",
- "INTEGER",
- true,
- "1"
- ),
- FieldBundle(
- "artistId",
- "artistId",
- "INTEGER",
- true,
- null
- )
- ),
- PrimaryKeyBundle(
- false,
- mutableListOf("id")
- ),
- emptyList(),
- emptyList()
- ),
- EntityBundle(
- "Artist",
- "CREATE TABLE IF NOT EXISTS `Artist` (`id` INTEGER NOT NULL, " +
- "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`id`))",
- listOf(
- FieldBundle(
- "id",
- "id",
- "INTEGER",
- true,
- "1"
- ),
- FieldBundle(
- "title",
- "title",
- "TEXT",
- true,
- ""
- ),
- FieldBundle(
- "length",
- "length",
- "INTEGER",
- true,
- "1"
- )
- ),
- PrimaryKeyBundle(
- false,
- mutableListOf("id")
- ),
- mutableListOf(),
- mutableListOf()
- )
- ),
- mutableListOf(),
- mutableListOf()
- )
- )
-
/**
* Adding multiple columns, preserving the order in which they have been added.
*/
-
private val toColumnsAddedInOrder = SchemaBundle(
2,
DatabaseBundle(
@@ -773,7 +624,105 @@
"INTEGER",
true,
"1"
- )
+ ),
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ )
+
+ /**
+ * Adding multiple columns, preserving the order in which they have been added.
+ */
+ private val toColumnsAddedWithSameName = SchemaBundle(
+ 2,
+ DatabaseBundle(
+ 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, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "newColumn",
+ "newColumn",
+ "INTEGER",
+ true,
+ "0"
+ ),
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ emptyList(),
+ emptyList()
+ ),
+ EntityBundle(
+ "Artist",
+ "CREATE TABLE IF NOT EXISTS `Artist` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "newColumn",
+ "newColumn",
+ "INTEGER",
+ true,
+ "0"
+ ),
),
PrimaryKeyBundle(
false,
@@ -791,8 +740,6 @@
/**
* Renaming the length column to duration.
*/
- // TODO: We currently do not support column renames as we can't detect rename or deletion
- // yet.
private val toColumnRenamed = SchemaBundle(
2,
DatabaseBundle(
@@ -1322,4 +1269,179 @@
mutableListOf()
)
)
+
+ //endregion
+
+ //region Invalid "to" Schemas (These are expected to throw an error.)
+
+ /**
+ * The length column is removed from the first version. No other changes made.
+ */
+ private val toColumnRemoved = SchemaBundle(
+ 2,
+ DatabaseBundle(
+ 2,
+ "",
+ listOf(
+ EntityBundle(
+ "Song",
+ "CREATE TABLE IF NOT EXISTS `Song` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ emptyList(),
+ emptyList()
+ ),
+ EntityBundle(
+ "Artist",
+ "CREATE TABLE IF NOT EXISTS `Artist` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ )
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ )
+
+ /**
+ * If the user declared the default value in the SQL statement and not used a @ColumnInfo,
+ * Room will put null for that default value in the exported schema. In this case we
+ * can't migrate.
+ */
+ private val toColumnAddedWithNoDefaultValue = SchemaBundle(
+ 2,
+ DatabaseBundle(
+ 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, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "artistId",
+ "artistId",
+ "INTEGER",
+ true,
+ null
+ )
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ emptyList(),
+ emptyList()
+ ),
+ EntityBundle(
+ "Artist",
+ "CREATE TABLE IF NOT EXISTS `Artist` (`id` INTEGER NOT NULL, " +
+ "`title` TEXT NOT NULL, `length` INTEGER NOT NULL, PRIMARY KEY(`id`))",
+ listOf(
+ FieldBundle(
+ "id",
+ "id",
+ "INTEGER",
+ true,
+ "1"
+ ),
+ FieldBundle(
+ "title",
+ "title",
+ "TEXT",
+ true,
+ ""
+ ),
+ FieldBundle(
+ "length",
+ "length",
+ "INTEGER",
+ true,
+ "1"
+ )
+ ),
+ PrimaryKeyBundle(
+ false,
+ mutableListOf("id")
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ ),
+ mutableListOf(),
+ mutableListOf()
+ )
+ )
+
+ //endregion
}
\ No newline at end of file
diff --git a/room/room-compiler/src/test/kotlin/androidx/room/writer/AutoMigrationWriterTest.kt b/room/room-compiler/src/test/kotlin/androidx/room/writer/AutoMigrationWriterTest.kt
index 0d0afec..28407f2 100644
--- a/room/room-compiler/src/test/kotlin/androidx/room/writer/AutoMigrationWriterTest.kt
+++ b/room/room-compiler/src/test/kotlin/androidx/room/writer/AutoMigrationWriterTest.kt
@@ -57,18 +57,15 @@
from = 1,
to = 2,
schemaDiff = SchemaDiffResult(
- addedColumns = linkedMapOf(
- Pair(
- "artistId",
- AutoMigration.AddedColumn(
- "Song",
- FieldBundle(
- "artistId",
- "artistId",
- "INTEGER",
- false,
- ""
- )
+ addedColumns = listOf(
+ AutoMigration.AddedColumn(
+ "Song",
+ FieldBundle(
+ "artistId",
+ "artistId",
+ "INTEGER",
+ false,
+ ""
)
)
),
@@ -118,18 +115,15 @@
from = 1,
to = 2,
schemaDiff = SchemaDiffResult(
- addedColumns = linkedMapOf(
- Pair(
- "artistId",
- AutoMigration.AddedColumn(
- "Song",
- FieldBundle(
- "artistId",
- "artistId",
- "INTEGER",
- false,
- ""
- )
+ addedColumns = listOf(
+ AutoMigration.AddedColumn(
+ "Song",
+ FieldBundle(
+ "artistId",
+ "artistId",
+ "INTEGER",
+ false,
+ ""
)
)
),
