Add ability to get path to modified collections in object notifications

jedelbo · jedelbo · commit f92baa3e4549 · 2024-02-22T15:36:57.000+01:00
diff --git a/src/realm.h b/src/realm.h
@@ -1953,6 +1953,13 @@ RLM_API bool realm_object_changes_is_deleted(const realm_object_changes_t*);
  */
 RLM_API size_t realm_object_changes_get_num_modified_properties(const realm_object_changes_t*);
 
+/**
+ * Get the number of paths to embedded collections that were modified.
+ *
+ * This function cannot fail.
+ */
+RLM_API size_t realm_object_changes_get_num_modified_paths(const realm_object_changes_t*);
+
 /**
  * Get the column keys for the properties that were modified in an object
  * notification.
@@ -1967,6 +1974,20 @@ RLM_API size_t realm_object_changes_get_num_modified_properties(const realm_obje
 RLM_API size_t realm_object_changes_get_modified_properties(const realm_object_changes_t*,
                                                             realm_property_key_t* out_modified, size_t max);
 
+/**
+ * Get the column keys for the properties that were modified in an object
+ * notification.
+ *
+ * This function cannot fail.
+ *
+ * @param out_modified Where the paths should be written. May be NULL.
+ * @param max The maximum number of paths to write.
+ * @return The number of paths written to @a out_modified, or the number
+ *         of modified paths if @a out_modified is NULL.
+ */
+RLM_API size_t realm_object_changes_get_modified_paths(const realm_object_changes_t*, realm_string_t* out_modified,
+                                                       size_t max);
+
 /**
  * Get the number of various types of changes in a collection notification.
  *
diff --git a/src/realm/collection.hpp b/src/realm/collection.hpp
@@ -39,6 +39,7 @@ class DummyParent : public CollectionParent {
     {
         return {};
     }
+    void translate_path(const StablePath&, Path&) const final {}
     void add_index(Path&, const Index&) const noexcept final {}
     size_t find_index(const Index&) const noexcept final
     {
diff --git a/src/realm/collection_parent.hpp b/src/realm/collection_parent.hpp
@@ -95,6 +95,8 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
     /// Get table of owning object
     virtual TableRef get_table() const noexcept = 0;
 
+    virtual void translate_path(const StablePath&, Path&) const = 0;
+
 protected:
     friend class Collection;
     template <class>
diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp
@@ -638,6 +638,28 @@ Dictionary::Iterator Dictionary::find(Mixed key) const noexcept
     return end();
 }
 
+
+void Dictionary::translate_path(const StablePath& stable_path, Path& path) const
+{
+    auto& index = stable_path[m_level];
+    auto ndx = find_index(index);
+    StringData key = do_get_key(ndx).get_string();
+    path.emplace_back(key);
+    if (stable_path.size() > m_level + 1) {
+        Mixed val = do_get(ndx);
+        if (val.is_type(type_Dictionary)) {
+            DummyParent parent(this->get_table(), val.get_ref());
+            Dictionary dict(parent, 0);
+            dict.translate_path(stable_path, path);
+        }
+        else if (val.is_type(type_List)) {
+            DummyParent parent(this->get_table(), val.get_ref());
+            Lst<Mixed> list(parent, 0);
+            list.translate_path(stable_path, path);
+        }
+    }
+}
+
 void Dictionary::add_index(Path& path, const Index& index) const
 {
     auto ndx = m_values->find_key(index.get_salt());
diff --git a/src/realm/dictionary.hpp b/src/realm/dictionary.hpp
@@ -205,6 +205,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
     {
         return Base::get_stable_path();
     }
+    void translate_path(const StablePath& stable_path, Path& path) const final;
 
     void add_index(Path& path, const Index& ndx) const final;
     size_t find_index(const Index&) const final;
diff --git a/src/realm/list.cpp b/src/realm/list.cpp
@@ -790,6 +790,26 @@ void Lst<Mixed>::set_collection_ref(Index index, ref_type ref, CollectionType ty
     m_tree->set(ndx, Mixed(ref, type));
 }
 
+void Lst<Mixed>::translate_path(const StablePath& stable_path, Path& path) const
+{
+    auto& index = stable_path[m_level];
+    auto ndx = find_index(index);
+    path.emplace_back(ndx);
+    if (stable_path.size() > m_level + 1) {
+        Mixed val = get(ndx);
+        if (val.is_type(type_Dictionary)) {
+            DummyParent parent(this->get_table(), val.get_ref());
+            Dictionary dict(parent, 0);
+            dict.translate_path(stable_path, path);
+        }
+        else if (val.is_type(type_List)) {
+            DummyParent parent(this->get_table(), val.get_ref());
+            Lst<Mixed> list(parent, 0);
+            list.translate_path(stable_path, path);
+        }
+    }
+}
+
 void Lst<Mixed>::add_index(Path& path, const Index& index) const
 {
     auto ndx = m_tree->find_key(index.get_salt());
diff --git a/src/realm/list.hpp b/src/realm/list.hpp
@@ -484,6 +484,7 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
     {
         return Base::get_stable_path();
     }
+    void translate_path(const StablePath& stable_path, Path& path) const final;
 
     ColKey get_col_key() const noexcept override
     {
diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp
@@ -2079,6 +2079,16 @@ CollectionPtr Obj::get_collection_ptr(const Path& path) const
     return collection;
 }
 
+void Obj::translate_path(const StablePath& stable_path, Path& path) const
+{
+    ColKey col_key = m_table->get_column_key(stable_path[0]);
+    path.emplace_back(m_table->get_column_name(col_key));
+    if (stable_path.size() > 1) {
+        CollectionBasePtr collection = get_collection_ptr(col_key);
+        dynamic_cast<CollectionParent*>(collection.get())->translate_path(stable_path, path);
+    }
+}
+
 CollectionPtr Obj::get_collection_by_stable_path(const StablePath& path) const
 {
     // First element in path is phony column key
diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp
@@ -78,6 +78,7 @@ class Obj : public CollectionParent {
     Path get_short_path() const noexcept final;
     ColKey get_col_key() const noexcept final;
     StablePath get_stable_path() const noexcept final;
+    void translate_path(const StablePath&, Path&) const final;
     void add_index(Path& path, const Index& ndx) const final;
     size_t find_index(const Index&) const final
     {
diff --git a/src/realm/object-store/binding_context.hpp b/src/realm/object-store/binding_context.hpp
@@ -165,7 +165,7 @@ class BindingContext {
         // Populated with information about which columns were changed
         // May be shorter than the actual number of columns if the later columns
         // are not modified
-        std::unordered_map<int64_t, ColumnInfo> changes;
+        std::unordered_map<ColKey, ColumnInfo> changes;
 
         // Simple lexographic ordering
         friend bool operator<(ObserverState const& lft, ObserverState const& rgt)
diff --git a/src/realm/object-store/c_api/notifications.cpp b/src/realm/object-store/c_api/notifications.cpp
@@ -113,6 +113,11 @@ RLM_API size_t realm_object_changes_get_num_modified_properties(const realm_obje
     return changes->columns.size();
 }
 
+RLM_API size_t realm_object_changes_get_num_modified_paths(const realm_object_changes_t* changes)
+{
+    return changes->modified_paths.size();
+}
+
 RLM_API size_t realm_object_changes_get_modified_properties(const realm_object_changes_t* changes,
                                                             realm_property_key_t* out_properties, size_t max)
 {
@@ -130,6 +135,39 @@ RLM_API size_t realm_object_changes_get_modified_properties(const realm_object_c
     return i;
 }
 
+RLM_API size_t realm_object_changes_get_modified_paths(const realm_object_changes_t* const_changes,
+                                                       realm_string_t* out_paths, size_t max)
+{
+    if (!out_paths)
+        return const_changes->modified_paths.size();
+
+    realm_object_changes_t* changes = const_cast<realm_object_changes_t*>(const_changes);
+    changes->path_buffer.resize(changes->modified_paths.size());
+    size_t i = 0;
+    for (const auto& p : changes->modified_paths) {
+        if (i >= max) {
+            break;
+        }
+        std::string& path = changes->path_buffer[i];
+        path = p[0].get_key();
+        for (auto path_elem = p.begin() + 1; path_elem != p.end(); ++path_elem) {
+            if (path_elem->is_key()) {
+                path += '.';
+                path += path_elem->get_key();
+            }
+            else {
+                char buffer[10];
+                sprintf(buffer, "[%u]", unsigned(path_elem->get_ndx()));
+                path += buffer;
+            }
+        }
+        out_paths[i].data = path.data();
+        out_paths[i].size = path.size();
+        ++i;
+    }
+    return i;
+}
+
 RLM_API realm_notification_token_t* realm_list_add_notification_callback(realm_list_t* list,
                                                                          realm_userdata_t userdata,
                                                                          realm_free_userdata_func_t free,
diff --git a/src/realm/object-store/c_api/types.hpp b/src/realm/object-store/c_api/types.hpp
@@ -428,6 +428,7 @@ struct realm_object_changes : realm::c_api::WrapC, realm::CollectionChangeSet {
     {
         return new realm_object_changes{static_cast<const realm::CollectionChangeSet&>(*this)};
     }
+    std::vector<std::string> path_buffer;
 };
 
 struct realm_collection_changes : realm::c_api::WrapC, realm::CollectionChangeSet {
diff --git a/src/realm/object-store/collection_notifications.hpp b/src/realm/object-store/collection_notifications.hpp
@@ -105,8 +105,9 @@ struct CollectionChangeSet {
 
     // Per-column version of `modifications`
     std::unordered_map<int64_t, IndexSet> columns;
+    std::vector<Path> modified_paths;
 
-    std::set<StablePath> paths;
+    std::set<StablePath> stable_paths;
 
     bool empty() const noexcept
     {
diff --git a/src/realm/object-store/impl/collection_change_builder.cpp b/src/realm/object-store/impl/collection_change_builder.cpp
@@ -704,5 +704,5 @@ CollectionChangeSet CollectionChangeBuilder::finalize() &&
 
     return {std::move(deletions),     std::move(insertions), std::move(modifications_in_old),
             std::move(modifications), std::move(moves),      collection_root_was_deleted,
-            collection_was_cleared,   std::move(columns)};
+            collection_was_cleared,   std::move(columns),    std::move(modified_paths)};
 }
diff --git a/src/realm/object-store/impl/list_notifier.cpp b/src/realm/object-store/impl/list_notifier.cpp
@@ -142,9 +142,9 @@ void ListNotifier::run()
         }
     }
 
-    if (m_change.paths.size()) {
+    if (m_change.stable_paths.size()) {
         if (auto coll = dynamic_cast<CollectionParent*>(m_list.get())) {
-            for (auto& p : m_change.paths) {
+            for (auto& p : m_change.stable_paths) {
                 // Report changes in substructure as modifications on this list
                 auto ndx = coll->find_index(p[0]);
                 if (ndx != realm::not_found)
diff --git a/src/realm/object-store/impl/object_notifier.cpp b/src/realm/object-store/impl/object_notifier.cpp
@@ -109,13 +109,19 @@ void ObjectNotifier::run()
 
     const auto& change = it->second;
 
-    auto column_modifications = change.get_columns_modified(m_obj_key);
-    if (!column_modifications)
+    auto path_modifications = change.get_paths_modified(m_obj_key);
+    if (!path_modifications)
         return;
 
     // Finally we add all changes to `m_change` which is later used to notify about the changed columns.
     m_change.modifications.add(0);
-    for (auto col : *column_modifications) {
-        m_change.columns[col.value].add(0);
+    auto obj = m_table->get_object(m_obj_key);
+    for (const StablePath& stable_path : *path_modifications) {
+        m_change.columns[m_table->get_column_key(stable_path[0]).value].add(0);
+        if (stable_path.size() > 1) {
+            Path path;
+            obj.translate_path(stable_path, path);
+            m_change.modified_paths.emplace_back(std::move(path));
+        }
     }
 }
diff --git a/src/realm/object-store/impl/results_notifier.cpp b/src/realm/object-store/impl/results_notifier.cpp
@@ -393,9 +393,9 @@ void ListResultsNotifier::run()
         std::iota(m_run_indices->begin(), m_run_indices->end(), 0);
     }
 
-    if (m_change.paths.size()) {
+    if (m_change.stable_paths.size()) {
         if (auto coll = dynamic_cast<CollectionParent*>(m_list.get())) {
-            for (auto& p : m_change.paths) {
+            for (auto& p : m_change.stable_paths) {
                 // Report changes in substructure as modifications on this list
                 auto ndx = coll->find_index(p[0]);
                 if (ndx != realm::not_found)
diff --git a/src/realm/object-store/impl/transact_log_handler.cpp b/src/realm/object-store/impl/transact_log_handler.cpp
@@ -110,10 +110,10 @@ void KVOAdapter::before(Transaction& sg)
             m_invalidated.push_back(observer.info);
             continue;
         }
-        auto column_modifications = table.get_columns_modified(key);
-        if (column_modifications) {
-            for (auto col : *column_modifications) {
-                observer.changes[col.value].kind = BindingContext::ColumnInfo::Kind::Set;
+        auto tbl = sg.get_table(observer.table_key);
+        if (auto path_modifications = table.get_paths_modified(key)) {
+            for (const StablePath& path : *path_modifications) {
+                observer.changes[tbl->get_column_key(path[0])].kind = BindingContext::ColumnInfo::Kind::Set;
             }
         }
     }
@@ -123,7 +123,7 @@ void KVOAdapter::before(Transaction& sg)
             // We may have pre-emptively marked the column as modified if the
             // LinkList was selected but the actual changes made ended up being
             // a no-op
-            list.observer->changes.erase(list.col_key.value);
+            list.observer->changes.erase(list.col_key);
             continue;
         }
         // If the containing row was deleted then changes will be empty
@@ -132,7 +132,7 @@ void KVOAdapter::before(Transaction& sg)
             continue;
         }
         // otherwise the column should have been marked as modified
-        auto it = list.observer->changes.find(list.col_key.value);
+        auto it = list.observer->changes.find(list.col_key);
         REALM_ASSERT(it != list.observer->changes.end());
         auto& builder = list.builder;
         auto& changes = it->second;
@@ -364,17 +364,19 @@ class TransactLogObserver : public TransactLogValidationMixin {
         return true;
     }
 
-    bool select_collection(ColKey col, ObjKey obj, const StablePath& path)
+    bool select_collection(ColKey, ObjKey obj, const StablePath& path)
     {
-        modify_object(col, obj);
+        if (m_active_table) {
+            m_active_table->modifications_add(obj, path);
+        }
         auto table = current_table();
         for (auto& c : m_info.collections) {
 
             if (c.table_key == table && c.obj_key == obj && c.path.is_prefix_of(path)) {
                 if (c.path.size() != path.size()) {
                     StablePath sub_path;
                     sub_path.insert(sub_path.begin(), path.begin() + c.path.size(), path.end());
-                    c.changes->paths.insert(std::move(sub_path));
+                    c.changes->stable_paths.insert(std::move(sub_path));
                 }
                 else {
                     m_active_collection = c.changes;
@@ -465,8 +467,11 @@ class TransactLogObserver : public TransactLogValidationMixin {
 
     bool modify_object(ColKey col, ObjKey key)
     {
-        if (m_active_table)
-            m_active_table->modifications_add(key, col);
+        if (m_active_table) {
+            StablePath path;
+            path.push_back(StableIndex(col, 0));
+            m_active_table->modifications_add(key, path);
+        }
         return true;
     }
 
diff --git a/src/realm/object-store/object_changeset.cpp b/src/realm/object-store/object_changeset.cpp
@@ -25,11 +25,11 @@ void ObjectChangeSet::insertions_add(ObjKey obj)
     m_insertions.insert(obj);
 }
 
-void ObjectChangeSet::modifications_add(ObjKey obj, ColKey col)
+void ObjectChangeSet::modifications_add(ObjKey obj, const StablePath& path)
 {
     // don't report modifications on new objects
     if (m_insertions.find(obj) == m_insertions.end()) {
-        m_modifications[obj].insert(col);
+        m_modifications[obj].insert(path);
     }
 }
 
@@ -82,17 +82,21 @@ bool ObjectChangeSet::modifications_contains(ObjKey obj, const std::vector<ColKe
     }
 
     // If a filter was set we need to check if the changed column is part of this filter.
-    const std::unordered_set<ColKey>& changed_columns_for_object = m_modifications.at(obj);
+    const std::set<StablePath>& changed_paths_for_object = m_modifications.at(obj);
     for (const auto& column_key_in_filter : filtered_column_keys) {
-        if (changed_columns_for_object.count(column_key_in_filter)) {
+        auto it =
+            std::find_if(changed_paths_for_object.begin(), changed_paths_for_object.end(), [&](const StablePath& p) {
+                return p[0].get_index().val == column_key_in_filter.get_index().val;
+            });
+        if (it != changed_paths_for_object.end()) {
             return true;
         }
     }
 
     return false;
 }
 
-const ObjectChangeSet::ColumnSet* ObjectChangeSet::get_columns_modified(ObjKey obj) const
+const ObjectChangeSet::PathSet* ObjectChangeSet::get_paths_modified(ObjKey obj) const
 {
     auto it = m_modifications.find(obj);
     if (it == m_modifications.end()) {
diff --git a/src/realm/object-store/object_changeset.hpp b/src/realm/object-store/object_changeset.hpp
diff --git a/test/object-store/c_api/c_api.cpp b/test/object-store/c_api/c_api.cpp
diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp
diff --git a/test/object-store/realm.cpp b/test/object-store/realm.cpp
diff --git a/test/object-store/transaction_log_parsing.cpp b/test/object-store/transaction_log_parsing.cpp

Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@ class DummyParent : public CollectionParent {`
`39`	`39`	`{`
`40`	`40`	`return {};`
`41`	`41`	`}`
	`42`	`+ void translate_path(const StablePath&, Path&) const final {}`
`42`	`43`	`void add_index(Path&, const Index&) const noexcept final {}`
`43`	`44`	`size_t find_index(const Index&) const noexcept final`
`44`	`45`	`{`
Original file line number	Diff line number	Diff line change
`@@ -205,6 +205,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle`
`205`	`205`	`{`
`206`	`206`	`return Base::get_stable_path();`
`207`	`207`	`}`
	`208`	`+ void translate_path(const StablePath& stable_path, Path& path) const final;`
`208`	`209`
`209`	`210`	`void add_index(Path& path, const Index& ndx) const final;`
`210`	`211`	`size_t find_index(const Index&) const final;`
Original file line number	Diff line number	Diff line change
`@@ -484,6 +484,7 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa`
`484`	`484`	`{`
`485`	`485`	`return Base::get_stable_path();`
`486`	`486`	`}`
	`487`	`+ void translate_path(const StablePath& stable_path, Path& path) const final;`
`487`	`488`
`488`	`489`	`ColKey get_col_key() const noexcept override`
`489`	`490`	`{`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,7 @@ class Obj : public CollectionParent {`
`78`	`78`	`Path get_short_path() const noexcept final;`
`79`	`79`	`ColKey get_col_key() const noexcept final;`
`80`	`80`	`StablePath get_stable_path() const noexcept final;`
	`81`	`+ void translate_path(const StablePath&, Path&) const final;`
`81`	`82`	`void add_index(Path& path, const Index& ndx) const final;`
`82`	`83`	`size_t find_index(const Index&) const final`
`83`	`84`	`{`