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

Set interface nested collections #6648

Merged
merged 4 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
15 changes: 15 additions & 0 deletions src/realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1814,6 +1814,15 @@ RLM_API bool realm_list_set_collection(realm_list_t* list, size_t index, realm_c
*/
RLM_API realm_list_t* realm_list_get_list(realm_list_t* list, size_t index);

/**
* Returns a nested set if such collection exists and it is a leaf collection, NULL otherwise.
*
* @param list pointer to the list that containes the nested collection into
* @param index position of collection in the list
* @return a pointer to the the nested dictionary found at index passed as argument
*/
RLM_API realm_set_t* realm_list_get_set(realm_list_t* list, size_t index);

/**
* Returns a nested dictionary if such collection exists, NULL otherwise.
*
Expand Down Expand Up @@ -2311,6 +2320,12 @@ RLM_API bool realm_dictionary_insert_collection(realm_dictionary_t*, realm_value
*/
RLM_API realm_list_t* realm_dictionary_get_list(realm_dictionary_t* dictionary, realm_value_t key);

/**
* Fetch a set from a dictionary.
* @return a valid dictionary that needs to be deleted by the caller or nullptr in case of an error.
*/
RLM_API realm_set_t* realm_dictionary_get_set(realm_dictionary_t* dictionary, realm_value_t key);

/**
* Fetch a dictioanry from a dictionary.
* @return a valid dictionary that needs to be deleted by the caller or nullptr in case of an error.
Expand Down
8 changes: 6 additions & 2 deletions src/realm/collection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,15 @@ class CollectionBase : public Collection {

virtual DictionaryPtr get_dictionary(const PathElement&) const
{
return nullptr;
throw IllegalOperation("get_dictionary for this collection is not allowed");
}
virtual SetMixedPtr get_set(const PathElement&) const
{
throw IllegalOperation("get_set for this collection is not allowed");
}
virtual ListMixedPtr get_list(const PathElement&) const
{
return nullptr;
throw IllegalOperation("get_list for this collection is not allowed");
}

virtual void set_owner(const Obj& obj, ColKey) = 0;
Expand Down
4 changes: 4 additions & 0 deletions src/realm/collection_parent.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class LstBase;
class SetBase;
class Dictionary;

template <class T>
class Set;

template <class T>
class Lst;

Expand All @@ -49,6 +52,7 @@ using CollectionBasePtr = std::shared_ptr<CollectionBase>;
using CollectionListPtr = std::shared_ptr<CollectionList>;
using ListMixedPtr = std::shared_ptr<Lst<Mixed>>;
using DictionaryPtr = std::shared_ptr<Dictionary>;
using SetMixedPtr = std::shared_ptr<Set<Mixed>>;

/// The status of an accessor after a call to `update_if_needed()`.
enum class UpdateStatus {
Expand Down
16 changes: 16 additions & 0 deletions src/realm/dictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <realm/array_ref.hpp>
#include <realm/group.hpp>
#include <realm/list.hpp>
#include <realm/set.hpp>
#include <realm/replication.hpp>

#include <algorithm>
Expand Down Expand Up @@ -439,6 +440,15 @@ DictionaryPtr Dictionary::get_dictionary(const PathElement& path_elem) const
return ret;
}

SetMixedPtr Dictionary::get_set(const PathElement& path_elem) const
{
auto weak = const_cast<Dictionary*>(this)->weak_from_this();
auto shared = weak.expired() ? std::make_shared<Dictionary>(*this) : weak.lock();
auto ret = std::make_shared<Set<Mixed>>(m_obj_mem, m_col_key);
ret->set_owner(shared, path_elem.get_key());
return ret;
}

std::shared_ptr<Lst<Mixed>> Dictionary::get_list(const PathElement& path_elem) const
{
auto weak = const_cast<Dictionary*>(this)->weak_from_this();
Expand Down Expand Up @@ -1031,6 +1041,12 @@ void Dictionary::to_json(std::ostream& out, size_t link_depth, JSONOutputMode ou
Lst<Mixed> list(parent, 0);
list.to_json(out, link_depth, output_mode, fn);
}
else if (val.is_type(type_Set)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use same constructs as above?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not want to turn Sets into parent collections, that's why... but I guess if I don't expose the interface Sets can inherent from ParentCollection and it should be fine

Copy link
Member Author

@nicola-cab nicola-cab May 22, 2023

Choose a reason for hiding this comment

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

Actually, it turns out that all I need is CollectionParent::Index.. I fixed the code.

auto parent = std::make_shared<DummyParent>(this->get_table(), val.get_ref());
Set<Mixed> set(m_obj_mem, m_col_key);
set.set_owner(parent, i);
set.to_json(out, link_depth, output_mode, fn);
}
else {
val.to_json(out, output_mode);
}
Expand Down
1 change: 1 addition & 0 deletions src/realm/dictionary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle

void insert_collection(const PathElement&, CollectionType dict_or_list) override;
DictionaryPtr get_dictionary(const PathElement& path_elem) const override;
SetMixedPtr get_set(const PathElement&) const override;
ListMixedPtr get_list(const PathElement& path_elem) const override;

// throws std::out_of_range if key is not found
Expand Down
2 changes: 1 addition & 1 deletion src/realm/exceptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ struct InvalidArgument : LogicError {
struct InvalidColumnKey : InvalidArgument {
template <class T>
InvalidColumnKey(const T& name)
: InvalidArgument(ErrorCodes::InvalidProperty, util::format("Invalid property for object type %1", name))
: InvalidArgument(ErrorCodes::InvalidProperty, util::format("Invalid property for object type: %1", name))
{
}
InvalidColumnKey()
Expand Down
15 changes: 15 additions & 0 deletions src/realm/list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,15 @@ DictionaryPtr Lst<Mixed>::get_dictionary(const PathElement& path_elem) const
return ret;
}

SetMixedPtr Lst<Mixed>::get_set(const PathElement& path_elem) const
{
auto weak = const_cast<Lst<Mixed>*>(this)->weak_from_this();
auto shared = weak.expired() ? std::make_shared<Lst<Mixed>>(*this) : weak.lock();
auto ret = std::make_shared<Set<Mixed>>(m_obj_mem, m_col_key);
ret->set_owner(shared, m_tree->get_key(path_elem.get_ndx()));
return ret;
}

std::shared_ptr<Lst<Mixed>> Lst<Mixed>::get_list(const PathElement& path_elem) const
{
auto weak = const_cast<Lst<Mixed>*>(this)->weak_from_this();
Expand Down Expand Up @@ -619,6 +628,12 @@ void Lst<Mixed>::to_json(std::ostream& out, size_t link_depth, JSONOutputMode ou
Lst<Mixed> list(parent, i);
list.to_json(out, link_depth, output_mode, fn);
}
else if (val.is_type(type_Set)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above

auto parent = std::make_shared<DummyParent>(this->get_table(), val.get_ref());
Set<Mixed> set(m_obj_mem, m_col_key);
set.set_owner(parent, i);
set.to_json(out, link_depth, output_mode, fn);
}
else {
val.to_json(out, output_mode);
}
Expand Down
2 changes: 2 additions & 0 deletions src/realm/list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,10 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
void insert_collection(const PathElement&, CollectionType dict_or_list) override;
void set_collection(const PathElement& path_element, CollectionType dict_or_list) override;
DictionaryPtr get_dictionary(const PathElement& path_elem) const override;
SetMixedPtr get_set(const PathElement& path_elem) const override;
ListMixedPtr get_list(const PathElement& path_elem) const override;


// Overriding members of CollectionBase:
size_t size() const final
{
Expand Down
2 changes: 1 addition & 1 deletion src/realm/mixed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ int Mixed::compare(const Mixed& b) const noexcept
if (type == type_TypeOfValue && b.get_type() == type_TypeOfValue) {
return TypeOfValue(int_val).matches(TypeOfValue(b.int_val)) ? 0 : compare_generic(int_val, b.int_val);
}
if ((type == type_List || type == type_Dictionary)) {
if ((type == type_List || type == type_Dictionary || type == type_Set)) {
return m_type == b.m_type ? 0 : m_type < b.m_type ? -1 : 1;
}
REALM_ASSERT_RELEASE(false && "Compare not supported for this column type");
Expand Down
9 changes: 9 additions & 0 deletions src/realm/obj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,12 @@ void Obj::to_json(std::ostream& out, size_t link_depth, const std::map<std::stri
Dictionary dict(parent, 0);
dict.to_json(out, link_depth, output_mode, print_link);
}
else if (val.is_type(type_Set)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Again

auto parent = std::make_shared<DummyParent>(this->get_table(), val.get_ref());
Set<Mixed> set(*this, ck);
set.set_owner(parent, 0);
set.to_json(out, link_depth, output_mode, print_link);
}
else if (val.is_type(type_List)) {
DummyParent parent(m_table, val.get_ref());
Lst<Mixed> list(parent, 0);
Expand Down Expand Up @@ -2050,6 +2056,9 @@ CollectionBasePtr Obj::get_collection_ptr(ColKey col_key) const
if (val.is_type(type_List)) {
return std::make_shared<Lst<Mixed>>(*this, col_key);
}
else if (val.is_type(type_Set)) {
return std::make_shared<Set<Mixed>>(*this, col_key);
}
REALM_ASSERT(val.is_type(type_Dictionary));
return std::make_shared<Dictionary>(*this, col_key);
}
Expand Down
12 changes: 12 additions & 0 deletions src/realm/object-store/c_api/dictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ RLM_API realm_dictionary_t* realm_dictionary_get_dictionary(realm_dictionary_t*
});
}

RLM_API realm_set_t* realm_dictionary_get_set(realm_dictionary_t* dictionary, realm_value_t key)
{
return wrap_err([&]() {
if (key.type != RLM_TYPE_STRING) {
throw InvalidArgument{"Only string keys are supported in dictionaries"};
}

StringData k{key.string.data, key.string.size};
return new realm_set_t{dictionary->get_set(k)};
});
}

RLM_API realm_object_t* realm_dictionary_get_linked_object(realm_dictionary_t* dict, realm_value_t key)
{
return wrap_err([&]() {
Expand Down
7 changes: 7 additions & 0 deletions src/realm/object-store/c_api/list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ RLM_API realm_list_t* realm_list_get_list(realm_list_t* list, size_t index)
});
}

RLM_API realm_set_t* realm_list_get_set(realm_list_t* list, size_t index)
{
return wrap_err([&]() {
return new realm_set_t{list->get_set(index)};
});
}

RLM_API realm_dictionary_t* realm_list_get_dictionary(realm_list_t* list, size_t index)
{
return wrap_err([&]() {
Expand Down
7 changes: 7 additions & 0 deletions src/realm/object-store/collection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <realm/object-store/shared_realm.hpp>
#include <realm/object-store/list.hpp>
#include <realm/object-store/dictionary.hpp>
#include <realm/object-store/set.hpp>

namespace realm::object_store {

Expand Down Expand Up @@ -285,4 +286,10 @@ Dictionary Collection::get_dictionary(const PathElement& path) const
return Dictionary{m_realm, m_coll_base->get_dictionary(path)};
}

Set Collection::get_set(const PathElement& path) const
{
return Set{m_realm, m_coll_base->get_set(path)};
}


} // namespace realm::object_store
2 changes: 2 additions & 0 deletions src/realm/object-store/collection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class ListNotifier;

namespace object_store {
class Dictionary;
class Set;
class Collection {
public:
Collection(PropertyType type) noexcept;
Expand Down Expand Up @@ -119,6 +120,7 @@ class Collection {
void set_collection(const PathElement&, CollectionType);
List get_list(const PathElement&) const;
Dictionary get_dictionary(const PathElement&) const;
Set get_set(const PathElement&) const;

protected:
std::shared_ptr<Realm> m_realm;
Expand Down
2 changes: 1 addition & 1 deletion src/realm/set.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class Set final : public CollectionBaseImpl<SetBase> {
Set(ColKey col_key)
: Base(col_key)
{
if (!col_key.is_set()) {
if (!(col_key.is_set() || col_key.get_type() == col_type_Mixed)) {
throw InvalidArgument(ErrorCodes::TypeMismatch, "Property not a set");
}

Expand Down
20 changes: 18 additions & 2 deletions test/object-store/c_api/c_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4920,9 +4920,17 @@ TEST_CASE("C API: nested collections", "[c_api]") {
auto dict2 = cptr_checked(realm_dictionary_get_dictionary(dict.get(), rlm_str_val("Hi")));
checked(realm_dictionary_insert(dict2.get(), rlm_str_val("Nested-Hello"), rlm_str_val("Nested-World"),
nullptr, nullptr));
// dict -> set
realm_dictionary_insert_collection(dict.get(), rlm_str_val("Leaf-Set"), RLM_COLLECTION_TYPE_SET);
auto set = cptr_checked(realm_dictionary_get_set(dict.get(), rlm_str_val("Leaf-Set")));
bool inserted;
size_t index;
realm_set_insert(set.get(), rlm_str_val("Set-Hello"), &index, &inserted);
CHECK(index == 0);
CHECK(inserted);
size_t size;
checked(realm_dictionary_size(dict.get(), &size));
REQUIRE(size == 3);
REQUIRE(size == 4);
}

SECTION("list") {
Expand All @@ -4942,9 +4950,17 @@ TEST_CASE("C API: nested collections", "[c_api]") {
auto list2 = cptr_checked(realm_list_get_list(list.get(), 2));
realm_list_insert(list2.get(), 0, rlm_str_val("Nested-Hello"));
realm_list_insert(list2.get(), 1, rlm_str_val("Nested-World"));
// list -> set
realm_list_insert_collection(list.get(), 3, RLM_COLLECTION_TYPE_SET);
auto set = cptr_checked(realm_list_get_set(list.get(), 3));
bool inserted;
size_t index;
realm_set_insert(set.get(), rlm_str_val("Set-Hello"), &index, &inserted);
CHECK(index == 0);
CHECK(inserted);
size_t size;
checked(realm_list_size(list.get(), &size));
REQUIRE(size == 4);
REQUIRE(size == 5);
}
realm_release(realm);
}
Expand Down
56 changes: 51 additions & 5 deletions test/object-store/nested_collections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <realm/object-store/schema.hpp>

#include <realm/object-store/list.hpp>
#include <realm/object-store/set.hpp>
#include <realm/object-store/dictionary.hpp>

#include <realm/db.hpp>
Expand All @@ -34,14 +35,15 @@

using namespace realm;

TEST_CASE("nested-list-mixed", "[nested-colllections]") {
TEST_CASE("nested-list-mixed", "[nested-collections]") {
InMemoryTestFile config;
config.cache = false;
config.automatic_change_notifications = false;
auto r = Realm::get_shared_realm(config);
r->update_schema({
{"any", {{"any_val", PropertyType::Mixed | PropertyType::Nullable}}},
});
r->update_schema({{
"any",
{{"any_val", PropertyType::Mixed | PropertyType::Nullable}},
}});

r->begin_transaction();

Expand All @@ -64,8 +66,52 @@ TEST_CASE("nested-list-mixed", "[nested-colllections]") {
nested_list1.add(Mixed{6});
nested_list1.add(Mixed{7});
nested_list1.add(Mixed{"World"});
const char* json_doc_list = "{\"_key\":0,\"any_val\":[[5,10,\"Hello\"],[6,7,\"World\"],{}]}";
auto nested_dict = list_os.get_dictionary(2);
nested_dict.insert("Test", Mixed{"val"});
nested_dict.insert_collection("Set", CollectionType::Set);
auto nested_set_dict = nested_dict.get_set("Set");
nested_set_dict.insert(Mixed{10});
const char* json_doc_list =
"{\"_key\":0,\"any_val\":[[5,10,\"Hello\"],[6,7,\"World\"],{\"Set\":[10],\"Test\":\"val\"}]}";
REQUIRE(list_os.get_impl().get_obj().to_string() == json_doc_list);
// add a set, this is doable, but it cannnot contain a nested collection in it.
list_os.insert_collection(3, CollectionType::Set);
auto nested_set = list_os.get_set(3);
nested_set.insert(Mixed{5});
nested_set.insert(Mixed{"Hello"});
const char* json_doc_list_with_set = "{\"_key\":0,\"any_val\":[[5,10,\"Hello\"],[6,7,\"World\"],{\"Set\":[10]"
",\"Test\":\"val\"},[5,\"Hello\"]]}";
REQUIRE(list_os.get_impl().get_obj().to_string() == json_doc_list_with_set);
}

// Set
{
obj.set_collection(col, CollectionType::Set);
object_store::Set set{r, obj, col};
// this should fail, sets cannot have nested collections, thus can only be leaf collections
REQUIRE_THROWS(set.insert_collection(0, CollectionType::List));

// create a set and try to add the previous set as Mixed that contains a link to an object.
auto col2 = table->add_column(type_Mixed, "any_val2");
obj.set_collection(col2, CollectionType::Set);
object_store::Set set2{r, obj, col2};
set2.insert_any(set.get_impl().get_obj());
// trying to get a collection from a SET is not allowed.
REQUIRE_THROWS(set2.get_set(0));

// try to extract the obj and construct the SET, this is technically doable if you know the index.
auto mixed = set2.get_any(0);
auto link = mixed.get_link();
auto hidden_obj = table->get_object(link.get_obj_key());
object_store::Set other_set{r, hidden_obj, col};
REQUIRE_THROWS(other_set.insert_collection(0, CollectionType::List));
other_set.insert_any(Mixed{42});

const char* json_doc =
"{\"_key\":0,\"any_val\":[42],\"any_val2\":[{ \"table\": \"class_any\", \"key\": 0 }]}";
REQUIRE(set.get_impl().get_obj().to_string() == json_doc);

table->remove_column(col2);
}

// Dictionary.
Expand Down