Allow null PKs for strings

CHANGELOG.md

@@ -23,6 +23,7 @@ that allows you to open the old Realm file in a dynamic mode and migrate any req
 account data associated with that user. ([#1573](https://github.com/realm/realm-dotnet/pull/1573))
 - Introduced a new method - `User.LogOutAsync` to replace the now-deprecated synchronous call. ([#1574](https://github.com/realm/realm-dotnet/pull/1574))
 - Exposed `BacklinksCount` property on `RealmObject` that returns the number of objects that refer to the current object via a to-one or a to-many relationship. ([#1578](https://github.com/realm/realm-dotnet/pull/1578))
+- String primary keys now support `null` as a value. ([#1579](https://github.com/realm/realm-dotnet/pull/1579))
 
 ### Bug fixes
 - `Realm.GetInstance` will now advance the Realm to the latest version, so you no longer have to call `Refresh` manually after that. ([#1523](https://github.com/realm/realm-dotnet/pull/1523))

Realm/Realm/Handles/SharedRealmHandle.cs

@@ -94,7 +94,7 @@ public static extern IntPtr open(Native.Configuration configuration,
             public static extern IntPtr create_object(SharedRealmHandle sharedRealm, TableHandle table, out NativeException ex);
 
             [DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_create_object_int_unique", CallingConvention = CallingConvention.Cdecl)]
-            public static extern IntPtr create_object_unique(SharedRealmHandle sharedRealm, TableHandle table, long key,
+            public static extern IntPtr create_object_unique(SharedRealmHandle sharedRealm, TableHandle table, long key, [MarshalAs(UnmanagedType.I1)] bool has_value,
                                                              [MarshalAs(UnmanagedType.I1)] bool is_nullable,
                                                              [MarshalAs(UnmanagedType.I1)] bool update,
                                                              [MarshalAs(UnmanagedType.I1)] out bool is_new, out NativeException ex);
@@ -105,11 +105,6 @@ public static extern IntPtr create_object_unique(SharedRealmHandle sharedRealm,
                                                              [MarshalAs(UnmanagedType.I1)] bool update,
                                                              [MarshalAs(UnmanagedType.I1)] out bool is_new, out NativeException ex);
 
-            [DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_create_object_null_unique", CallingConvention = CallingConvention.Cdecl)]
-            public static extern IntPtr create_object_unique(SharedRealmHandle sharedRealm, TableHandle table,
-                                                             [MarshalAs(UnmanagedType.I1)] bool update,
-                                                             [MarshalAs(UnmanagedType.I1)] out bool is_new, out NativeException ex);
-
             [DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_get_schema", CallingConvention = CallingConvention.Cdecl)]
             public static extern void get_schema(SharedRealmHandle sharedRealm, IntPtr callback, out NativeException ex);
         }
@@ -264,55 +259,34 @@ public IntPtr CreateObject(TableHandle table)
 
         public IntPtr CreateObjectWithPrimaryKey(Property pkProperty, object primaryKey, TableHandle table, string parentType, bool update, out bool isNew)
         {
+            if (primaryKey == null && !pkProperty.Type.IsNullable())
+            {
+                throw new ArgumentException($"{parentType}'s primary key is defined as non-nullable, but the value passed is null");
+            }
+
             switch (pkProperty.Type.UnderlyingType())
             {
                 case PropertyType.String:
-                    if (primaryKey == null)
-                    {
-                        throw new ArgumentNullException(nameof(primaryKey), "Object identifiers cannot be null");
-                    }
-
-                    if (primaryKey is string stringKey)
-                    {
-                        return CreateObjectWithPrimaryKey(table, stringKey, update, out isNew);
-                    }
-
-                    throw new ArgumentException($"{parentType}'s primary key is defined as string, but the value passed is {primaryKey.GetType().Name}");
+                    var stringKey = (string)primaryKey;
+                    return CreateObjectWithPrimaryKey(table, stringKey, update, out isNew);
                 case PropertyType.Int:
-                    if (primaryKey == null)
-                    {
-                        if (!pkProperty.Type.IsNullable())
-                        {
-                            throw new ArgumentException($"{parentType}'s primary key is defined as non-nullable, but the value passed is null");
-                        }
-
-                        return CreateObjectWithPrimaryKey(table, update, out isNew);
-                    }
-
-                    var longKey = Convert.ToInt64(primaryKey);
+                    var longKey = primaryKey == null ? (long?)null : Convert.ToInt64(primaryKey);
                     return CreateObjectWithPrimaryKey(table, longKey, pkProperty.Type.IsNullable(), update, out isNew);
                 default:
                     throw new NotSupportedException($"Unexpected primary key of type: {pkProperty.Type}");
             }
         }
 
-        private IntPtr CreateObjectWithPrimaryKey(TableHandle table, long key, bool isNullable, bool update, out bool isNew)
+        private IntPtr CreateObjectWithPrimaryKey(TableHandle table, long? key, bool isNullable, bool update, out bool isNew)
         {
-            var result = NativeMethods.create_object_unique(this, table, key, isNullable, update, out isNew, out var ex);
+            var result = NativeMethods.create_object_unique(this, table, key ?? 0, key.HasValue, isNullable, update, out isNew, out var ex);
             ex.ThrowIfNecessary();
             return result;
         }
 
         private IntPtr CreateObjectWithPrimaryKey(TableHandle table, string key, bool update, out bool isNew)
         {
-            var result = NativeMethods.create_object_unique(this, table, key, (IntPtr)key.Length, update, out isNew, out var ex);
-            ex.ThrowIfNecessary();
-            return result;
-        }
-
-        private IntPtr CreateObjectWithPrimaryKey(TableHandle table, bool update, out bool isNew)
-        {
-            var result = NativeMethods.create_object_unique(this, table, update, out isNew, out var ex);
+            var result = NativeMethods.create_object_unique(this, table, key, (IntPtr)(key?.Length ?? 0), update, out isNew, out var ex);
             ex.ThrowIfNecessary();
             return result;
         }

Realm/Realm/Handles/TableHandle.cs

@@ -108,12 +108,17 @@ public IntPtr CreateSortedResults(SharedRealmHandle sharedRealmHandle, SortDescr
 
         public IntPtr Find(SharedRealmHandle realmHandle, string id)
         {
+            NativeException nativeException;
+            IntPtr result;
             if (id == null)
             {
-                throw new ArgumentNullException(nameof(id));
+                result = NativeMethods.object_for_null_primarykey(this, realmHandle, out nativeException);
+            }
+            else
+            {
+                result = NativeMethods.object_for_string_primarykey(this, realmHandle, id, (IntPtr)id.Length, out nativeException);
             }
 
-            var result = NativeMethods.object_for_string_primarykey(this, realmHandle, id, (IntPtr)id.Length, out var nativeException);
             nativeException.ThrowIfNecessary();
             return result;
         }
@@ -154,4 +159,4 @@ public IntPtr Find(SharedRealmHandle realmHandle, long? id)
               if (ret ==0 && !myHandle.IsNull() && myHandle != invalidHandle)
             mySafeHandle.SetHandle(myHandle);
         }// End CER
-*/
+*/

Realm/Realm/Helpers/Operator.cs

@@ -76,13 +76,13 @@ private static Func<T, U> CreateConvert()
                     Expression convertFrom = input;
                     var typeOfT = typeof(T);
                     var isTNullable = false;
-                    if (ReflectionExtensions.IsClosedGeneric(typeOfT, typeof(Nullable<>), out var arguments))
+                    if (typeOfT.IsClosedGeneric(typeof(Nullable<>), out var arguments))
                     {
                         typeOfT = arguments.Single();
                         isTNullable = true;
                     }
 
-                    if (ReflectionExtensions.IsClosedGeneric(typeOfT, typeof(RealmInteger<>), out arguments))
+                    if (typeOfT.IsClosedGeneric(typeof(RealmInteger<>), out arguments))
                     {
                         var intermediateType = arguments.Single();
                         if (isTNullable)
@@ -95,15 +95,15 @@ private static Func<T, U> CreateConvert()
 
                     var typeOfU = typeof(U);
                     var isUNullable = false;
-                    if (typeOfU.IsConstructedGenericType && typeOfU.GetGenericTypeDefinition() == typeof(Nullable<>))
+                    if (typeOfU.IsClosedGeneric(typeof(Nullable<>), out arguments))
                     {
-                        typeOfU = typeOfU.GenericTypeArguments.Single();
+                        typeOfU = arguments.Single();
                         isUNullable = true;
                     }
 
-                    if (typeOfU.IsConstructedGenericType && typeOfU.GetGenericTypeDefinition() == typeof(RealmInteger<>))
+                    if (typeOfU.IsClosedGeneric(typeof(RealmInteger<>), out arguments))
                     {
-                        var intermediateType = typeOfU.GenericTypeArguments.Single();
+                        var intermediateType = arguments.Single();
                         if (isUNullable)
                         {
                             intermediateType = typeof(Nullable<>).MakeGenericType(intermediateType);

Tests/Tests.Shared/AddOrUpdateTests.cs

@@ -663,33 +663,49 @@ public void AddOrUpdate_WhenChildHasNoPKAndGrandchildHasPK_ShouldAddChildUpdateG
         }
 
         [Test]
-        public void AddOrUpdate_WhenPKIsNull_ShouldUpdate()
+        public void AddOrUpdate_WhenPKIsIntAndNull_ShouldUpdate()
         {
             var first = new NullablePrimaryKeyObject
             {
                 StringValue = "first"
             };
 
-            _realm.Write(() =>
-            {
-                _realm.Add(first, update: true);
-            });
+            _realm.Write(() => _realm.Add(first, update: true));
 
             var second = new NullablePrimaryKeyObject
             {
                 StringValue = "second"
             };
 
-            _realm.Write(() =>
-            {
-                _realm.Add(second, update: true);
-            });
+            _realm.Write(() => _realm.Add(second, update: true));
 
             Assert.That(first.StringValue, Is.EqualTo("second"));
             Assert.That(second.StringValue, Is.EqualTo("second"));
             Assert.That(_realm.All<NullablePrimaryKeyObject>().Count(), Is.EqualTo(1));
         }
 
+        [Test]
+        public void AddOrUpdate_WhenPKIsStringAndNull_ShouldUpdate()
+        {
+            var first = new PrimaryKeyStringObject
+            {
+                Value = "first"
+            };
+
+            _realm.Write(() => _realm.Add(first, update: true));
+
+            var second = new PrimaryKeyStringObject
+            {
+                Value = "second"
+            };
+
+            _realm.Write(() => _realm.Add(second, update: true));
+
+            Assert.That(first.Value, Is.EqualTo("second"));
+            Assert.That(second.Value, Is.EqualTo("second"));
+            Assert.That(_realm.All<PrimaryKeyStringObject>().Count(), Is.EqualTo(1));
+        }
+
         [Test]
         public void Add_ShouldReturnPassedInObject()
         {
@@ -786,6 +802,7 @@ public void AddOrUpdate_ShouldReturnManaged()
         [TestCase(typeof(PrimaryKeyNullableInt16Object))]
         [TestCase(typeof(PrimaryKeyNullableInt32Object))]
         [TestCase(typeof(PrimaryKeyNullableInt64Object))]
+        [TestCase(typeof(PrimaryKeyStringObject))]
         public void Add_WhenPKIsDefaultAndDuplicate_ShouldThrow(Type type)
         {
             Assert.That(() =>
@@ -798,18 +815,6 @@ public void Add_WhenPKIsDefaultAndDuplicate_ShouldThrow(Type type)
             }, Throws.TypeOf<RealmDuplicatePrimaryKeyValueException>());
         }
 
-        [Test]
-        public void Add_WhenPKIsStringAndNull_ShouldThrow()
-        {
-            Assert.That(() =>
-            {
-                _realm.Write(() =>
-                {
-                    _realm.Add(new PrimaryKeyStringObject());
-                });
-            }, Throws.TypeOf<ArgumentNullException>());
-        }
-
         [Test]
         public void Add_WhenPKIsNotDefaultButDuplicate_ShouldThrow()
         {

Tests/Tests.Shared/PrimaryKeyTests.cs

@@ -49,6 +49,10 @@ public class PrimaryKeyTests : RealmInstanceTest
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L, true)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null, true)]
         [TestCase(typeof(PrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(PrimaryKeyStringObject), null, false)]
+        [TestCase(typeof(PrimaryKeyStringObject), "", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "", false)]
         public void FindByPrimaryKeyDynamicTests(Type type, object primaryKeyValue, bool isIntegerPK)
         {
             var obj = (RealmObject)Activator.CreateInstance(type);
@@ -79,6 +83,10 @@ public void FindByPrimaryKeyDynamicTests(Type type, object primaryKeyValue, bool
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L, true)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null, true)]
         [TestCase(typeof(PrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(PrimaryKeyStringObject), null, false)]
+        [TestCase(typeof(PrimaryKeyStringObject), "", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "", false)]
         public void FailToFindByPrimaryKeyDynamicTests(Type type, object primaryKeyValue, bool isIntegerPK)
         {
             var foundObj = FindByPKDynamic(type, primaryKeyValue, isIntegerPK);
@@ -101,19 +109,17 @@ public void FailToFindByPrimaryKeyDynamicTests(Type type, object primaryKeyValue
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null)]
         [TestCase(typeof(PrimaryKeyStringObject), "key")]
+        [TestCase(typeof(PrimaryKeyStringObject), null)]
+        [TestCase(typeof(PrimaryKeyStringObject), "")]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key")]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "")]
         public void CreateObject_WhenPKExists_ShouldFail(Type type, object primaryKeyValue)
         {
-            _realm.Write(() =>
-            {
-                _realm.CreateObject(type.Name, primaryKeyValue);
-            });
+            _realm.Write(() => _realm.CreateObject(type.Name, primaryKeyValue));
 
             Assert.That(() =>
             {
-                _realm.Write(() =>
-                {
-                    _realm.CreateObject(type.Name, primaryKeyValue);
-                });
+                _realm.Write(() => _realm.CreateObject(type.Name, primaryKeyValue));
             }, Throws.TypeOf<RealmDuplicatePrimaryKeyValueException>());
         }
 
@@ -133,25 +139,23 @@ public void CreateObject_WhenPKExists_ShouldFail(Type type, object primaryKeyVal
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null)]
         [TestCase(typeof(PrimaryKeyStringObject), "key")]
+        [TestCase(typeof(PrimaryKeyStringObject), null)]
+        [TestCase(typeof(PrimaryKeyStringObject), "")]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key")]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "")]
         public void ManageObject_WhenPKExists_ShouldFail(Type type, object primaryKeyValue)
         {
             var pkProperty = type.GetProperties().Single(p => p.GetCustomAttribute<PrimaryKeyAttribute>() != null);
             var first = (RealmObject)Activator.CreateInstance(type);
             pkProperty.SetValue(first, primaryKeyValue);
 
-            _realm.Write(() =>
-            {
-                _realm.Add(first);
-            });
+            _realm.Write(() => _realm.Add(first));
 
             Assert.That(() =>
             {
                 var second = (RealmObject)Activator.CreateInstance(type);
                 pkProperty.SetValue(second, primaryKeyValue);
-                _realm.Write(() =>
-                {
-                    _realm.Add(second);
-                });
+                _realm.Write(() => _realm.Add(second));
             }, Throws.TypeOf<RealmDuplicatePrimaryKeyValueException>());
         }
 
@@ -191,6 +195,10 @@ private RealmObject FindByPKDynamic(Type type, object primaryKeyValue, bool isIn
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L, true)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null, true)]
         [TestCase(typeof(PrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(PrimaryKeyStringObject), null, false)]
+        [TestCase(typeof(PrimaryKeyStringObject), "", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "", false)]
         public void FindByPrimaryKeyGenericTests(Type type, object primaryKeyValue, bool isIntegerPK)
         {
             var obj = (RealmObject)Activator.CreateInstance(type);
@@ -221,6 +229,10 @@ public void FindByPrimaryKeyGenericTests(Type type, object primaryKeyValue, bool
         [TestCase(typeof(PrimaryKeyNullableInt64Object), 42000042L, true)]
         [TestCase(typeof(PrimaryKeyNullableInt64Object), null, true)]
         [TestCase(typeof(PrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(PrimaryKeyStringObject), null, false)]
+        [TestCase(typeof(PrimaryKeyStringObject), "", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "key", false)]
+        [TestCase(typeof(RequiredPrimaryKeyStringObject), "", false)]
         public void FailToFindByPrimaryKeyGenericTests(Type type, object primaryKeyValue, bool isIntegerPK)
         {
             var foundObj = FindByPKGeneric(type, primaryKeyValue, isIntegerPK);
@@ -307,18 +319,6 @@ public void PrimaryKeyStringObjectIsUnique()
             }, Throws.TypeOf<RealmDuplicatePrimaryKeyValueException>());
         }
 
-        [Test]
-        public void NullPrimaryKeyStringObjectThrows()
-        {
-            Assert.That(() =>
-            {
-                _realm.Write(() =>
-                {
-                    _realm.Add(new PrimaryKeyStringObject { StringProperty = null });
-                });
-            }, Throws.TypeOf<ArgumentNullException>());
-        }
-
         [Test]
         public void NullAndNotNullIntPKsWorkTogether()
         {
@@ -329,10 +329,18 @@ public void NullAndNotNullIntPKsWorkTogether()
             });
 
             Assert.That(_realm.All<PrimaryKeyNullableInt64Object>().Count, Is.EqualTo(2));
+
+            _realm.Write(() =>
+            {
+                _realm.Add(new PrimaryKeyStringObject { StringProperty = "123" });
+                _realm.Add(new PrimaryKeyStringObject { StringProperty = null });
+            });
+
+            Assert.That(_realm.All<PrimaryKeyStringObject>().Count, Is.EqualTo(2));
         }
 
         [Test]
-        public void PrimaryKeyFailsIfClassNotinRealm()
+        public void PrimaryKeyFailsIfClassNotInRealm()
         {
             var conf = RealmConfiguration.DefaultConfiguration.ConfigWithPath(Path.GetTempFileName());
             conf.ObjectClasses = new[] { typeof(Person) };
@@ -348,5 +356,11 @@ public void PrimaryKeyFailsIfClassNotinRealm()
                 Realm.DeleteRealm(conf);
             }
         }
+
+        [Test]
+        public void StringPrimaryKey_WhenRequiredDoesntAllowNull()
+        {
+            Assert.That(() => _realm.Write(() => _realm.Add(new RequiredPrimaryKeyStringObject())), Throws.TypeOf<ArgumentException>());
+        }
     }
 }