Add ability to get path to modified collections in object notifications

jedelbo · jedelbo · commit 5f8cca1fb25f · 2024-03-13T09:41:07.000+01:00
diff --git a/src/realm.h b/src/realm.h
@@ -1946,6 +1946,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.
@@ -1960,6 +1967,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
@@ -90,6 +90,7 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
     // Return path from owning object
     virtual StablePath get_stable_path() const = 0;
     // Add a translation of Index to PathElement
+    virtual void translate_path(const StablePath&, Path&) const = 0;
     virtual void add_index(Path& path, const Index& ndx) const = 0;
     // Return position of Index held by child
     virtual size_t find_index(const Index& ndx) const = 0;
@@ -110,9 +111,9 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
     friend class CollectionList;
 
 #ifdef REALM_DEBUG
-    static constexpr size_t s_max_level = 4;
+    static constexpr int s_max_level = 4;
 #else
-    static constexpr size_t s_max_level = 100;
+    static constexpr int s_max_level = 100;
 #endif
     uint8_t m_level = 0;
 
diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp
@@ -634,6 +634,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() > size_t(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
@@ -202,6 +202,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
@@ -763,6 +763,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() > size_t(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
@@ -466,6 +466,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
@@ -2090,6 +2090,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
@@ -72,6 +72,7 @@ class Obj {
     Path get_short_path() const noexcept;
     ColKey get_col_key() const noexcept;
     StablePath get_stable_path() const noexcept;
+    void translate_path(const StablePath&, Path&) const;
     void add_index(Path& path, const CollectionParent::Index& ndx) const;
 
     TableRef get_table() const noexcept
@@ -441,6 +442,10 @@ class ObjCollectionParent final : public Obj, public CollectionParent {
     {
         return Obj::get_stable_path();
     }
+    void translate_path(const StablePath& stable_path, Path& path) const override
+    {
+        Obj::translate_path(stable_path, path);
+    }
     void add_index(Path& path, const Index& ndx) const override
     {
         Obj::add_index(path, ndx);
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<StableIndex> paths;
+    std::set<StableIndex> stable_indexes;
 
     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
@@ -132,10 +132,10 @@ void ListNotifier::run()
 
     // Modifications to nested values in Mixed are recorded in replication as
     // StableIndex and we have to look up the actual index afterwards
-    if (m_change.paths.size()) {
+    if (m_change.stable_indexes.size()) {
         REALM_ASSERT(m_collection_parent);
         REALM_ASSERT(m_type == PropertyType::Mixed);
-        for (auto& p : m_change.paths) {
+        for (auto& p : m_change.stable_indexes) {
             if (auto ndx = m_collection_parent->find_index(p); ndx != realm::not_found)
                 m_change.modifications.add(ndx);
         }
diff --git a/src/realm/object-store/impl/object_notifier.cpp b/src/realm/object-store/impl/object_notifier.cpp
@@ -100,13 +100,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
@@ -375,9 +375,9 @@ void ListResultsNotifier::run()
 
     // Modifications to nested values in Mixed are recorded in replication as
     // StableIndex and we have to look up the actual index afterwards
-    if (m_change.paths.size()) {
+    if (m_change.stable_indexes.size()) {
         if (auto coll = dynamic_cast<CollectionParent*>(m_list.get())) {
-            for (auto& p : m_change.paths) {
+            for (auto& p : m_change.stable_indexes) {
                 if (auto ndx = coll->find_index(p); ndx != realm::not_found)
                     m_change.modifications.add(ndx);
             }
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,15 +364,17 @@ 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();
         m_active_collection = nullptr;
         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()) {
-                    c.changes->paths.insert(path[c.path.size()]);
+                    c.changes->stable_indexes.insert(path[c.path.size()]);
                 }
                 // If there are multiple exact matches for this collection we
                 // use the first and then propagate the data to the others later
@@ -463,8 +465,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
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
`@@ -202,6 +202,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle`
`202`	`202`	`{`
`203`	`203`	`return Base::get_stable_path();`
`204`	`204`	`}`
	`205`	`+ void translate_path(const StablePath& stable_path, Path& path) const final;`
`205`	`206`
`206`	`207`	`void add_index(Path& path, const Index& ndx) const final;`
`207`	`208`	`size_t find_index(const Index&) const final;`
Original file line number	Diff line number	Diff line change
`@@ -466,6 +466,7 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa`
`466`	`466`	`{`
`467`	`467`	`return Base::get_stable_path();`
`468`	`468`	`}`
	`469`	`+ void translate_path(const StablePath& stable_path, Path& path) const final;`
`469`	`470`
`470`	`471`	`ColKey get_col_key() const noexcept override`
`471`	`472`	`{`
Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,7 @@ class Obj {`
`72`	`72`	`Path get_short_path() const noexcept;`
`73`	`73`	`ColKey get_col_key() const noexcept;`
`74`	`74`	`StablePath get_stable_path() const noexcept;`
	`75`	`+ void translate_path(const StablePath&, Path&) const;`
`75`	`76`	`void add_index(Path& path, const CollectionParent::Index& ndx) const;`
`76`	`77`
`77`	`78`	`TableRef get_table() const noexcept`
`@@ -441,6 +442,10 @@ class ObjCollectionParent final : public Obj, public CollectionParent {`
`441`	`442`	`{`
`442`	`443`	`return Obj::get_stable_path();`
`443`	`444`	`}`
	`445`	`+ void translate_path(const StablePath& stable_path, Path& path) const override`
	`446`	`+ {`
	`447`	`+ Obj::translate_path(stable_path, path);`
	`448`	`+ }`
`444`	`449`	`void add_index(Path& path, const Index& ndx) const override`
`445`	`450`	`{`
`446`	`451`	`Obj::add_index(path, ndx);`