Resolving Auto-Migration error when a new column is added to an embedded Entity between versions.

This fix is particularly for the issue when the nullability of a column that is in an embedded entity is misinterpreted, generating an auto migration with missing default values.

Bug: 193798291
Test: AutoMigrationTest.java
Change-Id: I5fcb1bcf6bf0af9763ed90aea6ec4f25581ca63d
diff --git a/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/1.json b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/1.json
new file mode 100644
index 0000000..4710de2
--- /dev/null
+++ b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/1.json
@@ -0,0 +1,105 @@
+{
+  "formatVersion": 1,
+  "database": {
+    "version": 1,
+    "identityHash": "5eac636b72950b38807bb59226d793cc",
+    "entities": [
+      {
+        "tableName": "Entity1",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER NOT NULL, `name` TEXT, `addedInV1` INTEGER NOT NULL DEFAULT 1, `embeddedId1` INTEGER, `embeddedId2` INTEGER, PRIMARY KEY(`id`))",
+        "fields": [
+          {
+            "fieldPath": "id",
+            "columnName": "id",
+            "affinity": "INTEGER",
+            "notNull": true
+          },
+          {
+            "fieldPath": "name",
+            "columnName": "name",
+            "affinity": "TEXT",
+            "notNull": false
+          },
+          {
+            "fieldPath": "addedInV1",
+            "columnName": "addedInV1",
+            "affinity": "INTEGER",
+            "notNull": true,
+            "defaultValue": "1"
+          },
+          {
+            "fieldPath": "embeddedEntity1.embeddedId1",
+            "columnName": "embeddedId1",
+            "affinity": "INTEGER",
+            "notNull": false
+          },
+          {
+            "fieldPath": "embeddedEntity1.embeddedEntity2.embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": false
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "id"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      },
+      {
+        "tableName": "EmbeddedEntity1",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`embeddedId1` INTEGER NOT NULL, `embeddedId2` INTEGER, PRIMARY KEY(`embeddedId1`))",
+        "fields": [
+          {
+            "fieldPath": "embeddedId1",
+            "columnName": "embeddedId1",
+            "affinity": "INTEGER",
+            "notNull": true
+          },
+          {
+            "fieldPath": "embeddedEntity2.embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": false
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "embeddedId1"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      },
+      {
+        "tableName": "EmbeddedEntity2",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`embeddedId2` INTEGER NOT NULL, PRIMARY KEY(`embeddedId2`))",
+        "fields": [
+          {
+            "fieldPath": "embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": true
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "embeddedId2"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      }
+    ],
+    "views": [],
+    "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, '5eac636b72950b38807bb59226d793cc')"
+    ]
+  }
+}
\ No newline at end of file
diff --git a/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/2.json b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/2.json
new file mode 100644
index 0000000..47ca69d
--- /dev/null
+++ b/room/integration-tests/testapp/schemas/androidx.room.integration.testapp.migration.EmbeddedAutoMigrationDb/2.json
@@ -0,0 +1,119 @@
+{
+  "formatVersion": 1,
+  "database": {
+    "version": 2,
+    "identityHash": "2823f19c55f1bf691ff98bd51cb38bf3",
+    "entities": [
+      {
+        "tableName": "Entity1",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER NOT NULL, `name` TEXT, `addedInV1` INTEGER NOT NULL DEFAULT 1, `embeddedId1` INTEGER, `addedInV2` INTEGER DEFAULT 1, `embeddedId2` INTEGER, PRIMARY KEY(`id`))",
+        "fields": [
+          {
+            "fieldPath": "id",
+            "columnName": "id",
+            "affinity": "INTEGER",
+            "notNull": true
+          },
+          {
+            "fieldPath": "name",
+            "columnName": "name",
+            "affinity": "TEXT",
+            "notNull": false
+          },
+          {
+            "fieldPath": "addedInV1",
+            "columnName": "addedInV1",
+            "affinity": "INTEGER",
+            "notNull": true,
+            "defaultValue": "1"
+          },
+          {
+            "fieldPath": "embeddedEntity1.embeddedId1",
+            "columnName": "embeddedId1",
+            "affinity": "INTEGER",
+            "notNull": false
+          },
+          {
+            "fieldPath": "embeddedEntity1.addedInV2",
+            "columnName": "addedInV2",
+            "affinity": "INTEGER",
+            "notNull": false,
+            "defaultValue": "1"
+          },
+          {
+            "fieldPath": "embeddedEntity1.embeddedEntity2.embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": false
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "id"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      },
+      {
+        "tableName": "EmbeddedEntity1",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`embeddedId1` INTEGER NOT NULL, `addedInV2` INTEGER NOT NULL DEFAULT 1, `embeddedId2` INTEGER, PRIMARY KEY(`embeddedId1`))",
+        "fields": [
+          {
+            "fieldPath": "embeddedId1",
+            "columnName": "embeddedId1",
+            "affinity": "INTEGER",
+            "notNull": true
+          },
+          {
+            "fieldPath": "addedInV2",
+            "columnName": "addedInV2",
+            "affinity": "INTEGER",
+            "notNull": true,
+            "defaultValue": "1"
+          },
+          {
+            "fieldPath": "embeddedEntity2.embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": false
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "embeddedId1"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      },
+      {
+        "tableName": "EmbeddedEntity2",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`embeddedId2` INTEGER NOT NULL, PRIMARY KEY(`embeddedId2`))",
+        "fields": [
+          {
+            "fieldPath": "embeddedId2",
+            "columnName": "embeddedId2",
+            "affinity": "INTEGER",
+            "notNull": true
+          }
+        ],
+        "primaryKey": {
+          "columnNames": [
+            "embeddedId2"
+          ],
+          "autoGenerate": false
+        },
+        "indices": [],
+        "foreignKeys": []
+      }
+    ],
+    "views": [],
+    "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, '2823f19c55f1bf691ff98bd51cb38bf3')"
+    ]
+  }
+}
\ No newline at end of file
diff --git a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/AutoMigrationTest.java b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/AutoMigrationTest.java
index fb3cd7c..754cf15 100644
--- a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/AutoMigrationTest.java
+++ b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/AutoMigrationTest.java
@@ -93,6 +93,29 @@
         }
     }
 
