Resolving bug where defaultValue with parenthesis fails onValidateSchema.
Test: MigrationTest.java
Bug: 182284899
Change-Id: I845abfa898cfc579e0def75c901ca234999f059f
diff --git a/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.MigrationDb/13.json b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.MigrationDb/13.json
new file mode 100644
index 0000000..ede0c4f
--- /dev/null
+++ b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.MigrationDb/13.json
@@ -0,0 +1,153 @@
+{
+ "formatVersion": 1,
+ "database": {
+ "version": 13,
+ "identityHash": "243330983fa2a2c935ba4427cbd2238b",
+ "entities": [
+ {
+ "tableName": "Entity1",
+ "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER NOT NULL, `name` TEXT, `addedInV10` INTEGER NOT NULL DEFAULT 0, `added1InV13` INTEGER NOT NULL DEFAULT (0), PRIMARY KEY(`id`))",
+ "fields": [
+ {
+ "fieldPath": "id",
+ "columnName": "id",
+ "affinity": "INTEGER",
+ "notNull": true
+ },
+ {
+ "fieldPath": "name",
+ "columnName": "name",
+ "affinity": "TEXT",
+ "notNull": false
+ },
+ {
+ "fieldPath": "addedInV10",
+ "columnName": "addedInV10",
+ "affinity": "INTEGER",
+ "notNull": true,
+ "defaultValue": "0"
+ },
+ {
+ "fieldPath": "added1InV13",
+ "columnName": "added1InV13",
+ "affinity": "INTEGER",
+ "notNull": true,
+ "defaultValue": "(0)"
+ }
+ ],
+ "primaryKey": {
+ "columnNames": [
+ "id"
+ ],
+ "autoGenerate": false
+ },
+ "indices": [
+ {
+ "name": "index_Entity1_name",
+ "unique": true,
+ "columnNames": [
+ "name"
+ ],
+ "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_Entity1_name` ON `${TABLE_NAME}` (`name`)"
+ },
+ {
+ "name": "index_Entity1_addedInV10",
+ "unique": false,
+ "columnNames": [
+ "addedInV10"
+ ],
+ "createSql": "CREATE INDEX IF NOT EXISTS `index_Entity1_addedInV10` ON `${TABLE_NAME}` (`addedInV10`)"
+ }
+ ],
+ "foreignKeys": []
+ },
+ {
+ "tableName": "Entity2",
+ "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `addedInV3` TEXT, `name` TEXT DEFAULT 'Unknown', `addedInV9` TEXT)",
+ "fields": [
+ {
+ "fieldPath": "id",
+ "columnName": "id",
+ "affinity": "INTEGER",
+ "notNull": true
+ },
+ {
+ "fieldPath": "addedInV3",
+ "columnName": "addedInV3",
+ "affinity": "TEXT",
+ "notNull": false
+ },
+ {
+ "fieldPath": "name",
+ "columnName": "name",
+ "affinity": "TEXT",
+ "notNull": false,
+ "defaultValue": "'Unknown'"
+ },
+ {
+ "fieldPath": "addedInV9",
+ "columnName": "addedInV9",
+ "affinity": "TEXT",
+ "notNull": false
+ }
+ ],
+ "primaryKey": {
+ "columnNames": [
+ "id"
+ ],
+ "autoGenerate": true
+ },
+ "indices": [],
+ "foreignKeys": []
+ },
+ {
+ "tableName": "Entity4",
+ "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER NOT NULL, `name` TEXT COLLATE NOCASE, PRIMARY KEY(`id`), FOREIGN KEY(`name`) REFERENCES `Entity1`(`name`) ON UPDATE NO ACTION ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED)",
+ "fields": [
+ {
+ "fieldPath": "id",
+ "columnName": "id",
+ "affinity": "INTEGER",
+ "notNull": true
+ },
+ {
+ "fieldPath": "name",
+ "columnName": "name",
+ "affinity": "TEXT",
+ "notNull": false
+ }
+ ],
+ "primaryKey": {
+ "columnNames": [
+ "id"
+ ],
+ "autoGenerate": false
+ },
+ "indices": [],
+ "foreignKeys": [
+ {
+ "table": "Entity1",
+ "onDelete": "NO ACTION",
+ "onUpdate": "NO ACTION",
+ "columns": [
+ "name"
+ ],
+ "referencedColumns": [
+ "name"
+ ]
+ }
+ ]
+ }
+ ],
+ "views": [
+ {
+ "viewName": "View1",
+ "createSql": "CREATE VIEW `${VIEW_NAME}` AS SELECT Entity4.id, Entity4.name, Entity1.id AS entity1Id FROM Entity4 INNER JOIN Entity1 ON Entity4.name = Entity1.name"
+ }
+ ],
+ "setupQueries": [
+ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
+ "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '243330983fa2a2c935ba4427cbd2238b')"
+ ]
+ }
+}
\ No newline at end of file
diff --git a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationDb.java b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationDb.java
index 45ed433..6dce4f2 100644
--- a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationDb.java
+++ b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationDb.java
@@ -42,7 +42,7 @@
MigrationDb.Entity4.class},
views = {MigrationDb.View1.class})
public abstract class MigrationDb extends RoomDatabase {
- static final int LATEST_VERSION = 12;
+ static final int LATEST_VERSION = 13;
static final int MAX_VERSION = 1000;
abstract MigrationDao dao();
@Entity(indices = {
@@ -55,6 +55,8 @@
public String name;
@ColumnInfo(defaultValue = "0")
public int addedInV10;
+ @ColumnInfo(defaultValue = "(0)")
+ public int added1InV13;
}
@Entity
diff --git a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationTest.java b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationTest.java
index a9d78c8..6b90459 100644
--- a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationTest.java
+++ b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/MigrationTest.java
@@ -296,6 +296,29 @@
}
@Test
+ public void validateDefaultValueWithSurroundingParenthesis() throws IOException {
+ SupportSQLiteDatabase database = helper.createDatabase(TEST_DB, 12);
+ database.close();
+ Context targetContext = ApplicationProvider.getApplicationContext();
+ MigrationDb db = Room.databaseBuilder(targetContext, MigrationDb.class, TEST_DB)
+ .addMigrations(MIGRATION_12_13)
+ .build();
+
+ assertThat(db.dao().loadAllEntity1s(), is(notNullValue()));
+ helper.closeWhenFinished(db);
+
+ // Confirm if this is also works with the Migration Test Helper.
+ Throwable throwable = null;
+ try {
+ helper.runMigrationsAndValidate(TEST_DB, 13, true,
+ MIGRATION_12_13);
+ } catch (Throwable t) {
+ throwable = t;
+ }
+ assertThat(throwable, is(nullValue()));
+ }
+
+ @Test
public void missingMigration_onUpgrade() throws IOException {
SupportSQLiteDatabase database = helper.createDatabase(TEST_DB, 1);
database.close();
@@ -510,7 +533,7 @@
database.close();
Context targetContext = ApplicationProvider.getApplicationContext();
MigrationDb db = Room.databaseBuilder(targetContext, MigrationDb.class, TEST_DB)
- .addMigrations(new EmptyMigration(11, 12))
+ .addMigrations(new EmptyMigration(11, MIGRATION_MAX_LATEST.endVersion))
.build();
try {
db.dao().loadAllEntity1s();
@@ -657,6 +680,14 @@
}
};
+ private static final Migration MIGRATION_12_13 = new Migration(12, 13) {
+ @Override
+ public void migrate(@NonNull SupportSQLiteDatabase database) {
+ database.execSQL("ALTER TABLE Entity1 "
+ + "ADD COLUMN added1InV13 INTEGER NOT NULL DEFAULT (0)");
+ }
+ };
+
/**
* Downgrade migration from {@link MigrationDb#MAX_VERSION} to
* {@link MigrationDb#LATEST_VERSION} that uses the schema file and re-creates the tables such
@@ -697,7 +728,8 @@
private static final Migration[] ALL_MIGRATIONS = new Migration[]{MIGRATION_1_2,
MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6, MIGRATION_6_7,
- MIGRATION_7_8, MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, MIGRATION_11_12};
+ MIGRATION_7_8, MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, MIGRATION_11_12,
+ MIGRATION_12_13};
static final class EmptyMigration extends Migration {
EmptyMigration(int startVersion, int endVersion) {
diff --git a/room/runtime/api/restricted_current.txt b/room/runtime/api/restricted_current.txt
index 9d4e534..fa0e02d8 100644
--- a/room/runtime/api/restricted_current.txt
+++ b/room/runtime/api/restricted_current.txt
@@ -300,6 +300,7 @@
public static final class TableInfo.Column {
ctor @Deprecated public TableInfo.Column(String!, String!, boolean, int);
ctor public TableInfo.Column(String!, String!, boolean, int, String!, int);
+ method public static boolean defaultValueEquals(String, String?);
method public boolean isPrimaryKey();
field @androidx.room.ColumnInfo.SQLiteTypeAffinity public final int affinity;
field public final String! defaultValue;
diff --git a/room/runtime/src/androidTest/java/androidx/room/migration/TableInfoTest.java b/room/runtime/src/androidTest/java/androidx/room/migration/TableInfoTest.java
index b71367d..f7eb2f8 100644
--- a/room/runtime/src/androidTest/java/androidx/room/migration/TableInfoTest.java
+++ b/room/runtime/src/androidTest/java/androidx/room/migration/TableInfoTest.java
@@ -17,6 +17,8 @@
package androidx.room.migration;
+import static com.google.common.truth.Truth.assertThat;
+
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
@@ -46,6 +48,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
@SuppressWarnings("ArraysAsListWithZeroOrOneArgument")
@@ -401,4 +404,74 @@
}).build()
).getWritableDatabase();
}
+
+ @Test
+ public void testSurroundingParenthesis() {
+ mDb = createDatabase(
+ "CREATE TABLE foo (name INTEGER NOT NULL DEFAULT ((0) + (1 + 2)))");
+ TableInfo dbInfo = TableInfo.read(mDb, "foo");
+
+ assertThat("((0) + (1 + 2))")
+ .isNotEqualTo(Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue);
+
+ assertThat(TableInfo.Column.defaultValueEquals(
+ "((0) + (1 + 2))",
+ Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue)).isTrue();
+ }
+
+ @Test
+ public void testDoubleSurroundingParenthesis() {
+ mDb = createDatabase(
+ "CREATE TABLE foo (name INTEGER NOT NULL DEFAULT (((0) + (1 + 2))))");
+ TableInfo dbInfo = TableInfo.read(mDb, "foo");
+
+ assertThat("(((0) + (1 + 2)))")
+ .isNotEqualTo(Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue);
+
+ assertThat(TableInfo.Column.defaultValueEquals(
+ "(((0) + (1 + 2)))",
+ Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue));
+ }
+
+ @Test
+ public void testMultipleParenthesisWithSurrounding() {
+ mDb = createDatabase(
+ "CREATE TABLE foo (name INTEGER NOT NULL DEFAULT (((3 + 5) + (2 + 1)) + (1 + 2)))");
+ TableInfo dbInfo = TableInfo.read(mDb, "foo");
+
+ assertThat("(((3 + 5) + (2 + 1)) + (1 + 2))")
+ .isNotEqualTo(Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue);
+
+ assertThat(TableInfo.Column.defaultValueEquals(
+ "(((3 + 5) + (2 + 1)) + (1 + 2))",
+ Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue));
+ }
+
+ @Test
+ public void testSurroundingParenthesisWithSpacesBefore() {
+ mDb = createDatabase(
+ "CREATE TABLE foo (name INTEGER NOT NULL DEFAULT ( (0) + (1 + 2)))");
+ TableInfo dbInfo = TableInfo.read(mDb, "foo");
+
+ assertThat("( (0) + (1 + 2))")
+ .isNotEqualTo(Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue);
+
+ assertThat(TableInfo.Column.defaultValueEquals(
+ "( (0) + (1 + 2))",
+ Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue));
+ }
+
+ @Test
+ public void testSurroundingParenthesisWithSpacesAfter() {
+ mDb = createDatabase(
+ "CREATE TABLE foo (name INTEGER NOT NULL DEFAULT ((0) + (1 + 2) ))");
+ TableInfo dbInfo = TableInfo.read(mDb, "foo");
+
+ assertThat("((0) + (1 + 2) )")
+ .isNotEqualTo(Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue);
+
+ assertThat(TableInfo.Column.defaultValueEquals(
+ "((0) + (1 + 2) )",
+ Objects.requireNonNull(dbInfo.columns.get("name")).defaultValue)).isTrue();
+ }
}
diff --git a/room/runtime/src/main/java/androidx/room/util/TableInfo.java b/room/runtime/src/main/java/androidx/room/util/TableInfo.java
index f884f3f..37b41d6 100644
--- a/room/runtime/src/main/java/androidx/room/util/TableInfo.java
+++ b/room/runtime/src/main/java/androidx/room/util/TableInfo.java
@@ -237,6 +237,7 @@
private static Map<String, Column> readColumns(SupportSQLiteDatabase database,
String tableName) {
+
Cursor cursor = database
.query("PRAGMA table_info(`" + tableName + "`)");
//noinspection TryFinallyCanBeTryWithResources
@@ -456,15 +457,18 @@
// from the compiler itself has it. b/136019383
if (mCreatedFrom == CREATED_FROM_ENTITY
&& column.mCreatedFrom == CREATED_FROM_DATABASE
- && (defaultValue != null && !defaultValue.equals(column.defaultValue))) {
+ && (defaultValue != null && !defaultValueEquals(defaultValue,
+ column.defaultValue))) {
return false;
} else if (mCreatedFrom == CREATED_FROM_DATABASE
&& column.mCreatedFrom == CREATED_FROM_ENTITY
- && (column.defaultValue != null && !column.defaultValue.equals(defaultValue))) {
+ && (column.defaultValue != null && !defaultValueEquals(
+ column.defaultValue, defaultValue))) {
return false;
} else if (mCreatedFrom != CREATED_FROM_UNKNOWN
&& mCreatedFrom == column.mCreatedFrom
- && (defaultValue != null ? !defaultValue.equals(column.defaultValue)
+ && (defaultValue != null ? !defaultValueEquals(defaultValue,
+ column.defaultValue)
: column.defaultValue != null)) {
return false;
}
@@ -473,6 +477,56 @@
}
/**
+ * Checks if the default values provided match. Handles the special case in which the
+ * default value is surrounded by parenthesis (e.g. encountered in b/182284899).
+ *
+ * Surrounding parenthesis are removed by SQLite when reading from the database, hence
+ * this function will check if they are present in the actual value, if so, it will
+ * compare the two values by ignoring the surrounding parenthesis.
+ *
+ */
+ public static boolean defaultValueEquals(@NonNull String actual, @Nullable String other) {
+ if (other == null) {
+ return false;
+ }
+
+ if (actual.equals(other)) {
+ return true;
+ } else if (containsSurroundingParenthesis(actual)) {
+ return actual.substring(1, actual.length() - 1).trim().equals(other);
+ }
+ return false;
+ }
+
+ /**
+ * Checks for potential surrounding parenthesis, if found, removes them and checks if
+ * remaining paranthesis are balanced. If so, the surrounding parenthesis are redundant,
+ * and returns true.
+ */
+ private static boolean containsSurroundingParenthesis(@NonNull String actual) {
+ if (actual.length() == 0) {
+ return false;
+ }
+ int surroundingParenthesis = 0;
+ for (int i = 0; i < actual.length(); i++) {
+ char c = actual.charAt(i);
+ if (i == 0 && c != '(') {
+ return false;
+ }
+
+ if (c == '(') {
+ surroundingParenthesis++;
+ } else if (c == ')') {
+ surroundingParenthesis--;
+ if (surroundingParenthesis == 0 && i != actual.length() - 1) {
+ return false;
+ }
+ }
+ }
+ return surroundingParenthesis == 0;
+ }
+
+ /**
* Returns whether this column is part of the primary key or not.
*
* @return True if this column is part of the primary key, false otherwise.
