@NotNull as alias for @required (#5161)

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

Author: cmelchior

CHANGELOG.md

@@ -16,6 +16,7 @@ and `SyncUser#retrieveInfoForUserAsync` which returns a `SyncUserInfo` with mode
 * [ObjectServer] APIs of `UserStore` have been changed to support same user identity but different authentication server scenario.
 * [ObjectServer] Added `SyncUser.allSessions` to retrieve the all valid sessions belonging to the user (#4783).
 * Added `Nullable` annotation to methods that may return `null` in order to improve Kotlin usability. This also introduced a dependency to `com.google.code.findbugs:jsr305`.
+* `org.jetbrains.annotations.NotNull` is now an alias for `@Required`. This means that the Realm Schema now fully understand Kotlin non-null types.
 * Added support for new data type `MutableRealmIntegers`. The new type behaves almost exactly as a reference to a Long (mutable nullable, etc) but supports `increment` and `decrement` methods, which implement a Conflict Free Replicated Data Type, whose value will converge even when changed across distributed devices with poor connections (#4266).
 * Added more detailed exception message for `RealmMigrationNeeded`.
 * Bumping schema version only without any actual schema changes will just succeed even when the migration block is not supplied. It threw an `RealmMigrationNeededException` before in the same case.

realm/realm-annotations-processor/src/main/java/io/realm/processor/ClassMetaData.java

@@ -25,6 +25,7 @@
 import java.util.Set;
 
 import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
@@ -375,7 +376,7 @@ private boolean categorizeField(Element element) {
             if (!categorizeIndexField(element, field)) { return false; }
         }
 
-        if (field.getAnnotation(Required.class) != null) {
+        if (isRequiredField(field)) {
             categorizeRequiredField(element, field);
         } else {
             // The field doesn't have the @Required annotation.
@@ -408,6 +409,23 @@ private boolean categorizeField(Element element) {
         return true;
     }
 
+    private boolean isRequiredField(VariableElement field) {
+        if (field.getAnnotation(Required.class) != null) {
+            return true;
+        }
+
+        // Kotlin uses the `org.jetbrains.annotations.NotNull` annotation to mark non-null fields.
+        // In order to fully support the Kotlin type system we interpret `@NotNull` as an alias
+        // for `@Required`
+        for (AnnotationMirror annotation : field.getAnnotationMirrors()) {
+            if (annotation.getAnnotationType().toString().equals("org.jetbrains.annotations.NotNull")) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     // The field has the @Index annotation. It's only valid for column types:
     // STRING, DATE, INTEGER, BOOLEAN, and RealmMutableInteger
     private boolean categorizeIndexField(Element element, VariableElement variableElement) {
@@ -441,13 +459,13 @@ private boolean categorizeIndexField(Element element, VariableElement variableEl
     private void categorizeRequiredField(Element element, VariableElement variableElement) {
         if (Utils.isPrimitiveType(variableElement)) {
             Utils.error(String.format(Locale.US,
-                    "@Required annotation is unnecessary for primitive field \"%s\".", element));
+                    "@Required or @NotNull annotation is unnecessary for primitive field \"%s\".", element));
             return;
         }
 
-        if (Utils.isRealmList(variableElement) || Utils.isRealmModel(variableElement)) {
+        if (Utils.isRealmModel(variableElement)) {
             Utils.error(String.format(Locale.US,
-                    "Field \"%s\" with type \"%s\" cannot be @Required.", element, element.asType()));
+                    "Field \"%s\" with type \"%s\" cannot be @Required or @NotNull.", element, element.asType()));
             return;
         }
 

realm/realm-library/src/androidTest/kotlin/io/realm/KotlinSchemaTests.kt

@@ -51,20 +51,37 @@ class KotlinSchemaTests {
     fun kotlinTypeNonNull() {
         val objSchema = realm.schema.get(AllKotlinTypes::class.simpleName)!!
 
-        // Document current nullability. Ideally all should be non-nullable. This is currently
-        // not the case.
-        // TODO We should fix this. Tracked by https://github.com/realm/realm-java/issues/4701
-        assertTrue(objSchema.isNullable(AllKotlinTypes::nonNullBinary.name));
+        assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullBinary.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullBoolean.name));
-        assertTrue(objSchema.isNullable(AllKotlinTypes::nonNullString.name));
+        assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullString.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullLong.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullInt.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullShort.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullByte.name));
-        assertTrue(objSchema.isNullable(AllKotlinTypes::nonNullDate.name));
+        assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullDate.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullDouble.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullFloat.name));
         assertFalse(objSchema.isNullable(AllKotlinTypes::nonNullList.name));
-        assertTrue(objSchema.isNullable(AllKotlinTypes::nonNullObject.name));
+        // We cannot enforce this constraint inside the schema right now.
+        // If people maintain the variant themselves they need a custom getter
+        // assertTrue(objSchema.isNullable(AllKotlinTypes::nonNullObject.name));
     }
-}
+
+    @Test
+    fun kotlinTypeNull() {
+        val objSchema = realm.schema.get(AllKotlinTypes::class.simpleName)!!
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullBinary.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullBoolean.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullString.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullLong.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullInt.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullShort.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullByte.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullDate.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullDouble.name));
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullFloat.name));
+        assertFalse(objSchema.isNullable(AllKotlinTypes::nullList.name)); // Managed realm objects do not allow null lists
+        assertTrue(objSchema.isNullable(AllKotlinTypes::nullObject.name));
+    }
+
+}

realm/realm-library/src/androidTest/kotlin/io/realm/entities/AllKotlinTypes.kt

@@ -62,17 +62,23 @@ open class AllKotlinTypes : RealmObject() {
     var nullBinary: ByteArray? = null
     var nonNullBinary: ByteArray = ByteArray(0)
 
-// This turns into Byte[] which we dont support for some reason?
-//    var nullBoxedBinary: Array<Byte>? = null
-//    var nonNullBoxedBinary: Array<Byte> = emptyArray()
+    // This turns into Byte[] which we dont support for some reason?
+    // var nullBoxedBinary: Array<Byte>? = null
+    // var nonNullBoxedBinary: Array<Byte> = emptyArray()
 
     var nullObject: AllKotlinTypes? = null
-    var nonNullObject: AllKotlinTypes = AllKotlinTypes()
 
-    var nullList: RealmList<AllKotlinTypes>? = null // This should not be allowed
+    // Not-null object references cannot be enforced generically at the schema level, e.g. sync that
+    // removes an object reference.
+    // If people can maintain the variant themselves they can just expose a custom non-null getter.
+    // var nonNullObject: AllKotlinTypes = AllKotlinTypes()
+
+    // This is only possible in unmanaged objects, managed objects are never null
+    // For now we allow this anyway.
+    var nullList: RealmList<AllKotlinTypes>? = null
     var nonNullList: RealmList<AllKotlinTypes> = RealmList()
 
-    @LinkingObjects("nonNullObject")
+    @LinkingObjects("nullObject")
     val objectParents: RealmResults<AllKotlinTypes>? = null;
 
     @LinkingObjects("nonNullList")