+    @Test
+    public void testAutoMigrationWithNewEmbeddedField() throws IOException {
+        MigrationTestHelper embeddedHelper = new MigrationTestHelper(
+                InstrumentationRegistry.getInstrumentation(),
+                EmbeddedAutoMigrationDb.class
+        );
+
+        SupportSQLiteDatabase db = embeddedHelper.createDatabase(
+                "embedded-auto-migration-test",
+                1
+        );
+        db.execSQL("INSERT INTO Entity1 (id, name) VALUES (1, 'row1')");
+
+        final TableInfo info = TableInfo.read(
+                embeddedHelper.runMigrationsAndValidate(
+                        "embedded-auto-migration-test",
+                        2,
+                        true
+                ),
+                EmbeddedAutoMigrationDb.EmbeddedEntity1.TABLE_NAME);
+        assertThat(info.columns.size()).isEqualTo(3);
+    }
+
     /**
      * Verifies that the user defined migration is selected over using an autoMigration.
      */
diff --git a/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/EmbeddedAutoMigrationDb.java b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/EmbeddedAutoMigrationDb.java
new file mode 100644
index 0000000..aa22528
--- /dev/null
+++ b/room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/migration/EmbeddedAutoMigrationDb.java
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.room.integration.testapp.migration;
+
+import androidx.room.AutoMigration;
+import androidx.room.ColumnInfo;
+import androidx.room.Dao;
+import androidx.room.Database;
+import androidx.room.Embedded;
+import androidx.room.Entity;
+import androidx.room.PrimaryKey;
+import androidx.room.Query;
+import androidx.room.RoomDatabase;
+import androidx.room.RoomWarnings;
+
+import java.io.Serializable;
+import java.util.List;
+
+@Database(
+        version = EmbeddedAutoMigrationDb.LATEST_VERSION,
+        entities = {
+                EmbeddedAutoMigrationDb.Entity1.class,
+                EmbeddedAutoMigrationDb.EmbeddedEntity1.class,
+                EmbeddedAutoMigrationDb.EmbeddedEntity2.class
+        },
+        autoMigrations = {
+                @AutoMigration(
+                        from = 1, to = 2
+                )
+        },
+        exportSchema = true
+)
+public abstract class EmbeddedAutoMigrationDb extends RoomDatabase {
+    static final int LATEST_VERSION = 2;
+    @Dao
+    interface EmbeddedAutoMigrationDao {
+        @Query("SELECT * from Entity1 ORDER BY id ASC")
+        List<EmbeddedAutoMigrationDb.Entity1> getAllEntity1s();
+    }
+
+    abstract EmbeddedAutoMigrationDb.EmbeddedAutoMigrationDao dao();
+
+    /**
+     * No change between versions.
+     */
+    @Entity
+    @SuppressWarnings(RoomWarnings.PRIMARY_KEY_FROM_EMBEDDED_IS_DROPPED)
+    static class Entity1 implements Serializable {
+        public static final String TABLE_NAME = "Entity1";
+        @PrimaryKey
+        public int id;
+        public String name;
+        @ColumnInfo(defaultValue = "1")
+        public int addedInV1;
+
+        public EmbeddedEntity1 getEmbeddedEntity1() {
+            return embeddedEntity1;
+        }
+
+        public void setEmbeddedEntity1(
+                EmbeddedEntity1 embeddedEntity1) {
+            this.embeddedEntity1 = embeddedEntity1;
+        }
+
+        @Embedded
+        private EmbeddedEntity1 embeddedEntity1;
+    }
+
+    @Entity
+    @SuppressWarnings(RoomWarnings.PRIMARY_KEY_FROM_EMBEDDED_IS_DROPPED)
+    static class EmbeddedEntity1 implements Serializable {
+        public static final String TABLE_NAME = "EmbeddedEntity1";
+        @PrimaryKey
+        public int embeddedId1;
+
+        @ColumnInfo(defaultValue = "1")
+        public int addedInV2;
+
+        public EmbeddedEntity2 getEmbeddedEntity2() {
+            return embeddedEntity2;
+        }
+
+        public void setEmbeddedEntity2(
+                EmbeddedEntity2 embeddedEntity2) {
+            this.embeddedEntity2 = embeddedEntity2;
+        }
+
+        @Embedded
+        private EmbeddedEntity2 embeddedEntity2;
+    }
+
+    @Entity
+    @SuppressWarnings(RoomWarnings.PRIMARY_KEY_FROM_EMBEDDED_IS_DROPPED)
+    static class EmbeddedEntity2 implements Serializable {
+        public static final String TABLE_NAME = "EmbeddedEntity2";
+
+        public int getEmbeddedId2() {
+            return embeddedId2;
+        }
+
+        public void setEmbeddedId2(int embeddedId2) {
+            this.embeddedId2 = embeddedId2;
+        }
+
+        @PrimaryKey
+        private int embeddedId2;
+    }
+}
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 290606e..c1c9a19 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
@@ -428,12 +428,18 @@
             val addNewColumnSql = buildString {
                 append(
                     "ALTER TABLE `${it.tableName}` ADD COLUMN `${it.fieldBundle.columnName}` " +
-                        "${it.fieldBundle.affinity} "
+                        "${it.fieldBundle.affinity}"
                 )
                 if (it.fieldBundle.isNonNull) {
-                    append("NOT NULL DEFAULT ${it.fieldBundle.defaultValue}")
+                    append(" NOT NULL")
+                }
+                if (it.fieldBundle.defaultValue?.isNotEmpty() == true) {
+                    append(" DEFAULT ${it.fieldBundle.defaultValue}")
                 } else {
-                    append("DEFAULT NULL")
+                    check(
+                        !it.fieldBundle.isNonNull
+                    ) { "A Non-Null field should always have a default value." }
+                    append(" DEFAULT NULL")
                 }
             }
             addDatabaseExecuteSqlStatement(