Rework KeyPathArray filters for notifications in the C-API. (#7087)

* Add support for keypaths crossing backlins --------- Co-authored-by: Jørgen Edelbo <[email protected]>
realm · Nov 23, 2023 · 3bc7ce9 · 3bc7ce9
1 parent 4be34a4
commit 3bc7ce9
Show file tree

Hide file tree

Showing 13 changed files with 189 additions and 114 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
-* None.
+* Refactored how KeyPathArrays are created in the C-API by adding `realm_create_key_path_array` which allows a SDK to pass in the string representation of the keypath and then let Core calculate the correct TableKey/ColKey pairs instead of doing this on the SDK side. (Issue [#7087](https://github.com/realm/realm-core/pull/7087))
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

diff --git a/src/realm.h b/src/realm.h
@@ -65,6 +65,7 @@ typedef void (*realm_free_userdata_func_t)(realm_userdata_t userdata);
 typedef realm_userdata_t (*realm_clone_userdata_func_t)(const realm_userdata_t userdata);
 typedef void (*realm_on_object_store_thread_callback_t)(realm_userdata_t userdata);
 typedef bool (*realm_on_object_store_error_callback_t)(realm_userdata_t userdata, const char*);
+typedef struct realm_key_path_array realm_key_path_array_t;
 
 /* Accessor types */
 typedef struct realm_object realm_object_t;
@@ -201,18 +202,6 @@ typedef struct realm_value {
     } RLM_ANON_UNION_MEMBER(values);
     realm_value_type_e type;
 } realm_value_t;
-typedef struct realm_key_path_elem {
-    realm_class_key_t object;
-    realm_property_key_t property;
-} realm_key_path_elem_t;
-typedef struct realm_key_path {
-    size_t nb_elements;
-    realm_key_path_elem_t* path_elements;
-} realm_key_path_t;
-typedef struct realm_key_path_array {
-    size_t nb_elements;
-    realm_key_path_t* paths;
-} realm_key_path_array_t;
 typedef struct realm_query_arg {
     size_t nb_args;
     bool is_list;
@@ -1606,14 +1595,24 @@ RLM_API realm_class_key_t realm_object_get_table(const realm_object_t* object);
  */
 RLM_API realm_link_t realm_object_as_link(const realm_object_t* object);
 
+/**
+ * Helper method for making it easier to to convert SDK input to the underlying
+ * `realm_key_path_array_t`.
+ *
+ * @return A pointer to the converted key path array. NULL in case of an error.
+ */
+RLM_API realm_key_path_array_t* realm_create_key_path_array(const realm_t* realm,
+                                                            const realm_class_key_t object_class_key,
+                                                            size_t num_key_paths, const char** user_key_paths);
+
 /**
  * Subscribe to notifications for this object.
  *
  * @return A non-null pointer if no exception occurred.
  */
 RLM_API realm_notification_token_t* realm_object_add_notification_callback(realm_object_t*, realm_userdata_t userdata,
                                                                            realm_free_userdata_func_t userdata_free,
-                                                                           realm_key_path_array_t*,
+                                                                           realm_key_path_array_t* key_path_array,
                                                                            realm_on_object_change_func_t on_change);
 
 /**
@@ -1872,7 +1871,7 @@ RLM_API bool realm_list_remove_all(realm_list_t*);
  */
 RLM_API realm_notification_token_t* realm_list_add_notification_callback(realm_list_t*, realm_userdata_t userdata,
                                                                          realm_free_userdata_func_t userdata_free,
-                                                                         realm_key_path_array_t*,
+                                                                         realm_key_path_array_t* key_path_array,
                                                                          realm_on_collection_change_func_t on_change);
 
 /**
@@ -2165,7 +2164,7 @@ RLM_API bool realm_set_remove_all(realm_set_t*);
  */
 RLM_API realm_notification_token_t* realm_set_add_notification_callback(realm_set_t*, realm_userdata_t userdata,
                                                                         realm_free_userdata_func_t userdata_free,
-                                                                        realm_key_path_array_t*,
+                                                                        realm_key_path_array_t* key_path_array,
                                                                         realm_on_collection_change_func_t on_change);
 /**
  * Get an set from a thread-safe reference, potentially originating in a
@@ -2345,10 +2344,9 @@ RLM_API bool realm_dictionary_clear(realm_dictionary_t*);
  *
  * @return A non-null pointer if no exception occurred.
  */
-RLM_API realm_notification_token_t*
-realm_dictionary_add_notification_callback(realm_dictionary_t*, realm_userdata_t userdata,
-                                           realm_free_userdata_func_t userdata_free, realm_key_path_array_t*,
-                                           realm_on_dictionary_change_func_t on_change);
+RLM_API realm_notification_token_t* realm_dictionary_add_notification_callback(
+    realm_dictionary_t*, realm_userdata_t userdata, realm_free_userdata_func_t userdata_free,
+    realm_key_path_array_t* key_path_array, realm_on_dictionary_change_func_t on_change);
 
 /**
  * Get an dictionary from a thread-safe reference, potentially originating in a
@@ -2696,7 +2694,7 @@ RLM_API bool realm_results_average(realm_results_t*, realm_property_key_t, realm
 RLM_API realm_notification_token_t* realm_results_add_notification_callback(realm_results_t*,
                                                                             realm_userdata_t userdata,
                                                                             realm_free_userdata_func_t userdata_free,
-                                                                            realm_key_path_array_t*,
+                                                                            realm_key_path_array_t* key_path_array,
                                                                             realm_on_collection_change_func_t);
 
 /**

diff --git a/src/realm/CMakeLists.txt b/src/realm/CMakeLists.txt
@@ -234,7 +234,6 @@ set(REALM_INSTALL_HEADERS
     util/memory_stream.hpp
     util/misc_errors.hpp
     util/misc_ext_errors.hpp
-    util/miscellaneous.hpp
     util/optional.hpp
     util/overload.hpp
     util/platform_info.hpp

diff --git a/src/realm/alloc_slab.cpp b/src/realm/alloc_slab.cpp
@@ -37,7 +37,6 @@
 
 #include <realm/util/errno.hpp>
 #include <realm/util/encrypted_file_mapping.hpp>
-#include <realm/util/miscellaneous.hpp>
 #include <realm/util/terminate.hpp>
 #include <realm/util/thread.hpp>
 #include <realm/util/scope_exit.hpp>

diff --git a/src/realm/group.cpp b/src/realm/group.cpp
@@ -27,7 +27,6 @@
 
 #include <realm/util/file_mapper.hpp>
 #include <realm/util/memory_stream.hpp>
-#include <realm/util/miscellaneous.hpp>
 #include <realm/util/thread.hpp>
 #include <realm/impl/destroy_guard.hpp>
 #include <realm/utilities.hpp>

diff --git a/src/realm/group_writer.cpp b/src/realm/group_writer.cpp
@@ -28,7 +28,6 @@
 #include <realm/disable_sync_to_disk.hpp>
 #include <realm/impl/destroy_guard.hpp>
 #include <realm/impl/simulated_failure.hpp>
-#include <realm/util/miscellaneous.hpp>
 #include <realm/util/safe_int_ops.hpp>
 
 using namespace realm;

diff --git a/src/realm/object-store/c_api/notifications.cpp b/src/realm/object-store/c_api/notifications.cpp
@@ -1,5 +1,6 @@
 #include <realm/object-store/c_api/types.hpp>
 #include <realm/object-store/c_api/util.hpp>
+#include <realm/object-store/keypath_helpers.hpp>
 
 namespace realm::c_api {
 namespace {
@@ -65,24 +66,79 @@ struct DictionaryNotificationsCallback {
 
 std::optional<KeyPathArray> build_key_path_array(realm_key_path_array_t* key_path_array)
 {
-    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();
-            KeyPath& kp = ret.back();
-            for (size_t j = 0; j < key_path->nb_elements; j++) {
-                realm_key_path_elem_t* path_elem = key_path->path_elements + j;
-                kp.emplace_back(TableKey(path_elem->object), ColKey(path_elem->property));
+    std::optional<KeyPathArray> ret;
+    if (key_path_array && key_path_array->size()) {
+        ret.emplace(std::move(*key_path_array));
+    }
+    return ret;
+}
+
+static KeyPathArray create_key_path_array(Group& g, const ObjectSchema& object_schema, const Schema& schema,
+                                          const char** all_key_paths_begin, const char** all_key_paths_end)
+{
+    KeyPathArray resolved_key_path_array;
+    for (auto it = all_key_paths_begin; it != all_key_paths_end; it++) {
+        KeyPath resolved_key_path;
+        const ObjectSchema* schema_at_index = &object_schema;
+        const Property* prop = nullptr;
+        // Split string based on '.'
+        const char* path = *it;
+        do {
+            auto p = find_chr(path, '.');
+            StringData property(path, p - path);
+            path = p;
+            if (!schema_at_index) {
+                throw InvalidArgument(
+                    util::format("Property '%1' in KeyPath '%2' is not a collection of objects or an object "
+                                 "reference, so it cannot be used as an intermediate keypath element.",
+                                 prop->public_name, *it));
             }
-        }
-        return ret;
+            prop = schema_at_index->property_for_public_name(property);
+            if (prop) {
+                ColKey col_key = prop->column_key;
+                TableKey table_key = schema_at_index->table_key;
+                if (prop->type == PropertyType::Object || prop->type == PropertyType::LinkingObjects) {
+                    auto found_schema = schema.find(prop->object_type);
+                    if (found_schema != schema.end()) {
+                        schema_at_index = &*found_schema;
+                        if (prop->type == PropertyType::LinkingObjects) {
+                            // Find backlink column key
+                            auto origin_prop = schema_at_index->property_for_name(prop->link_origin_property_name);
+                            auto origin_table = ObjectStore::table_for_object_type(g, schema_at_index->name);
+                            col_key = origin_table->get_opposite_column(origin_prop->column_key);
+                        }
+                    }
+                }
+                resolved_key_path.emplace_back(table_key, col_key);
+            }
+            else {
+                throw InvalidArgument(util::format("Property '%1' in KeyPath '%2' is not a valid property in %3.",
+                                                   property, *it, schema_at_index->name));
+            }
+        } while (*path++ == '.');
+        resolved_key_path_array.push_back(std::move(resolved_key_path));
     }
-    return std::nullopt;
+    return resolved_key_path_array;
 }
 
 } // namespace
 
+RLM_API realm_key_path_array_t* realm_create_key_path_array(const realm_t* realm,
+                                                            const realm_class_key_t object_class_key,
+                                                            size_t num_key_paths, const char** user_key_paths)
+{
+    return wrap_err([&]() {
+        KeyPathArray ret;
+        if (user_key_paths) {
+            const Schema& schema = (*realm)->schema();
+            const ObjectSchema& object_schema = schema_for_table(*realm, TableKey(object_class_key));
+            ret = create_key_path_array((*realm)->read_group(), object_schema, schema, user_key_paths,
+                                        user_key_paths + num_key_paths);
+        }
+        return new realm_key_path_array_t(std::move(ret));
+    });
+}
+
 RLM_API realm_notification_token_t* realm_object_add_notification_callback(realm_object_t* obj,
                                                                            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
@@ -411,6 +411,13 @@ struct realm_dictionary : realm::c_api::WrapC, realm::object_store::Dictionary {
     }
 };
 
+struct realm_key_path_array : realm::c_api::WrapC, realm::KeyPathArray {
+    explicit realm_key_path_array(realm::KeyPathArray kpa)
+        : realm::KeyPathArray(std::move(kpa))
+    {
+    }
+};
+
 struct realm_object_changes : realm::c_api::WrapC, realm::CollectionChangeSet {
     explicit realm_object_changes(realm::CollectionChangeSet changes)
         : realm::CollectionChangeSet(std::move(changes))

diff --git a/src/realm/query_engine.hpp b/src/realm/query_engine.hpp
@@ -88,7 +88,6 @@ TConditionValue:    Type of values in condition column. That is, int64_t, float,
 #include <realm/table.hpp>
 #include <realm/column_integer.hpp>
 #include <realm/unicode.hpp>
-#include <realm/util/miscellaneous.hpp>
 #include <realm/util/serializer.hpp>
 #include <realm/utilities.hpp>
 #include <realm/index_string.hpp>

diff --git a/src/realm/table.cpp b/src/realm/table.cpp
@@ -34,7 +34,6 @@
 #include <realm/replication.hpp>
 #include <realm/table_view.hpp>
 #include <realm/util/features.h>
-#include <realm/util/miscellaneous.hpp>
 #include <realm/util/serializer.hpp>
 
 #include <stdexcept>

diff --git a/src/realm/util/miscellaneous.hpp b/src/realm/util/miscellaneous.hpp
diff --git a/src/realm/utilities.hpp b/src/realm/utilities.hpp
@@ -361,6 +361,14 @@ constexpr inline size_t round_down(size_t p, size_t align)
     return r & (~(align - 1));
 }
 
+// return pointer to found character or to terminating NUL
+static inline const char* find_chr(const char* p, char c)
+{
+    while (*p && *p != c) {
+        ++p;
+    }
+    return p;
+}
 
 #ifdef _WIN32
 typedef HANDLE FileDesc;