Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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?.?.?)
Expand Down
38 changes: 18 additions & 20 deletions src/realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

/**
Expand Down
1 change: 0 additions & 1 deletion src/realm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,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
Expand Down
1 change: 0 additions & 1 deletion src/realm/alloc_slab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 0 additions & 1 deletion src/realm/group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 0 additions & 1 deletion src/realm/group_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
80 changes: 68 additions & 12 deletions src/realm/object-store/c_api/notifications.cpp
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't there be value in having this method available to SDKs that do not use the C-API? I guess it is already available since it is just a static helper method, but maybe the location is a bit wierd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A static function is actually private within the compilation unit. We might consider making it public somewhere. At least to the binding generator.

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,
Expand Down
7 changes: 7 additions & 0 deletions src/realm/object-store/c_api/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 0 additions & 1 deletion src/realm/query_engine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 0 additions & 1 deletion src/realm/table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
49 changes: 0 additions & 49 deletions src/realm/util/miscellaneous.hpp

This file was deleted.

8 changes: 8 additions & 0 deletions src/realm/utilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this utility? It seems to me that it is just a char *strchr(const char *string, int c);

Copy link
Contributor

@jedelbo jedelbo Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strchr will return nullptr if not found. We need it to return a pointer to terminating NUL character. GCC has a function called strchrnul, but that is not generally available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think it does not make a huge difference if we call strchr() and we check for nullptr in order to return a null terminated string or loop over like we do.

{
while (*p && *p != c) {
++p;
}
return p;
}

#ifdef _WIN32
typedef HANDLE FileDesc;
Expand Down
Loading