From a44c6d7f31a84a2256af6010128399b5fe20294b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 16 Dec 2021 17:40:02 +0100 Subject: [PATCH] Make comparing Groups more comprehensive We can now compare collections and embedded objects. We can compare Groups that are not in the same realm. --- .gitignore | 3 - src/realm/group.cpp | 24 +++--- src/realm/obj.cpp | 173 ++++++++++++++++++------------------------- src/realm/obj.hpp | 27 ++++--- src/realm/table.cpp | 31 +++++--- src/realm/table.hpp | 5 -- test/test_shared.cpp | 115 ++++++++++++++++++++++++++++ tools/coverage.sh | 21 +++--- 8 files changed, 245 insertions(+), 154 deletions(-) diff --git a/.gitignore b/.gitignore index 569951ce415..93c452d2ffc 100644 --- a/.gitignore +++ b/.gitignore @@ -8,12 +8,9 @@ GPATH GRTAGS GTAGS -/cover_html /gcovr.xml /tmp doc/html -/html -coverage-* # CMake artifacts CMakeFiles diff --git a/src/realm/group.cpp b/src/realm/group.cpp index 64382c60bf1..6b25f27540d 100644 --- a/src/realm/group.cpp +++ b/src/realm/group.cpp @@ -1423,19 +1423,21 @@ void Group::update_refs(ref_type top_ref) noexcept bool Group::operator==(const Group& g) const { - auto keys_this = get_table_keys(); - auto keys_g = g.get_table_keys(); - size_t n = keys_this.size(); - if (n != keys_g.size()) - return false; - for (size_t i = 0; i < n; ++i) { - const StringData& table_name_1 = get_table_name(keys_this[i]); - const StringData& table_name_2 = g.get_table_name(keys_g[i]); - if (table_name_1 != table_name_2) + for (auto tk : get_table_keys()) { + const StringData& table_name = get_table_name(tk); + + ConstTableRef table_1 = get_table(tk); + ConstTableRef table_2 = g.get_table(table_name); + if (!table_2) + return false; + if (table_1->get_primary_key_column().get_type() != table_2->get_primary_key_column().get_type()) { + return false; + } + if (table_1->is_embedded() != table_2->is_embedded()) return false; + if (table_1->is_embedded()) + continue; - ConstTableRef table_1 = get_table(keys_this[i]); - ConstTableRef table_2 = g.get_table(keys_g[i]); if (*table_1 != *table_2) return false; } diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index dbf412b7a91..3432deb4f2f 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -103,11 +103,63 @@ Replication* Obj::get_replication() const bool Obj::operator==(const Obj& other) const { - size_t col_cnt = get_spec().get_public_column_count(); - while (col_cnt--) { - ColKey key = m_table->spec_ndx2colkey(col_cnt); - if (cmp(other, key) != 0) { - return false; + for (auto ck : m_table->get_column_keys()) { + StringData col_name = m_table->get_column_name(ck); + + auto compare_values = [&](Mixed val1, Mixed val2) { + if (val1.is_null()) { + if (!val2.is_null()) + return false; + } + else { + if (val1.get_type() != val2.get_type()) + return false; + if (val1.is_type(type_Link, type_TypedLink)) { + auto o1 = _get_linked_object(ck, val1); + auto o2 = other._get_linked_object(col_name, val2); + if (o1.m_table->is_embedded()) { + return o1 == o2; + } + else { + return o1.get_primary_key() == o2.get_primary_key(); + } + } + else { + if (val1 != val2) + return false; + } + } + return true; + }; + + if (!ck.is_collection()) { + if (!compare_values(get_any(ck), other.get_any(col_name))) + return false; + } + else { + auto coll1 = get_collection_ptr(ck); + auto coll2 = other.get_collection_ptr(col_name); + size_t sz = coll1->size(); + if (coll2->size() != sz) + return false; + if (ck.is_list() || ck.is_set()) { + for (size_t i = 0; i < sz; i++) { + if (!compare_values(coll1->get_any(i), coll2->get_any(i))) + return false; + } + } + if (ck.is_dictionary()) { + auto dict1 = dynamic_cast(coll1.get()); + auto dict2 = dynamic_cast(coll2.get()); + for (size_t i = 0; i < sz; i++) { + auto [key, value] = dict1->get_pair(i); + auto val2 = dict2->try_get(key); + if (!val2) + return false; + if (!compare_values(value, *val2)) + return false; + } + } } } return true; @@ -439,94 +491,6 @@ Mixed Obj::get_primary_key() const return col ? get_any(col) : Mixed{get_key()}; } -template -inline int Obj::cmp(const Obj& other, ColKey::Idx col_ndx) const -{ - T val1 = _get(col_ndx); - T val2 = other._get(col_ndx); - - if (val1 < val2) { - return -1; - } - - if (val1 > val2) { - return 1; - } - - return 0; -} - -template <> -inline int Obj::cmp(const Obj& other, ColKey::Idx col_ndx) const -{ - StringData a = _get(col_ndx); - StringData b = other._get(col_ndx); - - if (a.is_null()) { - return b.is_null() ? 0 : -1; - } - - if (b.is_null()) { - return 1; - } - - if (a == b) { - return 0; - } - - return utf8_compare(a, b) ? -1 : 1; -} - -int Obj::cmp(const Obj& other, ColKey col_key) const -{ - other.check_valid(); - ColKey::Idx col_ndx = col_key.get_index(); - ColumnAttrMask attr = col_key.get_attrs(); - REALM_ASSERT(!attr.test(col_attr_List)); // TODO: implement comparison of lists - - switch (DataType(col_key.get_type())) { - case type_Int: - if (attr.test(col_attr_Nullable)) - return cmp>(other, col_ndx); - else - return cmp(other, col_ndx); - case type_Bool: - return cmp(other, col_ndx); - case type_Float: - return cmp(other, col_ndx); - case type_Double: - return cmp(other, col_ndx); - case type_String: - return cmp(other, col_ndx); - case type_Binary: - return cmp(other, col_ndx); - case type_Mixed: - return cmp(other, col_ndx); - case type_Timestamp: - return cmp(other, col_ndx); - case type_Decimal: - return cmp(other, col_ndx); - case type_ObjectId: - if (attr.test(col_attr_Nullable)) - return cmp>(other, col_ndx); - else - return cmp(other, col_ndx); - case type_UUID: - if (attr.test(col_attr_Nullable)) - return cmp>(other, col_ndx); - else - return cmp(other, col_ndx); - case type_Link: - return cmp(other, col_ndx); - case type_TypedLink: - return cmp(other, col_ndx); - case type_LinkList: - REALM_ASSERT(false); - break; - } - return 0; -} - /* FIXME: Make this one fast too! template <> ObjKey Obj::_get(size_t col_ndx) const @@ -535,13 +499,18 @@ ObjKey Obj::_get(size_t col_ndx) const } */ -Obj Obj::get_linked_object(ColKey link_col_key) const +Obj Obj::_get_linked_object(ColKey link_col_key, Mixed link) const { - TableRef target_table = get_target_table(link_col_key); - ObjKey key = get(link_col_key); Obj obj; - if (key) { - obj = target_table->get_object(key); + if (!link.is_null()) { + TableRef target_table; + if (link.is_type(type_TypedLink)) { + target_table = m_table->get_parent_group()->get_table(link.get_link().get_table_key()); + } + else { + target_table = get_target_table(link_col_key); + } + obj = target_table->get_object(link.get()); } return obj; } @@ -741,10 +710,9 @@ void Obj::traverse_path(Visitor v, PathSizer ps, size_t path_length) const index = Mixed(int64_t(i)); } else if (next_col_key.get_attrs().test(col_attr_Dictionary)) { - ObjLink link = get_link(); auto dict = obj.get_dictionary(next_col_key); for (auto it : dict) { - if (it.second.is_type(type_TypedLink) && it.second.get_link() == link) { + if (it.second.is_type(type_TypedLink) && it.second.get_link() == get_link()) { index = it.first; break; } @@ -2131,6 +2099,11 @@ CollectionBasePtr Obj::get_collection_ptr(ColKey col_key) const return {}; } +CollectionBasePtr Obj::get_collection_ptr(StringData col_name) const +{ + return get_collection_ptr(get_column_key(col_name)); +} + LinkCollectionPtr Obj::get_linkcollection_ptr(ColKey col_key) const { if (col_key.is_list()) { diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 35c8cad7aab..af203b4e9ae 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -157,7 +157,6 @@ class Obj { return get(get_column_key(col_name)); } bool is_unresolved(ColKey col_key) const; - int cmp(const Obj& other, ColKey col_key) const; size_t get_link_count(ColKey col_key) const; @@ -262,8 +261,14 @@ class Obj { void assign(const Obj& other); - Obj get_linked_object(ColKey link_col_key) const; - Obj get_linked_object(StringData link_col_name) const; + Obj get_linked_object(ColKey link_col_key) const + { + return _get_linked_object(link_col_key, get_any(link_col_key)); + } + Obj get_linked_object(StringData link_col_name) const + { + return get_linked_object(get_column_key(link_col_name)); + } template Lst get_list(ColKey col_key) const; @@ -304,6 +309,7 @@ class Obj { Dictionary get_dictionary(StringData col_name) const; CollectionBasePtr get_collection_ptr(ColKey col_key) const; + CollectionBasePtr get_collection_ptr(StringData col_name) const; LinkCollectionPtr get_linkcollection_ptr(ColKey col_key) const; void assign_pk_and_backlinks(const Obj& other); @@ -370,9 +376,6 @@ class Obj { template U _get(ColKey::Idx col_ndx) const; - template - int cmp(const Obj& other, ColKey::Idx col_ndx) const; - int cmp(const Obj& other, ColKey::Idx col_ndx) const; ObjKey get_backlink(ColKey backlink_col, size_t backlink_ndx) const; // Return all backlinks from a specific backlink column std::vector get_all_backlinks(ColKey backlink_col) const; @@ -399,6 +402,12 @@ class Obj { return m_row_ndx; } + Obj _get_linked_object(ColKey link_col_key, Mixed link) const; + Obj _get_linked_object(StringData link_col_name, Mixed link) const + { + return _get_linked_object(get_column_key(link_col_name), link); + } + void set_int(ColKey col_key, int64_t value); void add_backlink(ColKey backlink_col, ObjKey origin_key); bool remove_one_backlink(ColKey backlink_col, ObjKey origin_key); @@ -549,12 +558,6 @@ std::vector Obj::get_list_values(ColKey col_key) const return values; } -inline Obj Obj::get_linked_object(StringData link_col_name) const -{ - ColKey col = get_column_key(link_col_name); - return get_linked_object(col); -} - template inline Obj& Obj::_set(size_t col_ndx, Val v) { diff --git a/src/realm/table.cpp b/src/realm/table.cpp index 411a7e16686..e4796b24380 100644 --- a/src/realm/table.cpp +++ b/src/realm/table.cpp @@ -2716,25 +2716,32 @@ size_t Table::compute_aggregated_byte_size() const noexcept return stats_2.allocated; } - -bool Table::compare_objects(const Table& t) const +bool Table::operator==(const Table& t) const { if (size() != t.size()) { return false; } - - auto it1 = begin(); - auto it2 = t.begin(); - auto e = end(); - - while (it1 != e) { - if (*it1 == *it2) { - ++it1; - ++it2; + // Check columns + for (auto ck : this->get_column_keys()) { + auto name = get_column_name(ck); + auto other_ck = t.get_column_key(name); + auto attrs = ck.get_attrs(); + if (!other_ck || other_ck.get_attrs() != attrs) { + return false; + } + } + auto pk_col = get_primary_key_column(); + for (auto o : *this) { + Obj other_o; + if (pk_col) { + auto pk = o.get_any(pk_col); + other_o = t.get_object_with_primary_key(pk); } else { - return false; + other_o = t.get_object(o.get_key()); } + if (!(other_o && o == other_o)) + return false; } return true; diff --git a/src/realm/table.hpp b/src/realm/table.hpp index f222ce0b862..983d84a9abd 100644 --- a/src/realm/table.hpp +++ b/src/realm/table.hpp @@ -1268,11 +1268,6 @@ inline bool Table::is_group_level() const noexcept return bool(get_parent_group()); } -inline bool Table::operator==(const Table& t) const -{ - return m_spec == t.m_spec && compare_objects(t); // Throws -} - inline bool Table::operator!=(const Table& t) const { return !(*this == t); // Throws diff --git a/test/test_shared.cpp b/test/test_shared.cpp index c5450245873..42064dfbc5c 100644 --- a/test/test_shared.cpp +++ b/test/test_shared.cpp @@ -4066,4 +4066,119 @@ TEST(Shared_WriteCopy) CHECK_EQUAL(db->start_read()->get_table("foo")->size(), 1); } +TEST(Shared_CompareGroups) +{ + SHARED_GROUP_TEST_PATH(path1); + SHARED_GROUP_TEST_PATH(path2); + + DBRef db1 = DB::create(make_in_realm_history(), path1); + DBRef db2 = DB::create(make_in_realm_history(), path2); + + { + // Create schema in DB1 + auto wt = db1->start_write(); + auto embedded = wt->add_embedded_table("class_Embedded"); + embedded->add_column(type_Float, "float"); + // Embedded in embedded + embedded->add_column_dictionary(*embedded, "additional"); + wt->add_embedded_table("class_AnotherEmbedded"); + + auto baas = wt->add_table_with_primary_key("class_Baa", type_Int, "_id"); + auto foos = wt->add_table_with_primary_key("class_Foo", type_String, "_id"); + + baas->add_column(type_Bool, "bool"); + baas->add_column_list(type_Int, "list"); + baas->add_column_set(type_Int, "set"); + baas->add_column_dictionary(type_Int, "dictionary"); + baas->add_column(*embedded, "embedded"); + baas->add_column(type_Mixed, "any", true); + baas->add_column(*foos, "link"); + + foos->add_column(type_String, "str"); + foos->add_column_list(*embedded, "list_of_embedded"); + foos->add_column_list(type_Mixed, "list_of_any", true); + foos->add_column_list(*baas, "link_list"); + wt->commit(); + } + { + // Create schema in DB2 - slightly different + auto wt = db2->start_write(); + auto foos = wt->add_table_with_primary_key("class_Foo", type_String, "_id"); + + auto embedded = wt->add_embedded_table("class_Embedded"); + embedded->add_column(type_Float, "float"); + // Embedded in embedded + embedded->add_column_dictionary(*embedded, "additional"); + + auto baas = wt->add_table_with_primary_key("class_Baa", type_Int, "_id"); + + baas->add_column_set(type_Int, "set"); + baas->add_column(type_Mixed, "any", true); + baas->add_column_dictionary(type_Int, "dictionary"); + baas->add_column(*embedded, "embedded"); + baas->add_column(type_Bool, "bool"); + baas->add_column(*foos, "link"); + baas->add_column_list(type_Int, "list"); + + foos->add_column_list(*embedded, "list_of_embedded"); + foos->add_column(type_String, "str"); + foos->add_column_list(*baas, "link_list"); + foos->add_column_list(type_Mixed, "list_of_any", true); + + wt->add_embedded_table("class_AnotherEmbedded"); + wt->commit(); + } + auto create_objects = [&](DBRef db) { + auto wt = db->start_write(); + + auto foos = wt->get_table("class_Foo"); + auto baas = wt->get_table("class_Baa"); + + auto foo = foos->create_object_with_primary_key("123").set("str", "Hello"); + auto children = foo.get_linklist("list_of_embedded"); + children.create_and_insert_linked_object(0).set("float", 10.f); + children.create_and_insert_linked_object(1).set("float", 20.f); + + auto baa = baas->create_object_with_primary_key(999); + baa.set("link", foo.get_key()); + baa.set("bool", true); + auto obj = baa.create_and_set_linked_object(baas->get_column_key("embedded")); + obj.set("float", 42.f); + auto additional = obj.get_dictionary("additional"); + additional.create_and_insert_linked_object("One").set("float", 1.f); + additional.create_and_insert_linked_object("Two").set("float", 2.f); + additional.create_and_insert_linked_object("Three").set("float", 3.f); + + foo.get_linklist("link_list").add(baa.get_key()); + + // Basic collections + auto list = baa.get_list("list"); + list.add(1); + list.add(2); + list.add(3); + auto set = baa.get_set("set"); + set.insert(4); + set.insert(5); + set.insert(6); + auto dict = baa.get_dictionary("dictionary"); + dict.insert("key7", 7); + dict.insert("key8", 8); + dict.insert("key9", 9); + + wt->commit(); + }; + create_objects(db1); + create_objects(db2); + CHECK(*db1->start_read() == *db2->start_read()); + { + auto wt = db2->start_write(); + auto baas = wt->get_table("class_Baa"); + auto obj = baas->get_object_with_primary_key(999); + auto embedded = obj.get_linked_object("embedded"); + embedded.get_dictionary("additional").get_object("One").set("float", 555.f); + wt->commit(); + } + CHECK_NOT(*db1->start_read() == *db2->start_read()); +} + #endif // TEST_SHARED diff --git a/tools/coverage.sh b/tools/coverage.sh index 5d75b70a7be..273e3dae57c 100755 --- a/tools/coverage.sh +++ b/tools/coverage.sh @@ -4,22 +4,21 @@ PROJECT_DIR=$(git rev-parse --show-toplevel) MAX_NODE_SIZE=$1 if [ -z ${MAX_NODE_SIZE} ]; then MAX_NODE_SIZE=1000; fi cd ${PROJECT_DIR} -rm -rf html mkdir -p build.cov cd build.cov/ -echo cmake -G Ninja -D REALM_COVERAGE=ON -D CMAKE_CXX_FLAGS=-g -DREALM_MAX_BPNODE_SIZE=${MAX_NODE_SIZE} .. -cmake -G Ninja -D REALM_COVERAGE=ON -D CMAKE_CXX_FLAGS=-g -DREALM_MAX_BPNODE_SIZE=${MAX_NODE_SIZE} .. +if [ ! -f CMakeCache.txt ]; then + echo cmake -G Ninja -D REALM_COVERAGE=ON -D CMAKE_CXX_FLAGS=-g -DREALM_MAX_BPNODE_SIZE=${MAX_NODE_SIZE} .. + cmake -G Ninja -D REALM_COVERAGE=ON -D CMAKE_CXX_FLAGS=-g -DREALM_MAX_BPNODE_SIZE=${MAX_NODE_SIZE} .. +fi ninja -cd ${PROJECT_DIR} + if [ ! -f coverage-base.info ]; then - lcov --no-external --capture --initial --directory . --output-file ./coverage-base.info + lcov --capture --initial --directory src --output-file ./coverage-base.info fi -cd build.cov ctest -cd ${PROJECT_DIR} -lcov --no-external --directory . --capture --output-file ./coverage-test.info +lcov --directory src --capture --output-file ./coverage-test.info lcov --add-tracefile coverage-base.info --add-tracefile coverage-test.info --output-file coverage-total.info -lcov --remove coverage-total.info "/usr/*" "*/test/*" "*/external/*" "*/generated/*" "*query_flex*" --output-file coverage-filtered.info -genhtml coverage-filtered.info --output-directory html -firefox html/index.html > /dev/null 2>&1 & +lcov --remove coverage-total.info "/usr/*" "*/external/*" "*/generated/*" "*query_flex*" --output-file coverage-filtered.info +genhtml coverage-filtered.info +firefox index.html > /dev/null 2>&1 & cd ${CURRENT_DIR}