Enforcing a NON NULL constraint on the PrimaryKey
Intended behaviour:
1. If a key is not autogenerated, but is Primary key or is
part of Primary key we force the developer to add @NonNull annotation
2. if a key is autogenerate, we generate NOT NULL in table spec,
but we don't require @NonNull annotation on the field itself.
Bug: 64292391
Test: EntityProcessorTest, room integration tests
Change-Id: I0b76122680a8f78080a715919e855aa7f414f700
diff --git a/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/EntityProcessor.kt b/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/EntityProcessor.kt
index 4f4a7f4..c9e3e18 100644
--- a/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/EntityProcessor.kt
+++ b/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/EntityProcessor.kt
@@ -233,6 +233,19 @@
collectPrimaryKeysFromEmbeddedFields(embeddedFields)
context.checker.check(candidates.isNotEmpty(), element, ProcessorErrors.MISSING_PRIMARY_KEY)
+
+ // 1. If a key is not autogenerated, but is Primary key or is part of Primary key we
+ // force the @NonNull annotation
+ // 2. If a key is autogenerate, we generate NOT NULL in table spec, but we don't require
+ // @NonNull annotation on the field itself.
+ candidates.filter { candidate -> !candidate.autoGenerateId }
+ .map { candidate ->
+ candidate.fields.map { field ->
+ context.checker.check(field.nonNull, field.element,
+ ProcessorErrors.PRIMARY_KEY_NULL)
+ }
+ }
+
if (candidates.size == 1) {
// easy :)
return candidates.first()
diff --git a/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/ProcessorErrors.kt b/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/ProcessorErrors.kt
index 42a2f45..01a4c5f 100644
--- a/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/ProcessorErrors.kt
+++ b/room/compiler/src/main/kotlin/android/arch/persistence/room/processor/ProcessorErrors.kt
@@ -446,4 +446,8 @@
""".trim()
val PAGING_SPECIFY_DATA_SOURCE_TYPE = "For now, Room only supports TiledDataSource class."
+
+ val PRIMARY_KEY_NULL = "You must annotate primary keys with @NonNull. SQLite considers this a " +
+ "bug and Room does not allow it. See SQLite docs for details: " +
+ "https://www.sqlite.org/lang_createtable.html"
}
diff --git a/room/compiler/src/test/data/common/input/MultiPKeyEntity.java b/room/compiler/src/test/data/common/input/MultiPKeyEntity.java
index dccb9dd..59570ad 100644
--- a/room/compiler/src/test/data/common/input/MultiPKeyEntity.java
+++ b/room/compiler/src/test/data/common/input/MultiPKeyEntity.java
@@ -15,9 +15,12 @@
*/
package foo.bar;
+import android.support.annotation.NonNull;
import android.arch.persistence.room.*;
@Entity(primaryKeys = {"name", "lastName"})
public class MultiPKeyEntity {
+ @NonNull
String name;
+ @NonNull
String lastName;
}
diff --git a/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/BaseEntityParserTest.kt b/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/BaseEntityParserTest.kt
index 9bfc0e7..7893200 100644
--- a/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/BaseEntityParserTest.kt
+++ b/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/BaseEntityParserTest.kt
@@ -20,6 +20,7 @@
import android.arch.persistence.room.testing.TestInvocation
import android.arch.persistence.room.testing.TestProcessor
import android.arch.persistence.room.vo.Entity
+import android.support.annotation.NonNull
import com.google.auto.common.MoreElements
import com.google.common.truth.Truth
import com.google.testing.compile.CompileTester
@@ -68,7 +69,8 @@
android.arch.persistence.room.PrimaryKey::class,
android.arch.persistence.room.Ignore::class,
Embedded::class,
- android.arch.persistence.room.ColumnInfo::class)
+ android.arch.persistence.room.ColumnInfo::class,
+ NonNull::class)
.nextRunHandler { invocation ->
val entity = invocation.roundEnv
.getElementsAnnotatedWith(
diff --git a/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/EntityProcessorTest.kt b/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/EntityProcessorTest.kt
index 2238eb2..c25d001 100644
--- a/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/EntityProcessorTest.kt
+++ b/room/compiler/src/test/kotlin/android/arch/persistence/room/processor/EntityProcessorTest.kt
@@ -18,6 +18,7 @@
import COMMON
import android.arch.persistence.room.parser.SQLTypeAffinity
+import android.arch.persistence.room.processor.ProcessorErrors.PRIMARY_KEY_NULL
import android.arch.persistence.room.processor.ProcessorErrors.RELATION_IN_ENTITY
import android.arch.persistence.room.vo.CallType
import android.arch.persistence.room.vo.Field
@@ -1025,6 +1026,7 @@
@Embedded(prefix = "bar_")
@PrimaryKey
+ @NonNull
public Foo foo;
static class Foo {
@@ -1042,6 +1044,7 @@
val parent = JavaFileObjects.forSourceLines("foo.bar.Base",
"""
package foo.bar;
+ import android.support.annotation.NonNull;
import android.arch.persistence.room.*;
public class Base {
@@ -1049,6 +1052,7 @@
String name, lastName;
@Embedded(prefix = "bar_")
@PrimaryKey
+ @NonNull
public Foo foo;
static class Foo {
@@ -1086,6 +1090,7 @@
public int id;
@Embedded(prefix = "bar_")
@PrimaryKey
+ @NonNull
public Foo foo;
static class Foo {
@@ -1106,6 +1111,7 @@
val parent = JavaFileObjects.forSourceLines("foo.bar.Base",
"""
package foo.bar;
+ import android.support.annotation.NonNull;
import android.arch.persistence.room.*;
public class Base {
@@ -1113,6 +1119,7 @@
String name, lastName;
@Embedded(prefix = "bar_")
@PrimaryKey
+ @NonNull
public Foo foo;
static class Foo {
@@ -1135,6 +1142,80 @@
}
@Test
+ fun primaryKey_NonNull() {
+ singleEntity(
+ """
+ @PrimaryKey
+ @NonNull
+ public String id;
+ """) { entity, _ ->
+ assertThat(entity.primaryKey.fields.size, `is`(1))
+ assertThat(entity.primaryKey.fields.firstOrNull()?.name, `is`("id"))
+ }.compilesWithoutError()
+ }
+
+ @Test
+ fun primaryKey_Nullable() {
+ singleEntity(
+ """
+ @PrimaryKey
+ public String id;
+ """) { entity, _ ->
+ assertThat(entity.primaryKey.fields.size, `is`(1))
+ assertThat(entity.primaryKey.fields.firstOrNull()?.name, `is`("id"))
+ }.failsToCompile().withErrorContaining(PRIMARY_KEY_NULL)
+ }
+
+ @Test
+ fun primaryKey_MultipleNullable() {
+ singleEntity(
+ """
+ @PrimaryKey
+ public String id;
+ @PrimaryKey
+ public String anotherId;
+ """) { entity, _ ->
+ }.failsToCompile().withErrorContaining(PRIMARY_KEY_NULL)
+ }
+
+ @Test
+ fun primaryKey_MultipleNullableAndNonNullable() {
+ singleEntity(
+ """
+ @PrimaryKey
+ @NonNull
+ public String id;
+ @PrimaryKey
+ public String anotherId;
+ """) { entity, _ ->
+ }.failsToCompile().withErrorContaining(PRIMARY_KEY_NULL)
+ }
+
+ @Test
+ fun primaryKey_definedAsAttributesNullable() {
+ singleEntity(
+ """
+ public int id;
+ public String foo;
+ """,
+ attributes = mapOf("primaryKeys" to "{\"id\", \"foo\"}")) { _, _ ->
+ }.failsToCompile().withErrorContaining(PRIMARY_KEY_NULL)
+ }
+
+ @Test
+ fun primaryKey_definedAsAttributesNonNull() {
+ singleEntity(
+ """
+ public int id;
+ @NonNull
+ public String foo;
+ """,
+ attributes = mapOf("primaryKeys" to "{\"id\", \"foo\"}")) { entity, _ ->
+ assertThat(entity.primaryKey.fields.map { it.name }, `is`(listOf("id", "foo")))
+ }.compilesWithoutError()
+ }
+
+ @Test
fun relationInEntity() {
singleEntity(
"""
diff --git a/room/compiler/src/test/kotlin/android/arch/persistence/room/writer/SQLiteOpenHelperWriterTest.kt b/room/compiler/src/test/kotlin/android/arch/persistence/room/writer/SQLiteOpenHelperWriterTest.kt
index b93972f..2e86a00 100644
--- a/room/compiler/src/test/kotlin/android/arch/persistence/room/writer/SQLiteOpenHelperWriterTest.kt
+++ b/room/compiler/src/test/kotlin/android/arch/persistence/room/writer/SQLiteOpenHelperWriterTest.kt
@@ -20,6 +20,7 @@
import android.arch.persistence.room.testing.TestInvocation
import android.arch.persistence.room.testing.TestProcessor
import android.arch.persistence.room.vo.Database
+import android.support.annotation.NonNull
import com.google.auto.common.MoreElements
import com.google.common.truth.Truth
import com.google.testing.compile.CompileTester
@@ -36,6 +37,7 @@
companion object {
const val ENTITY_PREFIX = """
package foo.bar;
+ import android.support.annotation.NonNull;
import android.arch.persistence.room.*;
@Entity%s
public class MyEntity {
@@ -55,6 +57,7 @@
singleEntity(
"""
@PrimaryKey
+ @NonNull
String uuid;
String name;
int age;
@@ -63,7 +66,7 @@
val query = SQLiteOpenHelperWriter(database)
.createQuery(database.entities.first())
assertThat(query, `is`("CREATE TABLE IF NOT EXISTS" +
- " `MyEntity` (`uuid` TEXT, `name` TEXT, `age` INTEGER NOT NULL," +
+ " `MyEntity` (`uuid` TEXT NOT NULL, `name` TEXT, `age` INTEGER NOT NULL," +
" PRIMARY KEY(`uuid`))"))
}.compilesWithoutError()
}
@@ -72,7 +75,9 @@
fun multiplePrimaryKeys() {
singleEntity(
"""
+ @NonNull
String uuid;
+ @NonNull
String name;
int age;
""".trimIndent(), attributes = mapOf("primaryKeys" to "{\"uuid\", \"name\"}")
@@ -80,27 +85,49 @@
val query = SQLiteOpenHelperWriter(database)
.createQuery(database.entities.first())
assertThat(query, `is`("CREATE TABLE IF NOT EXISTS" +
- " `MyEntity` (`uuid` TEXT, `name` TEXT, `age` INTEGER NOT NULL," +
- " PRIMARY KEY(`uuid`, `name`))"))
+ " `MyEntity` (`uuid` TEXT NOT NULL, `name` TEXT NOT NULL, " +
+ "`age` INTEGER NOT NULL, PRIMARY KEY(`uuid`, `name`))"))
}.compilesWithoutError()
}
@Test
- fun autoIncrement() {
- singleEntity(
- """
+ fun autoIncrementObject() {
+ listOf("Long", "Integer").forEach { type ->
+ singleEntity(
+ """
@PrimaryKey(autoGenerate = true)
- int uuid;
+ $type uuid;
String name;
int age;
""".trimIndent()
- ) { database, _ ->
- val query = SQLiteOpenHelperWriter(database)
- .createQuery(database.entities.first())
- assertThat(query, `is`("CREATE TABLE IF NOT EXISTS" +
- " `MyEntity` (`uuid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL," +
- " `name` TEXT, `age` INTEGER NOT NULL)"))
- }.compilesWithoutError()
+ ) { database, _ ->
+ val query = SQLiteOpenHelperWriter(database)
+ .createQuery(database.entities.first())
+ assertThat(query, `is`("CREATE TABLE IF NOT EXISTS" +
+ " `MyEntity` (`uuid` INTEGER PRIMARY KEY AUTOINCREMENT," +
+ " `name` TEXT, `age` INTEGER NOT NULL)"))
+ }.compilesWithoutError()
+ }
+ }
+
+ @Test
+ fun autoIncrementPrimitives() {
+ listOf("long", "int").forEach { type ->
+ singleEntity(
+ """
+ @PrimaryKey(autoGenerate = true)
+ $type uuid;
+ String name;
+ int age;
+ """.trimIndent()
+ ) { database, _ ->
+ val query = SQLiteOpenHelperWriter(database)
+ .createQuery(database.entities.first())
+ assertThat(query, `is`("CREATE TABLE IF NOT EXISTS" +
+ " `MyEntity` (`uuid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL," +
+ " `name` TEXT, `age` INTEGER NOT NULL)"))
+ }.compilesWithoutError()
+ }
}
fun singleEntity(input: String, attributes: Map<String, String> = mapOf(),
@@ -119,7 +146,8 @@
), JavaFileObjects.forSourceString("foo.bar.MyDatabase",
DATABASE_CODE)))
.processedWith(TestProcessor.builder()
- .forAnnotations(android.arch.persistence.room.Database::class)
+ .forAnnotations(android.arch.persistence.room.Database::class,
+ NonNull::class)
.nextRunHandler { invocation ->
val db = MoreElements.asType(invocation.roundEnv
.getElementsAnnotatedWith(
diff --git a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/PKeyTestDatabase.java b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/PKeyTestDatabase.java
index e61d808..b6339a8 100644
--- a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/PKeyTestDatabase.java
+++ b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/PKeyTestDatabase.java
@@ -23,14 +23,17 @@
import android.arch.persistence.room.RoomDatabase;
import android.arch.persistence.room.integration.testapp.vo.IntAutoIncPKeyEntity;
import android.arch.persistence.room.integration.testapp.vo.IntegerAutoIncPKeyEntity;
+import android.arch.persistence.room.integration.testapp.vo.ObjectPKeyEntity;
import java.util.List;
-@Database(entities = {IntAutoIncPKeyEntity.class, IntegerAutoIncPKeyEntity.class}, version = 1,
+@Database(entities = {IntAutoIncPKeyEntity.class, IntegerAutoIncPKeyEntity.class,
+ ObjectPKeyEntity.class}, version = 1,
exportSchema = false)
public abstract class PKeyTestDatabase extends RoomDatabase {
public abstract IntPKeyDao intPKeyDao();
public abstract IntegerPKeyDao integerPKeyDao();
+ public abstract ObjectPKeyDao objectPKeyDao();
@Dao
public interface IntPKeyDao {
@@ -52,7 +55,8 @@
@Dao
public interface IntegerPKeyDao {
@Insert
- void insertMe(IntegerAutoIncPKeyEntity items);
+ void insertMe(IntegerAutoIncPKeyEntity item);
+
@Query("select * from IntegerAutoIncPKeyEntity WHERE pKey = :key")
IntegerAutoIncPKeyEntity getMe(int key);
@@ -65,4 +69,10 @@
@Query("select data from IntegerAutoIncPKeyEntity WHERE pKey IN(:ids)")
List<String> loadDataById(long... ids);
}
+
+ @Dao
+ public interface ObjectPKeyDao {
+ @Insert
+ void insertMe(ObjectPKeyEntity item);
+ }
}
diff --git a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/test/PrimaryKeyTest.java b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/test/PrimaryKeyTest.java
index 97ce10c..8d213f2 100644
--- a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/test/PrimaryKeyTest.java
+++ b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/test/PrimaryKeyTest.java
@@ -16,18 +16,21 @@
package android.arch.persistence.room.integration.testapp.test;
+import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
-
-import android.support.test.InstrumentationRegistry;
-import android.support.test.filters.SmallTest;
-import android.support.test.runner.AndroidJUnit4;
+import static org.junit.Assert.assertNotNull;
import android.arch.persistence.room.Room;
import android.arch.persistence.room.integration.testapp.PKeyTestDatabase;
import android.arch.persistence.room.integration.testapp.vo.IntAutoIncPKeyEntity;
import android.arch.persistence.room.integration.testapp.vo.IntegerAutoIncPKeyEntity;
+import android.arch.persistence.room.integration.testapp.vo.ObjectPKeyEntity;
+import android.database.sqlite.SQLiteConstraintException;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
import org.junit.Before;
import org.junit.Test;
@@ -39,6 +42,7 @@
@SmallTest
public class PrimaryKeyTest {
private PKeyTestDatabase mDatabase;
+
@Before
public void setup() {
mDatabase = Room.inMemoryDatabaseBuilder(InstrumentationRegistry.getTargetContext(),
@@ -111,4 +115,18 @@
final long[] ids = mDatabase.integerPKeyDao().insertAndGetIds(entity, entity2);
assertThat(mDatabase.integerPKeyDao().loadDataById(ids), is(Arrays.asList("foo", "foo2")));
}
+
+ @Test
+ public void insertNullPrimaryKey() throws Exception {
+ ObjectPKeyEntity o1 = new ObjectPKeyEntity(null, "1");
+
+ Throwable throwable = null;
+ try {
+ mDatabase.objectPKeyDao().insertMe(o1);
+ } catch (Throwable t) {
+ throwable = t;
+ }
+ assertNotNull("Was expecting an exception", throwable);
+ assertThat(throwable, instanceOf(SQLiteConstraintException.class));
+ }
}
diff --git a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/ObjectPKeyEntity.java b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/ObjectPKeyEntity.java
new file mode 100644
index 0000000..895a35a
--- /dev/null
+++ b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/ObjectPKeyEntity.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2017 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 android.arch.persistence.room.integration.testapp.vo;
+
+import android.arch.persistence.room.Entity;
+import android.arch.persistence.room.PrimaryKey;
+import android.support.annotation.NonNull;
+
+@Entity
+public class ObjectPKeyEntity {
+ @PrimaryKey
+ @NonNull
+ public String pKey;
+ public String data;
+
+ public ObjectPKeyEntity(String pKey, String data) {
+ this.pKey = pKey;
+ this.data = data;
+ }
+}
diff --git a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/PetCouple.java b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/PetCouple.java
index f27b131..e5208ed 100644
--- a/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/PetCouple.java
+++ b/room/integration-tests/testapp/src/androidTest/java/android/arch/persistence/room/integration/testapp/vo/PetCouple.java
@@ -20,11 +20,13 @@
import android.arch.persistence.room.Entity;
import android.arch.persistence.room.PrimaryKey;
import android.arch.persistence.room.RoomWarnings;
+import android.support.annotation.NonNull;
@Entity
@SuppressWarnings(RoomWarnings.PRIMARY_KEY_FROM_EMBEDDED_IS_DROPPED)
public class PetCouple {
@PrimaryKey
+ @NonNull
public String id;
@Embedded(prefix = "male_")
public Pet male;
diff --git a/room/integration-tests/testapp/src/main/java/android/arch/persistence/room/integration/testapp/database/SampleDatabase.java b/room/integration-tests/testapp/src/main/java/android/arch/persistence/room/integration/testapp/database/SampleDatabase.java
index eec59f6..9020eb1 100644
--- a/room/integration-tests/testapp/src/main/java/android/arch/persistence/room/integration/testapp/database/SampleDatabase.java
+++ b/room/integration-tests/testapp/src/main/java/android/arch/persistence/room/integration/testapp/database/SampleDatabase.java
@@ -18,10 +18,6 @@
import android.arch.persistence.room.Database;
import android.arch.persistence.room.RoomDatabase;
-import android.arch.persistence.room.TypeConverter;
-import android.arch.persistence.room.TypeConverters;
-
-import java.util.Date;
/**
* Sample database of customers.