From 9f2388592152b16e450396d5da1d83da383c3f91 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Tue, 17 Jan 2023 15:23:22 -0800 Subject: [PATCH 1/2] Make getting a complete view of the schema optional Rather than tying this to the Additive schema mode, make it an option which can be used with all applicable schema modes. This also fixes several bugs in the implementation which would result in it failing to actually have a complete view of the schema in some cases, and could result in frozen Realms having an incorrect schema. This also incidentally improves the performance of freezing Realms as part of the fix for the schema management and fixes a few race conditions related to cached frozen Realms. --- CHANGELOG.md | 6 +- .../object-store/impl/realm_coordinator.cpp | 36 ++- .../object-store/impl/realm_coordinator.hpp | 7 +- src/realm/object-store/object_accessor.hpp | 2 +- src/realm/object-store/object_store.cpp | 1 - src/realm/object-store/schema.cpp | 50 ++-- src/realm/object-store/schema.hpp | 41 ++- src/realm/object-store/shared_realm.cpp | 92 +++--- src/realm/object-store/shared_realm.hpp | 12 +- test/object-store/migrations.cpp | 274 ++++-------------- test/object-store/object.cpp | 19 ++ test/object-store/realm.cpp | 258 +++++++++++++++-- 12 files changed, 459 insertions(+), 339 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be936ea45ea..b3d200b56f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,16 +2,20 @@ ### Enhancements * `SyncSession::pause()` and `SyncSession::resume()` allow users to suspend a Realm's sync session until it is explicitly resumed in ([#6183](https://github.com/realm/realm-core/pull/6183)). Previously `SyncSession::log_out()` and `SyncSession::close()` could be resumed under a number of circumstances where `SyncSession::revive_if_needed()` were called (like when freezing a realm) - fixes ([#6085](https://github.com/realm/realm-core/issues/6085)) +* Improve the performance of `Realm::freeze()` by eliminating some redudant work around schema initialization and validation. These optimizations do not apply to Realm::get_frozen_realm() ([PR #6211](https://github.com/realm/realm-core/pull/6211)). ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) * "find first" on Decimal128 field with value NaN does not find objects ([6182](https://github.com/realm/realm-core/issues/6182), since v6.0.0) * Value in List of Mixed would not be updated if new value is Binary and old value is StringData and the values otherwise matches ([#6201](https://github.com/realm/realm-core/issues/6201), since v6.0.0) * When client reset with recovery is used and the recovery does not actually result in any new local commits, the sync client may have gotten stuck in a cycle with a `A fatal error occured during client reset: 'A previous 'Recovery' mode reset from did not succeed, giving up on 'Recovery' mode to prevent a cycle'` error message. ([#6195](https://github.com/realm/realm-core/issues/6195), since v11.16.0) +* Fix several data races when opening cached frozen Realms. New frozen Realms were added to the cache and the lock released before they were fully initialized, resulting in races if they were immediately read from the cache on another thread ([PR #6211](https://github.com/realm/realm-core/pull/6211), since v6.0.0). +* Properties and types not present in the requested schema would be missing from the reported schema in several scenarios, such as if the Realm was being opened with a different schema version than the persisted one, and if the new tables or columns were added while the Realm instance did not have an active read transaction ([PR #6211](https://github.com/realm/realm-core/pull/6211), since v13.2.0). ### Breaking changes * `SyncSession::log_out()` has been renamed to `SyncSession::force_close()` to reflect what it actually does ([#6183](https://github.com/realm/realm-core/pull/6183)) * Passing an empty `key_path_array` to `add_notification_callback now` now ignores nested property changes. Pass `std::nullopt` to achieve the old meaning. ([#6122](https://github.com/realm/realm-core/pull/6122)) +* Whether to report the file's complete schema or only the requested schema is now an option on RealmConfig (schema_subset_mode) rather than always being enabled for Additive schema modes. All schema modes which this applies to are now supported ([PR #6211](https://github.com/realm/realm-core/pull/6211)). ### Compatibility * Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5. @@ -77,7 +81,7 @@ ### Breaking changes * FLX Subscription API reworked to better match SDK consumption patterns ([#6065](https://github.com/realm/realm-core/pull/6065)). Not all changes are breaking, but listing them all here together. - * `Subscription` is now a plain struct with public fields rather than getter functions * `has_name()` and `name()` were merged into a single `optional name` field + * `Subscription` is now a plain struct with public fields rather than getter functions * `has_name()` and `name()` were merged into a single `optional name` field * `SubscriptionSet` now uses the same types for `iterator` and `const_iterator` since neither was intended to support direct mutability * `SubscriptionSet::get_state_change_notification()` now offers a callback-taking overload diff --git a/src/realm/object-store/impl/realm_coordinator.cpp b/src/realm/object-store/impl/realm_coordinator.cpp index c3708813ccd..36bffba9277 100644 --- a/src/realm/object-store/impl/realm_coordinator.cpp +++ b/src/realm/object-store/impl/realm_coordinator.cpp @@ -247,9 +247,7 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config, util::O util::CheckedUniqueLock lock(m_realm_mutex); set_config(config); if ((realm = do_get_cached_realm(config))) { - if (version) { - REALM_ASSERT(realm->read_transaction_version() == *version); - } + REALM_ASSERT(!version || realm->read_transaction_version() == *version); return realm; } do_get_realm(std::move(config), realm, version, lock); @@ -269,15 +267,34 @@ std::shared_ptr RealmCoordinator::get_realm(std::shared_ptr RealmCoordinator::freeze_realm(const Realm& source_realm) +{ + std::shared_ptr realm; + util::CheckedUniqueLock lock(m_realm_mutex); + + auto version = source_realm.read_transaction_version(); + auto scheduler = util::Scheduler::make_frozen(version); + if ((realm = do_get_cached_realm(source_realm.config(), scheduler))) { + return realm; + } + + auto config = source_realm.config(); + config.scheduler = scheduler; + realm = Realm::make_shared_realm(std::move(config), version, shared_from_this()); + Realm::Internal::copy_schema(*realm, source_realm); + m_weak_realm_notifiers.emplace_back(realm, config.cache); + return realm; +} + ThreadSafeReference RealmCoordinator::get_unbound_realm() { std::shared_ptr realm; util::CheckedUniqueLock lock(m_realm_mutex); - do_get_realm(m_config, realm, none, lock); + do_get_realm(RealmConfig(m_config), realm, none, lock); return ThreadSafeReference(realm); } -void RealmCoordinator::do_get_realm(Realm::Config config, std::shared_ptr& realm, +void RealmCoordinator::do_get_realm(RealmConfig&& config, std::shared_ptr& realm, util::Optional version, util::CheckedUniqueLock& realm_lock) { open_db(); @@ -305,6 +322,15 @@ void RealmCoordinator::do_get_realm(Realm::Config config, std::shared_ptr REALM_TERMINATE("Cannot use Audit interface if Realm Core is built without Sync"); #endif + // Cached frozen Realms need to initialize their schema before releasing + // the lock as otherwise they could be read from the cache on another thread + // before the schema initialization happens. They'll never perform a write + // transaction, so unlike with live Realms this is safe to do. + if (config.cache && version && schema) { + realm->update_schema(std::move(*schema)); + schema.reset(); + } + realm_lock.unlock_unchecked(); if (schema) { realm->update_schema(std::move(*schema), config.schema_version, std::move(migration_function), diff --git a/src/realm/object-store/impl/realm_coordinator.hpp b/src/realm/object-store/impl/realm_coordinator.hpp index dbc5eed947c..1ec0169ebe7 100644 --- a/src/realm/object-store/impl/realm_coordinator.hpp +++ b/src/realm/object-store/impl/realm_coordinator.hpp @@ -60,6 +60,11 @@ class RealmCoordinator : public std::enable_shared_from_this { REQUIRES(!m_realm_mutex, !m_schema_cache_mutex); std::shared_ptr get_realm(std::shared_ptr = nullptr) REQUIRES(!m_realm_mutex, !m_schema_cache_mutex); + + // Return a frozen copy of the source Realm. May return a cached instance + // if the source Realm has caching enabled. + std::shared_ptr freeze_realm(const Realm& source_realm) REQUIRES(!m_realm_mutex); + #if REALM_ENABLE_SYNC // Get a thread-local shared Realm with the given configuration // If the Realm is not already present, it will be fully downloaded before being returned. @@ -262,7 +267,7 @@ class RealmCoordinator : public std::enable_shared_from_this { std::shared_ptr do_get_cached_realm(Realm::Config const& config, std::shared_ptr scheduler = nullptr) REQUIRES(m_realm_mutex); - void do_get_realm(Realm::Config config, std::shared_ptr& realm, util::Optional version, + void do_get_realm(Realm::Config&& config, std::shared_ptr& realm, util::Optional version, util::CheckedUniqueLock& realm_lock) REQUIRES(m_realm_mutex); void run_async_notifiers() REQUIRES(!m_notifier_mutex, m_running_notifiers_mutex); void clean_up_dead_notifiers() REQUIRES(m_notifier_mutex); diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index e377ed9f50f..0c6a88d8b86 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -264,7 +264,7 @@ Object Object::create(ContextType& ctx, std::shared_ptr const& realm, Obj // considered a primary key by core, and so will need to be set. bool skip_primary = true; // If the input value is missing values for any of the properties we want to - // set the propery to the default value for new objects, but leave it + // set the property to the default value for new objects, but leave it // untouched for existing objects. bool created = false; diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index 8128f2d6b0e..a33e1a807f9 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -875,7 +875,6 @@ void ObjectStore::apply_schema_changes(Transaction& group, uint64_t schema_versi } if (mode == SchemaMode::Manual) { - set_schema_keys(group, target_schema); if (migration_function) { migration_function(); } diff --git a/src/realm/object-store/schema.cpp b/src/realm/object-store/schema.cpp index 97a7422a919..00e54d9f163 100644 --- a/src/realm/object-store/schema.cpp +++ b/src/realm/object-store/schema.cpp @@ -313,12 +313,10 @@ void Schema::zip_matching(T&& a, U&& b, Func&& func) ++j; } } - for (; i < a.size(); ++i) { + for (; i < a.size(); ++i) func(&a[i], nullptr); - } - for (; j < b.size(); ++j) { + for (; j < b.size(); ++j) func(nullptr, &b[j]); - } } std::vector Schema::compare(Schema const& target_schema, SchemaMode mode, @@ -343,10 +341,8 @@ std::vector Schema::compare(Schema const& target_schema, SchemaMod // Modify columns zip_matching(target_schema, *this, [&](const ObjectSchema* target, const ObjectSchema* existing) { - if (target && existing) { + if (target && existing) ::compare(*existing, *target, changes); - } - else if (target && !orphans.count(target->name)) { // Target is a new table -- add all properties changes.emplace_back(schema_change::AddInitialProperties{target}); @@ -364,38 +360,34 @@ std::vector Schema::compare(Schema const& target_schema, SchemaMod return changes; } -void Schema::copy_keys_from(Schema const& other, bool is_schema_additive) +void Schema::copy_keys_from(realm::Schema const& other, SchemaSubsetMode subset_mode) { - std::vector other_classes; - zip_matching(*this, other, [&](ObjectSchema const* existing, ObjectSchema const* other) { - if (is_schema_additive && !existing && other) { - other_classes.push_back(*other); - } + std::vector other_classes; + zip_matching(*this, other, [&](ObjectSchema* existing, const ObjectSchema* other) { + if (subset_mode.include_types && !existing && other) + other_classes.push_back(other); if (!existing || !other) return; - update_or_append_properties(const_cast(existing), other, is_schema_additive); + + existing->table_key = other->table_key; + for (auto& current_prop : other->persisted_properties) { + if (auto target_prop = existing->property_for_name(current_prop.name)) { + target_prop->column_key = current_prop.column_key; + } + else if (subset_mode.include_properties) { + existing->persisted_properties.push_back(current_prop); + } + } }); if (!other_classes.empty()) { - insert(end(), other_classes.begin(), other_classes.end()); + reserve(size() + other_classes.size()); + for (auto other : other_classes) + push_back(*other); sort_schema(); } } -void Schema::update_or_append_properties(ObjectSchema* existing, const ObjectSchema* other, bool is_schema_additive) -{ - existing->table_key = other->table_key; - for (auto& current_prop : other->persisted_properties) { - auto target_prop = existing->property_for_name(current_prop.name); - if (target_prop) { - target_prop->column_key = current_prop.column_key; - } - else if (is_schema_additive) { - existing->persisted_properties.push_back(current_prop); - } - } -} - namespace realm { bool operator==(SchemaChange const& lft, SchemaChange const& rgt) noexcept { diff --git a/src/realm/object-store/schema.hpp b/src/realm/object-store/schema.hpp index 07f5b2b99de..0a134bdd4cf 100644 --- a/src/realm/object-store/schema.hpp +++ b/src/realm/object-store/schema.hpp @@ -114,6 +114,43 @@ enum class SchemaMode : uint8_t { Manual }; +// Options for how to handle the schema when the file has classes and/or +// properties not in the schema. +// +// Most schema modes allow the requested schema to be a subset of the actual +// schema of the Realm file. By default, any properties or object types not in +// the requested schema are simply ignored entirely and the Realm's in-memory +// schema will always exactly match the requested one. +struct SchemaSubsetMode { + // Add additional tables present in the Realm file to the schema. This is + // applicable to all schema modes except for Manual and ResetFile. + bool include_types : 1; + + // Add additional columns in the tables present in the Realm file to the + // object schema for those types. The additional properties are always + // added to the end of persisted_properties. This is only applicable to + // Additive and ReadOnly schema modes. + bool include_properties : 1; + + // The reported schema will always exactly match the requested one. + static const SchemaSubsetMode Strict; + // Additional object classes present in the Realm file are added to the + // requested schema, but all object types present in the requested schema + // will always exactly match even if there are additional columns in the + // tables. + static const SchemaSubsetMode AllClasses; + // Additional properties present in the Realm file are added to the + // requested schema, but tables not present in the schema are ignored. + static const SchemaSubsetMode AllProperties; + // Always report the complete schema. + static const SchemaSubsetMode Complete; +}; + +inline constexpr SchemaSubsetMode SchemaSubsetMode::Strict = {false, false}; +inline constexpr SchemaSubsetMode SchemaSubsetMode::AllClasses = {true, false}; +inline constexpr SchemaSubsetMode SchemaSubsetMode::AllProperties = {false, true}; +inline constexpr SchemaSubsetMode SchemaSubsetMode::Complete = {true, true}; + class Schema : private std::vector { private: @@ -151,7 +188,7 @@ class Schema : private std::vector { std::vector compare(Schema const&, SchemaMode = SchemaMode::Automatic, bool include_removals = false) const; - void copy_keys_from(Schema const&, bool is_schema_additive = false); + void copy_keys_from(Schema const&, SchemaSubsetMode subset_mode); friend bool operator==(Schema const&, Schema const&) noexcept; friend bool operator!=(Schema const& a, Schema const& b) noexcept @@ -171,8 +208,6 @@ class Schema : private std::vector { static void zip_matching(T&& a, U&& b, Func&& func); // sort all the classes by name in order to speed up find(StringData name) void sort_schema(); - // append missing properties and update matching properties for schema - void update_or_append_properties(ObjectSchema*, const ObjectSchema*, bool); }; namespace schema_change { diff --git a/src/realm/object-store/shared_realm.cpp b/src/realm/object-store/shared_realm.cpp index e2538ad9bef..71d913d5285 100644 --- a/src/realm/object-store/shared_realm.cpp +++ b/src/realm/object-store/shared_realm.cpp @@ -77,10 +77,13 @@ class CountGuard { Realm::Realm(Config config, util::Optional version, std::shared_ptr<_impl::RealmCoordinator> coordinator, MakeSharedTag) : m_config(std::move(config)) - , m_frozen_version(std::move(version)) + , m_frozen_version(version) , m_scheduler(m_config.scheduler) { - if (!coordinator->get_cached_schema(m_schema, m_schema_version, m_schema_transaction_version)) { + if (version) { + m_auto_refresh = false; + } + else if (!coordinator->get_cached_schema(m_schema, m_schema_version, m_schema_transaction_version)) { m_transaction = coordinator->begin_read(); read_schema_from_group_if_needed(); coordinator->cache_schema(m_schema, m_schema_version, m_schema_transaction_version); @@ -110,9 +113,8 @@ Group& Realm::read_group() Transaction& Realm::transaction() { verify_open(); - if (!m_transaction) { + if (!m_transaction) begin_read(m_frozen_version.value_or(VersionID{})); - } return *m_transaction; } @@ -160,9 +162,7 @@ SharedRealm Realm::get_shared_realm(Config config) SharedRealm Realm::get_frozen_realm(Config config, VersionID version) { auto coordinator = RealmCoordinator::get_coordinator(config.path); - SharedRealm realm = coordinator->get_realm(std::move(config), util::Optional(version)); - realm->set_auto_refresh(false); - return realm; + return coordinator->get_realm(std::move(config), version); } SharedRealm Realm::get_shared_realm(ThreadSafeReference ref, std::shared_ptr scheduler) @@ -218,7 +218,7 @@ sync::SubscriptionSet Realm::get_active_subscription_set() void Realm::set_schema(Schema const& reference, Schema schema) { m_dynamic_schema = false; - schema.copy_keys_from(reference, m_config.is_schema_additive()); + schema.copy_keys_from(reference, m_config.schema_subset_mode); m_schema = std::move(schema); notify_schema_changed(); } @@ -233,12 +233,12 @@ void Realm::read_schema_from_group_if_needed() } return; } + Group& group = read_group(); auto current_version = transaction().get_version_of_current_transaction().version; if (m_schema_transaction_version == current_version) return; - auto previous_transaction_version = m_schema_transaction_version; m_schema_transaction_version = current_version; m_schema_version = ObjectStore::get_schema_version(group); auto schema = ObjectStore::schema_from_group(group); @@ -249,20 +249,16 @@ void Realm::read_schema_from_group_if_needed() if (m_dynamic_schema) { if (m_schema == schema) { // The structure of the schema hasn't changed. Bring the table column indices up to date. - m_schema.copy_keys_from(schema); + m_schema.copy_keys_from(schema, SchemaSubsetMode::Strict); } else { // The structure of the schema has changed, so replace our copy of the schema. m_schema = std::move(schema); } } - else if (m_config.is_schema_additive() && m_schema_transaction_version < previous_transaction_version) { - // no verification of schema changes when opening a past version of the schema - m_schema.copy_keys_from(schema, m_config.is_schema_additive()); - } else { ObjectStore::verify_valid_external_changes(m_schema.compare(schema, m_config.schema_mode)); - m_schema.copy_keys_from(schema); + m_schema.copy_keys_from(schema, m_config.schema_subset_mode); } notify_schema_changed(); } @@ -339,16 +335,12 @@ Schema Realm::get_full_schema() do_refresh(); // If the user hasn't specified a schema previously then m_schema is always - // the full schema - if (m_dynamic_schema) { + // the full schema if it's been read + if (m_dynamic_schema && !m_schema.empty()) return m_schema; - } // Otherwise we may have a subset of the file's schema, so we need to get // the complete thing to calculate what changes to make - if (m_config.immutable()) - return ObjectStore::schema_from_group(read_group()); - Schema actual_schema; uint64_t actual_version; uint64_t version = -1; @@ -407,8 +399,16 @@ void Realm::update_schema(Schema schema, uint64_t version, MigrationFunction mig bool was_in_read_transaction = is_in_read_transaction(); Schema actual_schema = get_full_schema(); - std::vector required_changes = actual_schema.compare(schema, m_config.schema_mode); + // Frozen Realms never modify the schema on disk and we just need to verify + // that the requested schema is a subset of what actually exists + if (m_frozen_version) { + ObjectStore::verify_valid_external_changes(schema.compare(actual_schema, m_config.schema_mode, true)); + set_schema(actual_schema, std::move(schema)); + return; + } + + std::vector required_changes = actual_schema.compare(schema, m_config.schema_mode); if (!schema_change_needs_write_transaction(schema, required_changes, version)) { if (!was_in_read_transaction) m_transaction = nullptr; @@ -444,9 +444,13 @@ void Realm::update_schema(Schema schema, uint64_t version, MigrationFunction mig cache_new_schema(); } + schema.copy_keys_from(actual_schema, m_config.schema_subset_mode); + uint64_t old_schema_version = m_schema_version; - bool is_schema_additive = m_config.is_schema_additive(); - if (migration_function && !is_schema_additive) { + bool additive = m_config.schema_mode == SchemaMode::AdditiveDiscovered || + m_config.schema_mode == SchemaMode::AdditiveExplicit || + m_config.schema_mode == SchemaMode::ReadOnly; + if (migration_function && !additive) { auto wrapper = [&] { auto config = m_config; config.schema_mode = SchemaMode::ReadOnly; @@ -476,7 +480,7 @@ void Realm::update_schema(Schema schema, uint64_t version, MigrationFunction mig else { ObjectStore::apply_schema_changes(transaction(), m_schema_version, schema, version, m_config.schema_mode, required_changes, m_config.automatically_handle_backlinks_in_migrations); - REALM_ASSERT_DEBUG(is_schema_additive || + REALM_ASSERT_DEBUG(additive || (required_changes = ObjectStore::schema_from_group(read_group()).compare(schema)).empty()); } @@ -522,7 +526,7 @@ void Realm::add_schema_change_handler() m_schema = *m_new_schema; } else { - m_schema.copy_keys_from(*m_new_schema, m_config.is_schema_additive()); + m_schema.copy_keys_from(*m_new_schema, m_config.schema_subset_mode); } notify_schema_changed(); }); @@ -530,15 +534,17 @@ void Realm::add_schema_change_handler() void Realm::cache_new_schema() { - if (!is_closed()) { - auto new_version = transaction().get_version_of_current_transaction().version; - if (m_new_schema) - m_coordinator->cache_schema(std::move(*m_new_schema), m_schema_version, new_version); - else - m_coordinator->advance_schema_cache(m_schema_transaction_version, new_version); - m_schema_transaction_version = new_version; - m_new_schema = util::none; + if (is_closed()) { + return; } + + auto new_version = transaction().get_version_of_current_transaction().version; + if (m_new_schema) + m_coordinator->cache_schema(std::move(*m_new_schema), m_schema_version, new_version); + else + m_coordinator->advance_schema_cache(m_schema_transaction_version, new_version); + m_schema_transaction_version = new_version; + m_new_schema = util::none; } void Realm::translate_schema_error() @@ -1311,12 +1317,18 @@ bool Realm::is_frozen() const SharedRealm Realm::freeze() { - auto config = m_config; - auto version = read_transaction_version(); - config.scheduler = util::Scheduler::make_frozen(version); - auto frozen_realm = Realm::get_frozen_realm(std::move(config), version); - frozen_realm->set_schema(frozen_realm->m_schema, m_schema); - return frozen_realm; + read_group(); // Freezing requires a read transaction + return m_coordinator->freeze_realm(*this); +} + +void Realm::copy_schema_from(const Realm& source) +{ + REALM_ASSERT(is_frozen()); + REALM_ASSERT(m_frozen_version == source.read_transaction_version()); + m_schema = source.m_schema; + m_schema_version = source.m_schema_version; + m_schema_transaction_version = source.m_schema_transaction_version; + m_dynamic_schema = false; } void Realm::close() diff --git a/src/realm/object-store/shared_realm.hpp b/src/realm/object-store/shared_realm.hpp index 19c557ea70f..58cf7e8d478 100644 --- a/src/realm/object-store/shared_realm.hpp +++ b/src/realm/object-store/shared_realm.hpp @@ -104,6 +104,7 @@ struct RealmConfig { bool in_memory = false; SchemaMode schema_mode = SchemaMode::Automatic; + SchemaSubsetMode schema_subset_mode = SchemaSubsetMode::Strict; // Optional schema for the file. // If the schema and schema version are supplied, update_schema() is @@ -135,11 +136,6 @@ struct RealmConfig { { return schema_mode == SchemaMode::ReadOnly; } - bool is_schema_additive() const - { - return schema_mode == SchemaMode::AdditiveExplicit || schema_mode == SchemaMode::AdditiveDiscovered || - schema_mode == SchemaMode::ReadOnly; - } // If false, always return a new Realm instance, and don't return // that Realm instance for other requests for a cached Realm. Useful @@ -477,6 +473,11 @@ class Realm : public std::enable_shared_from_this { realm.run_writes(); } + static void copy_schema(Realm& target_realm, const Realm& source_realm) + { + target_realm.copy_schema_from(source_realm); + } + // CollectionNotifier needs to be able to access the owning // coordinator to wake up the worker thread when a callback is // added, and coordinators need to be able to get themselves from a Realm @@ -556,6 +557,7 @@ class Realm : public std::enable_shared_from_this { void cache_new_schema(); void translate_schema_error(); void notify_schema_changed(); + void copy_schema_from(const Realm&); Transaction& transaction(); Transaction& transaction() const; diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 0d622d69bd9..fffc5a485c4 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -211,199 +211,6 @@ auto create_objects(Table& table, size_t count) } } // anonymous namespace -TEST_CASE("migration: Additive mode returns OS schema - Automatic migration") { - - SECTION("Check OS schema returned in additive mode") { - InMemoryTestFile config; - config.automatic_change_notifications = false; - config.schema_mode = SchemaMode::AdditiveExplicit; - auto realm = Realm::get_shared_realm(config); - - auto update_schema = [](Realm& r, Schema& s, uint64_t version) { - REQUIRE_NOTHROW((r).update_schema(s, version)); - VERIFY_SCHEMA(r, false); - auto schema = (r).schema(); - for (const auto& other : s) { - REQUIRE(schema.find(other.name) != schema.end()); - } - }; - - Schema schema1 = {}; - Schema schema2 = add_table(schema1, {"A", {{"value", PropertyType::Int}}}); - Schema schema3 = add_table(schema2, {"B", {{"value", PropertyType::Int}}}); - Schema schema4 = add_table(schema3, {"C", {{"value", PropertyType::Int}}}); - Schema schema5 = add_table(schema4, {"Z", {{"value", PropertyType::Int}}}); - update_schema(*realm, schema1, 0); - REQUIRE(realm->schema().size() == 0); - update_schema(*realm, schema2, 0); - REQUIRE(realm->schema().size() == 1); - update_schema(*realm, schema3, 0); - REQUIRE(realm->schema().size() == 2); - update_schema(*realm, schema4, 0); - REQUIRE(realm->schema().size() == 3); - update_schema(*realm, schema5, 0); - REQUIRE(realm->schema().size() == 4); - - // schema size is decremented. - // after deletion the schema size is decremented but the just deleted object can still be found. - // the object that was just deleted is still there, thus find should return a valid iterator - SECTION("delete in reverse order") { - auto new_schema = schema5; - Schema delete_schema = remove_table(new_schema, "Z"); - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("Z") != schema.end()); - delete_schema = remove_table(schema4, "C"); - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - delete_schema = remove_table(schema3, "B"); - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - delete_schema = remove_table(schema2, "A"); - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - SECTION("delete 1 element") { - auto new_schema = schema5; - Schema delete_schema = remove_table(new_schema, "Z"); - // A B C Z vs A B C ==> Z (other classes) - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - delete_schema = remove_table(new_schema, "C"); - schema = realm->schema(); - // A B C vs A B Z => Z - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - delete_schema = remove_table(new_schema, "B"); - // A B Z vs A C Z => B - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - delete_schema = remove_table(new_schema, "A"); - // A B Z vs B C Z => A - update_schema(*realm, delete_schema, 0); - schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - SECTION("delete 2 elements") { - auto new_schema = schema5; - Schema delete_schema; - delete_schema = remove_table(new_schema, "Z"); - delete_schema = remove_table(delete_schema, "A"); - // A B C Z vs B C ==> A,Z (other classes) - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - SECTION("delete 3 elements") { - auto new_schema = schema5; - Schema delete_schema; - delete_schema = remove_table(new_schema, "Z"); - delete_schema = remove_table(delete_schema, "A"); - delete_schema = remove_table(delete_schema, "C"); - // A B C Z vs B ==> A,C,Z (other classes) - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - SECTION("delete all elements") { - auto new_schema = schema5; - Schema delete_schema; - delete_schema = remove_table(new_schema, "Z"); - delete_schema = remove_table(delete_schema, "A"); - delete_schema = remove_table(delete_schema, "C"); - delete_schema = remove_table(delete_schema, "B"); - // A B C Z vs None ==> A,C,Z,B (other classes) - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - SECTION("unsorted schema object names") { - InMemoryTestFile config; - config.automatic_change_notifications = false; - config.schema_mode = SchemaMode::AdditiveExplicit; - auto realm = Realm::get_shared_realm(config); - - Schema schema1 = {}; - Schema schema2 = add_table(schema1, {"Z", {{"value", PropertyType::Int}}}); - Schema schema3 = add_table(schema2, {"B", {{"value", PropertyType::Int}}}); - Schema schema4 = add_table(schema3, {"A", {{"value", PropertyType::Int}}}); - Schema schema5 = add_table(schema4, {"C", {{"value", PropertyType::Int}}}); - update_schema(*realm, schema5, 0); - - Schema delete_schema; - delete_schema = remove_table(schema5, "Z"); - delete_schema = remove_table(delete_schema, "A"); - // Z B A C vs Z A => B C (others) - update_schema(*realm, delete_schema, 0); - auto schema = realm->schema(); - REQUIRE(schema.size() == 4); - REQUIRE(schema.find("C") != schema.end()); - REQUIRE(schema.find("Z") != schema.end()); - REQUIRE(schema.find("A") != schema.end()); - REQUIRE(schema.find("B") != schema.end()); - } - - SECTION("frozen realm schema can be updated if it is a subset of OS schema") { - InMemoryTestFile config; - config.automatic_change_notifications = false; - config.schema_mode = SchemaMode::AdditiveExplicit; - auto realm = Realm::get_shared_realm(config); - Schema schema1 = {}; - Schema schema2 = add_table(schema1, {"Z", {{"value", PropertyType::Int}}}); - Schema schema3 = add_table(schema2, {"B", {{"value", PropertyType::Int}}}); - Schema schema4 = add_table(schema3, {"A", {{"value", PropertyType::Int}}}); - update_schema(*realm, schema4, 0); - auto frozen_realm = realm->freeze(); - auto subset_schema = remove_table(schema4, "Z"); - update_schema(*realm, subset_schema, 0); - REQUIRE_NOTHROW(frozen_realm->update_schema(realm->schema())); - } - } -} - TEST_CASE("migration: Automatic") { InMemoryTestFile config; config.automatic_change_notifications = false; @@ -2537,7 +2344,7 @@ TEST_CASE("migration: Additive") { REQUIRE_NOTHROW(realm->update_schema(remove_property(schema, "object", "value"))); REQUIRE(ObjectStore::table_for_object_type(realm->read_group(), "object")->get_column_count() == 2); auto const& properties = realm->schema().find("object")->persisted_properties; - REQUIRE(properties.size() == 2); + REQUIRE(properties.size() == 1); auto col_keys = table->get_column_keys(); REQUIRE(col_keys.size() == 2); REQUIRE(properties[0].column_key == col_keys[1]); @@ -2592,10 +2399,7 @@ TEST_CASE("migration: Additive") { })); } - SECTION("add new columns from different SG") { - using vec = std::vector; - using namespace schema_change; - + SECTION("new columns added externally are ignored") { auto realm2 = Realm::get_shared_realm(config); auto& group = realm2->read_group(); realm2->begin_transaction(); @@ -2605,19 +2409,17 @@ TEST_CASE("migration: Additive") { realm2->commit_transaction(); REQUIRE_NOTHROW(realm->refresh()); - auto schema_diff = schema.compare(realm->schema()); - REQUIRE(schema_diff.size() == 1); - REQUIRE(schema_diff == vec{(AddProperty{&*schema.find("object"), - &realm->schema().find("object")->persisted_properties[2]})}); + REQUIRE(realm->schema() == schema); REQUIRE(realm->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); REQUIRE(realm->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); - REQUIRE(realm->schema().find("object")->persisted_properties[2].column_key == col_keys[2]); + + auto frozen = realm->freeze(); + REQUIRE(frozen->schema() == schema); + REQUIRE(frozen->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); + REQUIRE(frozen->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); } SECTION("opening new Realms uses the correct schema after an external change") { - using vec = std::vector; - using namespace schema_change; - auto realm2 = Realm::get_shared_realm(config); auto& group = realm2->read_group(); realm2->begin_transaction(); @@ -2627,13 +2429,9 @@ TEST_CASE("migration: Additive") { realm2->commit_transaction(); REQUIRE_NOTHROW(realm->refresh()); - auto schema_diff = schema.compare(realm->schema()); - REQUIRE(schema_diff.size() == 1); - REQUIRE(schema_diff == vec{(AddProperty{&*schema.find("object"), - &realm->schema().find("object")->persisted_properties[2]})}); + REQUIRE(realm->schema() == schema); REQUIRE(realm->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); REQUIRE(realm->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); - REQUIRE(realm->schema().find("object")->persisted_properties[2].column_key == col_keys[2]); // Gets the schema from the RealmCoordinator auto realm3 = Realm::get_shared_realm(config); @@ -2645,17 +2443,49 @@ TEST_CASE("migration: Additive") { realm2.reset(); realm3.reset(); - // In case of additive schemas, changes to an external realm are on purpose - // propagated between different realm instances. realm = Realm::get_shared_realm(config); - schema_diff = schema.compare(realm->schema()); - REQUIRE(schema_diff.size() == 1); - REQUIRE(schema_diff == vec{(AddProperty{&*schema.find("object"), - &realm->schema().find("object")->persisted_properties[2]})}); - REQUIRE(realm->schema().find("object")->persisted_properties.size() == 3); + REQUIRE(realm->schema() == schema); REQUIRE(realm->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); REQUIRE(realm->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); - REQUIRE(realm->schema().find("object")->persisted_properties[2].column_key == col_keys[2]); + } + + SECTION("obtaining a frozen Realm from before an external schema change") { + auto realm2 = Realm::get_shared_realm(config); + realm->read_group(); + realm2->read_group(); + auto table = ObjectStore::table_for_object_type(realm->read_group(), "object"); + auto col_keys = table->get_column_keys(); + + { + auto write_realm = Realm::get_shared_realm(config); + write_realm->begin_transaction(); + auto table = ObjectStore::table_for_object_type(write_realm->read_group(), "object"); + table->add_column(type_Double, "newcol"); + write_realm->commit_transaction(); + } + + // Before refreshing when we haven't seen the new version at all + auto frozen = realm->freeze(); + REQUIRE(frozen->schema() == schema); + REQUIRE(frozen->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); + REQUIRE(frozen->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); + frozen = Realm::get_frozen_realm(config, realm->read_transaction_version()); + REQUIRE(frozen->schema() == schema); + REQUIRE(frozen->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); + REQUIRE(frozen->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); + + // Refresh the other instance so that the schema cache is updated, and + // then repeat + realm2->refresh(); + + frozen = realm->freeze(); + REQUIRE(frozen->schema() == schema); + REQUIRE(frozen->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); + REQUIRE(frozen->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); + frozen = Realm::get_frozen_realm(config, realm->read_transaction_version()); + REQUIRE(frozen->schema() == schema); + REQUIRE(frozen->schema().find("object")->persisted_properties[0].column_key == col_keys[0]); + REQUIRE(frozen->schema().find("object")->persisted_properties[1].column_key == col_keys[1]); } SECTION("can have different subsets of columns in different Realm instances") { @@ -2671,11 +2501,11 @@ TEST_CASE("migration: Additive") { auto realm3 = Realm::get_shared_realm(config3); REQUIRE(realm->schema().find("object")->persisted_properties.size() == 2); REQUIRE(realm2->schema().find("object")->persisted_properties.size() == 3); - REQUIRE(realm3->schema().find("object")->persisted_properties.size() == 3); + REQUIRE(realm3->schema().find("object")->persisted_properties.size() == 1); realm->refresh(); realm2->refresh(); - REQUIRE(realm->schema().find("object")->persisted_properties.size() == 3); + REQUIRE(realm->schema().find("object")->persisted_properties.size() == 2); REQUIRE(realm2->schema().find("object")->persisted_properties.size() == 3); // No schema specified; should see all of them @@ -2722,7 +2552,7 @@ TEST_CASE("migration: Additive") { REQUIRE_THROWS_CONTAINING( realm->update_schema(add_property(schema, "object", {"value 3", PropertyType::Float})), "Property 'object.value 3' has been changed from 'int' to 'float'."); - REQUIRE(realm->schema().find("object")->persisted_properties.size() == 3); + REQUIRE(realm->schema().find("object")->persisted_properties.size() == 2); } SECTION("update_schema() does not begin a write transaction when extra columns are present") { diff --git a/test/object-store/object.cpp b/test/object-store/object.cpp index 8322e228d32..7197370a329 100644 --- a/test/object-store/object.cpp +++ b/test/object-store/object.cpp @@ -164,6 +164,7 @@ TEST_CASE("object") { InMemoryTestFile config; config.cache = false; config.automatic_change_notifications = false; + config.schema_mode = SchemaMode::AdditiveExplicit; config.schema = Schema{ {"table", { @@ -1774,6 +1775,24 @@ TEST_CASE("object") { REQUIRE(Results(r, r->read_group().get_table("class_nullable object id pk")).size() == 2); } + SECTION("create only requires properties explicitly in the schema") { + config.schema = Schema{{"all types", {{"_id", PropertyType::Int, Property::IsPrimary{true}}}}}; + auto subset_realm = Realm::get_shared_realm(config); + subset_realm->begin_transaction(); + REQUIRE_NOTHROW(Object::create(d, subset_realm, "all types", std::any(AnyDict{{"_id", INT64_C(123)}}))); + subset_realm->commit_transaction(); + + r->refresh(); + auto obj = *r->read_group().get_table("class_all types")->begin(); + REQUIRE(obj.get("_id") == 123); + + // Other columns should have the default unset values + REQUIRE(obj.get("bool") == false); + REQUIRE(obj.get("int") == 0); + REQUIRE(obj.get("float") == 0); + REQUIRE(obj.get("string") == ""); + } + SECTION("getters and setters") { r->begin_transaction(); diff --git a/test/object-store/realm.cpp b/test/object-store/realm.cpp index 61886516ce1..b7b094ec306 100644 --- a/test/object-store/realm.cpp +++ b/test/object-store/realm.cpp @@ -19,8 +19,6 @@ #include #include -#include - #include "util/event_loop.hpp" #include "util/test_file.hpp" #include "util/test_utils.hpp" @@ -360,35 +358,6 @@ TEST_CASE("SharedRealm: get_shared_realm()") { REQUIRE(old_realm->schema().size() == 1); } - SECTION("should skip schema verification with mode additive and transaction version less than current version") { - - auto realm1 = Realm::get_shared_realm(config); - auto& db1 = TestHelper::get_db(realm1); - auto rt1 = db1->start_read(); - // grab the initial transaction version. - const auto version1 = rt1->get_version_of_current_transaction(); - realm1->close(); - - // update the schema - config.schema_mode = SchemaMode::AdditiveExplicit; - config.schema = Schema{ - {"object", {{"value", PropertyType::Int}}}, - {"object1", {{"value", PropertyType::Int}}}, - }; - auto realm2 = Realm::get_shared_realm(config); - - // no verification if the version chosen is less than the current transaction schema version. - // the schemas should be just merged - TestHelper::begin_read(realm2, version1); - auto& group = realm2->read_group(); - auto schema = realm2->schema(); - REQUIRE(schema == config.schema); - auto table_obj = group.get_table("class_object"); - auto table_obj1 = group.get_table("class_object1"); - REQUIRE(table_obj); // empty schema always has class_object - REQUIRE_FALSE(table_obj1); // class_object1 should not be present - } - SECTION("should sensibly handle opening an uninitialized file without a schema specified") { SECTION("cached") { } @@ -561,6 +530,20 @@ TEST_CASE("SharedRealm: get_shared_realm()") { REQUIRE(object_schema == &*realm->schema().find("object")); } + SECTION("should reuse cached frozen Realm if versions match") { + config.cache = true; + auto realm = Realm::get_shared_realm(config); + realm->read_group(); + auto frozen = realm->freeze(); + frozen->read_group(); + + REQUIRE(frozen != realm); + REQUIRE(realm->read_transaction_version() == frozen->read_transaction_version()); + + REQUIRE(realm->freeze() == frozen); + REQUIRE(Realm::get_frozen_realm(config, realm->read_transaction_version()) == frozen); + } + SECTION("should not use cached frozen Realm if versions don't match") { config.cache = true; auto realm = Realm::get_shared_realm(config); @@ -614,6 +597,30 @@ TEST_CASE("SharedRealm: get_shared_realm()") { REQUIRE(frozen_schema == subset_schema); } + SECTION("frozen realm should have the correct schema even if more properties are added later") { + config.schema_mode = SchemaMode::AdditiveExplicit; + auto full_schema = Schema{ + {"object", {{"value1", PropertyType::Int}, {"value2", PropertyType::Int}}}, + }; + + auto subset_schema = Schema{ + {"object", {{"value1", PropertyType::Int}}}, + }; + + config.schema = subset_schema; + auto realm = Realm::get_shared_realm(config); + realm->read_group(); + + config.schema = full_schema; + auto realm2 = Realm::get_shared_realm(config); + realm2->read_group(); + + auto frozen_realm = realm->freeze(); + REQUIRE(realm->schema() == subset_schema); + REQUIRE(realm2->schema() == full_schema); + REQUIRE(frozen_realm->schema() == subset_schema); + } + SECTION("freeze with orphaned embedded tables") { auto schema = Schema{ {"object1", {{"value", PropertyType::Int}}}, @@ -628,6 +635,195 @@ TEST_CASE("SharedRealm: get_shared_realm()") { } } +TEST_CASE("SharedRealm: schema_subset_mode") { + TestFile config; + config.schema_mode = SchemaMode::AdditiveExplicit; + config.schema_version = 1; + config.schema_subset_mode = SchemaSubsetMode::Complete; + config.encryption_key.clear(); + + // Use a DB directly to simulate changes made by another process + auto db = DB::create(make_in_realm_history(), config.path); + + // Changing the schema version results in update_schema() hitting a very + // different code path for Additive modes, so test both with the schema version + // matching and not matching + auto set_schema_version = GENERATE(false, true); + INFO("Matching schema version: " << set_schema_version); + if (set_schema_version) { + auto tr = db->start_write(); + ObjectStore::set_schema_version(*tr, 1); + tr->commit(); + } + + SECTION("additional properties are added at the end") { + { + auto tr = db->start_write(); + auto table = tr->add_table("class_object"); + for (int i = 0; i < 5; ++i) { + table->add_column(type_Int, util::format("col %1", i)); + } + tr->commit(); + } + + // missing col 0 and 4, and order is different from column order + config.schema = Schema{{"object", + { + {"col 2", PropertyType::Int}, + {"col 3", PropertyType::Int}, + {"col 1", PropertyType::Int}, + }}}; + + auto realm = Realm::get_shared_realm(config); + auto& properties = realm->schema().find("object")->persisted_properties; + REQUIRE(properties.size() == 5); + REQUIRE(properties[0].name == "col 2"); + REQUIRE(properties[1].name == "col 3"); + REQUIRE(properties[2].name == "col 1"); + REQUIRE(properties[3].name == "col 0"); + REQUIRE(properties[4].name == "col 4"); + + for (auto& property : properties) { + REQUIRE(property.column_key != ColKey{}); + } + + config.schema_subset_mode.include_properties = false; + realm = Realm::get_shared_realm(config); + REQUIRE(realm->schema().find("object")->persisted_properties.size() == 3); + } + + SECTION("additional tables are added in sorted order") { + { + auto tr = db->start_write(); + // In reverse order so that just using the table order doesn't + // work accidentally + tr->add_table("class_F")->add_column(type_Int, "value"); + tr->add_table("class_E")->add_column(type_Int, "value"); + tr->add_table("class_D")->add_column(type_Int, "value"); + tr->add_table("class_C")->add_column(type_Int, "value"); + tr->add_table("class_B")->add_column(type_Int, "value"); + tr->add_table("class_A")->add_column(type_Int, "value"); + tr->commit(); + } + + config.schema = Schema{ + {"A", {{"value", PropertyType::Int}}}, + {"E", {{"value", PropertyType::Int}}}, + {"D", {{"value", PropertyType::Int}}}, + }; + auto realm = Realm::get_shared_realm(config); + auto& schema = realm->schema(); + REQUIRE(schema.size() == 6); + REQUIRE(std::is_sorted(schema.begin(), schema.end(), [](auto& a, auto& b) { + return a.name < b.name; + })); + + config.schema_subset_mode.include_types = false; + realm = Realm::get_shared_realm(config); + REQUIRE(realm->schema().size() == 3); + } + + SECTION("schema is updated when refreshing over a schema change") { + config.schema = Schema{{"object", {{"value", PropertyType::Int}}}}; + auto realm = Realm::get_shared_realm(config); + realm->read_group(); + auto& schema = realm->schema(); + + { + auto tr = db->start_write(); + tr->get_table("class_object")->add_column(type_Int, "value 2"); + tr->commit(); + } + + REQUIRE(schema.find("object")->persisted_properties.size() == 1); + realm->refresh(); + REQUIRE(schema.find("object")->persisted_properties.size() == 2); + + { + auto tr = db->start_write(); + tr->add_table("class_object 2")->add_column(type_Int, "value"); + tr->commit(); + } + + REQUIRE(schema.size() == 1); + realm->refresh(); + REQUIRE(schema.size() == 2); + } + + SECTION("schema is updated when schema is modified while not in a read transaction") { + config.schema = Schema{{"object", {{"value", PropertyType::Int}}}}; + auto realm = Realm::get_shared_realm(config); + auto& schema = realm->schema(); + + { + auto tr = db->start_write(); + tr->get_table("class_object")->add_column(type_Int, "value 2"); + tr->commit(); + } + + REQUIRE(schema.find("object")->persisted_properties.size() == 1); + realm->read_group(); + REQUIRE(schema.find("object")->persisted_properties.size() == 2); + realm->invalidate(); + + { + auto tr = db->start_write(); + tr->add_table("class_object 2")->add_column(type_Int, "value"); + tr->commit(); + } + + REQUIRE(schema.size() == 1); + realm->read_group(); + REQUIRE(schema.size() == 2); + } + + SECTION("frozen Realm sees the correct schema for each version") { + config.schema = Schema{{"object", {{"value", PropertyType::Int}}}}; + std::vector> realms; + for (int i = 0; i < 10; ++i) { + realms.push_back(Realm::get_shared_realm(config)); + realms.back()->read_group(); + auto tr = db->start_write(); + tr->add_table(util::format("class_object %1", i))->add_column(type_Int, "value"); + tr->commit(); + } + + for (size_t i = 0; i < 10; ++i) { + auto& r = *realms[i]; + REQUIRE(r.schema().size() == i + 1); + REQUIRE(r.freeze()->schema().size() == i + 1); + REQUIRE(Realm::get_frozen_realm(config, r.read_transaction_version())->schema().size() == i + 1); + } + } + + SECTION("obtaining a frozen realm with an incompatible schema throws") { + config.schema = Schema{{"object", {{"value", PropertyType::Int}}}}; + auto old_realm = Realm::get_shared_realm(config); + old_realm->read_group(); + + { + auto tr = db->start_write(); + tr->add_table("class_object 2")->add_column(type_Int, "value"); + tr->commit(); + } + + config.schema = Schema{ + {"object", {{"value", PropertyType::Int}}}, + {"object 2", {{"value", PropertyType::Int}}}, + }; + auto new_realm = Realm::get_shared_realm(config); + new_realm->read_group(); + + REQUIRE(old_realm->freeze()->schema().size() == 1); + REQUIRE(new_realm->freeze()->schema().size() == 2); + REQUIRE(Realm::get_frozen_realm(config, new_realm->read_transaction_version())->schema().size() == 2); + // Fails because the requested version doesn't have the "object 2" table + // required by the config + REQUIRE_THROWS_AS(Realm::get_frozen_realm(config, old_realm->read_transaction_version()), + InvalidExternalSchemaChangeException); + } +} + #if REALM_ENABLE_SYNC TEST_CASE("Get Realm using Async Open", "[asyncOpen]") { if (!util::EventLoop::has_implementation()) From 19817a53aaf65a402b90303cd49b57ea90b0e26e Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 19 Jan 2023 11:31:11 -0800 Subject: [PATCH 2/2] Make add_key_based_notification_callback() take an optional keypath array --- src/realm/object-store/dictionary.cpp | 5 +++-- src/realm/object-store/dictionary.hpp | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/realm/object-store/dictionary.cpp b/src/realm/object-store/dictionary.cpp index b70c39be180..a5ef5bcf7bb 100644 --- a/src/realm/object-store/dictionary.cpp +++ b/src/realm/object-store/dictionary.cpp @@ -335,9 +335,10 @@ class NotificationHandler { Dictionary::CBFunc m_cb; }; -NotificationToken Dictionary::add_key_based_notification_callback(CBFunc cb, KeyPathArray key_path_array) & +NotificationToken Dictionary::add_key_based_notification_callback(CBFunc cb, + std::optional key_path_array) & { - return add_notification_callback(NotificationHandler(dict(), std::move(cb)), key_path_array); + return add_notification_callback(NotificationHandler(dict(), std::move(cb)), std::move(key_path_array)); } Dictionary Dictionary::freeze(const std::shared_ptr& frozen_realm) const diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index be4004b7e12..801df837a6c 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -118,7 +118,8 @@ class Dictionary : public object_store::Collection { Results get_values() const; using CBFunc = util::UniqueFunction; - NotificationToken add_key_based_notification_callback(CBFunc cb, KeyPathArray key_path_array = {}) &; + NotificationToken + add_key_based_notification_callback(CBFunc cb, std::optional key_path_array = std::nullopt) &; Iterator begin() const; Iterator end() const;