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.

Sign up for GitHub

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

Allow Dictionary to contain a collection #6584

Merged
merged 2 commits into from
May 10, 2023
Merged

Allow Dictionary to contain a collection #6584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce04b6f
Allow Dictionary to contain a collection
jedelbo May 3, 2023
0039e14
Remove not used code
jedelbo May 9, 2023
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
44 changes: 43 additions & 1 deletion src/realm/collection.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,41 @@ namespace realm {
template <class L>
struct CollectionIterator;

// Used in Cluster when removing owning object
class DummyParent : public CollectionParent {
public:
DummyParent(TableRef t, ref_type ref)
: m_obj(t, MemRef(), ObjKey(), 0)
, m_ref(ref)
{
}
TableRef get_table() const noexcept final
{
return m_obj.get_table();
}
const Obj& get_object() const noexcept final
{
return m_obj;
}

protected:
Obj m_obj;
ref_type m_ref;
UpdateStatus update_if_needed_with_status() const noexcept final
{
return UpdateStatus::Updated;
}
bool update_if_needed() const final
{
return true;
}
ref_type get_collection_ref(Index, CollectionType) const final
{
return m_ref;
}
void set_collection_ref(Index, ref_type, CollectionType) {}
};

class Collection {
public:
virtual ~Collection();
Expand Down Expand Up @@ -109,7 +144,7 @@ class CollectionBase : public Collection {
}

/// Get the table of the object that owns this collection.
virtual ConstTableRef get_table() const noexcept final
ConstTableRef get_table() const noexcept
{
return get_obj().get_table();
}
Expand Down Expand Up @@ -463,6 +498,13 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
{
}

CollectionBaseImpl(DummyParent& parent) noexcept
: m_obj_mem(parent.get_object())
, m_parent(&parent)
, m_alloc(&m_obj_mem.get_alloc())
{
}

CollectionBaseImpl& operator=(const CollectionBaseImpl& other)
{
if (this != &other) {
Expand Down
4 changes: 2 additions & 2 deletions src/realm/collection_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ namespace realm {

CollectionList::CollectionList(std::shared_ptr<CollectionParent> parent, ColKey col_key, Index index,
CollectionType coll_type)
: m_owned_parent(parent)
: CollectionParent(parent->get_level() + 1)
, m_owned_parent(parent)
, m_parent(m_owned_parent.get())
, m_index(index)
, m_level(parent->get_level() + 1)
, m_alloc(&get_table()->get_alloc())
, m_col_key(col_key)
, m_top(*m_alloc)
Expand Down
10 changes: 1 addition & 9 deletions src/realm/collection_list.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ using CollectionListPtr = std::shared_ptr<CollectionList>;
* by either an integer index or a string key.
*/

class CollectionList final : public Collection,
public CollectionParent,
protected ArrayParent,
public std::enable_shared_from_this<CollectionList> {
class CollectionList final : public Collection, public CollectionParent, protected ArrayParent {
public:
[[nodiscard]] static CollectionListPtr create(std::shared_ptr<CollectionParent> parent, ColKey col_key,
Index index, CollectionType coll_type)
Expand All @@ -59,10 +56,6 @@ class CollectionList final : public Collection,

bool init_from_parent(bool allow_create) const;

size_t get_level() const noexcept final
{
return m_level;
}
UpdateStatus update_if_needed_with_status() const noexcept final;
bool update_if_needed() const final;
TableRef get_table() const noexcept final
Expand Down Expand Up @@ -106,7 +99,6 @@ class CollectionList final : public Collection,
std::shared_ptr<CollectionParent> m_owned_parent;
CollectionParent* m_parent;
CollectionParent::Index m_index;
size_t m_level = 0;
Allocator* m_alloc;
ColKey m_col_key;
mutable Array m_top;
Expand Down
14 changes: 1 addition & 13 deletions src/realm/collection_parent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,6 @@ SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key) const
REALM_TERMINATE("Unsupported column type.");
}

DictionaryPtr CollectionParent::get_dictionary_ptr(ColKey col_key) const
{
return std::make_unique<Dictionary>(col_key);
}

CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key) const
{
if (col_key.is_list()) {
Expand All @@ -255,16 +250,9 @@ CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key) const
return get_setbase_ptr(col_key);
}
else if (col_key.is_dictionary()) {
return get_dictionary_ptr(col_key);
return std::make_unique<Dictionary>(col_key);
}
return {};
}


const Obj& DummyParent::get_object() const noexcept
{
static Obj dummy_obj;
return dummy_obj;
}

} // namespace realm
54 changes: 13 additions & 41 deletions src/realm/collection_parent.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ using LstBasePtr = std::unique_ptr<LstBase>;
using SetBasePtr = std::unique_ptr<SetBase>;
using CollectionBasePtr = std::unique_ptr<CollectionBase>;
using CollectionListPtr = std::shared_ptr<CollectionList>;
using DictionaryPtr = std::unique_ptr<Dictionary>;
using DictionaryPtr = std::shared_ptr<Dictionary>;

/// The status of an accessor after a call to `update_if_needed()`.
enum class UpdateStatus {
Expand Down Expand Up @@ -75,12 +75,14 @@ struct FullPath {
Path path_from_top;
};

class CollectionParent {
class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
public:
using Index = mpark::variant<ColKey, int64_t, std::string>;

// Return the nesting level of the parent
virtual size_t get_level() const noexcept = 0;
size_t get_level() const noexcept
{
return m_level;
}
/// Get table of owning object
virtual TableRef get_table() const noexcept = 0;

Expand All @@ -89,6 +91,13 @@ class CollectionParent {
friend class CollectionBaseImpl;
friend class CollectionList;

size_t m_level = 0;

constexpr CollectionParent(size_t level = 0)
: m_level(level)
{
}

virtual ~CollectionParent();
/// Update the accessor (and return `UpdateStatus::Detached` if the parent
/// is no longer valid, rather than throwing an exception).
Expand All @@ -112,46 +121,9 @@ class CollectionParent {

LstBasePtr get_listbase_ptr(ColKey col_key) const;
SetBasePtr get_setbase_ptr(ColKey col_key) const;
DictionaryPtr get_dictionary_ptr(ColKey col_key) const;
CollectionBasePtr get_collection_ptr(ColKey col_key) const;
};

// Used in Cluster when removing owning object
class DummyParent : public CollectionParent {
public:
DummyParent(TableRef t, ref_type ref)
: m_table(t)
, m_ref(ref)
{
}
size_t get_level() const noexcept final
{
return 0;
}
TableRef get_table() const noexcept final
{
return m_table;
}

protected:
TableRef m_table;
ref_type m_ref;
UpdateStatus update_if_needed_with_status() const noexcept final
{
return UpdateStatus::Updated;
}
bool update_if_needed() const final
{
return true;
}
const Obj& get_object() const noexcept final;
ref_type get_collection_ref(Index, CollectionType) const final
{
return m_ref;
}
void set_collection_ref(Index, ref_type, CollectionType) {}
};

} // namespace realm

#endif
89 changes: 82 additions & 7 deletions src/realm/dictionary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <realm/array_mixed.hpp>
#include <realm/array_ref.hpp>
#include <realm/group.hpp>
#include <realm/list.hpp>
#include <realm/replication.hpp>

#include <algorithm>
Expand Down Expand Up @@ -49,7 +50,7 @@ void validate_key_value(const Mixed& key)
Dictionary::Dictionary(ColKey col_key)
: Base(col_key)
{
if (!col_key.is_dictionary()) {
if (!(col_key.is_dictionary() || col_key.get_type() == col_type_Mixed)) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there a small chance that the user could sneak in a Mixed<int> and eventually crash in some other place in the code, but not during construction, at least not at this point?
Probably this is not a big issue, it would be nice to throw here though.

Although, identifying the type of the mixed (if it is a collection) requires exposing the collection types in the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot check more at this point. get_collection_ref should fail if the variable if not of the proper type.

throw InvalidArgument(ErrorCodes::TypeMismatch, "Property not a dictionary");
}
}
Expand Down Expand Up @@ -423,6 +424,38 @@ Obj Dictionary::create_and_insert_linked_object(Mixed key)
return o;
}

DictionaryPtr Dictionary::insert_dictionary(StringData key)
{
insert(key, Mixed(0, CollectionType::Dictionary));
return get_dictionary(key);
}

DictionaryPtr Dictionary::get_dictionary(StringData key) const
{
auto weak = const_cast<Dictionary*>(this)->weak_from_this();
REALM_ASSERT(!weak.expired());
auto shared = weak.lock();
DictionaryPtr ret = std::make_shared<Dictionary>(m_col_key);
ret->set_owner(shared, key);
return ret;
}

std::shared_ptr<Lst<Mixed>> Dictionary::insert_list(StringData key)
{
insert(key, Mixed(0, CollectionType::List));
return get_list(key);
}

std::shared_ptr<Lst<Mixed>> Dictionary::get_list(StringData key) const
{
auto weak = const_cast<Dictionary*>(this)->weak_from_this();
REALM_ASSERT(!weak.expired());
auto shared = weak.lock();
std::shared_ptr<Lst<Mixed>> ret = std::make_shared<Lst<Mixed>>(m_col_key);
ret->set_owner(shared, key);
return ret;
}

Mixed Dictionary::get(Mixed key) const
{
if (auto opt_val = try_get(key)) {
Expand Down Expand Up @@ -542,7 +575,7 @@ std::pair<Dictionary::Iterator, bool> Dictionary::insert(Mixed key, Mixed value)

if (new_link != old_link) {
CascadeState cascade_state(CascadeState::Mode::Strong);
bool recurse = replace_backlink(m_col_key, old_link, new_link, cascade_state);
bool recurse = Base::replace_backlink(m_col_key, old_link, new_link, cascade_state);
if (recurse)
_impl::TableFriend::remove_recursive(*my_table, cascade_state); // Throws
}
Expand Down Expand Up @@ -704,7 +737,7 @@ void Dictionary::clear()

bool Dictionary::init_from_parent(bool allow_create) const
{
auto ref = get_collection_ref();
auto ref = Base::get_collection_ref();

if ((ref || allow_create) && !m_dictionary_top) {
Allocator& alloc = get_alloc();
Expand All @@ -728,7 +761,7 @@ bool Dictionary::init_from_parent(bool allow_create) const
}

if (ref) {
m_dictionary_top->init_from_parent();
m_dictionary_top->init_from_ref(ref);
m_keys->init_from_parent();
m_values->init_from_parent();
}
Expand Down Expand Up @@ -852,7 +885,7 @@ std::pair<Mixed, Mixed> Dictionary::do_get_pair(size_t ndx) const
bool Dictionary::clear_backlink(Mixed value, CascadeState& state) const
{
if (value.is_type(type_TypedLink)) {
return remove_backlink(m_col_key, value.get_link(), state);
return Base::remove_backlink(m_col_key, value.get_link(), state);
}
return false;
}
Expand Down Expand Up @@ -935,12 +968,12 @@ void Dictionary::migrate()
ArrayParent* m_owner;
};

if (auto dict_ref = get_collection_ref()) {
if (auto dict_ref = Base::get_collection_ref()) {
Allocator& alloc = get_alloc();
DictionaryClusterTree cluster_tree(this, alloc, 0);
if (cluster_tree.init_from_parent()) {
// Create an empty dictionary in the old ones place
set_collection_ref(0);
Base::set_collection_ref(0);
ensure_created();

ArrayString keys(alloc); // We only support string type keys.
Expand Down Expand Up @@ -996,6 +1029,16 @@ void Dictionary::to_json(std::ostream& out, size_t link_depth, JSONOutputMode ou
if (val.is_type(type_TypedLink)) {
fn(val);
}
else if (val.is_type(type_Dictionary)) {
DummyParent parent(this->get_table(), val.get_ref());
Dictionary dict(parent);
dict.to_json(out, link_depth, output_mode, fn);
}
else if (val.is_type(type_List)) {
DummyParent parent(this->get_table(), val.get_ref());
Lst<Mixed> list(parent);
list.to_json(out, link_depth, output_mode, fn);
}
else {
val.to_json(out, output_mode);
}
Expand All @@ -1005,6 +1048,38 @@ void Dictionary::to_json(std::ostream& out, size_t link_depth, JSONOutputMode ou
out << close_str;
}

ref_type Dictionary::get_collection_ref(Index index, CollectionType type) const
{
auto ndx = do_find_key(StringData(mpark::get<std::string>(index)));
if (ndx != realm::not_found) {
auto val = m_values->get(ndx);
if (val.is_null() || !val.is_type(DataType(int(type)))) {
throw IllegalOperation("Not proper collection type");
}
return val.get_ref();
}

return 0;
}

void Dictionary::set_collection_ref(Index index, ref_type ref, CollectionType type)
{
auto ndx = do_find_key(StringData(mpark::get<std::string>(index)));
if (ndx == realm::not_found) {
throw StaleAccessor("Collection has been deleted");
}
m_values->set(ndx, Mixed(ref, type));
}

bool Dictionary::update_if_needed() const
{
auto status = update_if_needed_with_status();
if (status == UpdateStatus::Detached) {
throw StaleAccessor("CollectionList no longer exists");
}
return status == UpdateStatus::Updated;
}

/************************* DictionaryLinkValues *************************/

DictionaryLinkValues::DictionaryLinkValues(const Obj& obj, ColKey col_key)
Expand Down
Loading