From b338e7d7b7178dc7cc34646aaf6fc0e3ab12b5c8 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 31 Mar 2023 14:45:04 +0100 Subject: [PATCH 1/5] Schema support for nested collections --- src/realm/object-store/object_schema.cpp | 9 ++ src/realm/object-store/object_schema.hpp | 1 + src/realm/object-store/object_store.cpp | 53 +++++----- src/realm/object-store/property.hpp | 63 +++++++++++- src/realm/object-store/schema.cpp | 4 + test/object-store/CMakeLists.txt | 1 + test/object-store/migrations.cpp | 123 +++++++++++++++++++++++ test/test_list.cpp | 4 +- 8 files changed, 227 insertions(+), 31 deletions(-) diff --git a/src/realm/object-store/object_schema.cpp b/src/realm/object-store/object_schema.cpp index 5e9e9e8ccb9..9fd6bcb2245 100644 --- a/src/realm/object-store/object_schema.cpp +++ b/src/realm/object-store/object_schema.cpp @@ -151,6 +151,15 @@ ObjectSchema::ObjectSchema(Group const& group, StringData name, TableKey key) property.is_fulltext_indexed = table->search_index_type(col_key) == IndexType::Fulltext; property.column_key = col_key; + // account for nesting collections + const auto nesting_levels = table->get_nesting_levels(col_key); + if (nesting_levels > 0) { + property.nested_types.reserve(nesting_levels); + for (size_t i = 0; i < nesting_levels; ++i) { + property.nested_types.push_back(table->get_nested_column_type(col_key, i)); + } + } + if (property.type == PropertyType::Object) { // set link type for objects and arrays ConstTableRef linkTable = table->get_link_target(col_key); diff --git a/src/realm/object-store/object_schema.hpp b/src/realm/object-store/object_schema.hpp index aef4692aada..fb9b4f344e1 100644 --- a/src/realm/object-store/object_schema.hpp +++ b/src/realm/object-store/object_schema.hpp @@ -88,6 +88,7 @@ class ObjectSchema { static PropertyType from_core_type(ColumnType type); static PropertyType from_core_type(ColKey col); + static PropertyType from_core_type(CollectionType type); private: void set_primary_key_property() noexcept; diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index e7bd14bffa4..fbab11c8799 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -98,6 +98,30 @@ DataType to_core_type(PropertyType type) } } +std::vector process_nested_collection(const Property& property) +{ + std::vector collection_types; + // process the list of nested levels + for (const auto& prop_type : property.nested_types) { + collection_types.push_back(prop_type); + } + + // check if the final type is itself a collection. + if (is_array(property.type)) { + collection_types.push_back(CollectionType::List); + } + else if (is_set(property.type)) { + collection_types.push_back(CollectionType::Set); + } + else if (is_dictionary(property.type)) { + collection_types.push_back(CollectionType::Dictionary); + } + else if (!collection_types.empty()) { + throw InvalidColumnKey("Not a valid nested collection type"); + } + return collection_types; +} + ColKey add_column(Group& group, Table& table, Property const& property) { // Cannot directly insert a LinkingObjects column (a computed property). @@ -110,37 +134,16 @@ ColKey add_column(Group& group, Table& table, Property const& property) return col; } } + auto collection_types = process_nested_collection(property); if (property.type == PropertyType::Object) { auto target_name = ObjectStore::table_name_for_object_type(property.object_type); TableRef link_table = group.get_table(target_name); REALM_ASSERT(link_table); - if (is_array(property.type)) { - return table.add_column_list(*link_table, property.name); - } - else if (is_set(property.type)) { - return table.add_column_set(*link_table, property.name); - } - else if (is_dictionary(property.type)) { - return table.add_column_dictionary(*link_table, property.name); - } - else { - return table.add_column(*link_table, property.name); - } - } - else if (is_array(property.type)) { - return table.add_column_list(to_core_type(property.type & ~PropertyType::Flags), property.name, - is_nullable(property.type)); - } - else if (is_set(property.type)) { - return table.add_column_set(to_core_type(property.type & ~PropertyType::Flags), property.name, - is_nullable(property.type)); - } - else if (is_dictionary(property.type)) { - return table.add_column_dictionary(to_core_type(property.type & ~PropertyType::Flags), property.name, - is_nullable(property.type)); + return table.add_column(*link_table, property.name, collection_types); } else { - auto key = table.add_column(to_core_type(property.type), property.name, is_nullable(property.type)); + auto key = table.add_column(to_core_type(property.type), property.name, is_nullable(property.type), + collection_types); if (property.requires_index()) table.add_search_index(key); if (property.requires_fulltext_index()) diff --git a/src/realm/object-store/property.hpp b/src/realm/object-store/property.hpp index 8ec66878f30..f0b53660852 100644 --- a/src/realm/object-store/property.hpp +++ b/src/realm/object-store/property.hpp @@ -27,7 +27,6 @@ #include #include - namespace realm { class BinaryData; class Decimal128; @@ -59,6 +58,7 @@ enum class PropertyType : unsigned short { Required = 0, Nullable = 64, Array = 128, + Set = 256, Dictionary = 512, @@ -96,6 +96,8 @@ struct Property { IsPrimary is_primary = false; IsIndexed is_indexed = false; IsFulltextIndexed is_fulltext_indexed = false; + using NestedTypes = std::vector; + NestedTypes nested_types; ColKey column_key; @@ -107,9 +109,15 @@ struct Property { // Text property with fulltext index Property(std::string name, IsFulltextIndexed indexed, std::string public_name = ""); + // Link to another object Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name = "", std::string public_name = ""); + // Nested collections + Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string public_name = ""); + Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string object_type, + std::string link_origin_property_name = "", std::string public_name = ""); + Property(Property const&) = default; Property(Property&&) noexcept = default; Property& operator=(Property const&) = default; @@ -127,6 +135,7 @@ struct Property { bool type_is_indexable() const noexcept; bool type_is_nullable() const noexcept; + size_t type_nesting_levels() const noexcept; std::string type_string() const; }; @@ -336,6 +345,26 @@ inline Property::Property(std::string name, PropertyType type, std::string objec { } +inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, + std::string public_name) + : name(std::move(name)) + , public_name(std::move(public_name)) + , type(type) + , nested_types(nested_types) +{ +} + +inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, + std::string object_type, std::string link_origin_property_name, std::string public_name) + : name(std::move(name)) + , public_name(std::move(public_name)) + , type(type) + , object_type(std::move(object_type)) + , link_origin_property_name(std::move(link_origin_property_name)) + , nested_types(nested_types) +{ +} + inline bool Property::type_is_indexable() const noexcept { return !is_collection(type) && @@ -350,26 +379,52 @@ inline bool Property::type_is_nullable() const noexcept type != PropertyType::LinkingObjects; } +inline size_t Property::type_nesting_levels() const noexcept +{ + return nested_types.size(); +} + inline std::string Property::type_string() const { + std::string nested; + std::string closing_brackets; + for (auto& type : nested_types) { + switch (type) { + case CollectionType::List: + nested += "array<"; + closing_brackets += ">"; + break; + case CollectionType::Dictionary: + nested += "dictionary"; if (type == PropertyType::LinkingObjects) return "linking objects<" + object_type + ">"; - return std::string("array<") + string_for_property_type(type & ~PropertyType::Flags) + ">"; + const auto str = std::string("array<") + string_for_property_type(type & ~PropertyType::Flags) + ">"; + return nested + str + closing_brackets; } if (is_set(type)) { REALM_ASSERT(type != PropertyType::LinkingObjects); if (type == PropertyType::Object) return "set<" + object_type + ">"; - return std::string("set<") + string_for_property_type(type & ~PropertyType::Flags) + ">"; + const auto str = std::string("set<") + string_for_property_type(type & ~PropertyType::Flags) + ">"; + return nested + str + closing_brackets; } if (is_dictionary(type)) { REALM_ASSERT(type != PropertyType::LinkingObjects); if (type == PropertyType::Object) return "dictionary"; - return std::string("dictionary"; + const auto str = + std::string("dictionary"; + return nested + str + closing_brackets; } switch (auto base_type = (type & ~PropertyType::Flags)) { case PropertyType::Object: diff --git a/src/realm/object-store/schema.cpp b/src/realm/object-store/schema.cpp index 00e54d9f163..3e661e86f22 100644 --- a/src/realm/object-store/schema.cpp +++ b/src/realm/object-store/schema.cpp @@ -250,6 +250,10 @@ static void compare(ObjectSchema const& existing_schema, ObjectSchema const& tar changes.emplace_back(schema_change::RemoveProperty{&existing_schema, ¤t_prop}); continue; } + if (current_prop.nested_types != target_prop->nested_types) { + changes.emplace_back(schema_change::ChangePropertyType{&existing_schema, ¤t_prop, target_prop}); + continue; + } if (current_prop.type != target_prop->type || current_prop.object_type != target_prop->object_type || is_array(current_prop.type) != is_array(target_prop->type) || is_set(current_prop.type) != is_set(target_prop->type) || diff --git a/test/object-store/CMakeLists.txt b/test/object-store/CMakeLists.txt index 715b6f1aac3..34cf629f670 100644 --- a/test/object-store/CMakeLists.txt +++ b/test/object-store/CMakeLists.txt @@ -28,6 +28,7 @@ set(SOURCES thread_safe_reference.cpp transaction_log_parsing.cpp uuid.cpp + nested_collections.cpp c_api/c_api.cpp c_api/c_api.c diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index dcbfd1dad13..f9b19128295 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -2651,3 +2651,126 @@ TEST_CASE("migration: Manual") { Catch::Matchers::ContainsSubstring("Property 'object.value' has been removed.")); } } + +TEST_CASE("migration nested collection automatic") { + TestFile config; + config.schema_mode = SchemaMode::Automatic; + auto realm = Realm::get_shared_realm(config); + + Schema schema = { + {"nested_object", {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}}}}; + REQUIRE_UPDATE_SUCCEEDS(*realm, schema, 0); + + SECTION("Remove nested list") { + Schema new_schema = remove_property(schema, "nested_object", "nested_list"); + REQUIRE_UPDATE_SUCCEEDS(*realm, schema, 1); + } + + SECTION("Add nested collection") { + Schema new_schema = + add_property(schema, "nested_object", + {"another_nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } + + SECTION("Reorder property") { + Schema new_schema = + add_property(schema, "nested_object", + {"another_nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + auto& properties = new_schema.find("nested_object")->persisted_properties; + std::swap(properties[0], properties[1]); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 2); + } + + SECTION("Change property type should fail with schema in automatic mode") { + Schema new_schema = set_type(schema, "nested_object", "nested_list", PropertyType::Int); + REQUIRE_UPDATE_FAILS( + *realm, new_schema, + "Property 'nested_object.nested_list' has been changed from 'array>' to 'int'."); + } + + SECTION("add nested collection to same object") { + Schema new_schema = + add_property(schema, "nested_object", + {"another_nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } + + SECTION("add nested collection with nullable type") { + Schema schema = { + {"nested_object", {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}}}, + {"nested_object2", + {{"nested_list_optional", + PropertyType::Array | PropertyType::Int | PropertyType::Nullable, + {CollectionType::List}}}}, + }; + REQUIRE_UPDATE_SUCCEEDS(*realm, schema, 1); + } + + SECTION("change inner type") { + Schema new_schema = { + {"nested_object", + {{"nested_list", + PropertyType::Array | PropertyType::Mixed | PropertyType::Nullable, + {CollectionType::List}}}}, + }; + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } + + SECTION("change type of nested collection") { + Schema new_schema = { + {"nested_object", + {{"nested_list", + PropertyType::Array | PropertyType::Mixed | PropertyType::Nullable, + {CollectionType::Dictionary}}}}, + }; + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } +} + +TEST_CASE("migration nested collection additive") { + + TestFile config; + config.schema_mode = SchemaMode::AdditiveExplicit; + config.schema_subset_mode = SchemaSubsetMode::Strict; // ignore reporting complete schema + auto realm = Realm::get_shared_realm(config); + + Schema schema = { + {"nested_object", {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}}}}; + REQUIRE_UPDATE_SUCCEEDS(*realm, schema, 0); + + SECTION("Add new column") { + Schema new_schema = + add_property(schema, "nested_object", + {"another_nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } + + SECTION("Remove column") { + auto new_schema = remove_property(schema, "nested_object", "nested_list"); + REQUIRE_UPDATE_SUCCEEDS(*realm, new_schema, 1); + } + + SECTION("Change inner type") { + Schema new_schema = { + {"nested_object", + {{"nested_list", + PropertyType::Array | PropertyType::Mixed | PropertyType::Nullable, + {CollectionType::List}}}}, + }; + INVALID_SCHEMA_CHANGE(*realm, new_schema, + "Property 'nested_object.nested_list' has been changed from 'array>' to " + "'array>'"); + } + + SECTION("Change type of nested collection") { + Schema new_schema = { + {"nested_object", + {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::Dictionary}}}}, + }; + INVALID_SCHEMA_CHANGE(*realm, new_schema, + "Property 'nested_object.nested_list' has been changed from 'array>' to " + "'dictionary>'"); + } +} diff --git a/test/test_list.cpp b/test/test_list.cpp index 20b94231c1e..b421af3b02c 100644 --- a/test/test_list.cpp +++ b/test/test_list.cpp @@ -33,8 +33,8 @@ using namespace std::chrono; #include "test.hpp" #include "test_types_helper.hpp" -//#include -//#define PERFORMACE_TESTING +// #include +// #define PERFORMACE_TESTING using namespace realm; using namespace realm::util; From ca2495212e5ddd26780cd14dca5fac0a2bc4fb07 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 31 Mar 2023 14:50:46 +0100 Subject: [PATCH 2/5] nested collections testing at OS level --- test/object-store/nested_collections.cpp | 186 +++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 test/object-store/nested_collections.cpp diff --git a/test/object-store/nested_collections.cpp b/test/object-store/nested_collections.cpp new file mode 100644 index 00000000000..b79967125a6 --- /dev/null +++ b/test/object-store/nested_collections.cpp @@ -0,0 +1,186 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include + +#include "util/test_file.hpp" +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace realm; + +TEST_CASE("nested-list", "[nested-collections]") { + InMemoryTestFile config; + config.cache = false; + config.automatic_change_notifications = false; + auto r = Realm::get_shared_realm(config); + r->update_schema({ + {"list_of_list", {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}}}, + {"list_of_set", {{"nested_set", PropertyType::Set | PropertyType::Int, {CollectionType::List}}}}, + {"list_of_list_list", + {{"nested_list_list", + PropertyType::Array | PropertyType::Int, + {CollectionType::List, CollectionType::List}}}}, + {"list_of_dictonary_list", + {{"nested_dict_list", + PropertyType::Array | PropertyType::Int, + {CollectionType::List, CollectionType::Dictionary}}}}, + }); + auto table1 = r->read_group().get_table("class_list_of_list"); + auto table2 = r->read_group().get_table("class_list_of_set"); + auto table3 = r->read_group().get_table("class_list_of_list_list"); + auto table4 = r->read_group().get_table("class_list_of_dictonary_list"); + REQUIRE(table1); + REQUIRE(table2); + REQUIRE(table3); + REQUIRE(table4); + + // TODO use object store API + r->begin_transaction(); + // list of list + auto nested_obj = table1->create_object(); + auto list_col_key = table1->get_column_key("nested_list"); + auto list1 = nested_obj.get_collection_list(list_col_key); + CHECK(list1->is_empty()); + auto collection_list1 = list1->insert_collection(0); + auto storage_list = dynamic_cast*>(collection_list1.get()); + storage_list->add(5); + REQUIRE(storage_list->size() == 1); + + // list of set + auto nested_obj2 = table2->create_object(); + auto set_col_key = table2->get_column_key("nested_set"); + auto list2 = nested_obj2.get_collection_list(set_col_key); + CHECK(list2->is_empty()); + auto collection_set = list2->insert_collection(0); + auto storage_set = dynamic_cast*>(collection_set.get()); + storage_set->insert(5); + REQUIRE(storage_set->size() == 1); + + // list of list of list + auto nested_obj3 = table3->create_object(); + auto list_list_col_key = table3->get_column_key("nested_list_list"); + auto list3 = nested_obj3.get_collection_list(list_list_col_key); + CHECK(list3->is_empty()); + auto collection_list3 = list3->insert_collection_list(0); + auto collection3 = collection_list3->insert_collection(0); + auto storage_list3 = dynamic_cast*>(collection3.get()); + storage_list3->add(5); + REQUIRE(storage_list3->size() == 1); + REQUIRE(collection_list3->size() == 1); + + // list of dictionary of list + auto nested_obj4 = table4->create_object(); + auto nested_dict_col_key = table4->get_column_key("nested_dict_list"); + REQUIRE(table4->get_nesting_levels(nested_dict_col_key) == 2); + auto list4 = nested_obj4.get_collection_list(nested_dict_col_key); + CHECK(list4->is_empty()); + auto collection4_dict = list4->insert_collection_list(0); + auto collection4 = collection4_dict->insert_collection("Test"); + auto storage_list4 = dynamic_cast*>(collection4.get()); + storage_list4->add(5); + REQUIRE(storage_list4->size() == 1); + REQUIRE(collection4->size() == 1); + REQUIRE(collection4_dict->size() == 1); + + r->commit_transaction(); +} + +TEST_CASE("nested-dictionary", "[nested-collections]") { + InMemoryTestFile config; + config.cache = false; + config.automatic_change_notifications = false; + auto r = Realm::get_shared_realm(config); + r->update_schema({ + {"dictionary_of_list", + {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::Dictionary}}}}, + {"dictionary_of_set", {{"nested_set", PropertyType::Set | PropertyType::Int, {CollectionType::Dictionary}}}}, + {"dictionary_of_list_of_dictionary", + {{"nested_list_dict", + PropertyType::Dictionary | PropertyType::Int, + { + CollectionType::Dictionary, + CollectionType::List, + }}}}, + }); + auto table1 = r->read_group().get_table("class_dictionary_of_list"); + auto table2 = r->read_group().get_table("class_dictionary_of_set"); + auto table3 = r->read_group().get_table("class_dictionary_of_list_of_dictionary"); + REQUIRE(table1); + REQUIRE(table2); + REQUIRE(table3); + + // TODO use object store API + r->begin_transaction(); + + auto nested_obj = table1->create_object(); + auto nested_col_key = table1->get_column_key("nested_list"); + auto dict = nested_obj.get_collection_list(nested_col_key); + auto collection = dict->insert_collection("Foo"); + auto scollection = dynamic_cast*>(collection.get()); + scollection->add(5); + REQUIRE(scollection->size() == 1); + + auto nested_obj2 = table2->create_object(); + auto nested_col_key2 = table2->get_column_key("nested_set"); + auto dict2 = nested_obj2.get_collection_list(nested_col_key2); + auto collection2 = dict2->insert_collection("Foo"); + auto scollection2 = dynamic_cast*>(collection2.get()); + scollection2->insert(5); + REQUIRE(scollection2->size() == 1); + + auto nested_obj3 = table3->create_object(); + auto nested_col_key3 = table3->get_column_key("nested_list_dict"); + auto dict3 = nested_obj3.get_collection_list(nested_col_key3); + auto nested_array = dict3->insert_collection_list("Foo"); + auto collection3 = nested_array->insert_collection(0); + auto scollection3 = dynamic_cast(collection3.get()); + scollection3->insert("hello", 5); + REQUIRE(scollection3->size() == 1); + + r->commit_transaction(); +} + +TEST_CASE("nested-set", "[nested-collections]") { + // sets can't be parent nodes. Updating the schema should fail. + InMemoryTestFile config; + config.cache = false; + config.automatic_change_notifications = false; + auto r = Realm::get_shared_realm(config); + REQUIRE_NOTHROW(r->update_schema({{"set", {{"no_nested", PropertyType::Set | PropertyType::Int}}}})); + REQUIRE_THROWS(r->update_schema( + {{"set_of_set", {{"nested", PropertyType::Set | PropertyType::Int, {CollectionType::Set}}}}})); + REQUIRE_THROWS(r->update_schema( + {{"set_of_list", {{"nested", PropertyType::Array | PropertyType::Int, {CollectionType::Set}}}}})); + REQUIRE_THROWS(r->update_schema( + {{"set_of_dictionary", {{"nested", PropertyType::Dictionary | PropertyType::Int, {CollectionType::Set}}}}})); + REQUIRE_THROWS(r->update_schema( + {{"list_set_list", + {{"nested", PropertyType::Array | PropertyType::Int, {CollectionType::List, CollectionType::Set}}}}})); + REQUIRE_THROWS(r->update_schema({{"dictionary_set_list", + {{"nested", + PropertyType::Array | PropertyType::Int, + {CollectionType::Dictionary, CollectionType::Set}}}}})); +} From d728dee4e0c87ea865bc3e1bb3cf786e3e72b0d4 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 31 Mar 2023 15:25:41 +0100 Subject: [PATCH 3/5] fix copyright --- test/object-store/nested_collections.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/object-store/nested_collections.cpp b/test/object-store/nested_collections.cpp index b79967125a6..d5be42d1ce0 100644 --- a/test/object-store/nested_collections.cpp +++ b/test/object-store/nested_collections.cpp @@ -1,6 +1,6 @@ //////////////////////////////////////////////////////////////////////////// // -// Copyright 2015 Realm Inc. +// Copyright 2023 Realm Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 6f67fa87e9c296c39d28312ea5c8f1ab67b159e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Tue, 11 Apr 2023 17:03:56 +0200 Subject: [PATCH 4/5] Update after review --- src/realm/object-store/property.hpp | 14 +++++++++----- test/object-store/nested_collections.cpp | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/realm/object-store/property.hpp b/src/realm/object-store/property.hpp index f0b53660852..0aa1a3c255e 100644 --- a/src/realm/object-store/property.hpp +++ b/src/realm/object-store/property.hpp @@ -114,7 +114,10 @@ struct Property { std::string public_name = ""); // Nested collections - Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string public_name = ""); + // Although a collection property cannot be indexed, we need the flag anyway to differentiate from + // the linking object Property constructor. + Property(std::string name, PropertyType type, const NestedTypes& nested_types, IsIndexed indexed = false, + std::string public_name = ""); Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string object_type, std::string link_origin_property_name = "", std::string public_name = ""); @@ -345,13 +348,14 @@ inline Property::Property(std::string name, PropertyType type, std::string objec { } -inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, +inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, IsIndexed indexed, std::string public_name) : name(std::move(name)) , public_name(std::move(public_name)) , type(type) , nested_types(nested_types) { + REALM_ASSERT(indexed == IsIndexed{false}); } inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, @@ -392,15 +396,15 @@ inline std::string Property::type_string() const switch (type) { case CollectionType::List: nested += "array<"; - closing_brackets += ">"; break; case CollectionType::Dictionary: nested += "dictionaryupdate_schema({ + {"target", {{"value", PropertyType::Int}}}, + {"list_of_linklist", + {{"nested_linklist", PropertyType::Array | PropertyType::Object, {CollectionType::List}, "target"}}}, {"list_of_list", {{"nested_list", PropertyType::Array | PropertyType::Int, {CollectionType::List}}}}, {"list_of_set", {{"nested_set", PropertyType::Set | PropertyType::Int, {CollectionType::List}}}}, {"list_of_list_list", @@ -48,14 +51,18 @@ TEST_CASE("nested-list", "[nested-collections]") { PropertyType::Array | PropertyType::Int, {CollectionType::List, CollectionType::Dictionary}}}}, }); + + auto target = r->read_group().get_table("class_target"); auto table1 = r->read_group().get_table("class_list_of_list"); auto table2 = r->read_group().get_table("class_list_of_set"); auto table3 = r->read_group().get_table("class_list_of_list_list"); auto table4 = r->read_group().get_table("class_list_of_dictonary_list"); + auto table5 = r->read_group().get_table("class_list_of_linklist"); REQUIRE(table1); REQUIRE(table2); REQUIRE(table3); REQUIRE(table4); + REQUIRE(table5); // TODO use object store API r->begin_transaction(); @@ -105,6 +112,17 @@ TEST_CASE("nested-list", "[nested-collections]") { REQUIRE(collection4->size() == 1); REQUIRE(collection4_dict->size() == 1); + // list of linklist + auto target_obj = target->create_object(); + auto link_obj = table5->create_object(); + auto link_col_key = table5->get_column_key("nested_linklist"); + auto list5 = link_obj.get_collection_list(link_col_key); + CHECK(list5->is_empty()); + auto collection_list5 = list5->insert_collection(0); + auto link_list = dynamic_cast(collection_list5.get()); + link_list->add(target_obj.get_key()); + REQUIRE(link_list->size() == 1); + r->commit_transaction(); } From 4e0bb19ae5684cebbbab152b442274968a873d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Wed, 12 Apr 2023 08:51:21 +0200 Subject: [PATCH 5/5] Second update after review --- src/realm/object-store/property.hpp | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/realm/object-store/property.hpp b/src/realm/object-store/property.hpp index 0aa1a3c255e..acdb4074af2 100644 --- a/src/realm/object-store/property.hpp +++ b/src/realm/object-store/property.hpp @@ -114,12 +114,8 @@ struct Property { std::string public_name = ""); // Nested collections - // Although a collection property cannot be indexed, we need the flag anyway to differentiate from - // the linking object Property constructor. - Property(std::string name, PropertyType type, const NestedTypes& nested_types, IsIndexed indexed = false, + Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string object_type = "", std::string public_name = ""); - Property(std::string name, PropertyType type, const NestedTypes& nested_types, std::string object_type, - std::string link_origin_property_name = "", std::string public_name = ""); Property(Property const&) = default; Property(Property&&) noexcept = default; @@ -348,25 +344,16 @@ inline Property::Property(std::string name, PropertyType type, std::string objec { } -inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, IsIndexed indexed, - std::string public_name) - : name(std::move(name)) - , public_name(std::move(public_name)) - , type(type) - , nested_types(nested_types) -{ - REALM_ASSERT(indexed == IsIndexed{false}); -} - inline Property::Property(std::string name, PropertyType type, const NestedTypes& nested_types, - std::string object_type, std::string link_origin_property_name, std::string public_name) + std::string target_type, std::string public_name) : name(std::move(name)) , public_name(std::move(public_name)) , type(type) - , object_type(std::move(object_type)) - , link_origin_property_name(std::move(link_origin_property_name)) + , object_type(std::move(target_type)) , nested_types(nested_types) { + REALM_ASSERT(is_collection(type)); + REALM_ASSERT((type == PropertyType::Object) == (object_type.size() != 0)); } inline bool Property::type_is_indexable() const noexcept @@ -447,7 +434,8 @@ inline bool operator==(Property const& lft, Property const& rgt) return to_underlying(lft.type) == to_underlying(rgt.type) && lft.is_primary == rgt.is_primary && lft.requires_index() == rgt.requires_index() && lft.requires_fulltext_index() == rgt.requires_fulltext_index() && lft.name == rgt.name && - lft.object_type == rgt.object_type && lft.link_origin_property_name == rgt.link_origin_property_name; + lft.object_type == rgt.object_type && lft.link_origin_property_name == rgt.link_origin_property_name && + lft.nested_types == rgt.nested_types; } } // namespace realm