realm · JacobOscarGunnarsson · Jan 9, 2023 · Dec 14, 2022 · Dec 15, 2022 · Dec 19, 2022
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -96,6 +96,7 @@
 * Freelist may be corrupted if compaction was initiated ([#6054](https://github.com/realm/realm-core/pull/6054), since v13.0.0)
 
 ### Breaking changes
+* Passing an empty `key_path_array` to `add_notification_callback now` now ignores nested property changes. Pass `std::nullopt` to achieve the old meaning. ([#6122](https://github.com/realm/realm-core/pull/6122))
 * Updated `logger_factory` in SyncClientConfig to return a `shared_ptr` instead of a `unique_ptr` ([PR #5980](https://github.com/realm/realm-core/pull/5980))
 * `util::RootLogger` has been replaced with `util::Logger`
 

diff --git a/src/realm/object-store/c_api/notifications.cpp b/src/realm/object-store/c_api/notifications.cpp
@@ -43,10 +43,10 @@ struct CollectionNotificationsCallback {
     }
 };
 
-KeyPathArray build_key_path_array(realm_key_path_array_t* key_path_array)
+std::optional<KeyPathArray> build_key_path_array(realm_key_path_array_t* key_path_array)
 {
-    KeyPathArray ret;
     if (key_path_array) {
+        KeyPathArray ret;
         for (size_t i = 0; i < key_path_array->nb_elements; i++) {
             realm_key_path_t* key_path = key_path_array->paths + i;
             ret.emplace_back();
@@ -56,8 +56,9 @@ KeyPathArray build_key_path_array(realm_key_path_array_t* key_path_array)
                 kp.emplace_back(TableKey(path_elem->object), ColKey(path_elem->property));
             }
         }
+        return ret;
     }
-    return ret;
+    return std::nullopt;
 }
 
 } // namespace

diff --git a/src/realm/object-store/collection.cpp b/src/realm/object-store/collection.cpp
@@ -219,7 +219,7 @@ util::Optional<Mixed> Collection::average(ColKey col) const
 }
 
 NotificationToken Collection::add_notification_callback(CollectionChangeCallback callback,
-                                                        KeyPathArray key_path_array) &
+                                                        std::optional<KeyPathArray> key_path_array) &
 {
     verify_attached();
     m_realm->verify_notifications_available();

diff --git a/src/realm/object-store/collection.hpp b/src/realm/object-store/collection.hpp
@@ -122,7 +122,7 @@ class Collection {
      * @return A `NotificationToken` that is used to identify this callback.
      */
     NotificationToken add_notification_callback(CollectionChangeCallback callback,
-                                                KeyPathArray key_path_array = {}) &;
+                                                std::optional<KeyPathArray> key_path_array = std::nullopt) &;
 
     // The object being added to the collection is already a managed embedded object
     struct InvalidEmbeddedOperationException : public std::logic_error {

diff --git a/src/realm/object-store/impl/collection_notifier.cpp b/src/realm/object-store/impl/collection_notifier.cpp
@@ -103,14 +103,14 @@ void CollectionNotifier::recalculate_key_path_array()
     m_any_callbacks_filtered = false;
     m_key_path_array.clear();
     for (const auto& callback : m_callbacks) {
-        if (callback.key_path_array.empty()) {
+        if (!callback.key_path_array) {
             m_all_callbacks_filtered = false;
         }
         else {
             m_any_callbacks_filtered = true;
-        }
-        for (const auto& key_path : callback.key_path_array) {
-            m_key_path_array.push_back(key_path);
+            for (const auto& key_path : *callback.key_path_array) {
+                m_key_path_array.push_back(key_path);
+            }
         }
     }
 }
@@ -151,19 +151,20 @@ void CollectionNotifier::release_data() noexcept
 static bool all_have_filters(std::vector<NotificationCallback> const& callbacks) noexcept
 {
     return std::all_of(callbacks.begin(), callbacks.end(), [](auto& cb) {
-        return !cb.key_path_array.empty();
+        return !cb.key_path_array;
     });
 }
 
-uint64_t CollectionNotifier::add_callback(CollectionChangeCallback callback, KeyPathArray key_path_array)
+uint64_t CollectionNotifier::add_callback(CollectionChangeCallback callback,
+                                          std::optional<KeyPathArray> key_path_array)
 {
     m_realm->verify_thread();
 
     util::CheckedLockGuard lock(m_callback_mutex);
     // If we're adding a callback with a keypath filter or if previously all
     // callbacks had filters but this one doesn't we will need to recalculate
     // the related tables on the background thread.
-    if (!key_path_array.empty() || all_have_filters(m_callbacks)) {
+    if (!key_path_array || all_have_filters(m_callbacks)) {
         m_did_modify_callbacks = true;
     }
 
@@ -202,7 +203,7 @@ void CollectionNotifier::remove_callback(uint64_t token)
         // If we're removing a callback with a keypath filter or the last callback
         // without a keypath filter we will need to recalcuate the related tables
         // on next run.
-        if (!old.key_path_array.empty() || all_have_filters(m_callbacks)) {
+        if (!old.key_path_array || all_have_filters(m_callbacks)) {
             m_did_modify_callbacks = true;
         }
 

diff --git a/src/realm/object-store/impl/collection_notifier.hpp b/src/realm/object-store/impl/collection_notifier.hpp
@@ -51,9 +51,11 @@ struct NotificationCallback {
     // target thread.
     CollectionChangeBuilder changes_to_deliver;
     // The filter that this `NotificationCallback` is restricted to.
+    // if std::nullopt, then no restriction is enforced.
+    // if empty, then modifications to objects within the collection won't fire notifications.
     // If not empty, modifications of elements not part of the `key_path_array`
     // will not invoke a notification.
-    KeyPathArray key_path_array = {};
+    std::optional<KeyPathArray> key_path_array = std::nullopt;
     // A unique-per-notifier identifier used to unregister the callback.
     uint64_t token = 0;
     // We normally want to skip calling the callback if there's no changes,
@@ -94,7 +96,8 @@ class CollectionNotifier {
      *
      * @return A token which can be passed to `remove_callback()`.
      */
-    uint64_t add_callback(CollectionChangeCallback callback, KeyPathArray key_path_array) REQUIRES(!m_callback_mutex);
+    uint64_t add_callback(CollectionChangeCallback callback, std::optional<KeyPathArray> key_path_array)
+        REQUIRES(!m_callback_mutex);
 
     /**
      * Remove a previously added token.

diff --git a/src/realm/object-store/object.cpp b/src/realm/object-store/object.cpp
@@ -155,7 +155,8 @@ Object::Object(Object&&) = default;
 Object& Object::operator=(Object const&) = default;
 Object& Object::operator=(Object&&) = default;
 
-NotificationToken Object::add_notification_callback(CollectionChangeCallback callback, KeyPathArray key_path_array) &
+NotificationToken Object::add_notification_callback(CollectionChangeCallback callback,
+                                                    std::optional<KeyPathArray> key_path_array) &
 {
     verify_attached();
     m_realm->verify_notifications_available();

diff --git a/src/realm/object-store/object.hpp b/src/realm/object-store/object.hpp
@@ -127,7 +127,7 @@ class Object {
      * callback via `remove_callback`.
      */
     NotificationToken add_notification_callback(CollectionChangeCallback callback,
-                                                KeyPathArray key_path_array = {}) &;
+                                                std::optional<KeyPathArray> key_path_array = std::nullopt) &;
 
     template <typename ValueType>
     void set_column_value(StringData prop_name, ValueType&& value)

diff --git a/src/realm/object-store/results.cpp b/src/realm/object-store/results.cpp
@@ -1005,7 +1005,8 @@ void Results::prepare_async(ForCallback force) NO_THREAD_SAFETY_ANALYSIS
     _impl::RealmCoordinator::register_notifier(m_notifier);
 }
 
-NotificationToken Results::add_notification_callback(CollectionChangeCallback callback, KeyPathArray key_path_array) &
+NotificationToken Results::add_notification_callback(CollectionChangeCallback callback,
+                                                     std::optional<KeyPathArray> key_path_array) &
 {
     prepare_async(ForCallback{true});
     return {m_notifier, m_notifier->add_callback(std::move(callback), std::move(key_path_array))};

diff --git a/src/realm/object-store/results.hpp b/src/realm/object-store/results.hpp
@@ -304,7 +304,7 @@ class Results {
      * callback via `remove_callback`.
      */
     NotificationToken add_notification_callback(CollectionChangeCallback callback,
-                                                KeyPathArray key_path_array = {}) &;
+                                                std::optional<KeyPathArray> key_path_array = std::nullopt) &;
 
     // Returns whether the rows are guaranteed to be in table order.
     bool is_in_table_order() const;

diff --git a/src/realm/object-store/sectioned_results.cpp b/src/realm/object-store/sectioned_results.cpp
@@ -306,7 +306,7 @@ size_t ResultsSection::size()
 }
 
 NotificationToken ResultsSection::add_notification_callback(SectionedResultsNotificatonCallback callback,
-                                                            KeyPathArray key_path_array) &
+                                                            std::optional<KeyPathArray> key_path_array) &
 {
     return m_parent->add_notification_callback_for_section(m_key, std::move(callback), key_path_array);
 }
@@ -445,14 +445,14 @@ ResultsSection SectionedResults::operator[](Mixed key)
 }
 
 NotificationToken SectionedResults::add_notification_callback(SectionedResultsNotificatonCallback callback,
-                                                              KeyPathArray key_path_array) &
+                                                              std::optional<KeyPathArray> key_path_array) &
 {
     return m_results.add_notification_callback(SectionedResultsNotificationHandler(*this, std::move(callback)),
                                                std::move(key_path_array));
 }
 
 NotificationToken SectionedResults::add_notification_callback_for_section(
-    Mixed section_key, SectionedResultsNotificatonCallback callback, KeyPathArray key_path_array)
+    Mixed section_key, SectionedResultsNotificatonCallback callback, std::optional<KeyPathArray> key_path_array)
 {
     return m_results.add_notification_callback(
         SectionedResultsNotificationHandler(*this, std::move(callback), section_key), std::move(key_path_array));

diff --git a/src/realm/object-store/sectioned_results.hpp b/src/realm/object-store/sectioned_results.hpp
@@ -81,7 +81,7 @@ class ResultsSection {
      * callback via `remove_callback`.
      */
     NotificationToken add_notification_callback(SectionedResultsNotificatonCallback callback,
-                                                KeyPathArray key_path_array = {}) &;
+                                                std::optional<KeyPathArray> key_path_array = std::nullopt) &;
 
     bool is_valid() const;
 
@@ -139,7 +139,7 @@ class SectionedResults {
      * callback via `remove_callback`.
      */
     NotificationToken add_notification_callback(SectionedResultsNotificatonCallback callback,
-                                                KeyPathArray key_path_array = {}) &;
+                                                std::optional<KeyPathArray> key_path_array = std::nullopt) &;
 
     /// Return a new instance of SectionedResults that uses a snapshot of the underlying `Results`.
     /// The section key callback parameter will never be invoked.
@@ -186,9 +186,9 @@ class SectionedResults {
     void calculate_sections_if_required() REQUIRES(m_mutex);
     void calculate_sections() REQUIRES(m_mutex);
     bool m_has_performed_initial_evalutation = false;
-    NotificationToken add_notification_callback_for_section(Mixed section_key,
-                                                            SectionedResultsNotificatonCallback callback,
-                                                            KeyPathArray key_path_array = {});
+    NotificationToken
+    add_notification_callback_for_section(Mixed section_key, SectionedResultsNotificatonCallback callback,
+                                          std::optional<KeyPathArray> key_path_array = std::nullopt);
 
     friend class realm::ResultsSection;
     Results m_results;

diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp
@@ -937,7 +937,8 @@ TEMPLATE_TEST_CASE("dictionary of objects", "[dictionary][links]", cf::MixedVal,
         Obj target_obj = target->create_object().set(col_target_value, T(values[i]));
         dict.insert(keys[i], target_obj);
     }
-
+    r->commit_transaction();
+    r->begin_transaction();
     SECTION("min()") {
         if (!TestType::
3D11
can_minmax()) {
             REQUIRE_THROWS_AS(dict.min(col_target_value), Results::UnsupportedColumnTypeException);
@@ -1347,3 +1348,85 @@ TEST_CASE("dictionary aggregate", "[dictionary]") {
     auto sum = res.sum("intCol");
     REQUIRE(*sum == 16);
 }
+
+TEST_CASE("callback with empty keypatharray") {
+    InMemoryTestFile config;
+    config.schema = Schema{
+        {"object", {{"links", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, "target"}}},
+        {"target", {{"value", PropertyType::Int}}},
+    };
+
+    auto r = Realm::get_shared_realm(config);
+    auto table = r->read_group().get_table("class_object");
+    auto target = r->read_group().get_table("class_target");
+
+    r->begin_transaction();
+    Obj obj = table->create_object();
+    ColKey col_links = table->get_column_key("links");
+    ColKey col_target_value = target->get_column_key("value");
+    object_store::Dictionary dict(r, obj, col_links);
+    auto key = "key";
+    Obj target_obj = target->create_object().set(col_target_value, 1);
+    dict.insert(key, target_obj);
+    r->commit_transaction();
+
+    CollectionChangeSet change;
+    auto write = [&](auto&& f) {
+        r->begin_transaction();
+        f();
+        r->commit_transaction();
+        advance_and_notify(*r);
+    };
+
+    auto shallow_require_change = [&] {
+        auto token = dict.add_notification_callback(
+            [&](CollectionChangeSet c) {
+                change = c;
+            },
+            KeyPathArray());
+        advance_and_notify(*r);
+        return token;
+    };
+
+    auto shallow_require_no_change = [&] {
+        bool first = true;
+        auto token = dict.add_notification_callback(
+            [&first](CollectionChangeSet) mutable {
+                REQUIRE(first);
+                first = false;
+            },
+            KeyPathArray());
+        advance_and_notify(*r);
+        return token;
+    };
+
+    SECTION("insertion DOES send notification") {
+        auto token = shallow_require_change();
+        write([&] {
+            Obj target_obj = target->create_object().set(col_target_value, 1);
+            dict.insert("foo", target_obj);
+        });
+        REQUIRE_FALSE(change.insertions.empty());
+    }
+    SECTION("deletion DOES send notification") {
+        auto token = shallow_require_change();
+        write([&] {
+            dict.erase(key);
+        });
+        REQUIRE_FALSE(change.deletions.empty());
+    }
+    SECTION("replacement DOES send notification") {
+        auto token = shallow_require_change();
+        write([&] {
+            Obj target_obj = target->create_object().set(col_target_value, 1);
+            dict.insert(key, target_obj);
+        });
+        REQUIRE_FALSE(change.modifications.empty());
+    }
+    SECTION("modification does NOT send notification") {
+        auto token = shallow_require_no_change();
+        write([&] {
+            dict.get<Obj>(key).set(col_target_value, 2);
+        });
+    }
+}
diff --git a/test/object-store/list.cpp b/test/object-store/list.cpp
@@ -712,6 +712,7 @@ TEST_CASE("list") {
         // - some callbacks have filters
         // - all callbacks have filters
         CollectionChangeSet collection_change_set_without_filter;
+        CollectionChangeSet collection_change_set_with_empty_filter;
         CollectionChangeSet collection_change_set_with_filter_on_target_value;
 
         // Note that in case not all callbacks have filters we do accept false positive notifications by design.
@@ -807,6 +808,73 @@ TEST_CASE("list") {
             }
         }
 
+        SECTION("callback with empty keypatharray") {
+            auto shallow_require_change = [&] {
+                auto token = list.add_notification_callback(
+                    [&](CollectionChangeSet c) {
+                        collection_change_set_with_empty_filter = c;
+                    },
+                    KeyPathArray());
+                advance_and_notify(*r);
+                return token;
+            };
+
+            auto shallow_require_no_change = [&] {
+                bool first = true;
+                auto token = list.add_notification_callback(
+                    [&first](CollectionChangeSet) mutable {
+                        REQUIRE(first);
+                        first = false;
+                    },
+                    KeyPathArray());
+                advance_and_notify(*r);
+                return token;
+            };
+
+            SECTION("modifying table 'target', property 'value' "
+                    "-> does NOT send a notification for 'value'") {
+                auto token = shallow_require_no_change();
+                write([&] {
+                    list.get(0).set(col_target_value, 42);
+                });
+            }
+
+            SECTION("modifying table 'target', property 'value' "
+                    "-> does NOT send a notification for 'value'") {
+                auto token = shallow_require_no_change();
+                write([&] {
+                    list.get(0).set(col_target_value, 42);
+                });
+            }
+
+            SECTION("deleting a target row with shallow listener sends a change notification") {
+                auto token = shallow_require_change();
+                write([&] {
+                    list.remove(5);
+                });
+                REQUIRE_INDICES(collection_change_set_with_empty_filter.deletions, 5);
+            }
+
+            SECTION("adding a row with shallow listener sends a change notifcation") {
+                auto token = shallow_require_change();
+                write([&] {
+                    Obj obj = target->get_object(target_keys[5]);
+                    list.add(obj);
+                });
+                REQUIRE_INDICES(collection_change_set_with_empty_filter.insertions, 10);
+            }
+
+            SECTION("moving a row with shallow listener sends a change notifcation") {
+                auto token = shallow_require_change();
+                write([&] {
+                    list.move(5, 8);
+                });
+                REQUIRE_INDICES(collection_change_set_with_empty_filter.insertions, 8);
+                REQUIRE_INDICES(collection_change_set_with_empty_filter.deletions, 5);
+                REQUIRE_MOVES(collection_change_set_with_empty_filter, {5, 8});
+            }
+        }
+
         SECTION("linked filter") {
             CollectionChangeSet collection_change_set_linked_filter;
             Object object(r, obj);