Relax IndexOf conditions to handle null and unmanaged objects (#3369)

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ### Enhancements
 * Added validation checks to the geospatial type constructors. This means that an exception will now be thrown when constructing an invalid geospatial shape rather than when using it in a query. (PR [#3362](https://github.com/realm/realm-dotnet/pull/3362)) 
+* Relaxed some validations when invoking `IndexOf(null)` on a collection of non-nullable types. Previously, this would throw an `ArgumentNullException` whereas now it will return `-1`. This is particularly useful for data-binding scenarios where the binding engine might invoke it as `IndexOf(SelectedItem)` which would throw an exception when `SelectedItem` is `null`. (PR [#3369](https://github.com/realm/realm-dotnet/pull/3369))
+* Changed `RealmSet.IndexOf` implementation to return the actual result rather than throw a `NotSupportedException`. The order of persisted sets is still non-deterministic, but is stable between write transactions. Again, this is mostly useful for data-binding scenarios where the set is passed as a binding context to a collection control. (PR [#3369](https://github.com/realm/realm-dotnet/pull/3369))
 
 ### Fixed
 * Fixed an issue on Unity on Windows when the weaver would trigger excessive terminal windows to open. (Issue [3364]https://github.com/realm/realm-dotnet/issues/3364)

Realm/Realm/DatabaseTypes/RealmList.cs

@@ -109,7 +109,7 @@ public override int IndexOf([AllowNull] T value)
 
             if (realmValue.Type == RealmValueType.Object && !realmValue.AsIRealmObject().IsManaged)
             {
-                throw new ArgumentException("Value does not belong to a realm", nameof(value));
+                return -1;
             }
 
             return _listHandle.Find(realmValue);

Realm/Realm/DatabaseTypes/RealmSet.cs

@@ -89,7 +89,21 @@ public bool Remove(T value)
             return _setHandle.Remove(realmValue);
         }
 
-        public override int IndexOf([AllowNull] T value) => throw new NotSupportedException();
+        // IndexOf is not available on ISet<T>, but is available on IList and IRealmCollection.
+        // We provide an implementation here because in data-binding scenarios, the binding engine
+        // may cast the collection to IList and attempt to invoke IndexOf on it - most notably when
+        // binding to a ListView and changing the SelectedItem.
+        public override int IndexOf([AllowNull] T value)
+        {
+            var realmValue = Operator.Convert<T?, RealmValue>(value);
+
+            if (realmValue.Type == RealmValueType.Object && !realmValue.AsIRealmObject().IsManaged)
+            {
+                return -1;
+            }
+
+            return _setHandle.Find(realmValue);
+        }
 
         public override bool Contains([AllowNull] T value)
         {

Realm/Realm/Handles/SetHandle.cs

@@ -102,6 +102,9 @@ public static extern IntPtr get_filtered_results(SetHandle results,
                 [MarshalAs(UnmanagedType.LPWStr)] string query_buf, IntPtr query_len,
                 [MarshalAs(UnmanagedType.LPArray), In] NativeQueryArgument[] arguments, IntPtr args_count,
                 out NativeException ex);
+
+            [DllImport(InteropConfig.DLL_NAME, EntryPoint = "set_find_value", CallingConvention = CallingConvention.Cdecl)]
+            public static extern IntPtr find_value(SetHandle setHandle, PrimitiveValue value, out NativeException ex);
         }
 
         public override bool IsValid
@@ -225,6 +228,17 @@ public bool Remove(in RealmValue value)
 
         public override void Unbind() => NativeMethods.destroy(handle);
 
+        public int Find(in RealmValue value)
+        {
+            EnsureIsOpen();
+
+            var (primitive, handles) = value.ToNative();
+            var result = NativeMethods.find_value(this, primitive, out var nativeException);
+            handles?.Dispose();
+            nativeException.ThrowIfNecessary();
+            return (int)result;
+        }
+
         protected override IntPtr SnapshotCore(out NativeException ex) => NativeMethods.snapshot(this, out ex);
 
         #region Set methods

Realm/Realm/Linq/RealmResults.cs

@@ -80,13 +80,11 @@ internal override CollectionHandleBase GetOrCreateHandle()
 
         public override int IndexOf(T? value)
         {
-            Argument.NotNull(value, nameof(value));
-
-            var realmValue = Operator.Convert<T, RealmValue>(value!);
+            var realmValue = Operator.Convert<T?, RealmValue>(value!);
 
             if (realmValue.Type == RealmValueType.Object && !realmValue.AsIRealmObject().IsManaged)
             {
-                throw new ArgumentException("Value does not belong to a realm", nameof(value));
+                return -1;
             }
 
             return ResultsHandle.Find(realmValue);

Tests/Realm.Tests/Database/RealmResults/SimpleLINQtests.cs

@@ -1312,7 +1312,7 @@ public void Queryable_IndexOf_WhenObjectIsNotManaged_ShouldThrow()
         {
             var query = _realm.All<IntPrimaryKeyWithValueObject>().AsRealmCollection();
             var nonManagedItem = new IntPrimaryKeyWithValueObject();
-            Assert.That(() => query.IndexOf(nonManagedItem), Throws.InstanceOf<ArgumentException>());
+            Assert.That(query.IndexOf(nonManagedItem), Is.EqualTo(-1));
         }
 
         [Test]
@@ -1331,10 +1331,10 @@ public void Queryable_IndexOf_WhenObjectBelongsToADifferentRealm_ShouldThrow()
         }
 
         [Test]
-        public void Queryable_IndexOf_WhenNull_ShouldThrow()
+        public void Queryable_IndexOf_WhenNull_ShouldNotThrow()
         {
             var query = _realm.All<IntPrimaryKeyWithValueObject>().AsRealmCollection();
-            Assert.That(() => query.IndexOf(null), Throws.InstanceOf<ArgumentNullException>());
+            Assert.That(query.IndexOf(null), Is.EqualTo(-1));
         }
 
         [Test]

Tests/Realm.Tests/Database/RealmSetTests.cs

@@ -1227,6 +1227,14 @@ private void RunManagedTests<T>(
             var managedSet = accessor(testObject);
             Assert.That(set, Is.Not.SameAs(managedSet));
 
+            // Assert RealmCollection.IndexOf
+            var managedCollection = managedSet.AsRealmCollection();
+            for (var i = 0; i < managedCollection.Count; i++)
+            {
+                var element = managedCollection[i];
+                Assert.That(managedCollection.IndexOf(element), Is.EqualTo(i));
+            }
+
             // Now we're testing set operations on RealmSet/HashSet
             testData.AssertCount(managedSet);
             testData.AssertExceptWith(managedSet);

wrappers/src/results_cs.cpp

@@ -154,6 +154,11 @@ REALM_EXPORT Results* results_snapshot(const Results& results, NativeException::
 REALM_EXPORT size_t results_find_value(Results& results, realm_value_t value, NativeException::Marshallable& ex)
 {
     return handle_errors(ex, [&]() {
+        auto results_type = results.get_type();
+        if (value.is_null() && !is_nullable(results_type)) {
+            return (size_t)-1;
+        }
+
         if (value.type == realm_value_type::RLM_TYPE_LINK) {
             if (results.get_realm() != value.link.object->realm()) {
                 throw ObjectManagedByAnotherRealmException("Can't look up index of an object that belongs to a different Realm.");

wrappers/src/set_cs.cpp

@@ -271,4 +271,27 @@ REALM_EXPORT Results* set_get_filtered_results(const object_store::Set& set, uin
     });
 }
 
+REALM_EXPORT size_t set_find_value(const object_store::Set& set, realm_value_t value, NativeException::Marshallable& ex)
+{
+    return handle_errors(ex, [&]() {
+        auto set_type = set.get_type();
+        // This doesn't use ensure_types to allow List<string>.Find(null) to return false
+        if (value.is_null() && !is_nullable(set_type)) {
+            return (size_t)-1;
+        }
+
+        if (!value.is_null() && set_type != PropertyType::Mixed && to_capi(set_type) != value.type) {
+            throw PropertyTypeMismatchException(to_string(set_type), to_string(value.type));
+        }
+
+        if (value.type == realm_value_type::RLM_TYPE_LINK) {
+            if (set.get_realm() != value.link.object->realm()) {
+                throw ObjectManagedByAnotherRealmException("Can't look up index of an object that belongs to a different Realm.");
+            }
+        }
+
+        return set.find_any(from_capi(value));
+    });
+}
+
 }   // extern "C"