From d09e4a3880309aae1b70c41b4779e1a08f7f70d4 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 27 Mar 2024 10:13:55 -0400 Subject: [PATCH 01/37] First round of changes for server-initiated bootstraps --- src/realm/sync/client.cpp | 37 ++++++++++++------- src/realm/sync/noinst/client_impl_base.cpp | 23 ++++++++++-- src/realm/sync/noinst/client_impl_base.hpp | 7 +++- .../sync/noinst/pending_bootstrap_store.cpp | 21 ++++++++--- .../object-store/util/sync/baas_admin_api.cpp | 11 ++++++ .../object-store/util/sync/baas_admin_api.hpp | 1 + 6 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 2a35c746aab..0c6e5309b27 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -825,12 +825,8 @@ void SessionImpl::update_subscription_version_info() bool SessionImpl::process_flx_bootstrap_message(const SyncProgress& progress, DownloadBatchState batch_state, int64_t query_version, const ReceivedChangesets& received_changesets) { - // Ignore the call if the session is not active - if (m_state != State::Active) { - return false; - } - - if (is_steady_state_download_message(batch_state, query_version)) { + // Ignore the message if the session is not active or a steady state message + if (m_state != State::Active || batch_state == DownloadBatchState::SteadyState) { return false; } @@ -1081,23 +1077,36 @@ SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, con return call_debug_hook(data); } -bool SessionImpl::is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version) +bool SessionImpl::is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, + bool same_server_version) { // Should never be called if session is not active REALM_ASSERT_EX(m_state == State::Active, m_state); - if (batch_state == DownloadBatchState::SteadyState) { - return true; - } + // Query version should always be the same or increasing + REALM_ASSERT_3(query_version, >=, m_wrapper.m_flx_active_version); - if (!m_is_flx_sync_session) { + // Return early if already steady state or PBS (doesn't use bootstraps) + if (batch_state == DownloadBatchState::SteadyState || !m_is_flx_sync_session) { return true; } - // If this is a steady state DOWNLOAD, no need for special handling. - if (batch_state == DownloadBatchState::LastInBatch && query_version == m_wrapper.m_flx_active_version) { - return true; + // Bootstrap messages (i.e. non-steady state) are identified by: + // * DownloadBatchState of MoreToCome + // * DownloadBatchState of LastInBatch, and + // * first LastInBatch after one or more MoreToCome messages + // * query_version greater than the active query_version + // * message has 2 or more changesets and all have the same remote_version + + // If this is a single LastInBatch message, check to see if it is a single + // message bootstrap or just a steady state download message + if (batch_state == DownloadBatchState::LastInBatch && + m_last_download_batch_state != DownloadBatchState::MoreToCome) { + if (query_version == m_wrapper.m_flx_active_version && !same_server_version) { + return true; + } } + // Otherwise this is a bootstrap message return false; } diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 1d73f893368..90a2e80f410 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2360,10 +2360,6 @@ Status Session::receive_download_message(const SyncProgress& progress, std::uint if (m_state != Active) return Status::OK(); - if (is_steady_state_download_message(batch_state, query_version)) { - batch_state = DownloadBatchState::SteadyState; - } - logger.debug("Received: DOWNLOAD(download_server_version=%1, download_client_version=%2, " "latest_server_version=%3, latest_server_version_salt=%4, " "upload_client_version=%5, upload_server_version=%6, downloadable_bytes=%7, " @@ -2390,6 +2386,11 @@ Status Session::receive_download_message(const SyncProgress& progress, std::uint } version_type server_version = m_progress.download.server_version; + // If there are 2 or more changesets in this message, track whether or not the changesets have the same + // download_server_version to help with determining if this is a bootstrap message or not. + bool same_server_version = + received_changesets.size() > 1 && progress.download.server_version == received_changesets[0].remote_version; + bool after_first_changeset = false; version_type last_integrated_client_version = m_progress.download.last_integrated_client_version; for (const RemoteChangeset& changeset : received_changesets) { // Check that per-changeset server version is strictly increasing, except in FLX sync where the server @@ -2403,7 +2404,16 @@ Status Session::receive_download_message(const SyncProgress& progress, std::uint util::format("Bad server version in changeset header (DOWNLOAD) (%1, %2, %3)", changeset.remote_version, server_version, progress.download.server_version)}; } + if (after_first_changeset) + // After the first changeset compare the previous changeset's server version to the current changeset + // server version. If they are different then this is definitely not a bootstrap message + same_server_version = same_server_version && server_version == changeset.remote_version; + else + // Skip the first changeset, since `server_version` contains the incorrect value for this check + after_first_changeset = true; + server_version = changeset.remote_version; + // Check that per-changeset last integrated client version is "weakly" // increasing. bool good_client_version = @@ -2428,6 +2438,11 @@ Status Session::receive_download_message(const SyncProgress& progress, std::uint } } + if (is_steady_state_download_message(batch_state, query_version, same_server_version)) { + batch_state = DownloadBatchState::SteadyState; + } + m_last_download_batch_state = batch_state; + auto hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageReceived, progress, query_version, batch_state, received_changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index a354e74761a..a271885aa60 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1131,6 +1131,10 @@ class ClientImpl::Session { // the detection of download completion. request_ident_type m_last_triggering_download_mark = 0; + // Keep track of the DownloadBatchState for the last download message to help + // determine if the current LastInBatch came after a message with MoreToCome. + DownloadBatchState m_last_download_batch_state = DownloadBatchState::SteadyState; + SessionWrapper& m_wrapper; request_ident_type m_last_pending_test_command_ident = 0; @@ -1197,7 +1201,8 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo&); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); - bool is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version); + bool is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, + bool same_server_version); friend class Connection; }; diff --git a/src/realm/sync/noinst/pending_bootstrap_store.cpp b/src/realm/sync/noinst/pending_bootstrap_store.cpp index 6a7d0aad2a2..6bc9fec439e 100644 --- a/src/realm/sync/noinst/pending_bootstrap_store.cpp +++ b/src/realm/sync/noinst/pending_bootstrap_store.cpp @@ -169,6 +169,7 @@ void PendingBootstrapStore::add_batch(int64_t query_version, util::Optionalcommit(); @@ -177,15 +178,18 @@ void PendingBootstrapStore::add_batch(int64_t query_version, util::Optionalstart_write(); + auto tr = m_db->start_read(); auto bootstrap_table = tr->get_table(m_table); + // Just make sure the state is reset if the bootstrap table is empty + if (bootstrap_table->is_empty()) { + return; + } + tr->promote_to_write(); bootstrap_table->clear(); m_has_pending = false; tr->commit(); diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index 41dd4337f8d..f1a1b8f637c 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -942,6 +942,17 @@ void AdminAPISession::create_schema(const std::string& app_id, const AppCreateCo } } +bool AdminAPISession::set_feature_flag(const std::string& app_id, const std::string& flag_name, bool enable) const +{ + auto endpoint = + AdminAPIEndpoint(util::format("%1/api/private/v1.0/features/%2", m_base_url, flag_name), m_access_token); + auto flag_response = endpoint.put_json( + nlohmann::json{{"app_ids", nlohmann::json::array({app_id})}, {"action", enable ? "enable" : "disable"}}); + auto actual_modified = flag_response["actual_modified"]; + // "actual_modified" should only be 1 since only 1 app_id was provided + return actual_modified.is_number_integer() && actual_modified.get() == 1; +} + static nlohmann::json convert_config(AdminAPISession::ServiceConfig config) { if (config.mode == AdminAPISession::ServiceConfig::SyncMode::Flexible) { diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index c45a150bb45..e9d554d3093 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -83,6 +83,7 @@ class AdminAPISession { void trigger_client_reset(const std::string& app_id, int64_t file_ident) const; void migrate_to_flx(const std::string& app_id, const std::string& service_id, bool migrate_to_flx) const; void create_schema(const std::string& app_id, const AppCreateConfig& config, bool use_draft = true) const; + bool set_feature_flag(const std::string& app_id, const std::string& flag_name, bool enable) const; struct Service { std::string id; From 43391caf47b5b57f96821154e28a4fda60e078f1 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 9 Apr 2024 23:59:46 -0400 Subject: [PATCH 02/37] Added test for role change bootstraps --- src/realm/object-store/sync/sync_session.cpp | 19 +++ src/realm/object-store/sync/sync_session.hpp | 5 +- test/object-store/sync/flx_sync.cpp | 160 ++++++++++++++++++ .../object-store/util/sync/baas_admin_api.cpp | 49 +++++- .../object-store/util/sync/baas_admin_api.hpp | 4 + .../util/sync/flx_sync_harness.hpp | 2 +- test/object-store/util/test_utils.hpp | 4 +- 7 files changed, 237 insertions(+), 6 deletions(-) diff --git a/src/realm/object-store/sync/sync_session.cpp b/src/realm/object-store/sync/sync_session.cpp index 9de4c49948a..7ad0aaa6ee1 100644 --- a/src/realm/object-store/sync/sync_session.cpp +++ b/src/realm/object-store/sync/sync_session.cpp @@ -39,6 +39,8 @@ #include #include +#include + using namespace realm; using namespace realm::_impl; @@ -47,6 +49,23 @@ using SessionWaiterPointer = void (sync::Session::*)(util::UniqueFunction #include +#include +#include #include #include -#include namespace realm { class DB; @@ -535,6 +536,8 @@ class SyncSession : public std::enable_shared_from_this { bool m_schema_migration_in_progress GUARDED_BY(m_state_mutex) = false; }; +std::ostream& operator<<(std::ostream& out, const SyncSession::ConnectionState& val); + } // namespace realm #endif // REALM_OS_SYNC_SESSION_HPP diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 9ba6e1688ef..9ecb8615ac7 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4788,6 +4788,166 @@ TEST_CASE("flx: pause and resume bootstrapping at query version 0", "[sync][flx] REQUIRE(active_sub_set.state() == sync::SubscriptionSet::State::Complete); } + +static void fill_person_schema(FLXSyncTestHarness& harness) +{ + harness.load_initial_data([&](SharedRealm realm) { + int cnt = 1; + { + CppContext c(realm); + for (int i = 0; i < 100; ++i) { + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{ + {"_id", ObjectId::gen()}, + {"age", static_cast(40 + i)}, + {"role", "manager"s}, + {"firstName", util::format("firstname-%1", cnt)}, + {"lastName", util::format("lastname-%1", cnt)}, + })); + ++cnt; + } + } + { + CppContext c(realm); + for (int i = 0; i < 5000; ++i) { + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{{"_id", ObjectId::gen()}, + {"age", static_cast(20 + i % 30)}, + {"role", "employee"s}, + {"firstName", util::format("firstname-%1", cnt)}, + {"lastName", util::format("lastname-%1", cnt)}})); + ++cnt; + } + } + { + CppContext c(realm); + for (int i = 0; i < 125; ++i) { + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{{"_id", ObjectId::gen()}, + {"age", static_cast(45 + i)}, + {"role", "director"s}, + {"firstName", util::format("firstname-9999%1", cnt)}, + {"lastName", util::format("lastname-9999%1", cnt)}})); + ++cnt; + } + } + }); +} + +TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { + const Schema person_schema{{"Person", + {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, + {"age", PropertyType::Int | PropertyType::Nullable}, + {"role", PropertyType::String}, + {"firstName", PropertyType::String}, + {"lastName", PropertyType::String}}}}; + enum TestState { initial, disconnected, connecting, connected }; + TestingStateMachine machina(initial); + + auto logger = util::Logger::get_default_logger(); + FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); + auto& app_session = harness.session().app_session(); + // Enable the role change bootstraps + REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); + // Ensure the feature flag is enabled + REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + + auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); + + // Initialize the realm with some data + fill_person_schema(harness); + + auto config = harness.make_test_file(); + auto realm = Realm::get_shared_realm(config); + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + + + // Set up the initial subscription + { + auto table = realm->read_group().get_table("class_Person"); + auto new_subs = realm->get_latest_subscription_set().make_mutable_copy(); + new_subs.insert_or_assign(Query(table)); + auto subs = new_subs.commit(); + + // Wait for subscription update and sync to complete + subs.get_state_change_notification(sync::SubscriptionSet::State::Complete).get(); + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + wait_for_advance(*realm); + } + { + auto table = realm->read_group().get_table("class_Person"); + Results results(realm, Query(table)); + CHECK(results.size() == 5225); + } + + // Register connection change callback to verify session was restarted + using ConnState = SyncSession::ConnectionState; + auto session = realm->sync_session(); + session->register_connection_change_callback([&machina, &logger](ConnState old_state, ConnState new_state) { + logger->info("Connection state: %1 -> %2", old_state, new_state); + machina.transition_with([new_state](TestState curr_state) { + switch (new_state) { + case ConnState::Disconnected: + return TestState::disconnected; + case ConnState::Connecting: + REQUIRE(curr_state == TestState::disconnected); + return TestState::connecting; + case ConnState::Connected: + REQUIRE(curr_state == TestState::connecting); + return TestState::connected; + default: + FAIL(util::format("Unknown connection state: %1", new_state)); + } + }); + }); + + auto update_perms = [&](std::optional doc_filter, size_t num_employees, size_t num_dirs_mgrs) { + // Updating the role to employees only + logger->trace(">>>>> Updating permissions...JSON: %1", doc_filter); + if (doc_filter) { + rule["roles"][0]["document_filters"]["read"] = *doc_filter; + rule["roles"][0]["document_filters"]["write"] = *doc_filter; + } + else { + rule["roles"][0]["document_filters"]["read"] = true; + rule["roles"][0]["document_filters"]["write"] = true; + } + app_session.admin_api.update_default_rule(app_session.server_app_id, rule); + logger->trace(">>>>> Complete"); + + // Client should receive an error and drop/reconnect the session + REQUIRE(machina.wait_for(TestState::disconnected)); + REQUIRE(machina.wait_for(TestState::connected)); + + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + wait_for_advance(*realm); + { + auto table = realm->read_group().get_table("class_Person"); + REQUIRE(table->size() == (num_employees + num_dirs_mgrs)); + auto role_col = table->get_column_key("role"); + Query query_mgrs(table); + query_mgrs.equal(role_col, "director").Or().equal(role_col, "manager"); + Results results_mgrs(realm, query_mgrs); + CHECK(results_mgrs.size() == num_dirs_mgrs); + Query query_emps(table); + query_emps.equal(role_col, "employee"); + Results results_emps(realm, query_emps); + CHECK(results_emps.size() == num_employees); + } + }; + { + nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; + update_perms(doc_filter, 0, 225); + } + { + nlohmann::json doc_filter = {{"role", "employee"}}; + update_perms(doc_filter, 5000, 0); + } +} + } // namespace realm::app #endif // REALM_ENABLE_AUTH_TESTS diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index f1a1b8f637c..9d38a3de5be 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -944,15 +944,60 @@ void AdminAPISession::create_schema(const std::string& app_id, const AppCreateCo bool AdminAPISession::set_feature_flag(const std::string& app_id, const std::string& flag_name, bool enable) const { - auto endpoint = + auto features_endpoint = AdminAPIEndpoint(util::format("%1/api/private/v1.0/features/%2", m_base_url, flag_name), m_access_token); - auto flag_response = endpoint.put_json( + auto flag_response = features_endpoint.post_json( nlohmann::json{{"app_ids", nlohmann::json::array({app_id})}, {"action", enable ? "enable" : "disable"}}); auto actual_modified = flag_response["actual_modified"]; // "actual_modified" should only be 1 since only 1 app_id was provided return actual_modified.is_number_integer() && actual_modified.get() == 1; } +bool AdminAPISession::get_feature_flag(const std::string& app_id, const std::string& flag_name) const +{ + auto settings_json = get_app_settings(app_id); + // Format: {..., "features": {"enabled": ["", "", ...], ...}, ...} + if (auto features = settings_json.find("features"); + features != settings_json.end() && features->is_object() && !features->empty()) { + if (auto enabled_list = features->find("enabled"); + enabled_list != features->end() && enabled_list->is_array() && !enabled_list->empty()) { + for (auto& item : *enabled_list) { + if (item.is_string() && item.get() == flag_name) { + return true; + } + } + } + } + return false; +} + +nlohmann::json AdminAPISession::get_default_rule(const std::string& app_id) const +{ + auto baas_sync_service = get_sync_service(app_id); + auto rule_endpoint = apps()[app_id]["services"][baas_sync_service.id]["default_rule"]; + auto rule = rule_endpoint.get_json(); + return rule; +} + +bool AdminAPISession::update_default_rule(const std::string& app_id, nlohmann::json rule_json) const +{ + if (auto id = rule_json.find("_id"); + id == rule_json.end() || !id->is_string() || id->get().empty()) { + return false; + } + + auto baas_sync_service = get_sync_service(app_id); + auto rule_endpoint = apps()[app_id]["services"][baas_sync_service.id]["default_rule"]; + auto response = rule_endpoint.put_json(rule_json); + return response.empty(); +} + +nlohmann::json AdminAPISession::get_app_settings(const std::string& app_id) const +{ + auto settings_endpoint = apps(APIFamily::Private)[app_id]["settings"]; + return settings_endpoint.get_json(); +} + static nlohmann::json convert_config(AdminAPISession::ServiceConfig config) { if (config.mode == AdminAPISession::ServiceConfig::SyncMode::Flexible) { diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index e9d554d3093..9745988c410 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -84,6 +84,9 @@ class AdminAPISession { void migrate_to_flx(const std::string& app_id, const std::string& service_id, bool migrate_to_flx) const; void create_schema(const std::string& app_id, const AppCreateConfig& config, bool use_draft = true) const; bool set_feature_flag(const std::string& app_id, const std::string& flag_name, bool enable) const; + bool get_feature_flag(const std::string& app_id, const std::string& flag_name) const; + nlohmann::json get_default_rule(const std::string& app_id) const; + bool update_default_rule(const std::string& app_id, nlohmann::json roles) const; struct Service { std::string id; @@ -141,6 +144,7 @@ class AdminAPISession { }; MigrationStatus get_migration_status(const std::string& app_id) const; + nlohmann::json get_app_settings(const std::string& app_id) const; const std::string& admin_url() const noexcept { diff --git a/test/object-store/util/sync/flx_sync_harness.hpp b/test/object-store/util/sync/flx_sync_harness.hpp index 7d08c861616..36004b14605 100644 --- a/test/object-store/util/sync/flx_sync_harness.hpp +++ b/test/object-store/util/sync/flx_sync_harness.hpp @@ -119,7 +119,7 @@ class FLXSyncTestHarness { template void load_initial_data(Func&& func) { - SyncTestFile config(m_test_session.app()->current_user(), schema(), SyncConfig::FLXSyncEnabled{}); + auto config = make_test_file(); auto realm = Realm::get_shared_realm(config); subscribe_to_all_and_bootstrap(*realm); diff --git a/test/object-store/util/test_utils.hpp b/test/object-store/util/test_utils.hpp index 89fc92e1b83..517463494bd 100644 --- a/test/object-store/util/test_utils.hpp +++ b/test/object-store/util/test_utils.hpp @@ -68,10 +68,10 @@ class TestingStateMachine { m_cv.notify_one(); } - void wait_for(E target) + bool wait_for(E target, std::chrono::milliseconds period = std::chrono::seconds(15)) { std::unique_lock lock{m_mutex}; - m_cv.wait(lock, [&] { + return m_cv.wait_for(lock, period, [&] { return m_cur_state == target; }); } From 3424431fc97c0a58eeeb719b2521d174336dbd4d Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 11 Apr 2024 18:45:03 -0400 Subject: [PATCH 03/37] Updated test for handle role bootstraps --- src/realm/sync/noinst/client_impl_base.cpp | 2 +- src/realm/sync/protocol.hpp | 7 +- test/object-store/sync/flx_sync.cpp | 151 ++++++++++++++++----- 3 files changed, 121 insertions(+), 39 deletions(-) diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 1736854a989..9fbf0c5bfcc 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -1891,7 +1891,7 @@ void Session::send_bind_message() session_ident_type session_ident = m_ident; bool need_client_file_ident = !have_client_file_ident(); const bool is_subserver = false; - + m_last_download_batch_state = DownloadBatchState::SteadyState; ClientProtocol& protocol = m_conn.get_client_protocol(); int protocol_version = m_conn.get_negotiated_protocol_version(); diff --git a/src/realm/sync/protocol.hpp b/src/realm/sync/protocol.hpp index 4142281fdab..bbc14807616 100644 --- a/src/realm/sync/protocol.hpp +++ b/src/realm/sync/protocol.hpp @@ -57,6 +57,11 @@ namespace sync { // Server replaces 'downloadable_bytes' (which was always zero prior this version) // with an estimated progress value (double from 0.0 to 1.0) for flx sessions // +// 13 Support for collections in mixed +// +// 14 Support for server initiated bootstraps, including bootstraps for role/ +// permissions changes instead of performing a client reset. +// // XX Changes: // - TBD // @@ -64,7 +69,7 @@ constexpr int get_current_protocol_version() noexcept { // Also update the current protocol version test in flx_sync.cpp when // updating this value - return 12; + return 14; } constexpr std::string_view get_pbs_websocket_protocol_prefix() noexcept diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index e00bcabd1f0..41dda597411 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -2581,7 +2581,7 @@ TEST_CASE("flx: writes work without waiting for sync", "[sync][flx][baas]") { TEST_CASE("flx: verify websocket protocol number and prefixes", "[sync][protocol]") { // Update the expected value whenever the protocol version is updated - this ensures // that the current protocol version does not change unexpectedly. - REQUIRE(12 == sync::get_current_protocol_version()); + REQUIRE(14 == sync::get_current_protocol_version()); // This was updated in Protocol V8 to use '#' instead of '/' to support the Web SDK REQUIRE("com.mongodb.realm-sync#" == sync::get_pbs_websocket_protocol_prefix()); REQUIRE("com.mongodb.realm-query-sync#" == sync::get_flx_websocket_protocol_prefix()); @@ -4791,14 +4791,23 @@ TEST_CASE("flx: pause and resume bootstrapping at query version 0", "[sync][flx] REQUIRE(active_sub_set.state() == sync::SubscriptionSet::State::Complete); } +TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { + const Schema person_schema{{"Person", + {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, + {"age", PropertyType::Int | PropertyType::Nullable}, + {"role", PropertyType::String}, + {"firstName", PropertyType::String}, + {"lastName", PropertyType::String}}}}; -static void fill_person_schema(FLXSyncTestHarness& harness) -{ - harness.load_initial_data([&](SharedRealm realm) { - int cnt = 1; + size_t num_emps = 5000; + size_t num_mgrs = 200; + size_t num_dirs = 25; + size_t num_total = num_emps + num_mgrs + num_dirs; + auto fill_person_schema = [num_emps, num_mgrs, num_dirs](SharedRealm realm) { + size_t cnt = 1; { CppContext c(realm); - for (int i = 0; i < 100; ++i) { + for (size_t i = 0; i < num_mgrs; ++i) { auto obj = Object::create(c, realm, "Person", std::any(AnyDict{ {"_id", ObjectId::gen()}, @@ -4812,7 +4821,7 @@ static void fill_person_schema(FLXSyncTestHarness& harness) } { CppContext c(realm); - for (int i = 0; i < 5000; ++i) { + for (size_t i = 0; i < num_emps; ++i) { auto obj = Object::create(c, realm, "Person", std::any(AnyDict{{"_id", ObjectId::gen()}, {"age", static_cast(20 + i % 30)}, @@ -4824,7 +4833,7 @@ static void fill_person_schema(FLXSyncTestHarness& harness) } { CppContext c(realm); - for (int i = 0; i < 125; ++i) { + for (size_t i = 0; i < num_dirs; ++i) { auto obj = Object::create(c, realm, "Person", std::any(AnyDict{{"_id", ObjectId::gen()}, {"age", static_cast(45 + i)}, @@ -4834,33 +4843,39 @@ static void fill_person_schema(FLXSyncTestHarness& harness) ++cnt; } } - }); -} + }; -TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { - const Schema person_schema{{"Person", - {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, - {"age", PropertyType::Int | PropertyType::Nullable}, - {"role", PropertyType::String}, - {"firstName", PropertyType::String}, - {"lastName", PropertyType::String}}}}; - enum TestState { initial, disconnected, connecting, connected }; + enum TestState { initial, disconnected, connecting, connected, bs_downloading, bs_downloaded, bs_complete }; TestingStateMachine machina(initial); auto logger = util::Logger::get_default_logger(); FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); auto& app_session = harness.session().app_session(); // Enable the role change bootstraps - REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); + // REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", + // true)); // Ensure the feature flag is enabled - REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + // REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); // Initialize the realm with some data - fill_person_schema(harness); + harness.load_initial_data(fill_person_schema); auto config = harness.make_test_file(); + + std::mutex hook_mutex; + std::function, const SyncClientHookData&)> hook_callback; + + config.sync_config->on_sync_client_event_hook = [&hook_callback, &hook_mutex](std::weak_ptr session, + const SyncClientHookData& data) { + std::lock_guard lock(hook_mutex); + if (hook_callback) { + return hook_callback(session, data); + } + return SyncClientHookAction::NoAction; + }; + auto realm = Realm::get_shared_realm(config); REQUIRE(!wait_for_download(*realm)); REQUIRE(!wait_for_upload(*realm)); @@ -4882,14 +4897,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra { auto table = realm->read_group().get_table("class_Person"); Results results(realm, Query(table)); - CHECK(results.size() == 5225); + CHECK(results.size() == num_total); } // Register connection change callback to verify session was restarted using ConnState = SyncSession::ConnectionState; auto session = realm->sync_session(); session->register_connection_change_callback([&machina, &logger](ConnState old_state, ConnState new_state) { - logger->info("Connection state: %1 -> %2", old_state, new_state); + logger->trace("Connection state: %1 -> %2", old_state, new_state); machina.transition_with([new_state](TestState curr_state) { switch (new_state) { case ConnState::Disconnected: @@ -4906,9 +4921,57 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }); - auto update_perms = [&](std::optional doc_filter, size_t num_employees, size_t num_dirs_mgrs) { + auto update_perms = [&](std::optional doc_filter, bool multi_msg, size_t cnt_emps, + size_t cnt_mgrs, size_t cnt_dirs) { + using BatchState = sync::DownloadBatchState; + BatchState last_state = BatchState::SteadyState; + size_t msg_count = 0; + auto bs_callback = [&last_state, &msg_count, &machina, multi_msg](std::weak_ptr, + const SyncClientHookData& data) { + machina.transition_with( + [&data, &last_state, &msg_count, multi_msg](TestState curr_state) -> std::optional { + // download message saved to bootstrap store + switch (data.event) { + // Multimessage bootstrap processing + case SyncClientHookEvent::BootstrapMessageProcessed: { + std::optional next_state; + REQUIRE(data.batch_state != BatchState::SteadyState); + REQUIRE((curr_state == TestState::connected || curr_state == TestState::bs_downloading)); + if (msg_count == 0) { + REQUIRE(last_state == BatchState::SteadyState); + } + else { + REQUIRE(last_state == BatchState::MoreToCome); + } + if (data.batch_state == BatchState::LastInBatch) { + next_state = TestState::bs_downloaded; + } + else { + next_state = TestState::bs_downloading; + } + last_state = data.batch_state; + ++msg_count; + return next_state; + } + case SyncClientHookEvent::BootstrapProcessed: + REQUIRE(last_state == BatchState::LastInBatch); + REQUIRE(curr_state == TestState::bs_downloaded); + REQUIRE((msg_count > 1 == multi_msg)); + return TestState::bs_complete; + case SyncClientHookEvent::DownloadMessageReceived: + case SyncClientHookEvent::DownloadMessageIntegrated: + default: + return std::nullopt; + } + }); + return SyncClientHookAction::NoAction; + }; + { + std::lock_guard lock(hook_mutex); + hook_callback = bs_callback; + } + // Updating the role to employees only - logger->trace(">>>>> Updating permissions...JSON: %1", doc_filter); if (doc_filter) { rule["roles"][0]["document_filters"]["read"] = *doc_filter; rule["roles"][0]["document_filters"]["write"] = *doc_filter; @@ -4918,7 +4981,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra rule["roles"][0]["document_filters"]["write"] = true; } app_session.admin_api.update_default_rule(app_session.server_app_id, rule); - logger->trace(">>>>> Complete"); // Client should receive an error and drop/reconnect the session REQUIRE(machina.wait_for(TestState::disconnected)); @@ -4929,25 +4991,40 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra wait_for_advance(*realm); { auto table = realm->read_group().get_table("class_Person"); - REQUIRE(table->size() == (num_employees + num_dirs_mgrs)); + REQUIRE(table->size() == (cnt_emps + cnt_mgrs + cnt_dirs)); auto role_col = table->get_column_key("role"); - Query query_mgrs(table); - query_mgrs.equal(role_col, "director").Or().equal(role_col, "manager"); - Results results_mgrs(realm, query_mgrs); - CHECK(results_mgrs.size() == num_dirs_mgrs); - Query query_emps(table); - query_emps.equal(role_col, "employee"); - Results results_emps(realm, query_emps); - CHECK(results_emps.size() == num_employees); + auto table_query = Query(table).equal(role_col, "director").Or().equal(role_col, "manager"); + auto results = Results(realm, table_query); + CHECK(results.size() == (cnt_mgrs + cnt_dirs)); + table_query = Query(table).equal(role_col, "employee"); + results = Results(realm, table_query); + CHECK(results.size() == cnt_emps); + table_query = Query(table).equal(role_col, "manager"); + results = Results(realm, table_query); + CHECK(results.size() == cnt_mgrs); + table_query = Query(table).equal(role_col, "director"); + results = Results(realm, table_query); + CHECK(results.size() == cnt_dirs); + } + { + // Clear out the hook callback before leaving + std::lock_guard lock(hook_mutex); + hook_callback = nullptr; } }; { + // Multi-message subtractive bootstrap nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; - update_perms(doc_filter, 0, 225); + update_perms(doc_filter, true, 0, num_mgrs, num_dirs); } { + // Multi-message additive (and a little bit subtractive) bootstrap nlohmann::json doc_filter = {{"role", "employee"}}; - update_perms(doc_filter, 5000, 0); + update_perms(doc_filter, true, num_emps, 0, 0); + } + { + // Single message, single changeset additive bootstrap + update_perms(std::nullopt, false, num_emps, num_mgrs, num_dirs); } } From e0d6ac16df85619b3c0e43b7ccb32a0053c592f9 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 11 Apr 2024 18:45:32 -0400 Subject: [PATCH 04/37] Updated baas/baasaas to use branch with fixes --- dependencies.yml | 3 ++- evergreen/config.yml | 3 ++- evergreen/install_baas.sh | 5 ++++- test/object-store/util/sync/baas_admin_api.cpp | 15 ++++++++------- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 71ba18f8ebd..d2cfb2eb967 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -4,4 +4,5 @@ OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits # 6bf5e1 is 2024 Mar 20 -BAAS_VERSION: 6bf5e111499ba32d04224e2fc10acca3595edb20 +BAAS_VERSION: 66171338e32da7000754c772 +BAAS_VERSION_TYPE: patchid diff --git a/evergreen/config.yml b/evergreen/config.yml index 2b4831daf87..eb553aebe69 100644 --- a/evergreen/config.yml +++ b/evergreen/config.yml @@ -216,8 +216,9 @@ functions: shell: bash env: BAASAAS_API_KEY: "${baasaas_api_key}" + # BAAS_VERSION and VERSION_TYPE are set by realm-core/dependencies.yml BAASAAS_REF_SPEC: "${BAAS_VERSION}" - BAASAAS_START_MODE: "githash" + BAASAAS_START_MODE: "${BAAS_VERSION_TYPE|githash}" script: |- set -o errexit set -o verbose diff --git a/evergreen/install_baas.sh b/evergreen/install_baas.sh index 5ae34178635..9aac95aa0ab 100755 --- a/evergreen/install_baas.sh +++ b/evergreen/install_baas.sh @@ -402,9 +402,12 @@ if [[ -z "${BAAS_VERSION}" ]]; then exit 1 fi +BAAS_VERSION="BAAS-28604-remove-ff" + # Clone the baas repo and check out the specified version if [[ ! -d "${BAAS_DIR}/.git" ]]; then - git clone git@github.com:10gen/baas.git "${BAAS_DIR}" +# git clone git@github.com:10gen/baas.git "${BAAS_DIR}" + git clone git@github.com:sean-brandenburg/baas.git "${BAAS_DIR}" pushd "${BAAS_DIR}" > /dev/null else pushd "${BAAS_DIR}" > /dev/null diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index d3fcf324ad4..573e9c286df 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -672,8 +672,8 @@ app::Response AdminAPIEndpoint::del() const nlohmann::json AdminAPIEndpoint::get_json(const std::vector>& params) const { auto resp = get(params); - REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, - util::format("url: %1, reply: %2", m_url, resp.body)); + REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, m_url, resp.http_status_code, + resp.body); return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); } @@ -689,7 +689,8 @@ app::Response AdminAPIEndpoint::post(std::string body) const nlohmann::json AdminAPIEndpoint::post_json(nlohmann::json body) const { auto resp = post(body.dump()); - REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, m_url, body.dump(), resp.body); + REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, m_url, body.dump(), + resp.http_status_code, resp.body); return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); } @@ -705,8 +706,8 @@ app::Response AdminAPIEndpoint::put(std::string body) const nlohmann::json AdminAPIEndpoint::put_json(nlohmann::json body) const { auto resp = put(body.dump()); - REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, - util::format("url: %1 request: %2, reply: %3", m_url, body.dump(), resp.body)); + REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, m_url, body.dump(), + resp.http_status_code, resp.body); return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); } @@ -722,8 +723,8 @@ app::Response AdminAPIEndpoint::patch(std::string body) const nlohmann::json AdminAPIEndpoint::patch_json(nlohmann::json body) const { auto resp = patch(body.dump()); - REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, - util::format("url: %1 request: %2, reply: %3", m_url, body.dump(), resp.body)); + REALM_ASSERT_EX(resp.http_status_code >= 200 && resp.http_status_code < 300, m_url, body.dump(), + resp.http_status_code, resp.body); return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); } From 7356ba222db10a6fcaedf8b8c228a56055902379 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 11 Apr 2024 18:54:00 -0400 Subject: [PATCH 05/37] updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 412a89a1899..a2f9aebad4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ * (PR [#????](https://github.com/realm/realm-core/pull/????)) * Add `SyncClientConfig::security_access_group` which allows specifying the access group to use for the sync metadata Realm's encryption key. Setting this is required when sharing the metadata Realm between apps on Apple platforms ([#7552](https://github.com/realm/realm-core/pull/7552)). * When connecting to multiple server apps, a unique encryption key is used for each of the metadata Realms rather than sharing one between them ([#7552](https://github.com/realm/realm-core/pull/7552)). -* Introduce the new `SyncUser` interface which can be implemented by SDKs to use sync without the core App Services implementation (or just for greater control over user behavior in tests). ([PR #7300](https://github.com/realm/realm-core/pull/7300). +* Introduce the new `SyncUser` interface which can be implemented by SDKs to use sync without the core App Services implementation (or just for greater control over user behavior in tests). ([PR #7300](https://github.com/realm/realm-core/pull/7300)). +* Added support for server initiated bootstraps. ([PR #7440](https://github.com/realm/realm-core/pull/7440)) ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) From 44319859439a35100e201312a1c049c949c3190e Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 12 Apr 2024 09:42:34 -0400 Subject: [PATCH 06/37] Updated test to verify bootstrap actually occurred --- test/object-store/sync/flx_sync.cpp | 50 +++++++++++++++++------------ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 41dda597411..b6e990bd2f6 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4921,15 +4921,16 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }); - auto update_perms = [&](std::optional doc_filter, bool multi_msg, size_t cnt_emps, - size_t cnt_mgrs, size_t cnt_dirs) { + auto update_perms = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, + size_t cnt_dirs) { using BatchState = sync::DownloadBatchState; + std::mutex callback_mutex; BatchState last_state = BatchState::SteadyState; size_t msg_count = 0; - auto bs_callback = [&last_state, &msg_count, &machina, multi_msg](std::weak_ptr, - const SyncClientHookData& data) { + auto bs_callback = [&callback_mutex, &last_state, &msg_count, &machina](std::weak_ptr, + const SyncClientHookData& data) { machina.transition_with( - [&data, &last_state, &msg_count, multi_msg](TestState curr_state) -> std::optional { + [&data, &callback_mutex, &last_state, &msg_count](TestState curr_state) -> std::optional { // download message saved to bootstrap store switch (data.event) { // Multimessage bootstrap processing @@ -4937,26 +4938,28 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra std::optional next_state; REQUIRE(data.batch_state != BatchState::SteadyState); REQUIRE((curr_state == TestState::connected || curr_state == TestState::bs_downloading)); - if (msg_count == 0) { - REQUIRE(last_state == BatchState::SteadyState); - } - else { - REQUIRE(last_state == BatchState::MoreToCome); - } if (data.batch_state == BatchState::LastInBatch) { next_state = TestState::bs_downloaded; } else { next_state = TestState::bs_downloading; } - last_state = data.batch_state; - ++msg_count; + { + std::lock_guard lock(callback_mutex); + if (msg_count == 0) { + REQUIRE(last_state == BatchState::SteadyState); + } + else { + REQUIRE(last_state == BatchState::MoreToCome); + } + last_state = data.batch_state; + ++msg_count; + } return next_state; } case SyncClientHookEvent::BootstrapProcessed: REQUIRE(last_state == BatchState::LastInBatch); REQUIRE(curr_state == TestState::bs_downloaded); - REQUIRE((msg_count > 1 == multi_msg)); return TestState::bs_complete; case SyncClientHookEvent::DownloadMessageReceived: case SyncClientHookEvent::DownloadMessageIntegrated: @@ -4987,6 +4990,13 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(machina.wait_for(TestState::connected)); REQUIRE(!wait_for_download(*realm)); + { + std::lock_guard lock(callback_mutex); + if (msg_count > 1) { + // Verify a bootstrap occurred if expected multiple messages + REQUIRE(machina.get() == TestState::bs_complete); + } + } REQUIRE(!wait_for_upload(*realm)); wait_for_advance(*realm); { @@ -5013,18 +5023,18 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } }; { - // Multi-message subtractive bootstrap + // Single message subtractive bootstrap nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; - update_perms(doc_filter, true, 0, num_mgrs, num_dirs); + update_perms(doc_filter, 0, num_mgrs, num_dirs); } { - // Multi-message additive (and a little bit subtractive) bootstrap + // Multi-message additive/subtractive bootstrap nlohmann::json doc_filter = {{"role", "employee"}}; - update_perms(doc_filter, true, num_emps, 0, 0); + update_perms(doc_filter, num_emps, 0, 0); } { - // Single message, single changeset additive bootstrap - update_perms(std::nullopt, false, num_emps, num_mgrs, num_dirs); + // Single message additive bootstrap + update_perms(std::nullopt, num_emps, num_mgrs, num_dirs); } } From b87bc6574339f92112c514eb6e3ceb794af54499 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 12 Apr 2024 11:17:11 -0400 Subject: [PATCH 07/37] Fixed tsan warning --- test/object-store/sync/flx_sync.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index b6e990bd2f6..c450834b12e 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4990,13 +4990,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(machina.wait_for(TestState::connected)); REQUIRE(!wait_for_download(*realm)); + bool multi_msg = false; { std::lock_guard lock(callback_mutex); - if (msg_count > 1) { - // Verify a bootstrap occurred if expected multiple messages - REQUIRE(machina.get() == TestState::bs_complete); - } + if (msg_count > 1) + multi_msg = true; } + // Verify a bootstrap occurred if multiple messages were received + REQUIRE((!multi_msg || machina.get() == TestState::bs_complete)); + REQUIRE(!wait_for_upload(*realm)); wait_for_advance(*realm); { From 844746168548610d12a8988dad2204c26db61808 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 12 Apr 2024 11:19:10 -0400 Subject: [PATCH 08/37] Move instead of copy --- test/object-store/sync/flx_sync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index c450834b12e..cff63fc0fce 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4971,7 +4971,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }; { std::lock_guard lock(hook_mutex); - hook_callback = bs_callback; + hook_callback = std::move(bs_callback); } // Updating the role to employees only From 44d8e124840ad44940059b07954b46ea04d93f40 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 15 Apr 2024 18:59:15 -0400 Subject: [PATCH 09/37] Updates from review; added comments to clarify bootstrap detection logic --- src/realm/object-store/sync/sync_session.cpp | 2 +- src/realm/sync/client.cpp | 42 ++++++++++++++------ src/realm/sync/noinst/client_impl_base.cpp | 42 +++++++++++--------- src/realm/sync/noinst/client_impl_base.hpp | 2 +- 4 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/realm/object-store/sync/sync_session.cpp b/src/realm/object-store/sync/sync_session.cpp index 76fbdb544a3..93c73da7a08 100644 --- a/src/realm/object-store/sync/sync_session.cpp +++ b/src/realm/object-store/sync/sync_session.cpp @@ -39,7 +39,7 @@ #include #include -#include +#include using namespace realm; using namespace realm::_impl; diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 8787a706640..d1d08e1be49 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -1100,7 +1100,7 @@ SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, con } bool SessionImpl::is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, - bool same_server_version) + bool same_remote_versions) { // Should never be called if session is not active REALM_ASSERT_EX(m_state == State::Active, m_state); @@ -1109,27 +1109,43 @@ bool SessionImpl::is_steady_state_download_message(DownloadBatchState batch_stat // Return early if already steady state or PBS (doesn't use bootstraps) if (batch_state == DownloadBatchState::SteadyState || !m_is_flx_sync_session) { - return true; + return true; // Steady state } // Bootstrap messages (i.e. non-steady state) are identified by: // * DownloadBatchState of MoreToCome // * DownloadBatchState of LastInBatch, and - // * first LastInBatch after one or more MoreToCome messages + // * first LastInBatch=true after one or more MoreToCome messages // * query_version greater than the active query_version - // * message has 2 or more changesets and all have the same remote_version + // * LastInBatch=true message has 2 or more changesets and all have the same + // remote_version - // If this is a single LastInBatch message, check to see if it is a single - // message bootstrap or just a steady state download message - if (batch_state == DownloadBatchState::LastInBatch && - m_last_download_batch_state != DownloadBatchState::MoreToCome) { - if (query_version == m_wrapper.m_flx_active_version && !same_server_version) { - return true; - } + // LastInBatch=False messages are always a bootstrap message + if (batch_state == DownloadBatchState::MoreToCome) { + return false; // Not steady state + } + + // Messages with query_version greater than the active version are always a + // bootstrap message + if (query_version > m_wrapper.m_flx_active_version) { + return false; // Not steady state + } + + // If this is the first LastInBatch=true message after one or more + // LastInBatch=false messages, this is the end of a bootstrap + if (m_last_download_batch_state == DownloadBatchState::MoreToCome) { + return false; // Not steady state + } + + // Otherwise, if this is a LastInBatch=true message whose query version matches + // the current active version and all the changesets in the message have the + // same remote version, then this is a server initiated single message bootstrap + if (same_remote_versions) { + return false; // Not steady state } - // Otherwise this is a bootstrap message - return false; + // If none of the previous checks were successful, then this is a steady state msg + return true; // Steady state } void SessionImpl::init_progress_handler() diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index d54ea668cfd..ad3b6771510 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2412,34 +2412,38 @@ Status Session::receive_download_message(const DownloadMessage& message) return status; } - version_type server_version = m_progress.download.server_version; - // If there are 2 or more changesets in this message, track whether or not the changesets have the same - // download_server_version to help with determining if this is a bootstrap message or not. - bool same_server_version = - message.changesets.size() > 1 && progress.download.server_version == message.changesets[0].remote_version; - bool after_first_changeset = false; + // Start with the download server version from the last download message, since the changesets in the new + // download message must have a remote version greater than (or equal to for FLX) this value. + version_type last_remote_version = m_progress.download.server_version; version_type last_integrated_client_version = m_progress.download.last_integrated_client_version; + // If there are 2 or more changesets in this message, track whether or not the changesets all have the same + // download_server_version to help with determining if this is a single message server-initiated bootstrap or not. + bool same_remote_version = last_in_batch && message.changesets.size() > 1; + bool after_first_changeset = false; + for (const RemoteChangeset& changeset : message.changesets) { - // Check that per-changeset server version is strictly increasing, except in FLX sync where the server - // version must be increasing, but can stay the same during bootstraps. - bool good_server_version = m_is_flx_sync_session ? (changeset.remote_version >= server_version) - : (changeset.remote_version > server_version); + // Check that per-changeset server version is strictly increasing since the last download server version, + // except in FLX sync where the server version must be increasing, but can stay the same during bootstraps. + bool good_server_version = m_is_flx_sync_session ? (changeset.remote_version >= last_remote_version) + : (changeset.remote_version > last_remote_version); // Each server version cannot be greater than the one in the header of the download message. good_server_version = good_server_version && (changeset.remote_version <= progress.download.server_version); if (!good_server_version) { return {ErrorCodes::SyncProtocolInvariantFailed, util::format("Bad server version in changeset header (DOWNLOAD) (%1, %2, %3)", - changeset.remote_version, server_version, progress.download.server_version)}; + changeset.remote_version, last_remote_version, progress.download.server_version)}; } - if (after_first_changeset) + // Check to see if all the changesets in this LastInBatch=true message have the same remote_version + // If so, this is a server-initiated single message bootstrap + if (same_remote_version && after_first_changeset) { // After the first changeset compare the previous changeset's server version to the current changeset // server version. If they are different then this is definitely not a bootstrap message - same_server_version = same_server_version && server_version == changeset.remote_version; - else - // Skip the first changeset, since `server_version` contains the incorrect value for this check - after_first_changeset = true; - - server_version = changeset.remote_version; + same_remote_version = changeset.remote_version == last_remote_version; + } + // Skip the first changeset, since `server_version` contains the incorrect value for this check + after_first_changeset = true; + // Save the remote version for the current changeset to compare against the next changeset + last_remote_version = changeset.remote_version; // Check that per-changeset last integrated client version is "weakly" // increasing. @@ -2465,7 +2469,7 @@ Status Session::receive_download_message(const DownloadMessage& message) } } - if (is_steady_state_download_message(batch_state, query_version, same_server_version)) { + if (is_steady_state_download_message(batch_state, query_version, same_remote_version)) { batch_state = DownloadBatchState::SteadyState; } m_last_download_batch_state = batch_state; diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 3b797e53f48..94d55841026 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1202,7 +1202,7 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(const SyncClientHookData& data); bool is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, - bool same_server_version); + bool same_remote_versions); void init_progress_handler(); void enable_progress_notifications(); From 8d9557d0bac13f3173d73511d702ae78df64d4eb Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 30 Apr 2024 10:23:14 -0400 Subject: [PATCH 10/37] Updates from review --- .../sync/noinst/pending_bootstrap_store.cpp | 8 +- test/object-store/sync/flx_sync.cpp | 271 +++++++++--------- 2 files changed, 134 insertions(+), 145 deletions(-) diff --git a/src/realm/sync/noinst/pending_bootstrap_store.cpp b/src/realm/sync/noinst/pending_bootstrap_store.cpp index 714238b3055..65bb87072c4 100644 --- a/src/realm/sync/noinst/pending_bootstrap_store.cpp +++ b/src/realm/sync/noinst/pending_bootstrap_store.cpp @@ -202,13 +202,7 @@ bool PendingBootstrapStore::has_pending() void PendingBootstrapStore::clear() { - auto tr = m_db->start_read(); - auto bootstrap_table = tr->get_table(m_table); - // Just make sure the state is reset if the bootstrap table is empty - if (bootstrap_table->is_empty()) { - return; - } - tr->promote_to_write(); + auto tr = m_db->start_write(); clear(*tr); tr->commit(); } diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 59731127f24..27ea09fba3b 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4939,51 +4939,23 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t num_mgrs = 200; size_t num_dirs = 25; size_t num_total = num_emps + num_mgrs + num_dirs; - auto fill_person_schema = [num_emps, num_mgrs, num_dirs](SharedRealm realm) { - size_t cnt = 1; - { - CppContext c(realm); - for (size_t i = 0; i < num_mgrs; ++i) { - auto obj = Object::create(c, realm, "Person", - std::any(AnyDict{ - {"_id", ObjectId::gen()}, - {"age", static_cast(40 + i)}, - {"role", "manager"s}, - {"firstName", util::format("firstname-%1", cnt)}, - {"lastName", util::format("lastname-%1", cnt)}, - })); - ++cnt; - } - } - { - CppContext c(realm); - for (size_t i = 0; i < num_emps; ++i) { - auto obj = Object::create(c, realm, "Person", - std::any(AnyDict{{"_id", ObjectId::gen()}, - {"age", static_cast(20 + i % 30)}, - {"role", "employee"s}, - {"firstName", util::format("firstname-%1", cnt)}, - {"lastName", util::format("lastname-%1", cnt)}})); - ++cnt; - } - } - { - CppContext c(realm); - for (size_t i = 0; i < num_dirs; ++i) { - auto obj = Object::create(c, realm, "Person", - std::any(AnyDict{{"_id", ObjectId::gen()}, - {"age", static_cast(45 + i)}, - {"role", "director"s}, - {"firstName", util::format("firstname-9999%1", cnt)}, - {"lastName", util::format("lastname-9999%1", cnt)}})); - ++cnt; + auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { + CppContext c(realm); + for (size_t i = 0; i < count; ++i) { + // Recreate the context (transaction) for every 100 entries + if (count % 100 == 0) { + c = CppContext(realm); } + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{ + {"_id", ObjectId::gen()}, + {"age", static_cast(i)}, + {"role", role}, + {"firstName", util::format("%1-%2", role, i)}, + {"lastName", util::format("last-name-%1", i)}, + })); } }; - - enum TestState { initial, disconnected, connecting, connected, bs_downloading, bs_downloaded, bs_complete }; - TestingStateMachine machina(initial); - auto logger = util::Logger::get_default_logger(); FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); auto& app_session = harness.session().app_session(); @@ -4993,30 +4965,39 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Ensure the feature flag is enabled // REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + // Get the current rules so it can be updated during the test auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); // Initialize the realm with some data - harness.load_initial_data(fill_person_schema); + harness.load_initial_data([&fill_person_schema, num_emps, num_mgrs, num_dirs](SharedRealm realm) { + fill_person_schema(realm, "employee", num_emps); + fill_person_schema(realm, "manager", num_mgrs); + fill_person_schema(realm, "director", num_dirs); + }); auto config = harness.make_test_file(); - std::mutex hook_mutex; + std::mutex callback_mutex; std::function, const SyncClientHookData&)> hook_callback; + bool did_client_reset = false; - config.sync_config->on_sync_client_event_hook = [&hook_callback, &hook_mutex](std::weak_ptr session, - const SyncClientHookData& data) { - std::lock_guard lock(hook_mutex); - if (hook_callback) { - return hook_callback(session, data); - } - return SyncClientHookAction::NoAction; + config.sync_config->on_sync_client_event_hook = + [&hook_callback, &callback_mutex](std::weak_ptr session, const SyncClientHookData& data) { + std::lock_guard lock(callback_mutex); + if (hook_callback) { + return hook_callback(session, data); + } + return SyncClientHookAction::NoAction; + }; + config.sync_config->notify_before_client_reset = [&did_client_reset, &callback_mutex](std::shared_ptr) { + std::lock_guard lock(callback_mutex); + did_client_reset = true; }; auto realm = Realm::get_shared_realm(config); REQUIRE(!wait_for_download(*realm)); REQUIRE(!wait_for_upload(*realm)); - // Set up the initial subscription { auto table = realm->read_group().get_table("class_Person"); @@ -5033,111 +5014,125 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra { auto table = realm->read_group().get_table("class_Person"); Results results(realm, Query(table)); - CHECK(results.size() == num_total); + REQUIRE(results.size() == num_total); } - // Register connection change callback to verify session was restarted - using ConnState = SyncSession::ConnectionState; auto session = realm->sync_session(); - session->register_connection_change_callback([&machina, &logger](ConnState old_state, ConnState new_state) { - logger->trace("Connection state: %1 -> %2", old_state, new_state); - machina.transition_with([new_state](TestState curr_state) { - switch (new_state) { - case ConnState::Disconnected: - return TestState::disconnected; - case ConnState::Connecting: - REQUIRE(curr_state == TestState::disconnected); - return TestState::connecting; - case ConnState::Connected: - REQUIRE(curr_state == TestState::connecting); - return TestState::connected; - default: - FAIL(util::format("Unknown connection state: %1", new_state)); - } - }); - }); + int64_t query_version = realm->get_active_subscription_set().version(); - auto update_perms = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, - size_t cnt_dirs) { + auto update_perms_and_verify = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, + size_t cnt_dirs) { using BatchState = sync::DownloadBatchState; - std::mutex callback_mutex; - BatchState last_state = BatchState::SteadyState; + using Event = SyncClientHookEvent; + std::mutex bootstrap_mutex; size_t msg_count = 0; - auto bs_callback = [&callback_mutex, &last_state, &msg_count, &machina](std::weak_ptr, - const SyncClientHookData& data) { - machina.transition_with( - [&data, &callback_mutex, &last_state, &msg_count](TestState curr_state) -> std::optional { - // download message saved to bootstrap store - switch (data.event) { - // Multimessage bootstrap processing - case SyncClientHookEvent::BootstrapMessageProcessed: { - std::optional next_state; - REQUIRE(data.batch_state != BatchState::SteadyState); - REQUIRE((curr_state == TestState::connected || curr_state == TestState::bs_downloading)); - if (data.batch_state == BatchState::LastInBatch) { - next_state = TestState::bs_downloaded; - } - else { - next_state = TestState::bs_downloading; - } - { - std::lock_guard lock(callback_mutex); - if (msg_count == 0) { - REQUIRE(last_state == BatchState::SteadyState); - } - else { - REQUIRE(last_state == BatchState::MoreToCome); - } - last_state = data.batch_state; - ++msg_count; + bool role_change_bootstrap = false; + + enum TestState { start, reconnect_received, downloading, downloaded, complete }; + TestingStateMachine state_machina(TestState::start); + + auto bootstrap_callback = [&bootstrap_mutex, &msg_count, query_version, &role_change_bootstrap, + &state_machina](std::weak_ptr, const SyncClientHookData& data) { + state_machina.transition_with([&](TestState cur_state) -> std::optional { + switch (data.event) { + case Event::ErrorMessageReceived: + REQUIRE(cur_state == TestState::start); + REQUIRE(data.error_info); + REQUIRE(data.error_info->raw_error_code == 200); + REQUIRE(data.error_info->server_requests_action == + sync::ProtocolErrorInfo::Action::Transient); + REQUIRE_FALSE(data.error_info->is_fatal); + return TestState::reconnect_received; + + // A bootstrap message was processed - used for multi-message or + // single message multi changeset bootstraps + case Event::BootstrapMessageProcessed: { + REQUIRE(data.batch_state != BatchState::SteadyState); + REQUIRE((cur_state == TestState::reconnect_received || cur_state == TestState::downloading)); + if (data.batch_state != BatchState::SteadyState) { + std::lock_guard lock(bootstrap_mutex); + ++msg_count; + if (data.query_version == query_version) { + role_change_bootstrap = true; } - return next_state; } - case SyncClientHookEvent::BootstrapProcessed: - REQUIRE(last_state == BatchState::LastInBatch); - REQUIRE(curr_state == TestState::bs_downloaded); - return TestState::bs_complete; - case SyncClientHookEvent::DownloadMessageReceived: - case SyncClientHookEvent::DownloadMessageIntegrated: - default: - return std::nullopt; + // multi-message bootstrap in progress.. + if (data.batch_state == BatchState::MoreToCome) { + return TestState::downloading; + } + // single bootstrap message or last message in the multi-message bootstrap + else if (data.batch_state == BatchState::LastInBatch) { + return TestState::downloaded; + } + return std::nullopt; } - }); + // The bootstrap has been received and processed + case Event::BootstrapProcessed: + REQUIRE(cur_state == TestState::downloaded); + return TestState::complete; + default: + return std::nullopt; + } + }); return SyncClientHookAction::NoAction; }; { - std::lock_guard lock(hook_mutex); - hook_callback = std::move(bs_callback); + std::lock_guard lock(callback_mutex); + hook_callback = std::move(bootstrap_callback); + did_client_reset = false; } - // Updating the role to employees only - if (doc_filter) { - rule["roles"][0]["document_filters"]["read"] = *doc_filter; - rule["roles"][0]["document_filters"]["write"] = *doc_filter; - } - else { - rule["roles"][0]["document_filters"]["read"] = true; - rule["roles"][0]["document_filters"]["write"] = true; - } + rule["roles"][0]["document_filters"]["read"] = doc_filter.value_or(true); + rule["roles"][0]["document_filters"]["write"] = doc_filter.value_or(true); + + // Update the permissions on the server - should send an error to the client to force + // it to reconnect app_session.admin_api.update_default_rule(app_session.server_app_id, rule); - // Client should receive an error and drop/reconnect the session - REQUIRE(machina.wait_for(TestState::disconnected)); - REQUIRE(machina.wait_for(TestState::connected)); + // After updating the permissions (if they are different), the server should send an + // error that will disconnect/reconnect the session - verify the reconnect occurs. + REQUIRE(state_machina.wait_for(TestState::reconnect_received)); + // Assuming the session disconnects and reconnects, the server initiated role change + // bootstrap download will take place when the session is re-established and will + // complete before the server sends the initial MARK response. REQUIRE(!wait_for_download(*realm)); + + // The tricky part here is that a single server initiated bootstrap message + // with one changeset will be treated as a typical download message and the + // bootstrap operation cannot easily be tracked. Adding back the employees to + // the local data via the permissions should produce a trackable bootstrap + // with multiple messages which can be verified. + bool multi_msg = false; { - std::lock_guard lock(callback_mutex); - if (msg_count > 1) + std::lock_guard lock(bootstrap_mutex); + if (msg_count > 1) { multi_msg = true; + // verify a role change server initiated bootstrap was performed if it + // contained more than one bootstrap message + REQUIRE(role_change_bootstrap); + } } - // Verify a bootstrap occurred if multiple messages were received - REQUIRE((!multi_msg || machina.get() == TestState::bs_complete)); + // Make sure a client reset did not occur while waiting for the role change to + // be applied + { + std::lock_guard lock(callback_mutex); + CHECK_FALSE(did_client_reset); + } + + // If a multi-message bootstrap was received, verify it was applied (it should be + // applied when the last download message of the bootstrap is received). By the + // time the MARK response is received and wait_for_download() returns, the bootstrap + // should have already been applied. + REQUIRE((!multi_msg || state_machina.get() == TestState::complete)); + + // Wait for the upload to complete and the updated data to be ready in the local realm. REQUIRE(!wait_for_upload(*realm)); wait_for_advance(*realm); { + // Validate the expected number of entries for each role type after the role change auto table = realm->read_group().get_table("class_Person"); REQUIRE(table->size() == (cnt_emps + cnt_mgrs + cnt_dirs)); auto role_col = table->get_column_key("role"); @@ -5156,23 +5151,23 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } { // Clear out the hook callback before leaving - std::lock_guard lock(hook_mutex); + std::lock_guard lock(callback_mutex); hook_callback = nullptr; } }; { - // Single message subtractive bootstrap + // Single message bootstrap - remove employees, keep mgrs/dirs nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; - update_perms(doc_filter, 0, num_mgrs, num_dirs); + update_perms_and_verify(doc_filter, 0, num_mgrs, num_dirs); } { - // Multi-message additive/subtractive bootstrap + // Multi-message bootstrap - add employeees, remove managers and directors nlohmann::json doc_filter = {{"role", "employee"}}; - update_perms(doc_filter, num_emps, 0, 0); + update_perms_and_verify(doc_filter, num_emps, 0, 0); } { - // Single message additive bootstrap - update_perms(std::nullopt, num_emps, num_mgrs, num_dirs); + // Single message bootstrap - add back managers and directors + update_perms_and_verify(std::nullopt, num_emps, num_mgrs, num_dirs); } } From 400aebae96119f370af768689785aa987fccc72b Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 30 Apr 2024 15:48:34 -0400 Subject: [PATCH 11/37] Reworked test to fix msvc failure --- test/object-store/sync/flx_sync.cpp | 150 +++++++++++++--------------- 1 file changed, 69 insertions(+), 81 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 27ea09fba3b..186d01e331b 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4956,6 +4956,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra })); } }; + + enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; + TestingStateMachine state_machina(TestState::not_ready); + std::mutex callback_mutex; + int64_t query_version = 0; + bool did_client_reset = false; + size_t msg_count = 0; + bool role_change_bootstrap = false; + auto logger = util::Logger::get_default_logger(); FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); auto& app_session = harness.session().app_session(); @@ -4977,19 +4986,58 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto config = harness.make_test_file(); - std::mutex callback_mutex; - std::function, const SyncClientHookData&)> hook_callback; - bool did_client_reset = false; + config.sync_config->on_sync_client_event_hook = [&state_machina, &callback_mutex, &msg_count, + &role_change_bootstrap, &query_version]( + std::weak_ptr, const SyncClientHookData& data) { + state_machina.transition_with([&](TestState cur_state) -> std::optional { + if (cur_state == TestState::not_ready) + return std::nullopt; - config.sync_config->on_sync_client_event_hook = - [&hook_callback, &callback_mutex](std::weak_ptr session, const SyncClientHookData& data) { - std::lock_guard lock(callback_mutex); - if (hook_callback) { - return hook_callback(session, data); + using BatchState = sync::DownloadBatchState; + using Event = SyncClientHookEvent; + switch (data.event) { + case Event::ErrorMessageReceived: + REQUIRE(cur_state == TestState::start); + REQUIRE(data.error_info); + REQUIRE(data.error_info->raw_error_code == 200); + REQUIRE(data.error_info->server_requests_action == sync::ProtocolErrorInfo::Action::Transient); + REQUIRE_FALSE(data.error_info->is_fatal); + return TestState::reconnect_received; + + // A bootstrap message was processed - used for multi-message or + // single message multi changeset bootstraps + case Event::BootstrapMessageProcessed: { + REQUIRE(data.batch_state != BatchState::SteadyState); + REQUIRE((cur_state == TestState::reconnect_received || cur_state == TestState::downloading)); + if (data.batch_state != BatchState::SteadyState) { + std::lock_guard lock(callback_mutex); + ++msg_count; + if (data.query_version == query_version) { + role_change_bootstrap = true; + } + } + // multi-message bootstrap in progress.. + if (data.batch_state == BatchState::MoreToCome) { + return TestState::downloading; + } + // single bootstrap message or last message in the multi-message bootstrap + else if (data.batch_state == BatchState::LastInBatch) { + return TestState::downloaded; + } + return std::nullopt; + } + // The bootstrap has been received and processed + case Event::BootstrapProcessed: + REQUIRE(cur_state == TestState::downloaded); + return TestState::complete; + default: + return std::nullopt; } - return SyncClientHookAction::NoAction; - }; - config.sync_config->notify_before_client_reset = [&did_client_reset, &callback_mutex](std::shared_ptr) { + }); + return SyncClientHookAction::NoAction; + }; + + config.sync_config->notify_before_client_reset = [&callback_mutex, &did_client_reset](std::shared_ptr) { std::lock_guard lock(callback_mutex); did_client_reset = true; }; @@ -5017,70 +5065,17 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(results.size() == num_total); } - auto session = realm->sync_session(); - int64_t query_version = realm->get_active_subscription_set().version(); - auto update_perms_and_verify = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, size_t cnt_dirs) { - using BatchState = sync::DownloadBatchState; - using Event = SyncClientHookEvent; - std::mutex bootstrap_mutex; - size_t msg_count = 0; - bool role_change_bootstrap = false; - - enum TestState { start, reconnect_received, downloading, downloaded, complete }; - TestingStateMachine state_machina(TestState::start); - - auto bootstrap_callback = [&bootstrap_mutex, &msg_count, query_version, &role_change_bootstrap, - &state_machina](std::weak_ptr, const SyncClientHookData& data) { - state_machina.transition_with([&](TestState cur_state) -> std::optional { - switch (data.event) { - case Event::ErrorMessageReceived: - REQUIRE(cur_state == TestState::start); - REQUIRE(data.error_info); - REQUIRE(data.error_info->raw_error_code == 200); - REQUIRE(data.error_info->server_requests_action == - sync::ProtocolErrorInfo::Action::Transient); - REQUIRE_FALSE(data.error_info->is_fatal); - return TestState::reconnect_received; - - // A bootstrap message was processed - used for multi-message or - // single message multi changeset bootstraps - case Event::BootstrapMessageProcessed: { - REQUIRE(data.batch_state != BatchState::SteadyState); - REQUIRE((cur_state == TestState::reconnect_received || cur_state == TestState::downloading)); - if (data.batch_state != BatchState::SteadyState) { - std::lock_guard lock(bootstrap_mutex); - ++msg_count; - if (data.query_version == query_version) { - role_change_bootstrap = true; - } - } - // multi-message bootstrap in progress.. - if (data.batch_state == BatchState::MoreToCome) { - return TestState::downloading; - } - // single bootstrap message or last message in the multi-message bootstrap - else if (data.batch_state == BatchState::LastInBatch) { - return TestState::downloaded; - } - return std::nullopt; - } - // The bootstrap has been received and processed - case Event::BootstrapProcessed: - REQUIRE(cur_state == TestState::downloaded); - return TestState::complete; - default: - return std::nullopt; - } - }); - return SyncClientHookAction::NoAction; - }; { std::lock_guard lock(callback_mutex); - hook_callback = std::move(bootstrap_callback); did_client_reset = false; + msg_count = 0; + role_change_bootstrap = false; + query_version = realm->get_active_subscription_set().version(); } + // Reset the state machine + state_machina.transition_to(TestState::start); rule["roles"][0]["document_filters"]["read"] = doc_filter.value_or(true); rule["roles"][0]["document_filters"]["write"] = doc_filter.value_or(true); @@ -5106,19 +5101,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra bool multi_msg = false; { - std::lock_guard lock(bootstrap_mutex); + std::lock_guard lock(callback_mutex); if (msg_count > 1) { multi_msg = true; // verify a role change server initiated bootstrap was performed if it // contained more than one bootstrap message REQUIRE(role_change_bootstrap); } - } - - // Make sure a client reset did not occur while waiting for the role change to - // be applied - { - std::lock_guard lock(callback_mutex); + // Make sure a client reset did not occur while waiting for the role change to + // be applied CHECK_FALSE(did_client_reset); } @@ -5149,11 +5140,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra results = Results(realm, table_query); CHECK(results.size() == cnt_dirs); } - { - // Clear out the hook callback before leaving - std::lock_guard lock(callback_mutex); - hook_callback = nullptr; - } + // Reset the state machine to "not ready" before leaving + state_machina.transition_to(TestState::not_ready); }; { // Single message bootstrap - remove employees, keep mgrs/dirs From 3ad02b643038351076f99fa34f6a99a823eb453c Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 2 May 2024 16:28:56 -0400 Subject: [PATCH 12/37] Reverted baas branch to master and protocol version to 12 --- dependencies.yml | 9 +++++---- evergreen/install_baas.sh | 5 +---- src/realm/sync/protocol.hpp | 2 +- test/object-store/sync/flx_sync.cpp | 6 +++--- test/object-store/util/sync/baas_admin_api.cpp | 15 +++++++-------- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 1ebba1ef43c..4dd2966c94a 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -2,7 +2,8 @@ PACKAGE_NAME: realm-core VERSION: 14.6.2 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 -# https://github.com/10gen/baas/commits -# 6bf5e1 is 2024 Mar 20 -BAAS_VERSION: 66171338e32da7000754c772 -BAAS_VERSION_TYPE: patchid +# 5138f5c is 2024 Apr 19 +# BAAS_VERSION: 5138f5c924f9f376d31cbc2b1819e1d934600277 +# BAAS_VERSION_TYPE: githash +BAAS_VERSION: master +BAAS_VERSION_TYPE: branch diff --git a/evergreen/install_baas.sh b/evergreen/install_baas.sh index 9aac95aa0ab..5ae34178635 100755 --- a/evergreen/install_baas.sh +++ b/evergreen/install_baas.sh @@ -402,12 +402,9 @@ if [[ -z "${BAAS_VERSION}" ]]; then exit 1 fi -BAAS_VERSION="BAAS-28604-remove-ff" - # Clone the baas repo and check out the specified version if [[ ! -d "${BAAS_DIR}/.git" ]]; then -# git clone git@github.com:10gen/baas.git "${BAAS_DIR}" - git clone git@github.com:sean-brandenburg/baas.git "${BAAS_DIR}" + git clone git@github.com:10gen/baas.git "${BAAS_DIR}" pushd "${BAAS_DIR}" > /dev/null else pushd "${BAAS_DIR}" > /dev/null diff --git a/src/realm/sync/protocol.hpp b/src/realm/sync/protocol.hpp index f857d565b9a..7f90537cff3 100644 --- a/src/realm/sync/protocol.hpp +++ b/src/realm/sync/protocol.hpp @@ -69,7 +69,7 @@ constexpr int get_current_protocol_version() noexcept { // Also update the current protocol version test in flx_sync.cpp when // updating this value - return 14; + return 12; } constexpr std::string_view get_pbs_websocket_protocol_prefix() noexcept diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 186d01e331b..5d0d3ed3484 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -2583,7 +2583,7 @@ TEST_CASE("flx: writes work without waiting for sync", "[sync][flx][baas]") { TEST_CASE("flx: verify websocket protocol number and prefixes", "[sync][protocol]") { // Update the expected value whenever the protocol version is updated - this ensures // that the current protocol version does not change unexpectedly. - REQUIRE(14 == sync::get_current_protocol_version()); + REQUIRE(12 == sync::get_current_protocol_version()); // This was updated in Protocol V8 to use '#' instead of '/' to support the Web SDK REQUIRE("com.mongodb.realm-sync#" == sync::get_pbs_websocket_protocol_prefix()); REQUIRE("com.mongodb.realm-query-sync#" == sync::get_flx_websocket_protocol_prefix()); @@ -4969,9 +4969,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); auto& app_session = harness.session().app_session(); // Enable the role change bootstraps - // REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", - // true)); + REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); // Ensure the feature flag is enabled + // TODO: Fix once an updated baasaas solution is in place // REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); // Get the current rules so it can be updated during the test diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index 2952463cb68..224f6ff9504 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -948,17 +948,15 @@ void AdminAPISession::create_schema(const std::string& app_id, const AppCreateCo bool AdminAPISession::set_feature_flag(const std::string& app_id, const std::string& flag_name, bool enable) const { - auto features_endpoint = - AdminAPIEndpoint(util::format("%1/api/private/v1.0/features/%2", m_base_url, flag_name), m_access_token); - auto flag_response = features_endpoint.post_json( - nlohmann::json{{"app_ids", nlohmann::json::array({app_id})}, {"action", enable ? "enable" : "disable"}}); - auto actual_modified = flag_response["actual_modified"]; - // "actual_modified" should only be 1 since only 1 app_id was provided - return actual_modified.is_number_integer() && actual_modified.get() == 1; + auto features = apps(APIFamily::Private)[app_id]["features"]; + auto flag_response = + features.post_json(nlohmann::json{{"action", enable ? "enable" : "disable"}, {"feature_flags", {flag_name}}}); + return flag_response.empty(); } -bool AdminAPISession::get_feature_flag(const std::string& app_id, const std::string& flag_name) const +bool AdminAPISession::get_feature_flag(const std::string& /*app_id*/, const std::string& /*flag_name*/) const { + /** TODO: Fix once an updated baasaas solution is in place auto settings_json = get_app_settings(app_id); // Format: {..., "features": {"enabled": ["", "", ...], ...}, ...} if (auto features = settings_json.find("features"); @@ -972,6 +970,7 @@ bool AdminAPISession::get_feature_flag(const std::string& app_id, const std::str } } } + */ return false; } From 3198ebde6869ad048cc5373c56035b9b6579ed70 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 2 May 2024 20:37:18 -0400 Subject: [PATCH 13/37] Added comments to changes needed when merging to master; update baas version to not use master --- dependencies.yml | 9 ++++----- test/object-store/sync/flx_sync.cpp | 4 +--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 4dd2966c94a..9d55886b19f 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -2,8 +2,7 @@ PACKAGE_NAME: realm-core VERSION: 14.6.2 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 -# 5138f5c is 2024 Apr 19 -# BAAS_VERSION: 5138f5c924f9f376d31cbc2b1819e1d934600277 -# BAAS_VERSION_TYPE: githash -BAAS_VERSION: master -BAAS_VERSION_TYPE: branch +# TODO: Remove when switching to use Protocol version in RCORE-1972 and merging to master +# 51d0e54 is BAAS-30091 - 2024 May 01 +BAAS_VERSION: 51d0e5411b9dab52cb24977d470c2c910d1b18c8 +BAAS_VERSION_TYPE: githash diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 5d0d3ed3484..bcc48594caf 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4968,11 +4968,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto logger = util::Logger::get_default_logger(); FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); auto& app_session = harness.session().app_session(); + /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ // Enable the role change bootstraps REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); - // Ensure the feature flag is enabled - // TODO: Fix once an updated baasaas solution is in place - // REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); // Get the current rules so it can be updated during the test auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); From fe86341f37bccf8a378d06b1fde826f5c9360018 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 14 May 2024 13:47:22 -0400 Subject: [PATCH 14/37] Merging in recommended changes --- src/realm/sync/client.cpp | 63 +++++++++---------- src/realm/sync/noinst/client_impl_base.cpp | 35 ++++------- src/realm/sync/noinst/client_impl_base.hpp | 8 +-- src/realm/sync/noinst/protocol_codec.hpp | 1 + test/object-store/sync/flx_sync.cpp | 62 ++++++++++++------ .../object-store/util/sync/baas_admin_api.cpp | 29 +++++---- .../object-store/util/sync/baas_admin_api.hpp | 1 + 7 files changed, 102 insertions(+), 97 deletions(-) diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 413e374c8c7..5d84f2e197b 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -936,7 +936,7 @@ void SessionImpl::process_pending_flx_bootstrap() auto pending_batch = bootstrap_store->peek_pending(m_wrapper.m_flx_bootstrap_batch_size_bytes); if (!pending_batch.progress) { logger.info("Incomplete pending bootstrap found for query version %1", pending_batch.query_version); - // Close the write transation before clearing the bootstrap store to avoid a deadlock because the + // Close the write transaction before clearing the bootstrap store to avoid a deadlock because the // bootstrap store requires a write transaction itself. transact->close(); bootstrap_store->clear(); @@ -1099,53 +1099,46 @@ SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, con return call_debug_hook(data); } -bool SessionImpl::is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, - bool same_remote_versions) +DownloadBatchState SessionImpl::derive_download_batch_state(const DownloadMessage& message, + bool has_duplicate_changeset_versons) { // Should never be called if session is not active REALM_ASSERT_EX(m_state == State::Active, m_state); - // Query version should always be the same or increasing - REALM_ASSERT_3(query_version, >=, m_wrapper.m_flx_active_version); - // Return early if already steady state or PBS (doesn't use bootstraps) - if (batch_state == DownloadBatchState::SteadyState || !m_is_flx_sync_session) { - return true; // Steady state + // PBS download messages are always steady state + if (!m_is_flx_sync_session) { + return DownloadBatchState::SteadyState; } // Bootstrap messages (i.e. non-steady state) are identified by: - // * DownloadBatchState of MoreToCome - // * DownloadBatchState of LastInBatch, and - // * first LastInBatch=true after one or more MoreToCome messages - // * query_version greater than the active query_version - // * LastInBatch=true message has 2 or more changesets and all have the same - // remote_version - - // LastInBatch=False messages are always a bootstrap message - if (batch_state == DownloadBatchState::MoreToCome) { - return false; // Not steady state - } + // * last_in_batch=false + // * last_in_batch=true, and + // * previous messages received had last_in_batch=false + // * query_version is greater than the active query_version + // * 2 or more changesets in the message have the same remote version - // Messages with query_version greater than the active version are always a - // bootstrap message - if (query_version > m_wrapper.m_flx_active_version) { - return false; // Not steady state - } + // Query version should always be the same or increasing + REALM_ASSERT_3(message.query_version, >=, m_wrapper.m_flx_active_version); + // FLX download messages should always have a last_in_batch value + REALM_ASSERT_EX(message.last_in_batch, "FLX download message missing last_in_batch value"); - // If this is the first LastInBatch=true message after one or more - // LastInBatch=false messages, this is the end of a bootstrap - if (m_last_download_batch_state == DownloadBatchState::MoreToCome) { - return false; // Not steady state + // If last_in_batch=false, this is a multi-message bootstrap and there are more to come + if (!*message.last_in_batch) { + return DownloadBatchState::MoreToCome; } - // Otherwise, if this is a LastInBatch=true message whose query version matches - // the current active version and all the changesets in the message have the - // same remote version, then this is a server initiated single message bootstrap - if (same_remote_versions) { - return false; // Not steady state + // This download message is the last message of a bootstrap (or a single + // message bootstrap) if last_in_batch=true and one of the following applies: + // * The query_version is greater than the active query_version + // * A bootstrap is in progress (i.e. previous MoreToCome messages received) + // * Two or more changesets in the download message have the same remote version + if (message.query_version > m_wrapper.m_flx_active_version || + m_wrapper.get_flx_pending_bootstrap_store()->has_pending() || has_duplicate_changeset_versons) { + return DownloadBatchState::LastInBatch; } - // If none of the previous checks were successful, then this is a steady state msg - return true; // Steady state + // Otherwise, not a bootstrap message, process as a normal download message + return DownloadBatchState::SteadyState; } void SessionImpl::init_progress_handler() diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index ad3b6771510..ca9126227c8 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -1891,7 +1891,6 @@ void Session::send_bind_message() session_ident_type session_ident = m_ident; bool need_client_file_ident = !have_client_file_ident(); const bool is_subserver = false; - m_last_download_batch_state = DownloadBatchState::SteadyState; ClientProtocol& protocol = m_conn.get_client_protocol(); int protocol_version = m_conn.get_negotiated_protocol_version(); @@ -2370,10 +2369,6 @@ Status Session::receive_download_message(const DownloadMessage& message) if (!is_flx || query_version > 0) enable_progress_notifications(); - // If this is a PBS connection, then every download message is its own complete batch. - bool last_in_batch = is_flx ? *message.last_in_batch : true; - auto batch_state = last_in_batch ? sync::DownloadBatchState::LastInBatch : sync::DownloadBatchState::MoreToCome; - auto&& progress = message.progress; if (is_flx) { logger.debug("Received: DOWNLOAD(download_server_version=%1, download_client_version=%2, " @@ -2383,7 +2378,8 @@ Status Session::receive_download_message(const DownloadMessage& message) progress.download.server_version, progress.download.last_integrated_client_version, progress.latest_server_version.version, progress.latest_server_version.salt, progress.upload.client_version, progress.upload.last_integrated_server_version, - message.progress_estimate, last_in_batch, query_version, message.changesets.size()); // Throws + message.progress_estimate, *message.last_in_batch, query_version, + message.changesets.size()); // Throws } else { logger.debug("Received: DOWNLOAD(download_server_version=%1, download_client_version=%2, " @@ -2416,10 +2412,8 @@ Status Session::receive_download_message(const DownloadMessage& message) // download message must have a remote version greater than (or equal to for FLX) this value. version_type last_remote_version = m_progress.download.server_version; version_type last_integrated_client_version = m_progress.download.last_integrated_client_version; - // If there are 2 or more changesets in this message, track whether or not the changesets all have the same - // download_server_version to help with determining if this is a single message server-initiated bootstrap or not. - bool same_remote_version = last_in_batch && message.changesets.size() > 1; - bool after_first_changeset = false; + version_type last_changeset_version = 0; // skips checking the first time through the loop + bool has_duplicate_changeset_versions = false; for (const RemoteChangeset& changeset : message.changesets) { // Check that per-changeset server version is strictly increasing since the last download server version, @@ -2433,17 +2427,17 @@ Status Session::receive_download_message(const DownloadMessage& message) util::format("Bad server version in changeset header (DOWNLOAD) (%1, %2, %3)", changeset.remote_version, last_remote_version, progress.download.server_version)}; } - // Check to see if all the changesets in this LastInBatch=true message have the same remote_version - // If so, this is a server-initiated single message bootstrap - if (same_remote_version && after_first_changeset) { - // After the first changeset compare the previous changeset's server version to the current changeset - // server version. If they are different then this is definitely not a bootstrap message - same_remote_version = changeset.remote_version == last_remote_version; + + if (last_changeset_version == changeset.remote_version) { + // last_changeset_version is 0 first time through the loop to prevent comparing the + // changeset remote_version to the download_server_version, which may lead to a false + // positive when checking for changesets with duplicate remote_version values + has_duplicate_changeset_versions = true; } - // Skip the first changeset, since `server_version` contains the incorrect value for this check - after_first_changeset = true; + // Save the remote version for the current changeset to compare against the next changeset last_remote_version = changeset.remote_version; + last_changeset_version = changeset.remote_version; // Check that per-changeset last integrated client version is "weakly" // increasing. @@ -2469,10 +2463,7 @@ Status Session::receive_download_message(const DownloadMessage& message) } } - if (is_steady_state_download_message(batch_state, query_version, same_remote_version)) { - batch_state = DownloadBatchState::SteadyState; - } - m_last_download_batch_state = batch_state; + auto batch_state = derive_download_batch_state(message, has_duplicate_changeset_versions); auto hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageReceived, progress, query_version, batch_state, message.changesets.size()); diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 84ddf01c19d..46625cad420 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1133,10 +1133,6 @@ class ClientImpl::Session { // the detection of download completion. request_ident_type m_last_triggering_download_mark = 0; - // Keep track of the DownloadBatchState for the last download message to help - // determine if the current LastInBatch came after a message with MoreToCome. - DownloadBatchState m_last_download_batch_state = DownloadBatchState::SteadyState; - SessionWrapper& m_wrapper; request_ident_type m_last_pending_test_command_ident = 0; @@ -1201,8 +1197,8 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo&); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); - bool is_steady_state_download_message(DownloadBatchState batch_state, int64_t query_version, - bool same_remote_versions); + DownloadBatchState derive_download_batch_state(const DownloadMessage& message, + bool has_duplicate_changeset_versons); void init_progress_handler(); void enable_progress_notifications(); diff --git a/src/realm/sync/noinst/protocol_codec.hpp b/src/realm/sync/noinst/protocol_codec.hpp index 9810d205ba5..225c80cd71e 100644 --- a/src/realm/sync/noinst/protocol_codec.hpp +++ b/src/realm/sync/noinst/protocol_codec.hpp @@ -403,6 +403,7 @@ class ClientProtocol { struct DownloadMessage { SyncProgress progress; + // For FLX sync, the query_version and last_in_batch are required to be provided std::optional query_version; std::optional last_in_batch; union { diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 7d28952bb52..58b847afcc8 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5023,12 +5023,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } }; + enum BootstrapMode { None, SingleMessage, SingleMessageMulti, MultiMessage }; enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; TestingStateMachine state_machina(TestState::not_ready); std::mutex callback_mutex; int64_t query_version = 0; bool did_client_reset = false; - size_t msg_count = 0; + BootstrapMode bootstrap_mode = BootstrapMode::None; + size_t download_msg_count = 0; + size_t bootstrap_msg_count = 0; bool role_change_bootstrap = false; auto logger = util::Logger::get_default_logger(); @@ -5037,6 +5040,11 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ // Enable the role change bootstraps REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); + REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, + {{"sync", {{"num_objects_before_bootstrap_flush", 1}}}})); + REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, + {{"sync", {{"download_loop_sleep_millis", 25}}}})); // Get the current rules so it can be updated during the test auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); @@ -5050,9 +5058,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto config = harness.make_test_file(); - config.sync_config->on_sync_client_event_hook = [&state_machina, &callback_mutex, &msg_count, - &role_change_bootstrap, &query_version]( - std::weak_ptr, const SyncClientHookData& data) { + config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, const SyncClientHookData& data) { state_machina.transition_with([&](TestState cur_state) -> std::optional { if (cur_state == TestState::not_ready) return std::nullopt; @@ -5068,26 +5074,42 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE_FALSE(data.error_info->is_fatal); return TestState::reconnect_received; + case Event::DownloadMessageReceived: { + std::lock_guard lock(callback_mutex); + // multi-message bootstrap in progress.. + if (data.batch_state == BatchState::MoreToCome) { + // More than 1 bootstrap message, always a multi-message + ++download_msg_count; + bootstrap_mode = BootstrapMode::MultiMessage; + return TestState::downloading; + } + // single bootstrap message or last message in the multi-message bootstrap + else if (data.batch_state == BatchState::LastInBatch) { + ++download_msg_count; + if (cur_state == TestState::reconnect_received) { + // If reconnect error was last state, then bootstrap has only one message + bootstrap_mode = BootstrapMode::SingleMessageMulti; + // Must have 2 or more changesets in message to get here... + REQUIRE(data.num_changesets > 1); + } + return TestState::downloaded; + } + // single message/single changeset bootstraps are treated as a steady state download + return std::nullopt; + } + // A bootstrap message was processed - used for multi-message or // single message multi changeset bootstraps case Event::BootstrapMessageProcessed: { REQUIRE(data.batch_state != BatchState::SteadyState); - REQUIRE((cur_state == TestState::reconnect_received || cur_state == TestState::downloading)); + REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); if (data.batch_state != BatchState::SteadyState) { std::lock_guard lock(callback_mutex); - ++msg_count; + ++bootstrap_msg_count; if (data.query_version == query_version) { role_change_bootstrap = true; } } - // multi-message bootstrap in progress.. - if (data.batch_state == BatchState::MoreToCome) { - return TestState::downloading; - } - // single bootstrap message or last message in the multi-message bootstrap - else if (data.batch_state == BatchState::LastInBatch) { - return TestState::downloaded; - } return std::nullopt; } // The bootstrap has been received and processed @@ -5130,11 +5152,12 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } auto update_perms_and_verify = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, - size_t cnt_dirs) { + size_t cnt_dirs, BootstrapMode expected_mode) { { std::lock_guard lock(callback_mutex); did_client_reset = false; - msg_count = 0; + bootstrap_msg_count = 0; + download_msg_count = 0; role_change_bootstrap = false; query_version = realm->get_active_subscription_set().version(); } @@ -5208,18 +5231,19 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra state_machina.transition_to(TestState::not_ready); }; { + // SingleMessage, SingleMessageMulti, MultiMessage // Single message bootstrap - remove employees, keep mgrs/dirs nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; - update_perms_and_verify(doc_filter, 0, num_mgrs, num_dirs); + update_perms_and_verify(doc_filter, 0, num_mgrs, num_dirs, BootstrapMode::SingleMessage); } { // Multi-message bootstrap - add employeees, remove managers and directors nlohmann::json doc_filter = {{"role", "employee"}}; - update_perms_and_verify(doc_filter, num_emps, 0, 0); + update_perms_and_verify(doc_filter, num_emps, 0, 0, BootstrapMode::MultiMessage); } { // Single message bootstrap - add back managers and directors - update_perms_and_verify(std::nullopt, num_emps, num_mgrs, num_dirs); + update_perms_and_verify(std::nullopt, num_emps, num_mgrs, num_dirs, , BootstrapMode::SingleMessageMulti); } } diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index 3c4fdaebca1..ab4eb57ba96 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -994,23 +994,15 @@ bool AdminAPISession::set_feature_flag(const std::string& app_id, const std::str return flag_response.empty(); } -bool AdminAPISession::get_feature_flag(const std::string& /*app_id*/, const std::string& /*flag_name*/) const +bool AdminAPISession::get_feature_flag(const std::string& app_id, const std::string& flag_name) const { - /** TODO: Fix once an updated baasaas solution is in place - auto settings_json = get_app_settings(app_id); - // Format: {..., "features": {"enabled": ["", "", ...], ...}, ...} - if (auto features = settings_json.find("features"); - features != settings_json.end() && features->is_object() && !features->empty()) { - if (auto enabled_list = features->find("enabled"); - enabled_list != features->end() && enabled_list->is_array() && !enabled_list->empty()) { - for (auto& item : *enabled_list) { - if (item.is_string() && item.get() == flag_name) { - return true; - } - } - } + auto features = apps(APIFamily::Private)[app_id]["features"]; + auto response = features.get_json(); + if (auto feature_list = response["enabled"]; !feature_list.empty()) { + return std::find_if(feature_list.begin(), feature_list.end(), [&flag_name](const auto& feature) { + return feature == flag_name; + }) != feature_list.end(); } - */ return false; } @@ -1041,6 +1033,13 @@ nlohmann::json AdminAPISession::get_app_settings(const std::string& app_id) cons return settings_endpoint.get_json(); } +bool AdminAPISession::patch_app_settings(const std::string& app_id, nlohmann::json&& json) const +{ + auto settings_endpoint = apps(APIFamily::Private)[app_id]["settings"]; + auto response = settings_endpoint.patch_json(std::move(json)); + return response.empty(); +} + static nlohmann::json convert_config(AdminAPISession::ServiceConfig config) { if (config.mode == AdminAPISession::ServiceConfig::SyncMode::Flexible) { diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index d9b2fbbc358..85b20306bea 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -145,6 +145,7 @@ class AdminAPISession { MigrationStatus get_migration_status(const std::string& app_id) const; nlohmann::json get_app_settings(const std::string& app_id) const; + bool patch_app_settings(const std::string& app_id, nlohmann::json&& new_settings) const; const std::string& admin_url() const noexcept { From 7a5269bb116546232851bba76f8ccc4e42326f4b Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 16 May 2024 10:43:12 -0400 Subject: [PATCH 15/37] Pulled over changes from other branch and tweaking download params --- test/object-store/sync/flx_sync.cpp | 225 +++++++++--------- .../object-store/util/sync/baas_admin_api.cpp | 57 ++--- .../object-store/util/sync/baas_admin_api.hpp | 15 +- 3 files changed, 153 insertions(+), 144 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 58b847afcc8..73cc6bd5c22 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -1639,18 +1639,9 @@ TEST_CASE("flx: uploading an object that is out-of-view results in compensating {"queryable_str_field", PropertyType::String | PropertyType::Nullable}, }}}; - AppCreateConfig::ServiceRole role; - role.name = "compensating_write_perms"; - - AppCreateConfig::ServiceRoleDocumentFilters doc_filters; - doc_filters.read = true; - doc_filters.write = {{"queryable_str_field", {{"$in", nlohmann::json::array({"foo", "bar"})}}}}; - role.document_filters = doc_filters; - - role.insert_filter = true; - role.delete_filter = true; - role.read = true; - role.write = true; + AppCreateConfig::ServiceRole role{"compensating_write_perms"}; + role.document_filters.write = {{"queryable_str_field", {{"$in", nlohmann::json::array({"foo", "bar"})}}}}; + FLXSyncTestHarness::ServerSchema server_schema{schema, {"queryable_str_field"}, {role}}; harness.emplace("flx_bad_query", server_schema); } @@ -3597,18 +3588,8 @@ TEST_CASE("flx: data ingest - dev mode", "[sync][flx][data ingest][baas]") { } TEST_CASE("flx: data ingest - write not allowed", "[sync][flx][data ingest][baas]") { - AppCreateConfig::ServiceRole role; - role.name = "asymmetric_write_perms"; - - AppCreateConfig::ServiceRoleDocumentFilters doc_filters; - doc_filters.read = true; - doc_filters.write = false; - role.document_filters = doc_filters; - - role.insert_filter = true; - role.delete_filter = true; - role.read = true; - role.write = true; + AppCreateConfig::ServiceRole role{"asymmetric_write_perms"}; + role.document_filters.write = false; Schema schema({ {"Asymmetric", @@ -4086,19 +4067,10 @@ TEST_CASE("flx: convert flx sync realm to bundled realm", "[app][flx][baas]") { } TEST_CASE("flx: compensating write errors get re-sent across sessions", "[sync][flx][compensating write][baas]") { - AppCreateConfig::ServiceRole role; - role.name = "compensating_write_perms"; - - AppCreateConfig::ServiceRoleDocumentFilters doc_filters; - doc_filters.read = true; - doc_filters.write = - nlohmann::json{{"queryable_str_field", nlohmann::json{{"$in", nlohmann::json::array({"foo", "bar"})}}}}; - role.document_filters = doc_filters; - - role.insert_filter = true; - role.delete_filter = true; - role.read = true; - role.write = true; + AppCreateConfig::ServiceRole role{"compensating_write_perms"}; + role.document_filters.write = { + {"queryable_str_field", nlohmann::json{{"$in", nlohmann::json::array({"foo", "bar"})}}}}; + FLXSyncTestHarness::ServerSchema server_schema{ g_simple_embedded_obj_schema, {"queryable_str_field", "queryable_int_field"}, {role}}; FLXSyncTestHarness::Config harness_config("flx_bad_query", server_schema); @@ -5001,10 +4973,10 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra {"firstName", PropertyType::String}, {"lastName", PropertyType::String}}}}; - size_t num_emps = 5000; - size_t num_mgrs = 200; - size_t num_dirs = 25; - size_t num_total = num_emps + num_mgrs + num_dirs; + constexpr size_t num_emps = 5000; + constexpr size_t num_mgrs = 200; + constexpr size_t num_dirs = 25; + constexpr size_t num_total = num_emps + num_mgrs + num_dirs; auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { CppContext c(realm); for (size_t i = 0; i < count; ++i) { @@ -5041,16 +5013,23 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Enable the role change bootstraps REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + auto app_settings = app_session.admin_api.get_app_settings(app_session.server_app_id); + std::cout << "APP SETTINGS: " << app_settings << std::endl; + REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - {{"sync", {{"num_objects_before_bootstrap_flush", 1}}}})); + {{"sync", {{"num_objects_before_bootstrap_flush", 200}}}})); REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - {{"sync", {{"download_loop_sleep_millis", 25}}}})); + {{"sync", {{"download_loop_sleep_millis", 1500}}}})); + // REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, + // {{"sync", {{"num_objects_before_bootstrap_flush", 0}}}})); + // REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, + // {{"sync", {{"download_loop_sleep_millis", 0}}}})); // Get the current rules so it can be updated during the test auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); // Initialize the realm with some data - harness.load_initial_data([&fill_person_schema, num_emps, num_mgrs, num_dirs](SharedRealm realm) { + harness.load_initial_data([&fill_person_schema](SharedRealm realm) { fill_person_schema(realm, "employee", num_emps); fill_person_schema(realm, "manager", num_mgrs); fill_person_schema(realm, "director", num_dirs); @@ -5128,12 +5107,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra did_client_reset = true; }; - auto realm = Realm::get_shared_realm(config); - REQUIRE(!wait_for_download(*realm)); - REQUIRE(!wait_for_upload(*realm)); - - // Set up the initial subscription - { + auto set_up_realm = [](SharedRealm realm, size_t expected_cnt) { + // Set up the initial subscription auto table = realm->read_group().get_table("class_Person"); auto new_subs = realm->get_latest_subscription_set().make_mutable_copy(); new_subs.insert_or_assign(Query(table)); @@ -5144,32 +5119,52 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(!wait_for_download(*realm)); REQUIRE(!wait_for_upload(*realm)); wait_for_advance(*realm); - } - { - auto table = realm->read_group().get_table("class_Person"); + + // Verify the data was downloaded + table = realm->read_group().get_table("class_Person"); Results results(realm, Query(table)); - REQUIRE(results.size() == num_total); - } + REQUIRE(results.size() == expected_cnt); + }; + + auto verify_records = [](SharedRealm check_realm, size_t cnt_emps, size_t cnt_mgrs, size_t cnt_dirs) { + // Validate the expected number of entries for each role type after the role change + auto table = check_realm->read_group().get_table("class_Person"); + REQUIRE(table->size() == (cnt_emps + cnt_mgrs + cnt_dirs)); + auto role_col = table->get_column_key("role"); + auto table_query = Query(table).equal(role_col, "director").Or().equal(role_col, "manager"); + auto results = Results(check_realm, table_query); + CHECK(results.size() == (cnt_mgrs + cnt_dirs)); + table_query = Query(table).equal(role_col, "employee"); + results = Results(check_realm, table_query); + CHECK(results.size() == cnt_emps); + table_query = Query(table).equal(role_col, "manager"); + results = Results(check_realm, table_query); + CHECK(results.size() == cnt_mgrs); + table_query = Query(table).equal(role_col, "director"); + results = Results(check_realm, table_query); + CHECK(results.size() == cnt_dirs); + }; - auto update_perms_and_verify = [&](std::optional doc_filter, size_t cnt_emps, size_t cnt_mgrs, - size_t cnt_dirs, BootstrapMode expected_mode) { + auto update_perms_and_verify = [&](nlohmann::json new_rules, SharedRealm check_realm, size_t cnt_emps, + size_t cnt_mgrs, size_t cnt_dirs, BootstrapMode expected_mode) { { std::lock_guard lock(callback_mutex); did_client_reset = false; bootstrap_msg_count = 0; download_msg_count = 0; role_change_bootstrap = false; - query_version = realm->get_active_subscription_set().version(); + query_version = check_realm->get_active_subscription_set().version(); } // Reset the state machine - state_machina.transition_to(TestState::start); - - rule["roles"][0]["document_filters"]["read"] = doc_filter.value_or(true); - rule["roles"][0]["document_filters"]["write"] = doc_filter.value_or(true); + state_machina.transition_with([&](TestState cur_state) { + REQUIRE(cur_state == TestState::not_ready); + return TestState::start; + }); // Update the permissions on the server - should send an error to the client to force // it to reconnect - app_session.admin_api.update_default_rule(app_session.server_app_id, rule); + logger->trace("New rule definitions: %1", new_rules); + app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); // After updating the permissions (if they are different), the server should send an // error that will disconnect/reconnect the session - verify the reconnect occurs. @@ -5178,7 +5173,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Assuming the session disconnects and reconnects, the server initiated role change // bootstrap download will take place when the session is re-established and will // complete before the server sends the initial MARK response. - REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_download(*check_realm)); // The tricky part here is that a single server initiated bootstrap message // with one changeset will be treated as a typical download message and the @@ -5186,64 +5181,74 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // the local data via the permissions should produce a trackable bootstrap // with multiple messages which can be verified. - bool multi_msg = false; { + // By the time the MARK response is received and wait_for_download() + // returns, the bootstrap should have already been applied. std::lock_guard lock(callback_mutex); - if (msg_count > 1) { - multi_msg = true; - // verify a role change server initiated bootstrap was performed if it - // contained more than one bootstrap message + if (expected_mode == BootstrapMode::SingleMessageMulti || expected_mode == BootstrapMode::MultiMessage) { + REQUIRE(expected_mode == bootstrap_mode); REQUIRE(role_change_bootstrap); + REQUIRE(state_machina.get() == TestState::complete); + if (expected_mode == BootstrapMode::SingleMessageMulti) { + REQUIRE(bootstrap_msg_count == 1); + } + else if (expected_mode == BootstrapMode::MultiMessage) { + REQUIRE(bootstrap_msg_count > 1); + } + } + else if (expected_mode == BootstrapMode::SingleMessage) { + REQUIRE(bootstrap_mode == BootstrapMode::None); + REQUIRE(bootstrap_msg_count == 0); + REQUIRE(state_machina.get() == TestState::reconnect_received); } // Make sure a client reset did not occur while waiting for the role change to // be applied - CHECK_FALSE(did_client_reset); + REQUIRE_FALSE(did_client_reset); } - // If a multi-message bootstrap was received, verify it was applied (it should be - // applied when the last download message of the bootstrap is received). By the - // time the MARK response is received and wait_for_download() returns, the bootstrap - // should have already been applied. - REQUIRE((!multi_msg || state_machina.get() == TestState::complete)); - // Wait for the upload to complete and the updated data to be ready in the local realm. - REQUIRE(!wait_for_upload(*realm)); - wait_for_advance(*realm); - { - // Validate the expected number of entries for each role type after the role change - auto table = realm->read_group().get_table("class_Person"); - REQUIRE(table->size() == (cnt_emps + cnt_mgrs + cnt_dirs)); - auto role_col = table->get_column_key("role"); - auto table_query = Query(table).equal(role_col, "director").Or().equal(role_col, "manager"); - auto results = Results(realm, table_query); - CHECK(results.size() == (cnt_mgrs + cnt_dirs)); - table_query = Query(table).equal(role_col, "employee"); - results = Results(realm, table_query); - CHECK(results.size() == cnt_emps); - table_query = Query(table).equal(role_col, "manager"); - results = Results(realm, table_query); - CHECK(results.size() == cnt_mgrs); - table_query = Query(table).equal(role_col, "director"); - results = Results(realm, table_query); - CHECK(results.size() == cnt_dirs); - } + REQUIRE(!wait_for_upload(*check_realm)); + wait_for_advance(*check_realm); + + // Validate the expected number of entries for each role type after the role change + verify_records(check_realm, cnt_emps, cnt_mgrs, cnt_dirs); + // Reset the state machine to "not ready" before leaving state_machina.transition_to(TestState::not_ready); }; - { - // SingleMessage, SingleMessageMulti, MultiMessage - // Single message bootstrap - remove employees, keep mgrs/dirs - nlohmann::json doc_filter = {{"role", {{"$in", {"manager", "director"}}}}}; - update_perms_and_verify(doc_filter, 0, num_mgrs, num_dirs, BootstrapMode::SingleMessage); - } - { - // Multi-message bootstrap - add employeees, remove managers and directors - nlohmann::json doc_filter = {{"role", "employee"}}; - update_perms_and_verify(doc_filter, num_emps, 0, 0, BootstrapMode::MultiMessage); - } - { - // Single message bootstrap - add back managers and directors - update_perms_and_verify(std::nullopt, num_emps, num_mgrs, num_dirs, , BootstrapMode::SingleMessageMulti); + + SECTION("Role changes lead to objects in/out of view without client reset") { + // Get the current rules so it can be updated during the test + auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); + + auto update_role = [&rule](nlohmann::json doc_filter) { + rule["roles"][0]["document_filters"]["read"] = doc_filter; + rule["roles"][0]["document_filters"]["write"] = doc_filter; + }; + + auto realm = Realm::get_shared_realm(config); + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + set_up_realm(realm, num_total); + + { + // Single message bootstrap - remove employees, keep mgrs/dirs + logger->info(">>>>> REMOVING EMPLOYEES"); + update_role({{"role", {{"$in", {"manager", "director"}}}}}); + update_perms_and_verify(rule, realm, 0, num_mgrs, num_dirs, BootstrapMode::SingleMessage); + } + { + // Multi-message bootstrap - add employeees, remove managers and directors + logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); + update_role({{"role", "employee"}}); + update_perms_and_verify(rule, realm, num_emps, 0, 0, BootstrapMode::SingleMessageMulti); + } + { + // Single message bootstrap - add back managers and directors + logger->info(">>>>> SWITCHING BACK TO ALL"); + update_role(true); + update_perms_and_verify(rule, realm, num_emps, num_mgrs, num_dirs, BootstrapMode::SingleMessageMulti); + } } } diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index ab4eb57ba96..b8a3b0b9b0b 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -704,14 +704,23 @@ app::Response AdminAPIEndpoint::get(const std::vector= 200 && resp.http_status_code < 300, m_url, body.dump(), + resp.http_status_code, resp.body); + return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); +} + nlohmann::json AdminAPIEndpoint::get_json(const std::vector>& params) const { auto resp = get(params); @@ -1426,6 +1435,23 @@ AppCreateConfig minimal_app_config(const std::string& name, const Schema& schema }; } +nlohmann::json transform_service_role(const AppCreateConfig::ServiceRole& role_def) +{ + return { + {"name", role_def.name}, + {"apply_when", role_def.apply_when}, + {"document_filters", + { + {"read", role_def.document_filters.read}, + {"write", role_def.document_filters.write}, + }}, + {"insert", role_def.insert_filter}, + {"delete", role_def.delete_filter}, + {"read", role_def.read}, + {"write", role_def.write}, + }; +} + AppSession create_app(const AppCreateConfig& config) { auto session = AdminAPISession::login(config); @@ -1564,36 +1590,11 @@ AppSession create_app(const AppCreateConfig& config) auto default_rule = services[mongo_service_id]["default_rule"]; auto service_roles = nlohmann::json::array(); if (config.service_roles.empty()) { - service_roles = nlohmann::json::array({{{"name", "default"}, - {"apply_when", nlohmann::json::object()}, - {"document_filters", - { - {"read", true}, - {"write", true}, - }}, - {"read", true}, - {"write", true}, - {"insert", true}, - {"delete", true}}}); + service_roles.push_back(transform_service_role({"default"})); } else { std::transform(config.service_roles.begin(), config.service_roles.end(), std::back_inserter(service_roles), - [](const AppCreateConfig::ServiceRole& role_def) { - nlohmann::json ret{ - {"name", role_def.name}, - {"apply_when", role_def.apply_when}, - {"document_filters", - { - {"read", role_def.document_filters.read}, - {"write", role_def.document_filters.write}, - }}, - {"insert", role_def.insert_filter}, - {"delete", role_def.delete_filter}, - {"read", role_def.read}, - {"write", role_def.write}, - }; - return ret; - }); + transform_service_role); } default_rule.post_json({{"roles", service_roles}}); diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index 85b20306bea..8564f19afca 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -43,11 +43,12 @@ class AdminAPIEndpoint { app::Response patch(std::string body) const; app::Response post(std::string body) const; app::Response put(std::string body) const; - app::Response del() const; + app::Response del(std::string body = {}) const; nlohmann::json get_json(const std::vector>& params = {}) const; nlohmann::json patch_json(nlohmann::json body) const; nlohmann::json post_json(nlohmann::json body) const; nlohmann::json put_json(nlohmann::json body) const; + nlohmann::json del_json(nlohmann::json body) const; AdminAPIEndpoint operator[](StringData name) const; @@ -146,6 +147,7 @@ class AdminAPISession { MigrationStatus get_migration_status(const std::string& app_id) const; nlohmann::json get_app_settings(const std::string& app_id) const; bool patch_app_settings(const std::string& app_id, nlohmann::json&& new_settings) const; + bool update_app_settings(const std::string& app_id, nlohmann::json&& new_settings) const; const std::string& admin_url() const noexcept { @@ -205,17 +207,17 @@ struct AppCreateConfig { // document_filters describe which objects can be read from/written to, as // specified by the below read and write expressions. Set both to true to give read/write // access on all objects - ServiceRoleDocumentFilters document_filters; + ServiceRoleDocumentFilters document_filters = {true, true}; // insert_filter and delete_filter describe which objects can be created and erased by the client, // respectively. Set both to true if all objects can be created/erased by the client - nlohmann::json insert_filter; - nlohmann::json delete_filter; + nlohmann::json insert_filter = true; + nlohmann::json delete_filter = true; // read and write describe the permissions for "read-all-fields"/"write-all-fields" behavior. Set both to true // if all fields should have read/write access - nlohmann::json read; - nlohmann::json write; + nlohmann::json read = true; + nlohmann::json write = true; // NB: for more granular field-level permissions, the "fields" and "additional_fields" keys can be included in // a service role to describe which fields individually can be read/written. These fields have been omitted @@ -256,6 +258,7 @@ struct AppCreateConfig { realm::Schema get_default_schema(); AppCreateConfig default_app_config(); AppCreateConfig minimal_app_config(const std::string& name, const Schema& schema); +nlohmann::json transform_service_role(const AppCreateConfig::ServiceRole& role_def); struct AppSession { std::string client_app_id; From c7a4739d9d7ba5eedcadc030862f167f108fb0ec Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 16 May 2024 23:12:47 -0400 Subject: [PATCH 16/37] Refactored tests to validate different bootstrap types --- test/object-store/sync/flx_sync.cpp | 305 ++++++++++++++++------------ 1 file changed, 175 insertions(+), 130 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 73cc6bd5c22..c756ef1c956 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4968,15 +4968,14 @@ TEST_CASE("flx: nested collections in mixed", "[sync][flx][baas]") { TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { const Schema person_schema{{"Person", {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, - {"age", PropertyType::Int | PropertyType::Nullable}, {"role", PropertyType::String}, - {"firstName", PropertyType::String}, - {"lastName", PropertyType::String}}}}; + {"firstName", PropertyType::String}}}}; - constexpr size_t num_emps = 5000; - constexpr size_t num_mgrs = 200; - constexpr size_t num_dirs = 25; + constexpr size_t num_emps = 1000; + constexpr size_t num_mgrs = 50; + constexpr size_t num_dirs = 10; constexpr size_t num_total = num_emps + num_mgrs + num_dirs; + auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { CppContext c(realm); for (size_t i = 0; i < count; ++i) { @@ -4987,16 +4986,22 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto obj = Object::create(c, realm, "Person", std::any(AnyDict{ {"_id", ObjectId::gen()}, - {"age", static_cast(i)}, {"role", role}, {"firstName", util::format("%1-%2", role, i)}, - {"lastName", util::format("last-name-%1", i)}, })); } }; enum BootstrapMode { None, SingleMessage, SingleMessageMulti, MultiMessage }; enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; + + struct ExpectedResults { + BootstrapMode bootstrap; + size_t emps; + size_t mgrs; + size_t dirs; + }; + TestingStateMachine state_machina(TestState::not_ready); std::mutex callback_mutex; int64_t query_version = 0; @@ -5005,106 +5010,107 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t download_msg_count = 0; size_t bootstrap_msg_count = 0; bool role_change_bootstrap = false; - auto logger = util::Logger::get_default_logger(); - FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); - auto& app_session = harness.session().app_session(); - /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ - // Enable the role change bootstraps - REQUIRE(app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); - REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); - auto app_settings = app_session.admin_api.get_app_settings(app_session.server_app_id); - std::cout << "APP SETTINGS: " << app_settings << std::endl; - - REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - {{"sync", {{"num_objects_before_bootstrap_flush", 200}}}})); - REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - {{"sync", {{"download_loop_sleep_millis", 1500}}}})); - // REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - // {{"sync", {{"num_objects_before_bootstrap_flush", 0}}}})); - // REQUIRE(app_session.admin_api.patch_app_settings(app_session.server_app_id, - // {{"sync", {{"download_loop_sleep_millis", 0}}}})); - - // Get the current rules so it can be updated during the test - auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); - - // Initialize the realm with some data - harness.load_initial_data([&fill_person_schema](SharedRealm realm) { - fill_person_schema(realm, "employee", num_emps); - fill_person_schema(realm, "manager", num_mgrs); - fill_person_schema(realm, "director", num_dirs); - }); - auto config = harness.make_test_file(); + auto setup_harness = [&](FLXSyncTestHarness& harness, std::optional num_objects = std::nullopt, + std::optional sleep_millis = std::nullopt) { + auto& app_session = harness.session().app_session(); + /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ + // Enable the role change bootstraps + REQUIRE( + app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); + REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); + + if (num_objects) { + REQUIRE(app_session.admin_api.patch_app_settings( + app_session.server_app_id, {{"sync", {{"num_objects_before_bootstrap_flush", *num_objects}}}})); + } + if (sleep_millis) { + REQUIRE(app_session.admin_api.patch_app_settings( + app_session.server_app_id, {{"sync", {{"download_loop_sleep_millis", *sleep_millis}}}})); + } - config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, const SyncClientHookData& data) { - state_machina.transition_with([&](TestState cur_state) -> std::optional { - if (cur_state == TestState::not_ready) - return std::nullopt; + // Initialize the realm with some data + harness.load_initial_data([&fill_person_schema](SharedRealm realm) { + fill_person_schema(realm, "employee", num_emps); + fill_person_schema(realm, "manager", num_mgrs); + fill_person_schema(realm, "director", num_dirs); + }); + }; - using BatchState = sync::DownloadBatchState; - using Event = SyncClientHookEvent; - switch (data.event) { - case Event::ErrorMessageReceived: - REQUIRE(cur_state == TestState::start); - REQUIRE(data.error_info); - REQUIRE(data.error_info->raw_error_code == 200); - REQUIRE(data.error_info->server_requests_action == sync::ProtocolErrorInfo::Action::Transient); - REQUIRE_FALSE(data.error_info->is_fatal); - return TestState::reconnect_received; - - case Event::DownloadMessageReceived: { - std::lock_guard lock(callback_mutex); - // multi-message bootstrap in progress.. - if (data.batch_state == BatchState::MoreToCome) { - // More than 1 bootstrap message, always a multi-message - ++download_msg_count; - bootstrap_mode = BootstrapMode::MultiMessage; - return TestState::downloading; - } - // single bootstrap message or last message in the multi-message bootstrap - else if (data.batch_state == BatchState::LastInBatch) { - ++download_msg_count; - if (cur_state == TestState::reconnect_received) { - // If reconnect error was last state, then bootstrap has only one message - bootstrap_mode = BootstrapMode::SingleMessageMulti; - // Must have 2 or more changesets in message to get here... - REQUIRE(data.num_changesets > 1); - } - return TestState::downloaded; - } - // single message/single changeset bootstraps are treated as a steady state download + auto setup_config_callbacks = [&](SyncTestFile& config) { + config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, + const SyncClientHookData& data) { + state_machina.transition_with([&](TestState cur_state) -> std::optional { + if (cur_state == TestState::not_ready) return std::nullopt; - } - // A bootstrap message was processed - used for multi-message or - // single message multi changeset bootstraps - case Event::BootstrapMessageProcessed: { - REQUIRE(data.batch_state != BatchState::SteadyState); - REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); - if (data.batch_state != BatchState::SteadyState) { + using BatchState = sync::DownloadBatchState; + using Event = SyncClientHookEvent; + switch (data.event) { + case Event::ErrorMessageReceived: + REQUIRE(cur_state == TestState::start); + REQUIRE(data.error_info); + REQUIRE(data.error_info->raw_error_code == 200); + REQUIRE(data.error_info->server_requests_action == + sync::ProtocolErrorInfo::Action::Transient); + REQUIRE_FALSE(data.error_info->is_fatal); + return TestState::reconnect_received; + + case Event::DownloadMessageReceived: { std::lock_guard lock(callback_mutex); - ++bootstrap_msg_count; - if (data.query_version == query_version) { - role_change_bootstrap = true; + // multi-message bootstrap in progress.. + if (data.batch_state == BatchState::MoreToCome) { + // More than 1 bootstrap message, always a multi-message + ++download_msg_count; + bootstrap_mode = BootstrapMode::MultiMessage; + return TestState::downloading; } + // single bootstrap message or last message in the multi-message bootstrap + else if (data.batch_state == BatchState::LastInBatch) { + ++download_msg_count; + if (cur_state == TestState::reconnect_received) { + // If reconnect error was last state, then bootstrap has only one message + bootstrap_mode = BootstrapMode::SingleMessageMulti; + // Must have 2 or more changesets in message to get here... + REQUIRE(data.num_changesets > 1); + } + return TestState::downloaded; + } + // single message/single changeset bootstraps are treated as a steady state download + return std::nullopt; } - return std::nullopt; + + // A bootstrap message was processed - used for multi-message or + // single message multi changeset bootstraps + case Event::BootstrapMessageProcessed: { + REQUIRE(data.batch_state != BatchState::SteadyState); + REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); + if (data.batch_state != BatchState::SteadyState) { + std::lock_guard lock(callback_mutex); + ++bootstrap_msg_count; + if (data.query_version == query_version) { + role_change_bootstrap = true; + } + } + return std::nullopt; + } + // The bootstrap has been received and processed + case Event::BootstrapProcessed: + REQUIRE(cur_state == TestState::downloaded); + return TestState::complete; + default: + return std::nullopt; } - // The bootstrap has been received and processed - case Event::BootstrapProcessed: - REQUIRE(cur_state == TestState::downloaded); - return TestState::complete; - default: - return std::nullopt; - } - }); - return SyncClientHookAction::NoAction; - }; + }); + return SyncClientHookAction::NoAction; + }; - config.sync_config->notify_before_client_reset = [&callback_mutex, &did_client_reset](std::shared_ptr) { - std::lock_guard lock(callback_mutex); - did_client_reset = true; + config.sync_config->notify_before_client_reset = [&callback_mutex, + &did_client_reset](std::shared_ptr) { + std::lock_guard lock(callback_mutex); + did_client_reset = true; + }; }; auto set_up_realm = [](SharedRealm realm, size_t expected_cnt) { @@ -5126,27 +5132,27 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(results.size() == expected_cnt); }; - auto verify_records = [](SharedRealm check_realm, size_t cnt_emps, size_t cnt_mgrs, size_t cnt_dirs) { + auto verify_records = [](SharedRealm check_realm, ExpectedResults& expected) { // Validate the expected number of entries for each role type after the role change auto table = check_realm->read_group().get_table("class_Person"); - REQUIRE(table->size() == (cnt_emps + cnt_mgrs + cnt_dirs)); + REQUIRE(table->size() == (expected.emps + expected.mgrs + expected.dirs)); auto role_col = table->get_column_key("role"); auto table_query = Query(table).equal(role_col, "director").Or().equal(role_col, "manager"); auto results = Results(check_realm, table_query); - CHECK(results.size() == (cnt_mgrs + cnt_dirs)); + CHECK(results.size() == (expected.mgrs + expected.dirs)); table_query = Query(table).equal(role_col, "employee"); results = Results(check_realm, table_query); - CHECK(results.size() == cnt_emps); + CHECK(results.size() == expected.emps); table_query = Query(table).equal(role_col, "manager"); results = Results(check_realm, table_query); - CHECK(results.size() == cnt_mgrs); + CHECK(results.size() == expected.mgrs); table_query = Query(table).equal(role_col, "director"); results = Results(check_realm, table_query); - CHECK(results.size() == cnt_dirs); + CHECK(results.size() == expected.dirs); }; - auto update_perms_and_verify = [&](nlohmann::json new_rules, SharedRealm check_realm, size_t cnt_emps, - size_t cnt_mgrs, size_t cnt_dirs, BootstrapMode expected_mode) { + auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, nlohmann::json new_rules, + ExpectedResults& expected) { { std::lock_guard lock(callback_mutex); did_client_reset = false; @@ -5163,7 +5169,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Update the permissions on the server - should send an error to the client to force // it to reconnect - logger->trace("New rule definitions: %1", new_rules); + logger->info("New rule definitions: %1", new_rules); + auto& app_session = harness.session().app_session(); app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); // After updating the permissions (if they are different), the server should send an @@ -5185,18 +5192,19 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // By the time the MARK response is received and wait_for_download() // returns, the bootstrap should have already been applied. std::lock_guard lock(callback_mutex); - if (expected_mode == BootstrapMode::SingleMessageMulti || expected_mode == BootstrapMode::MultiMessage) { - REQUIRE(expected_mode == bootstrap_mode); + if (expected.bootstrap == BootstrapMode::SingleMessageMulti || + expected.bootstrap == BootstrapMode::MultiMessage) { + REQUIRE(expected.bootstrap == bootstrap_mode); REQUIRE(role_change_bootstrap); REQUIRE(state_machina.get() == TestState::complete); - if (expected_mode == BootstrapMode::SingleMessageMulti) { + if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { REQUIRE(bootstrap_msg_count == 1); } - else if (expected_mode == BootstrapMode::MultiMessage) { + else if (expected.bootstrap == BootstrapMode::MultiMessage) { REQUIRE(bootstrap_msg_count > 1); } } - else if (expected_mode == BootstrapMode::SingleMessage) { + else if (expected.bootstrap == BootstrapMode::SingleMessage) { REQUIRE(bootstrap_mode == BootstrapMode::None); REQUIRE(bootstrap_msg_count == 0); REQUIRE(state_machina.get() == TestState::reconnect_received); @@ -5211,44 +5219,81 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra wait_for_advance(*check_realm); // Validate the expected number of entries for each role type after the role change - verify_records(check_realm, cnt_emps, cnt_mgrs, cnt_dirs); + verify_records(check_realm, expected); // Reset the state machine to "not ready" before leaving state_machina.transition_to(TestState::not_ready); }; SECTION("Role changes lead to objects in/out of view without client reset") { - // Get the current rules so it can be updated during the test - auto rule = app_session.admin_api.get_default_rule(app_session.server_app_id); - - auto update_role = [&rule](nlohmann::json doc_filter) { + auto update_role = [](nlohmann::json& rule, nlohmann::json doc_filter) { rule["roles"][0]["document_filters"]["read"] = doc_filter; rule["roles"][0]["document_filters"]["write"] = doc_filter; }; - auto realm = Realm::get_shared_realm(config); - REQUIRE(!wait_for_download(*realm)); - REQUIRE(!wait_for_upload(*realm)); - set_up_realm(realm, num_total); + auto run_test = [&](nlohmann::json initial_rules, size_t intitial_cnt, nlohmann::json test_rules, + ExpectedResults expected_results, std::optional num_objects = std::nullopt, + std::optional sleep_millis = std::nullopt) { + // Set up the test harness with the provided initial rules + FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName"}}); + setup_harness(harness, num_objects, sleep_millis); + + // Get the current rules so it can be updated during the test + auto& app_session = harness.session().app_session(); + auto default_rule = app_session.admin_api.get_default_rule(app_session.server_app_id); + if (!initial_rules.empty()) { + update_role(default_rule, initial_rules); + logger->info("Initial rule definitions: %1", default_rule); + app_session.admin_api.update_default_rule(app_session.server_app_id, default_rule); + } - { + // Set up and create a realm; wait for data sync + auto config = harness.make_test_file(); + setup_config_callbacks(config); + auto realm = Realm::get_shared_realm(config); + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + set_up_realm(realm, intitial_cnt); + + // Single message bootstrap - remove employees, keep mgrs/dirs + update_role(default_rule, test_rules); + update_perms_and_verify(harness, realm, default_rule, expected_results); + }; + + SECTION("Single-message bootstrap") { // Single message bootstrap - remove employees, keep mgrs/dirs logger->info(">>>>> REMOVING EMPLOYEES"); - update_role({{"role", {{"$in", {"manager", "director"}}}}}); - update_perms_and_verify(rule, realm, 0, num_mgrs, num_dirs, BootstrapMode::SingleMessage); + run_test({}, num_total, {{"role", {{"$in", {"manager", "director"}}}}}, + {BootstrapMode::SingleMessage, 0, num_mgrs, num_dirs}); } - { + SECTION("Multi-message bootstrap") { // Multi-message bootstrap - add employeees, remove managers and directors logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); - update_role({{"role", "employee"}}); - update_perms_and_verify(rule, realm, num_emps, 0, 0, BootstrapMode::SingleMessageMulti); + run_test({{"role", {{"$in", {"manager", "director"}}}}}, num_mgrs + num_dirs, {{"role", "employee"}}, + {BootstrapMode::MultiMessage, num_emps, 0, 0}, 10); } - { - // Single message bootstrap - add back managers and directors + SECTION("Single-message/Multi-changeset bootstrap") { + // Single message/multi-changeset bootstrap - add back managers and directors logger->info(">>>>> SWITCHING BACK TO ALL"); - update_role(true); - update_perms_and_verify(rule, realm, num_emps, num_mgrs, num_dirs, BootstrapMode::SingleMessageMulti); + run_test({{"role", "employee"}}, num_emps, true, + {BootstrapMode::SingleMessageMulti, num_emps, num_mgrs, num_dirs}, 10, 2000); } + /* + { + // Multi-message bootstrap - add employeees, remove managers and directors + logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); + update_role(default_rule, {{"role", "employee"}}); + expected_results = {BootstrapMode::MultiMessage, num_emps, 0, 0}; + update_perms_and_verify(harness, realm, default_rule, expected_results); + } + { + // Single message bootstrap - add back managers and directors + logger->info(">>>>> SWITCHING BACK TO ALL"); + update_role(default_rule, true); + expected_results = {BootstrapMode::SingleMessageMulti, num_emps, num_mgrs, num_dirs}; + update_perms_and_verify(harness, realm, default_rule, expected_results); + } + */ } } From 509c97b98538b84fb8a5373b75dfe97d66ffea87 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 17 May 2024 11:32:38 -0400 Subject: [PATCH 17/37] Address test failures --- test/object-store/sync/flx_sync.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index b8504f2165b..5802a6574c7 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4969,7 +4969,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra const Schema person_schema{{"Person", {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, {"role", PropertyType::String}, - {"firstName", PropertyType::String}}}}; + {"firstName", PropertyType::String}, + {"lastName", PropertyType::String}}}}; constexpr size_t num_emps = 1000; constexpr size_t num_mgrs = 50; @@ -4988,6 +4989,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra {"_id", ObjectId::gen()}, {"role", role}, {"firstName", util::format("%1-%2", role, i)}, + {"lastName", util::format("last-name-%1", i)}, })); } }; @@ -5031,7 +5033,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } // Initialize the realm with some data - harness.load_initial_data([&fill_person_schema](SharedRealm realm) { + harness.load_initial_data([&](SharedRealm realm) { fill_person_schema(realm, "employee", num_emps); fill_person_schema(realm, "manager", num_mgrs); fill_person_schema(realm, "director", num_dirs); @@ -5191,12 +5193,13 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra { // By the time the MARK response is received and wait_for_download() // returns, the bootstrap should have already been applied. + auto cur_state = state_machina.get(); // grab before locking callback_mutex std::lock_guard lock(callback_mutex); if (expected.bootstrap == BootstrapMode::SingleMessageMulti || expected.bootstrap == BootstrapMode::MultiMessage) { REQUIRE(expected.bootstrap == bootstrap_mode); REQUIRE(role_change_bootstrap); - REQUIRE(state_machina.get() == TestState::complete); + REQUIRE(cur_state == TestState::complete); if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { REQUIRE(bootstrap_msg_count == 1); } @@ -5207,7 +5210,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra else if (expected.bootstrap == BootstrapMode::SingleMessage) { REQUIRE(bootstrap_mode == BootstrapMode::None); REQUIRE(bootstrap_msg_count == 0); - REQUIRE(state_machina.get() == TestState::reconnect_received); + REQUIRE(cur_state == TestState::reconnect_received); } // Make sure a client reset did not occur while waiting for the role change to // be applied @@ -5235,7 +5238,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra ExpectedResults expected_results, std::optional num_objects = std::nullopt, std::optional sleep_millis = std::nullopt) { // Set up the test harness with the provided initial rules - FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName"}}); + FLXSyncTestHarness harness("flx_role_change_bootstrap", + {person_schema, {"role", "firstName", "lastName"}}); setup_harness(harness, num_objects, sleep_millis); // Get the current rules so it can be updated during the test From 8a0044766a1f132caa55bcdf733b0ef79be9441e Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 20 May 2024 11:48:32 -0400 Subject: [PATCH 18/37] Updated tests to get passing using the server params --- test/object-store/sync/flx_sync.cpp | 80 +++++++++++++---------------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 5802a6574c7..ea07734fecd 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4972,11 +4972,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra {"firstName", PropertyType::String}, {"lastName", PropertyType::String}}}}; - constexpr size_t num_emps = 1000; - constexpr size_t num_mgrs = 50; - constexpr size_t num_dirs = 10; - constexpr size_t num_total = num_emps + num_mgrs + num_dirs; - auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { CppContext c(realm); for (size_t i = 0; i < count; ++i) { @@ -5004,6 +4999,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t dirs; }; + struct TestParams { + size_t num_emps = 500; + size_t num_mgrs = 10; + size_t num_dirs = 5; + std::optional num_objects = std::nullopt; + std::optional sleep_millis = std::nullopt; + }; + TestingStateMachine state_machina(TestState::not_ready); std::mutex callback_mutex; int64_t query_version = 0; @@ -5014,8 +5017,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra bool role_change_bootstrap = false; auto logger = util::Logger::get_default_logger(); - auto setup_harness = [&](FLXSyncTestHarness& harness, std::optional num_objects = std::nullopt, - std::optional sleep_millis = std::nullopt) { + auto setup_harness = [&](FLXSyncTestHarness& harness, TestParams params) { auto& app_session = harness.session().app_session(); /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ // Enable the role change bootstraps @@ -5023,20 +5025,21 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); REQUIRE(app_session.admin_api.get_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap")); - if (num_objects) { + if (params.num_objects) { REQUIRE(app_session.admin_api.patch_app_settings( - app_session.server_app_id, {{"sync", {{"num_objects_before_bootstrap_flush", *num_objects}}}})); + app_session.server_app_id, + {{"sync", {{"num_objects_before_bootstrap_flush", *params.num_objects}}}})); } - if (sleep_millis) { + if (params.sleep_millis) { REQUIRE(app_session.admin_api.patch_app_settings( - app_session.server_app_id, {{"sync", {{"download_loop_sleep_millis", *sleep_millis}}}})); + app_session.server_app_id, {{"sync", {{"download_loop_sleep_millis", *params.sleep_millis}}}})); } // Initialize the realm with some data harness.load_initial_data([&](SharedRealm realm) { - fill_person_schema(realm, "employee", num_emps); - fill_person_schema(realm, "manager", num_mgrs); - fill_person_schema(realm, "director", num_dirs); + fill_person_schema(realm, "employee", params.num_emps); + fill_person_schema(realm, "manager", params.num_mgrs); + fill_person_schema(realm, "director", params.num_dirs); }); }; @@ -5139,11 +5142,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto table = check_realm->read_group().get_table("class_Person"); REQUIRE(table->size() == (expected.emps + expected.mgrs + expected.dirs)); auto role_col = table->get_column_key("role"); - auto table_query = Query(table).equal(role_col, "director").Or().equal(role_col, "manager"); + auto table_query = Query(table).equal(role_col, "employee"); auto results = Results(check_realm, table_query); - CHECK(results.size() == (expected.mgrs + expected.dirs)); - table_query = Query(table).equal(role_col, "employee"); - results = Results(check_realm, table_query); CHECK(results.size() == expected.emps); table_query = Query(table).equal(role_col, "manager"); results = Results(check_realm, table_query); @@ -5234,13 +5234,12 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra rule["roles"][0]["document_filters"]["write"] = doc_filter; }; - auto run_test = [&](nlohmann::json initial_rules, size_t intitial_cnt, nlohmann::json test_rules, - ExpectedResults expected_results, std::optional num_objects = std::nullopt, - std::optional sleep_millis = std::nullopt) { + auto run_test = [&](TestParams params, nlohmann::json initial_rules, size_t initial_count, + nlohmann::json test_rules, ExpectedResults expected_results) { // Set up the test harness with the provided initial rules FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); - setup_harness(harness, num_objects, sleep_millis); + setup_harness(harness, params); // Get the current rules so it can be updated during the test auto& app_session = harness.session().app_session(); @@ -5257,7 +5256,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto realm = Realm::get_shared_realm(config); REQUIRE(!wait_for_download(*realm)); REQUIRE(!wait_for_upload(*realm)); - set_up_realm(realm, intitial_cnt); + set_up_realm(realm, initial_count); // Single message bootstrap - remove employees, keep mgrs/dirs update_role(default_rule, test_rules); @@ -5267,37 +5266,28 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra SECTION("Single-message bootstrap") { // Single message bootstrap - remove employees, keep mgrs/dirs logger->info(">>>>> REMOVING EMPLOYEES"); - run_test({}, num_total, {{"role", {{"$in", {"manager", "director"}}}}}, - {BootstrapMode::SingleMessage, 0, num_mgrs, num_dirs}); + // 500 emps, 10 mgrs, 5 dirs + TestParams params{}; + auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; + run_test(params, {}, num_total, {{"role", {{"$in", {"manager", "director"}}}}}, + {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); } SECTION("Multi-message bootstrap") { // Multi-message bootstrap - add employeees, remove managers and directors logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); - run_test({{"role", {{"$in", {"manager", "director"}}}}}, num_mgrs + num_dirs, {{"role", "employee"}}, - {BootstrapMode::MultiMessage, num_emps, 0, 0}, 10); + // 5000 emps, 100 mgrs, 25 dirs + TestParams params{5000, 100, 25, 10}; + run_test(params, {{"role", {{"$in", {"manager", "director"}}}}}, params.num_mgrs + params.num_dirs, + {{"role", "employee"}}, {BootstrapMode::MultiMessage, params.num_emps, 0, 0}); } SECTION("Single-message/Multi-changeset bootstrap") { // Single message/multi-changeset bootstrap - add back managers and directors logger->info(">>>>> SWITCHING BACK TO ALL"); - run_test({{"role", "employee"}}, num_emps, true, - {BootstrapMode::SingleMessageMulti, num_emps, num_mgrs, num_dirs}, 10, 2000); + // 500 emps, 500 mgrs, 125 dirs + TestParams params{500, 500, 125, 100, 2000}; + run_test(params, {{"role", "employee"}}, params.num_emps, true, + {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, params.num_dirs}); } - /* - { - // Multi-message bootstrap - add employeees, remove managers and directors - logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); - update_role(default_rule, {{"role", "employee"}}); - expected_results = {BootstrapMode::MultiMessage, num_emps, 0, 0}; - update_perms_and_verify(harness, realm, default_rule, expected_results); - } - { - // Single message bootstrap - add back managers and directors - logger->info(">>>>> SWITCHING BACK TO ALL"); - update_role(default_rule, true); - expected_results = {BootstrapMode::SingleMessageMulti, num_emps, num_mgrs, num_dirs}; - update_perms_and_verify(harness, realm, default_rule, expected_results); - } - */ } } From b7b71805025c20c7b6ca167012f69b89137269bb Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 21 May 2024 10:05:08 -0400 Subject: [PATCH 19/37] Updated debug statements role change tests --- test/object-store/sync/flx_sync.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index ea07734fecd..50e3213cbfe 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5069,6 +5069,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // More than 1 bootstrap message, always a multi-message ++download_msg_count; bootstrap_mode = BootstrapMode::MultiMessage; + logger->debug("ROLE CHANGE: detected multi-message bootstrap"); return TestState::downloading; } // single bootstrap message or last message in the multi-message bootstrap @@ -5077,6 +5078,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra if (cur_state == TestState::reconnect_received) { // If reconnect error was last state, then bootstrap has only one message bootstrap_mode = BootstrapMode::SingleMessageMulti; + logger->debug("ROLE CHANGE: detected single-message/multi-changeset bootstrap"); // Must have 2 or more changesets in message to get here... REQUIRE(data.num_changesets > 1); } @@ -5171,7 +5173,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Update the permissions on the server - should send an error to the client to force // it to reconnect - logger->info("New rule definitions: %1", new_rules); + logger->trace("ROLE CHANGE: New rule definitions: %1", new_rules); auto& app_session = harness.session().app_session(); app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); @@ -5208,6 +5210,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } } else if (expected.bootstrap == BootstrapMode::SingleMessage) { + logger->debug("ROLE CHANGE: expected single-message/single-changeset bootstrap"); REQUIRE(bootstrap_mode == BootstrapMode::None); REQUIRE(bootstrap_msg_count == 0); REQUIRE(cur_state == TestState::reconnect_received); @@ -5246,7 +5249,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto default_rule = app_session.admin_api.get_default_rule(app_session.server_app_id); if (!initial_rules.empty()) { update_role(default_rule, initial_rules); - logger->info("Initial rule definitions: %1", default_rule); + logger->trace("ROLE CHANGE: Initial rule definitions: %1", default_rule); app_session.admin_api.update_default_rule(app_session.server_app_id, default_rule); } @@ -5265,7 +5268,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra SECTION("Single-message bootstrap") { // Single message bootstrap - remove employees, keep mgrs/dirs - logger->info(">>>>> REMOVING EMPLOYEES"); + logger->trace("ROLE CHANGE: Updating rules to remove employees"); // 500 emps, 10 mgrs, 5 dirs TestParams params{}; auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; @@ -5274,7 +5277,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } SECTION("Multi-message bootstrap") { // Multi-message bootstrap - add employeees, remove managers and directors - logger->info(">>>>> SWITCHING TO EMPLOYEES ONLY"); + logger->trace("ROLE CHANGE: Updating rules to employees only"); // 5000 emps, 100 mgrs, 25 dirs TestParams params{5000, 100, 25, 10}; run_test(params, {{"role", {{"$in", {"manager", "director"}}}}}, params.num_mgrs + params.num_dirs, @@ -5282,7 +5285,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } SECTION("Single-message/Multi-changeset bootstrap") { // Single message/multi-changeset bootstrap - add back managers and directors - logger->info(">>>>> SWITCHING BACK TO ALL"); + logger->trace("ROLE CHANGE: Updating rules to all records"); // 500 emps, 500 mgrs, 125 dirs TestParams params{500, 500, 125, 100, 2000}; run_test(params, {{"role", "employee"}}, params.num_emps, true, From 4a4f13fab7429bf4a47c2b3e269271588e844147 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 3 Jun 2024 11:02:01 -0400 Subject: [PATCH 20/37] Updated to support new batch_state protocol changes; updated tests --- dependencies.yml | 4 +- src/realm/object-store/sync/sync_session.hpp | 2 + src/realm/sync/client.cpp | 48 +------ src/realm/sync/noinst/client_impl_base.cpp | 47 +++---- src/realm/sync/noinst/client_impl_base.hpp | 7 +- src/realm/sync/noinst/protocol_codec.hpp | 16 ++- src/realm/sync/protocol.hpp | 19 ++- .../sync/tools/apply_to_state_command.cpp | 4 +- test/object-store/sync/flx_sync.cpp | 124 ++++++++++-------- .../object-store/util/sync/baas_admin_api.cpp | 11 +- .../object-store/util/sync/baas_admin_api.hpp | 4 +- 11 files changed, 122 insertions(+), 164 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 6e3ed1e223c..e4e547ede6b 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -3,6 +3,6 @@ VERSION: 14.8.0 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits -# 9d1b4d6 is 2024 May 8 -BAAS_VERSION: 9d1b4d628babadfb606ebcadb93b1e5cae3c9565 +# fed47fa is 2024 May 31 +BAAS_VERSION: fed47fa027f2f48c6912ee848deab0c12da2c4e7 BAAS_VERSION_TYPE: githash diff --git a/src/realm/object-store/sync/sync_session.hpp b/src/realm/object-store/sync/sync_session.hpp index 02d1cb076f5..981550cb061 100644 --- a/src/realm/object-store/sync/sync_session.hpp +++ b/src/realm/object-store/sync/sync_session.hpp @@ -346,6 +346,8 @@ class SyncSession : public std::enable_shared_from_this { return session.get_appservices_connection_id(); } + // Supported commands can be found in `handleTestCommandMessage()` + // in baas/devicesync/server/qbs_client_handler_functions.go static util::Future send_test_command(SyncSession& session, std::string request) { return session.send_test_command(std::move(request)); diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 670a8853835..4e1465b4e93 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -1042,7 +1042,7 @@ SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, con return call_debug_hook(data); } -SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo& error_info) +SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo* error_info) { if (REALM_LIKELY(!m_wrapper.m_debug_hook)) { return SyncClientHookAction::NoAction; @@ -1056,54 +1056,12 @@ SyncClientHookAction SessionImpl::call_debug_hook(SyncClientHookEvent event, con data.batch_state = DownloadBatchState::SteadyState; data.progress = m_progress; data.num_changesets = 0; - data.query_version = 0; - data.error_info = &error_info; + data.query_version = m_last_sent_flx_query_version; + data.error_info = error_info; return call_debug_hook(data); } -DownloadBatchState SessionImpl::derive_download_batch_state(const DownloadMessage& message, - bool has_duplicate_changeset_versons) -{ - // Should never be called if session is not active - REALM_ASSERT_EX(m_state == State::Active, m_state); - - // PBS download messages are always steady state - if (!m_is_flx_sync_session) { - return DownloadBatchState::SteadyState; - } - - // Bootstrap messages (i.e. non-steady state) are identified by: - // * last_in_batch=false - // * last_in_batch=true, and - // * previous messages received had last_in_batch=false - // * query_version is greater than the active query_version - // * 2 or more changesets in the message have the same remote version - - // Query version should always be the same or increasing - REALM_ASSERT_3(message.query_version, >=, m_wrapper.m_flx_active_version); - // FLX download messages should always have a last_in_batch value - REALM_ASSERT_EX(message.last_in_batch, "FLX download message missing last_in_batch value"); - - // If last_in_batch=false, this is a multi-message bootstrap and there are more to come - if (!*message.last_in_batch) { - return DownloadBatchState::MoreToCome; - } - - // This download message is the last message of a bootstrap (or a single - // message bootstrap) if last_in_batch=true and one of the following applies: - // * The query_version is greater than the active query_version - // * A bootstrap is in progress (i.e. previous MoreToCome messages received) - // * Two or more changesets in the download message have the same remote version - if (message.query_version > m_wrapper.m_flx_active_version || - m_wrapper.get_flx_pending_bootstrap_store()->has_pending() || has_duplicate_changeset_versons) { - return DownloadBatchState::LastInBatch; - } - - // Otherwise, not a bootstrap message, process as a normal download message - return DownloadBatchState::SteadyState; -} - void SessionImpl::init_progress_handler() { if (m_state != State::Unactivated && m_state != State::Active) diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index e53310cb73f..a734b97d1e6 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -1729,8 +1729,7 @@ void Session::activate() reset_protocol_state(); m_state = Active; - call_debug_hook(SyncClientHookEvent::SessionActivating, m_progress, m_last_sent_flx_query_version, - DownloadBatchState::SteadyState, 0); + call_debug_hook(SyncClientHookEvent::SessionActivating); REALM_ASSERT(!m_suspended); m_conn.one_more_active_unsuspended_session(); // Throws @@ -1828,12 +1827,7 @@ void Session::send_message() if (!m_bind_message_sent) return send_bind_message(); // Throws - if (!m_ident_message_sent) { - if (have_client_file_ident()) - send_ident_message(); // Throws - return; - } - + // Pending test commands can be sent any time after the BIND message is sent const auto has_pending_test_command = std::any_of(m_pending_test_commands.begin(), m_pending_test_commands.end(), [](const PendingTestCommand& command) { return command.pending; @@ -1842,6 +1836,12 @@ void Session::send_message() return send_test_command_message(); } + if (!m_ident_message_sent) { + if (have_client_file_ident()) + send_ident_message(); // Throws + return; + } + if (m_error_to_send) return send_json_error_message(); // Throws @@ -1948,8 +1948,7 @@ void Session::send_bind_message() m_conn.initiate_write_message(out, this); // Throws m_bind_message_sent = true; - call_debug_hook(SyncClientHookEvent::BindMessageSent, m_progress, m_last_sent_flx_query_version, - DownloadBatchState::SteadyState, 0); + call_debug_hook(SyncClientHookEvent::BindMessageSent); // Ready to send the IDENT message if the file identifier pair is already // available. @@ -2394,11 +2393,11 @@ Status Session::receive_download_message(const DownloadMessage& message) logger.debug("Received: DOWNLOAD(download_server_version=%1, download_client_version=%2, " "latest_server_version=%3, latest_server_version_salt=%4, " "upload_client_version=%5, upload_server_version=%6, progress_estimate=%7, " - "last_in_batch=%8, query_version=%9, num_changesets=%10, ...)", + "batch_state=%8, query_version=%9, num_changesets=%10, ...)", progress.download.server_version, progress.download.last_integrated_client_version, progress.latest_server_version.version, progress.latest_server_version.salt, progress.upload.client_version, progress.upload.last_integrated_server_version, - message.progress_estimate, *message.last_in_batch, query_version, + message.progress_estimate, message.batch_state, query_version, message.changesets.size()); // Throws } else { @@ -2432,8 +2431,6 @@ Status Session::receive_download_message(const DownloadMessage& message) // download message must have a remote version greater than (or equal to for FLX) this value. version_type last_remote_version = m_progress.download.server_version; version_type last_integrated_client_version = m_progress.download.last_integrated_client_version; - version_type last_changeset_version = 0; // skips checking the first time through the loop - bool has_duplicate_changeset_versions = false; for (const RemoteChangeset& changeset : message.changesets) { // Check that per-changeset server version is strictly increasing since the last download server version, @@ -2448,16 +2445,8 @@ Status Session::receive_download_message(const DownloadMessage& message) changeset.remote_version, last_remote_version, progress.download.server_version)}; } - if (last_changeset_version == changeset.remote_version) { - // last_changeset_version is 0 first time through the loop to prevent comparing the - // changeset remote_version to the download_server_version, which may lead to a false - // positive when checking for changesets with duplicate remote_version values - has_duplicate_changeset_versions = true; - } - // Save the remote version for the current changeset to compare against the next changeset last_remote_version = changeset.remote_version; - last_changeset_version = changeset.remote_version; // Check that per-changeset last integrated client version is "weakly" // increasing. @@ -2483,10 +2472,8 @@ Status Session::receive_download_message(const DownloadMessage& message) } } - auto batch_state = derive_download_batch_state(message, has_duplicate_changeset_versions); - auto hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageReceived, progress, query_version, - batch_state, message.changesets.size()); + message.batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } @@ -2495,16 +2482,16 @@ Status Session::receive_download_message(const DownloadMessage& message) if (is_flx) update_download_estimate(message.progress_estimate); - if (process_flx_bootstrap_message(progress, batch_state, query_version, message.changesets)) { + if (process_flx_bootstrap_message(progress, message.batch_state, query_version, message.changesets)) { clear_resumption_delay_state(); return Status::OK(); } uint64_t downloadable_bytes = is_flx ? 0 : message.downloadable_bytes; - initiate_integrate_changesets(downloadable_bytes, batch_state, progress, message.changesets); // Throws + initiate_integrate_changesets(downloadable_bytes, message.batch_state, progress, message.changesets); // Throws hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageIntegrated, progress, query_version, - batch_state, message.changesets.size()); + message.batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } @@ -2614,7 +2601,7 @@ Status Session::receive_error_message(const ProtocolErrorInfo& info) // Can't process debug hook actions once the Session is undergoing deactivation, since // the SessionWrapper may not be available if (m_state == Active) { - auto debug_action = call_debug_hook(SyncClientHookEvent::ErrorMessageReceived, info); + auto debug_action = call_debug_hook(SyncClientHookEvent::ErrorMessageReceived, &info); if (debug_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } @@ -2674,7 +2661,7 @@ void Session::suspend(const SessionErrorInfo& info) // Notify the application of the suspension of the session if the session is // still in the Active state if (m_state == Active) { - call_debug_hook(SyncClientHookEvent::SessionSuspended, info); + call_debug_hook(SyncClientHookEvent::SessionSuspended, &info); m_conn.one_less_active_unsuspended_session(); // Throws on_suspended(info); // Throws } diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index e9006b70239..49930e79d53 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -827,7 +827,7 @@ class ClientImpl::Session { /// To be used in connection with implementations of /// initiate_integrate_changesets(). void integrate_changesets(const SyncProgress&, std::uint_fast64_t downloadable_bytes, const ReceivedChangesets&, - VersionInfo&, DownloadBatchState last_in_batch); + VersionInfo&, DownloadBatchState batch_state); /// To be used in connection with implementations of /// initiate_integrate_changesets(). @@ -1179,12 +1179,9 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const SyncProgress&, int64_t, DownloadBatchState, size_t); - SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo&); + SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo* = nullptr); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); - DownloadBatchState derive_download_batch_state(const DownloadMessage& message, - bool has_duplicate_changeset_versons); - void init_progress_handler(); void enable_progress_notifications(); void notify_upload_progress(); diff --git a/src/realm/sync/noinst/protocol_codec.hpp b/src/realm/sync/noinst/protocol_codec.hpp index 225c80cd71e..4a0a884b464 100644 --- a/src/realm/sync/noinst/protocol_codec.hpp +++ b/src/realm/sync/noinst/protocol_codec.hpp @@ -403,9 +403,8 @@ class ClientProtocol { struct DownloadMessage { SyncProgress progress; - // For FLX sync, the query_version and last_in_batch are required to be provided - std::optional query_version; - std::optional last_in_batch; + std::optional query_version; // FLX sync only + sync::DownloadBatchState batch_state = sync::DownloadBatchState::SteadyState; union { uint64_t downloadable_bytes = 0; double progress_estimate; @@ -440,10 +439,15 @@ class ClientProtocol { if (is_flx) { message.query_version = msg.read_next(); if (message.query_version < 0) - return report_error(ErrorCodes::SyncProtocolInvariantFailed, "Bad query version", + return report_error(ErrorCodes::SyncProtocolInvariantFailed, "Bad query version: %1", message.query_version); - - message.last_in_batch = msg.read_next(); + int batch_state = msg.read_next(); + if (batch_state != static_cast(sync::DownloadBatchState::MoreToCome) && + batch_state != static_cast(sync::DownloadBatchState::LastInBatch) && + batch_state != static_cast(sync::DownloadBatchState::SteadyState)) { + return report_error(ErrorCodes::SyncProtocolInvariantFailed, "Bad batch state: %1", batch_state); + } + message.batch_state = static_cast(batch_state); message.progress_estimate = msg.read_next(); if (message.progress_estimate < 0 || message.progress_estimate > 1) diff --git a/src/realm/sync/protocol.hpp b/src/realm/sync/protocol.hpp index c5d813956a1..bcd2b1707a0 100644 --- a/src/realm/sync/protocol.hpp +++ b/src/realm/sync/protocol.hpp @@ -179,9 +179,9 @@ struct DownloadCursor { }; enum class DownloadBatchState { - MoreToCome, - LastInBatch, - SteadyState, + MoreToCome = 0, + LastInBatch = 1, + SteadyState = 2, }; /// Checks that `dc.last_integrated_client_version` is zero if @@ -467,6 +467,19 @@ inline std::ostream& operator<<(std::ostream& o, ProtocolErrorInfo::Action actio return o << "Invalid error action: " << int64_t(action); } +inline std::ostream& operator<<(std::ostream& o, DownloadBatchState batch_state) +{ + switch (batch_state) { + case DownloadBatchState::MoreToCome: + return o << "MoreToCome"; + case DownloadBatchState::LastInBatch: + return o << "LastInBatch"; + case DownloadBatchState::SteadyState: + return o << "SteadyState"; + } + return o << "Invalid batch state: " << int(batch_state); +} + } // namespace sync } // namespace realm diff --git a/src/realm/sync/tools/apply_to_state_command.cpp b/src/realm/sync/tools/apply_to_state_command.cpp index 4db2c47845b..68581c956fc 100644 --- a/src/realm/sync/tools/apply_to_state_command.cpp +++ b/src/realm/sync/tools/apply_to_state_command.cpp @@ -98,9 +98,7 @@ DownloadMessage DownloadMessage::parse(HeaderLineParser& msg, Logger& logger, bo ret.progress.upload.last_integrated_server_version = msg.read_next(); if (is_flx_sync) { ret.query_version = msg.read_next(); - auto last_in_batch = msg.read_next(); - ret.batch_state = - last_in_batch ? sync::DownloadBatchState::LastInBatch : sync::DownloadBatchState::MoreToCome; + ret.batch_state = static_cast(msg.read_next()); } else { ret.query_version = 0; diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 50e3213cbfe..0cadb43a397 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5043,11 +5043,17 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }; + // auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { + // nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : + // "RESUME_DOWNLOAD_BUILDER"}}; return SyncSession::OnlyForTesting::send_test_command(session, + // test_command.dump()); + // }; + auto setup_config_callbacks = [&](SyncTestFile& config) { config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, const SyncClientHookData& data) { state_machina.transition_with([&](TestState cur_state) -> std::optional { - if (cur_state == TestState::not_ready) + if (cur_state == TestState::not_ready || cur_state == TestState::complete) return std::nullopt; using BatchState = sync::DownloadBatchState; @@ -5063,33 +5069,38 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return TestState::reconnect_received; case Event::DownloadMessageReceived: { + logger->debug("ROLE CHANGE: download message: %1", data.batch_state); + std::lock_guard lock(callback_mutex); // multi-message bootstrap in progress.. + ++download_msg_count; + if (cur_state != TestState::reconnect_received && cur_state != TestState::downloading) + return std::nullopt; + if (data.batch_state == BatchState::MoreToCome) { // More than 1 bootstrap message, always a multi-message - ++download_msg_count; bootstrap_mode = BootstrapMode::MultiMessage; - logger->debug("ROLE CHANGE: detected multi-message bootstrap"); + logger->trace("ROLE CHANGE: detected multi-message bootstrap"); return TestState::downloading; } // single bootstrap message or last message in the multi-message bootstrap else if (data.batch_state == BatchState::LastInBatch) { - ++download_msg_count; - if (cur_state == TestState::reconnect_received) { - // If reconnect error was last state, then bootstrap has only one message - bootstrap_mode = BootstrapMode::SingleMessageMulti; - logger->debug("ROLE CHANGE: detected single-message/multi-changeset bootstrap"); - // Must have 2 or more changesets in message to get here... - REQUIRE(data.num_changesets > 1); + if (download_msg_count == 1) { + if (data.num_changesets == 1) { + logger->trace("ROLE CHANGE: detected single-message/single-changeset bootstrap"); + bootstrap_mode = BootstrapMode::SingleMessage; + } + else { + logger->trace("ROLE CHANGE: detected single-message/multi-changeset bootstrap"); + bootstrap_mode = BootstrapMode::SingleMessageMulti; + } } return TestState::downloaded; } - // single message/single changeset bootstraps are treated as a steady state download return std::nullopt; } - // A bootstrap message was processed - used for multi-message or - // single message multi changeset bootstraps + // A bootstrap message was processed case Event::BootstrapMessageProcessed: { REQUIRE(data.batch_state != BatchState::SteadyState); REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); @@ -5106,6 +5117,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra case Event::BootstrapProcessed: REQUIRE(cur_state == TestState::downloaded); return TestState::complete; + default: return std::nullopt; } @@ -5113,8 +5125,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return SyncClientHookAction::NoAction; }; - config.sync_config->notify_before_client_reset = [&callback_mutex, - &did_client_reset](std::shared_ptr) { + config.sync_config->notify_before_client_reset = [&](std::shared_ptr) { std::lock_guard lock(callback_mutex); did_client_reset = true; }; @@ -5155,8 +5166,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra CHECK(results.size() == expected.dirs); }; - auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, nlohmann::json new_rules, - ExpectedResults& expected) { + auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, + nlohmann::json doc_filter, ExpectedResults& expected) { { std::lock_guard lock(callback_mutex); did_client_reset = false; @@ -5173,8 +5184,11 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Update the permissions on the server - should send an error to the client to force // it to reconnect - logger->trace("ROLE CHANGE: New rule definitions: %1", new_rules); auto& app_session = harness.session().app_session(); + auto new_rules = app_session.admin_api.get_default_rule(app_session.server_app_id); + new_rules["roles"][0]["document_filters"]["read"] = doc_filter; + new_rules["roles"][0]["document_filters"]["write"] = doc_filter; + logger->trace("ROLE CHANGE: New rule definitions: %1", new_rules); app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); // After updating the permissions (if they are different), the server should send an @@ -5197,23 +5211,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // returns, the bootstrap should have already been applied. auto cur_state = state_machina.get(); // grab before locking callback_mutex std::lock_guard lock(callback_mutex); + REQUIRE(expected.bootstrap == bootstrap_mode); + REQUIRE(role_change_bootstrap); + REQUIRE(cur_state == TestState::complete); if (expected.bootstrap == BootstrapMode::SingleMessageMulti || - expected.bootstrap == BootstrapMode::MultiMessage) { - REQUIRE(expected.bootstrap == bootstrap_mode); - REQUIRE(role_change_bootstrap); - REQUIRE(cur_state == TestState::complete); - if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { - REQUIRE(bootstrap_msg_count == 1); - } - else if (expected.bootstrap == BootstrapMode::MultiMessage) { - REQUIRE(bootstrap_msg_count > 1); - } + expected.bootstrap == BootstrapMode::SingleMessage) { + REQUIRE(bootstrap_msg_count == 1); } - else if (expected.bootstrap == BootstrapMode::SingleMessage) { - logger->debug("ROLE CHANGE: expected single-message/single-changeset bootstrap"); - REQUIRE(bootstrap_mode == BootstrapMode::None); - REQUIRE(bootstrap_msg_count == 0); - REQUIRE(cur_state == TestState::reconnect_received); + else if (expected.bootstrap == BootstrapMode::MultiMessage) { + REQUIRE(bootstrap_msg_count > 1); } // Make sure a client reset did not occur while waiting for the role change to // be applied @@ -5237,11 +5243,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra rule["roles"][0]["document_filters"]["write"] = doc_filter; }; - auto run_test = [&](TestParams params, nlohmann::json initial_rules, size_t initial_count, - nlohmann::json test_rules, ExpectedResults expected_results) { + auto setup_test = [&](FLXSyncTestHarness& harness, TestParams params, nlohmann::json initial_rules, + size_t initial_count) { // Set up the test harness with the provided initial rules - FLXSyncTestHarness harness("flx_role_change_bootstrap", - {person_schema, {"role", "firstName", "lastName"}}); setup_harness(harness, params); // Get the current rules so it can be updated during the test @@ -5260,36 +5264,42 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(!wait_for_download(*realm)); REQUIRE(!wait_for_upload(*realm)); set_up_realm(realm, initial_count); - - // Single message bootstrap - remove employees, keep mgrs/dirs - update_role(default_rule, test_rules); - update_perms_and_verify(harness, realm, default_rule, expected_results); + return realm; }; SECTION("Single-message bootstrap") { // Single message bootstrap - remove employees, keep mgrs/dirs - logger->trace("ROLE CHANGE: Updating rules to remove employees"); - // 500 emps, 10 mgrs, 5 dirs - TestParams params{}; - auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; - run_test(params, {}, num_total, {{"role", {{"$in", {"manager", "director"}}}}}, - {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); - } - SECTION("Multi-message bootstrap") { - // Multi-message bootstrap - add employeees, remove managers and directors - logger->trace("ROLE CHANGE: Updating rules to employees only"); // 5000 emps, 100 mgrs, 25 dirs + // num_objects_before_bootstrap_flush: 10 TestParams params{5000, 100, 25, 10}; - run_test(params, {{"role", {{"$in", {"manager", "director"}}}}}, params.num_mgrs + params.num_dirs, - {{"role", "employee"}}, {BootstrapMode::MultiMessage, params.num_emps, 0, 0}); + auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; + FLXSyncTestHarness harness("flx_role_change_bootstrap", + {person_schema, {"role", "firstName", "lastName"}}); + auto realm = setup_test(harness, params, {}, num_total); + + // Single message bootstrap - remove employees, keep mgrs/dirs + ExpectedResults test_results = {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}; + logger->trace("ROLE CHANGE: Updating rules to remove employees"); + update_perms_and_verify(harness, realm, {{"role", {{"$in", {"manager", "director"}}}}}, test_results); + // Multi-message bootstrap - add employeees, remove managers and directors + test_results = {BootstrapMode::MultiMessage, params.num_emps, 0, 0}; + logger->trace("ROLE CHANGE: Updating rules to add back the employees and remove mgrs/dirs"); + update_perms_and_verify(harness, realm, {{"role", "employee"}}, test_results); } SECTION("Single-message/Multi-changeset bootstrap") { - // Single message/multi-changeset bootstrap - add back managers and directors - logger->trace("ROLE CHANGE: Updating rules to all records"); + /// TODO: Update this test to use the PAUSE_DOWNLOAD_BUILDER test command + /// once it can be sent prior to the IDENT message // 500 emps, 500 mgrs, 125 dirs TestParams params{500, 500, 125, 100, 2000}; - run_test(params, {{"role", "employee"}}, params.num_emps, true, - {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, params.num_dirs}); + FLXSyncTestHarness harness("flx_role_change_bootstrap", + {person_schema, {"role", "firstName", "lastName"}}); + auto realm = setup_test(harness, params, {{"role", "employee"}}, params.num_emps); + + // Single message/multi-changeset bootstrap - add back managers and directors + ExpectedResults test_results = {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, + params.num_dirs}; + logger->trace("ROLE CHANGE: Updating rules to all records"); + update_perms_and_verify(harness, realm, true, test_results); } } } diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index b8a3b0b9b0b..c16581af5a9 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -704,23 +704,14 @@ app::Response AdminAPIEndpoint::get(const std::vector= 200 && resp.http_status_code < 300, m_url, body.dump(), - resp.http_status_code, resp.body); - return nlohmann::json::parse(resp.body.empty() ? "{}" : resp.body); -} - nlohmann::json AdminAPIEndpoint::get_json(const std::vector>& params) const { auto resp = get(params); diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index 8564f19afca..78d67861593 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -43,12 +43,11 @@ class AdminAPIEndpoint { app::Response patch(std::string body) const; app::Response post(std::string body) const; app::Response put(std::string body) const; - app::Response del(std::string body = {}) const; + app::Response del() const; nlohmann::json get_json(const std::vector>& params = {}) const; nlohmann::json patch_json(nlohmann::json body) const; nlohmann::json post_json(nlohmann::json body) const; nlohmann::json put_json(nlohmann::json body) const; - nlohmann::json del_json(nlohmann::json body) const; AdminAPIEndpoint operator[](StringData name) const; @@ -147,7 +146,6 @@ class AdminAPISession { MigrationStatus get_migration_status(const std::string& app_id) const; nlohmann::json get_app_settings(const std::string& app_id) const; bool patch_app_settings(const std::string& app_id, nlohmann::json&& new_settings) const; - bool update_app_settings(const std::string& app_id, nlohmann::json&& new_settings) const; const std::string& admin_url() const noexcept { From 8ecfe34bc96fa313ada732e6e81faff1a6e1dda2 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 3 Jun 2024 20:20:26 -0400 Subject: [PATCH 21/37] Updated role change tests and merged test from separate PR --- test/object-store/sync/flx_sync.cpp | 271 +++++++++++++----- .../util/sync/sync_test_utils.cpp | 4 + 2 files changed, 200 insertions(+), 75 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 0cadb43a397..2f481d3d357 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4989,7 +4989,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } }; - enum BootstrapMode { None, SingleMessage, SingleMessageMulti, MultiMessage }; + enum BootstrapMode { NoReconnect, None, SingleMessage, SingleMessageMulti, MultiMessage, Any }; enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; struct ExpectedResults { @@ -5043,11 +5043,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }; - // auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { - // nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : - // "RESUME_DOWNLOAD_BUILDER"}}; return SyncSession::OnlyForTesting::send_test_command(session, - // test_command.dump()); - // }; +#if 0 + /// TODO: Add back once the server supports sending test commands before the IDENT message being sent + auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { + nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : + "RESUME_DOWNLOAD_BUILDER"}}; return SyncSession::OnlyForTesting::send_test_command(session, + test_command.dump()); + }; +#endif auto setup_config_callbacks = [&](SyncTestFile& config) { config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, @@ -5069,8 +5072,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return TestState::reconnect_received; case Event::DownloadMessageReceived: { - logger->debug("ROLE CHANGE: download message: %1", data.batch_state); - std::lock_guard lock(callback_mutex); // multi-message bootstrap in progress.. ++download_msg_count; @@ -5150,7 +5151,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(results.size() == expected_cnt); }; - auto verify_records = [](SharedRealm check_realm, ExpectedResults& expected) { + auto verify_records = [](SharedRealm check_realm, ExpectedResults expected) { // Validate the expected number of entries for each role type after the role change auto table = check_realm->read_group().get_table("class_Person"); REQUIRE(table->size() == (expected.emps + expected.mgrs + expected.dirs)); @@ -5166,8 +5167,13 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra CHECK(results.size() == expected.dirs); }; - auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, - nlohmann::json doc_filter, ExpectedResults& expected) { + auto update_role = [](nlohmann::json& rule, nlohmann::json doc_filter) { + rule["roles"][0]["document_filters"]["read"] = doc_filter; + rule["roles"][0]["document_filters"]["write"] = doc_filter; + }; + + auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, nlohmann::json new_rules, + ExpectedResults expected) { { std::lock_guard lock(callback_mutex); did_client_reset = false; @@ -5185,15 +5191,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Update the permissions on the server - should send an error to the client to force // it to reconnect auto& app_session = harness.session().app_session(); - auto new_rules = app_session.admin_api.get_default_rule(app_session.server_app_id); - new_rules["roles"][0]["document_filters"]["read"] = doc_filter; - new_rules["roles"][0]["document_filters"]["write"] = doc_filter; logger->trace("ROLE CHANGE: New rule definitions: %1", new_rules); app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); - // After updating the permissions (if they are different), the server should send an - // error that will disconnect/reconnect the session - verify the reconnect occurs. - REQUIRE(state_machina.wait_for(TestState::reconnect_received)); + if (expected.bootstrap != BootstrapMode::NoReconnect) { + // After updating the permissions (if they are different), the server should send an + // error that will disconnect/reconnect the session - verify the reconnect occurs. + REQUIRE(state_machina.wait_for(TestState::reconnect_received)); + } // Assuming the session disconnects and reconnects, the server initiated role change // bootstrap download will take place when the session is re-established and will @@ -5205,25 +5210,47 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // bootstrap operation cannot easily be tracked. Adding back the employees to // the local data via the permissions should produce a trackable bootstrap // with multiple messages which can be verified. - - { - // By the time the MARK response is received and wait_for_download() - // returns, the bootstrap should have already been applied. - auto cur_state = state_machina.get(); // grab before locking callback_mutex - std::lock_guard lock(callback_mutex); - REQUIRE(expected.bootstrap == bootstrap_mode); - REQUIRE(role_change_bootstrap); - REQUIRE(cur_state == TestState::complete); - if (expected.bootstrap == BootstrapMode::SingleMessageMulti || - expected.bootstrap == BootstrapMode::SingleMessage) { - REQUIRE(bootstrap_msg_count == 1); - } - else if (expected.bootstrap == BootstrapMode::MultiMessage) { - REQUIRE(bootstrap_msg_count > 1); - } - // Make sure a client reset did not occur while waiting for the role change to - // be applied - REQUIRE_FALSE(did_client_reset); + auto cur_state = state_machina.get(); // grab before locking callback_mutex + switch (expected.bootstrap) { + case BootstrapMode::NoReconnect: { + // Confirm that the session did receive an error and a bootstrap did not occur + REQUIRE(cur_state == TestState::start); + std::lock_guard lock(callback_mutex); + REQUIRE_FALSE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + } break; + case BootstrapMode::None: { + // Confirm that a bootstrap did not occur + REQUIRE(cur_state == TestState::reconnect_received); + std::lock_guard lock(callback_mutex); + REQUIRE_FALSE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + } break; + case BootstrapMode::Any: { + // Doesn't matter which one, just that a bootstrap occurred and not a client reset + REQUIRE(cur_state == TestState::complete); + std::lock_guard lock(callback_mutex); + REQUIRE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + } break; + default: { + // By the time the MARK response is received and wait_for_download() + // returns, the bootstrap should have already been applied. + std::lock_guard lock(callback_mutex); + REQUIRE(expected.bootstrap == bootstrap_mode); + REQUIRE(role_change_bootstrap); + REQUIRE(cur_state == TestState::complete); + if (expected.bootstrap == BootstrapMode::SingleMessageMulti || + expected.bootstrap == BootstrapMode::SingleMessage) { + REQUIRE(bootstrap_msg_count == 1); + } + else if (expected.bootstrap == BootstrapMode::MultiMessage) { + REQUIRE(bootstrap_msg_count > 1); + } + // Make sure a client reset did not occur while waiting for the role change to + // be applied + REQUIRE_FALSE(did_client_reset); + } break; } // Wait for the upload to complete and the updated data to be ready in the local realm. @@ -5237,69 +5264,163 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra state_machina.transition_to(TestState::not_ready); }; - SECTION("Role changes lead to objects in/out of view without client reset") { - auto update_role = [](nlohmann::json& rule, nlohmann::json doc_filter) { - rule["roles"][0]["document_filters"]["read"] = doc_filter; - rule["roles"][0]["document_filters"]["write"] = doc_filter; - }; - - auto setup_test = [&](FLXSyncTestHarness& harness, TestParams params, nlohmann::json initial_rules, - size_t initial_count) { - // Set up the test harness with the provided initial rules - setup_harness(harness, params); + auto setup_test = [&](FLXSyncTestHarness& harness, TestParams params, nlohmann::json initial_rules, + size_t initial_count) { + // Set up the test harness with the provided initial rules + setup_harness(harness, params); - // Get the current rules so it can be updated during the test + // If the intial rules are not empty, then set them now + if (!initial_rules.empty()) { + logger->trace("ROLE CHANGE: Initial rule definitions: %1", initial_rules); auto& app_session = harness.session().app_session(); - auto default_rule = app_session.admin_api.get_default_rule(app_session.server_app_id); - if (!initial_rules.empty()) { - update_role(default_rule, initial_rules); - logger->trace("ROLE CHANGE: Initial rule definitions: %1", default_rule); - app_session.admin_api.update_default_rule(app_session.server_app_id, default_rule); - } + app_session.admin_api.update_default_rule(app_session.server_app_id, initial_rules); + } - // Set up and create a realm; wait for data sync - auto config = harness.make_test_file(); - setup_config_callbacks(config); - auto realm = Realm::get_shared_realm(config); - REQUIRE(!wait_for_download(*realm)); - REQUIRE(!wait_for_upload(*realm)); - set_up_realm(realm, initial_count); - return realm; - }; + // Create and set up a new realm to be returned; wait for data sync + auto config = harness.make_test_file(); + setup_config_callbacks(config); + auto realm = Realm::get_shared_realm(config); + REQUIRE(!wait_for_download(*realm)); + REQUIRE(!wait_for_upload(*realm)); + set_up_realm(realm, initial_count); + return realm; + }; + + SECTION("Role changes lead to objects in/out of view without client reset") { + FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); + // Get the current rules so it can be updated during the test + auto& app_session = harness.session().app_session(); + auto test_rules = app_session.admin_api.get_default_rule(app_session.server_app_id); - SECTION("Single-message bootstrap") { + SECTION("Basic bootstraps") { // Single message bootstrap - remove employees, keep mgrs/dirs // 5000 emps, 100 mgrs, 25 dirs // num_objects_before_bootstrap_flush: 10 TestParams params{5000, 100, 25, 10}; auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; - FLXSyncTestHarness harness("flx_role_change_bootstrap", - {person_schema, {"role", "firstName", "lastName"}}); auto realm = setup_test(harness, params, {}, num_total); // Single message bootstrap - remove employees, keep mgrs/dirs - ExpectedResults test_results = {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}; logger->trace("ROLE CHANGE: Updating rules to remove employees"); - update_perms_and_verify(harness, realm, {{"role", {{"$in", {"manager", "director"}}}}}, test_results); + update_role(test_rules, {{"role", {{"$in", {"manager", "director"}}}}}); + update_perms_and_verify(harness, realm, test_rules, + {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); + // Write the same rules again - the client should not receive the reconnect (200) error + update_perms_and_verify(harness, realm, test_rules, + {BootstrapMode::NoReconnect, 0, params.num_mgrs, params.num_dirs}); // Multi-message bootstrap - add employeees, remove managers and directors - test_results = {BootstrapMode::MultiMessage, params.num_emps, 0, 0}; logger->trace("ROLE CHANGE: Updating rules to add back the employees and remove mgrs/dirs"); - update_perms_and_verify(harness, realm, {{"role", "employee"}}, test_results); + update_role(test_rules, {{"role", "employee"}}); + update_perms_and_verify(harness, realm, test_rules, {BootstrapMode::MultiMessage, params.num_emps, 0, 0}); } SECTION("Single-message/Multi-changeset bootstrap") { /// TODO: Update this test to use the PAUSE_DOWNLOAD_BUILDER test command /// once it can be sent prior to the IDENT message // 500 emps, 500 mgrs, 125 dirs TestParams params{500, 500, 125, 100, 2000}; - FLXSyncTestHarness harness("flx_role_change_bootstrap", - {person_schema, {"role", "firstName", "lastName"}}); - auto realm = setup_test(harness, params, {{"role", "employee"}}, params.num_emps); + update_role(test_rules, {{"role", "employee"}}); + auto realm = setup_test(harness, params, test_rules, params.num_emps); // Single message/multi-changeset bootstrap - add back managers and directors - ExpectedResults test_results = {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, - params.num_dirs}; logger->trace("ROLE CHANGE: Updating rules to all records"); - update_perms_and_verify(harness, realm, true, test_results); + update_role(test_rules, true); + update_perms_and_verify( + harness, realm, test_rules, + {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, params.num_dirs}); + } + } + SECTION("Role changes for one user do not change unaffected user") { + FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); + // Get the current rules so it can be updated during the test + auto& app_session = harness.session().app_session(); + auto default_rule = app_session.admin_api.get_default_rule(app_session.server_app_id); + // 500 emps, 10 mgrs, 5 dirs + TestParams params{}; + size_t num_total = params.num_emps + params.num_mgrs + params.num_dirs; + auto realm_1 = setup_test(harness, params, {}, num_total); + + // Get the config for the first user + auto config_1 = harness.make_test_file(); + // Create a second user and a new realm config for that user + create_user_and_log_in(harness.app()); + auto config_2 = harness.make_test_file(); + REQUIRE(config_1.sync_config->user->user_id() != config_2.sync_config->user->user_id()); + + // While the default realm only allows access to the employee records + AppCreateConfig::ServiceRole general_role{"default"}; + general_role.document_filters.read = {{"role", "employee"}}; + general_role.document_filters.write = {{"role", "employee"}}; + + auto rules = app_session.admin_api.get_default_rule(app_session.server_app_id); + rules["roles"][0] = {transform_service_role(general_role)}; + { + auto realm_2 = Realm::get_shared_realm(config_2); + REQUIRE(!wait_for_download(*realm_2)); + REQUIRE(!wait_for_upload(*realm_2)); + set_up_realm(realm_2, num_total); + + // Add the initial rule and verify the data in realm 1 and 2 (should be same) + update_perms_and_verify(harness, realm_1, rules, {BootstrapMode::Any, params.num_emps, 0, 0}); + REQUIRE(!wait_for_download(*realm_2)); + REQUIRE(!wait_for_upload(*realm_2)); + wait_for_advance(*realm_2); + verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); + } + { + // Reopen realm 2 with the hook callback + std::mutex realm2_mutex; + bool realm_2_bootstrap_detected = false; + config_2.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, + const SyncClientHookData& data) { + using Event = SyncClientHookEvent; + if ((data.event == Event::DownloadMessageReceived && + data.batch_state != sync::DownloadBatchState::SteadyState) || + data.event == Event::BootstrapMessageProcessed || data.event == Event::BootstrapProcessed) { + // If a download message was received or bootstrap was processed, then record it occurred + std::lock_guard lock(realm2_mutex); + realm_2_bootstrap_detected = true; + } + return SyncClientHookAction::NoAction; + }; + auto realm_2 = Realm::get_shared_realm(config_2); + REQUIRE(!wait_for_download(*realm_2)); + REQUIRE(!wait_for_upload(*realm_2)); + verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); + { + // Reset the realm_2 state for the next rule change + std::lock_guard lock(realm2_mutex); + realm_2_bootstrap_detected = false; + } + // The user1 realm allows access to all records + AppCreateConfig::ServiceRole user1_role{"user 1 role"}; + user1_role.apply_when = {{"%%user.id", config_1.sync_config->user->user_id()}}; + // Add a specific rule for the realm 1 user and verify the data + rules["roles"] = {transform_service_role(user1_role), transform_service_role(general_role)}; + update_perms_and_verify(harness, realm_1, rules, + {BootstrapMode::Any, params.num_emps, params.num_mgrs, params.num_dirs}); + + // Realm 2 data should not change (and there shouldn't be any bootstrap messages) + { + std::lock_guard lock(realm2_mutex); + REQUIRE_FALSE(realm_2_bootstrap_detected); + } + verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); + + // The user1 realm will be updated to only have access to employee and managers + AppCreateConfig::ServiceRole user1_role_2 = user1_role; + user1_role_2.document_filters.read = {{"role", {{"$in", {"employee", "manager"}}}}}; + user1_role_2.document_filters.write = {{"role", {{"$in", {"employee", "manager"}}}}}; + // Update the doc filter for the realm 1 user rules and verify the data + rules["roles"][0] = {transform_service_role(user1_role_2)}; + update_perms_and_verify(harness, realm_1, rules, + {BootstrapMode::Any, params.num_emps, params.num_mgrs, 0}); + + // Realm 2 data should not change (and there shouldn't be any bootstrap messages) + { + std::lock_guard lock(realm2_mutex); + REQUIRE_FALSE(realm_2_bootstrap_detected); + } + verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); } } } diff --git a/test/object-store/util/sync/sync_test_utils.cpp b/test/object-store/util/sync/sync_test_utils.cpp index 1d1adbaddf5..4ba39aa8e4b 100644 --- a/test/object-store/util/sync/sync_test_utils.cpp +++ b/test/object-store/util/sync/sync_test_utils.cpp @@ -269,6 +269,10 @@ void wait_for_advance(Realm& realm) , target_version(*realm.latest_snapshot_version()) , done(done) { + // Are we already there... + if (realm.read_transaction_version().version >= target_version) { + done = true; + } } void did_change(std::vector const&, std::vector const&, bool) override From e375bfdd1879fc4992c57e1bfaf14b8eb19a04a3 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 3 Jun 2024 22:42:19 -0400 Subject: [PATCH 22/37] Fixed issue with flx query verion 0 not being treated as a bootstrap --- src/realm/sync/client.cpp | 5 ++++ src/realm/sync/noinst/client_impl_base.cpp | 35 +++++++++++----------- src/realm/sync/noinst/client_impl_base.hpp | 3 ++ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index f2c36d3c7c0..9f72b3bfefe 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -989,6 +989,11 @@ void SessionImpl::on_flx_sync_version_complete(int64_t version) } } +bool SessionImpl::needs_initial_bootstrap() +{ + return m_wrapper.m_flx_active_version < 0; +} + SyncClientHookAction SessionImpl::call_debug_hook(const SyncClientHookData& data) { // Should never be called if session is not active diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index f2b63630c75..4bc72b5077d 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2386,6 +2386,12 @@ Status Session::receive_download_message(const DownloadMessage& message) bool is_flx = m_conn.is_flx_sync_connection(); int64_t query_version = is_flx ? *message.query_version : 0; + sync::DownloadBatchState batch_state = message.batch_state; + // Handle the case for the FLX query version 0 bootstrap message, which is reported as a steady state message + if (is_flx && query_version == 0 && batch_state == sync::DownloadBatchState::SteadyState && + needs_initial_bootstrap()) { + batch_state = sync::DownloadBatchState::LastInBatch; + } if (!is_flx || query_version > 0) enable_progress_notifications(); @@ -2399,7 +2405,7 @@ Status Session::receive_download_message(const DownloadMessage& message) progress.download.server_version, progress.download.last_integrated_client_version, progress.latest_server_version.version, progress.latest_server_version.salt, progress.upload.client_version, progress.upload.last_integrated_server_version, - message.progress_estimate, message.batch_state, query_version, + message.progress_estimate, batch_state, query_version, message.changesets.size()); // Throws } else { @@ -2429,26 +2435,21 @@ Status Session::receive_download_message(const DownloadMessage& message) return status; } - // Start with the download server version from the last download message, since the changesets in the new - // download message must have a remote version greater than (or equal to for FLX) this value. - version_type last_remote_version = m_progress.download.server_version; + version_type server_version = m_progress.download.server_version; version_type last_integrated_client_version = m_progress.download.last_integrated_client_version; - for (const RemoteChangeset& changeset : message.changesets) { - // Check that per-changeset server version is strictly increasing since the last download server version, - // except in FLX sync where the server version must be increasing, but can stay the same during bootstraps. - bool good_server_version = m_is_flx_sync_session ? (changeset.remote_version >= last_remote_version) - : (changeset.remote_version > last_remote_version); + // Check that per-changeset server version is strictly increasing, except in FLX sync where the server + // version must be increasing, but can stay the same during bootstraps. + bool good_server_version = m_is_flx_sync_session ? (changeset.remote_version >= server_version) + : (changeset.remote_version > server_version); // Each server version cannot be greater than the one in the header of the download message. good_server_version = good_server_version && (changeset.remote_version <= progress.download.server_version); if (!good_server_version) { return {ErrorCodes::SyncProtocolInvariantFailed, util::format("Bad server version in changeset header (DOWNLOAD) (%1, %2, %3)", - changeset.remote_version, last_remote_version, progress.download.server_version)}; + changeset.remote_version, server_version, progress.download.server_version)}; } - - // Save the remote version for the current changeset to compare against the next changeset - last_remote_version = changeset.remote_version; + server_version = changeset.remote_version; // Check that per-changeset last integrated client version is "weakly" // increasing. @@ -2475,7 +2476,7 @@ Status Session::receive_download_message(const DownloadMessage& message) } auto hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageReceived, progress, query_version, - message.batch_state, message.changesets.size()); + batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } @@ -2484,16 +2485,16 @@ Status Session::receive_download_message(const DownloadMessage& message) if (is_flx) update_download_estimate(message.progress_estimate); - if (process_flx_bootstrap_message(progress, message.batch_state, query_version, message.changesets)) { + if (process_flx_bootstrap_message(progress, batch_state, query_version, message.changesets)) { clear_resumption_delay_state(); return Status::OK(); } uint64_t downloadable_bytes = is_flx ? 0 : message.downloadable_bytes; - initiate_integrate_changesets(downloadable_bytes, message.batch_state, progress, message.changesets); // Throws + initiate_integrate_changesets(downloadable_bytes, batch_state, progress, message.changesets); // Throws hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageIntegrated, progress, query_version, - message.batch_state, message.changesets.size()); + batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index df8183c9c00..691a1dd94e0 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1181,6 +1181,9 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo* = nullptr); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); + // Return true if the session hasn't received the initial (version 0) bootstrap yet in FLX sync. + bool needs_initial_bootstrap(); + void init_progress_handler(); void enable_progress_notifications(); void notify_upload_progress(); From c4f03440f731821b29eda3bed6c101ee1630da68 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 3 Jun 2024 23:27:24 -0400 Subject: [PATCH 23/37] Cleaned up the tests a bit and reworked query version 0 handling --- src/realm/sync/client.cpp | 7 +- src/realm/sync/noinst/client_impl_base.cpp | 3 +- src/realm/sync/noinst/client_impl_base.hpp | 3 +- test/object-store/sync/flx_sync.cpp | 108 ++++++++++----------- 4 files changed, 60 insertions(+), 61 deletions(-) diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 9f72b3bfefe..8c5f6e60703 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -989,9 +989,12 @@ void SessionImpl::on_flx_sync_version_complete(int64_t version) } } -bool SessionImpl::needs_initial_bootstrap() +int64_t SessionImpl::flx_active_version() { - return m_wrapper.m_flx_active_version < 0; + if (m_state == State::Active) { + return m_wrapper.m_flx_active_version; + } + return 0; } SyncClientHookAction SessionImpl::call_debug_hook(const SyncClientHookData& data) diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 4bc72b5077d..96fc5498d0d 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2388,8 +2388,7 @@ Status Session::receive_download_message(const DownloadMessage& message) int64_t query_version = is_flx ? *message.query_version : 0; sync::DownloadBatchState batch_state = message.batch_state; // Handle the case for the FLX query version 0 bootstrap message, which is reported as a steady state message - if (is_flx && query_version == 0 && batch_state == sync::DownloadBatchState::SteadyState && - needs_initial_bootstrap()) { + if (is_flx && query_version != flx_active_version() && batch_state == sync::DownloadBatchState::SteadyState) { batch_state = sync::DownloadBatchState::LastInBatch; } diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 691a1dd94e0..8016bf8aa3c 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1181,8 +1181,7 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo* = nullptr); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); - // Return true if the session hasn't received the initial (version 0) bootstrap yet in FLX sync. - bool needs_initial_bootstrap(); + int64_t flx_active_version(); void init_progress_handler(); void enable_progress_notifications(); diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index c4a8b8dad12..730958a759c 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4967,39 +4967,31 @@ TEST_CASE("flx: nested collections in mixed", "[sync][flx][baas]") { CHECK(nested_list.get_any(1) == "foo"); } -TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { - const Schema person_schema{{"Person", - {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, - {"role", PropertyType::String}, - {"firstName", PropertyType::String}, - {"lastName", PropertyType::String}}}}; +const Schema g_person_schema{{"Person", + {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, + {"role", PropertyType::String}, + {"firstName", PropertyType::String}, + {"lastName", PropertyType::String}}}}; - auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { - CppContext c(realm); - for (size_t i = 0; i < count; ++i) { - // Recreate the context (transaction) for every 100 entries - if (count % 100 == 0) { - c = CppContext(realm); - } - auto obj = Object::create(c, realm, "Person", - std::any(AnyDict{ - {"_id", ObjectId::gen()}, - {"role", role}, - {"firstName", util::format("%1-%2", role, i)}, - {"lastName", util::format("last-name-%1", i)}, - })); +auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { + CppContext c(realm); + for (size_t i = 0; i < count; ++i) { + // Recreate the context (transaction) for every 100 entries + if (count % 100 == 0) { + c = CppContext(realm); } - }; + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{ + {"_id", ObjectId::gen()}, + {"role", role}, + {"firstName", util::format("%1-%2", role, i)}, + {"lastName", util::format("last-name-%1", i)}, + })); + } +}; +TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { enum BootstrapMode { NoReconnect, None, SingleMessage, SingleMessageMulti, MultiMessage, Any }; - enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; - - struct ExpectedResults { - BootstrapMode bootstrap; - size_t emps; - size_t mgrs; - size_t dirs; - }; struct TestParams { size_t num_emps = 500; @@ -5009,6 +5001,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra std::optional sleep_millis = std::nullopt; }; + struct ExpectedResults { + BootstrapMode bootstrap; + size_t emps; + size_t mgrs; + size_t dirs; + }; + + enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; TestingStateMachine state_machina(TestState::not_ready); std::mutex callback_mutex; int64_t query_version = 0; @@ -5055,6 +5055,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra #endif auto setup_config_callbacks = [&](SyncTestFile& config) { + // Use the sync client event hook to check for the error received and for tracking + // download messages and bootstraps config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, const SyncClientHookData& data) { state_machina.transition_with([&](TestState cur_state) -> std::optional { @@ -5128,6 +5130,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return SyncClientHookAction::NoAction; }; + // Add client reset callback to verify a client reset doesn't happen config.sync_config->notify_before_client_reset = [&](std::shared_ptr) { std::lock_guard lock(callback_mutex); did_client_reset = true; @@ -5177,6 +5180,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, nlohmann::json new_rules, ExpectedResults expected) { { + // Reset the test state std::lock_guard lock(callback_mutex); did_client_reset = false; bootstrap_msg_count = 0; @@ -5206,32 +5210,30 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // bootstrap download will take place when the session is re-established and will // complete before the server sends the initial MARK response. REQUIRE(!wait_for_download(*check_realm)); + REQUIRE(!wait_for_upload(*check_realm)); - // The tricky part here is that a single server initiated bootstrap message - // with one changeset will be treated as a typical download message and the - // bootstrap operation cannot easily be tracked. Adding back the employees to - // the local data via the permissions should produce a trackable bootstrap - // with multiple messages which can be verified. + // Now that the server initiated bootstrap should be complete, verify the operation + // performed matched what was expected. auto cur_state = state_machina.get(); // grab before locking callback_mutex switch (expected.bootstrap) { case BootstrapMode::NoReconnect: { // Confirm that the session did receive an error and a bootstrap did not occur - REQUIRE(cur_state == TestState::start); std::lock_guard lock(callback_mutex); + REQUIRE(cur_state == TestState::start); REQUIRE_FALSE(role_change_bootstrap); REQUIRE_FALSE(did_client_reset); } break; case BootstrapMode::None: { - // Confirm that a bootstrap did not occur - REQUIRE(cur_state == TestState::reconnect_received); + // Confirm that a bootstrap nor a client reset did not occur std::lock_guard lock(callback_mutex); + REQUIRE(cur_state == TestState::reconnect_received); REQUIRE_FALSE(role_change_bootstrap); REQUIRE_FALSE(did_client_reset); } break; case BootstrapMode::Any: { // Doesn't matter which one, just that a bootstrap occurred and not a client reset - REQUIRE(cur_state == TestState::complete); std::lock_guard lock(callback_mutex); + REQUIRE(cur_state == TestState::complete); REQUIRE(role_change_bootstrap); REQUIRE_FALSE(did_client_reset); } break; @@ -5255,11 +5257,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } break; } - // Wait for the upload to complete and the updated data to be ready in the local realm. - REQUIRE(!wait_for_upload(*check_realm)); - wait_for_advance(*check_realm); - // Validate the expected number of entries for each role type after the role change + wait_for_advance(*check_realm); verify_records(check_realm, expected); // Reset the state machine to "not ready" before leaving @@ -5268,10 +5267,10 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto setup_test = [&](FLXSyncTestHarness& harness, TestParams params, nlohmann::json initial_rules, size_t initial_count) { - // Set up the test harness with the provided initial rules + // Set up the test harness and data with the provided initial parameters setup_harness(harness, params); - // If the intial rules are not empty, then set them now + // If an intial set of rules are provided, then set them now if (!initial_rules.empty()) { logger->trace("ROLE CHANGE: Initial rule definitions: %1", initial_rules); auto& app_session = harness.session().app_session(); @@ -5282,14 +5281,12 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto config = harness.make_test_file(); setup_config_callbacks(config); auto realm = Realm::get_shared_realm(config); - REQUIRE(!wait_for_download(*realm)); - REQUIRE(!wait_for_upload(*realm)); set_up_realm(realm, initial_count); return realm; }; SECTION("Role changes lead to objects in/out of view without client reset") { - FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); + FLXSyncTestHarness harness("flx_role_change_bootstrap", {g_person_schema, {"role", "firstName", "lastName"}}); // Get the current rules so it can be updated during the test auto& app_session = harness.session().app_session(); auto test_rules = app_session.admin_api.get_default_rule(app_session.server_app_id); @@ -5308,6 +5305,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra update_perms_and_verify(harness, realm, test_rules, {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); // Write the same rules again - the client should not receive the reconnect (200) error + logger->trace("ROLE CHANGE: Updating same rules again and verify reconnect doesn't happen"); update_perms_and_verify(harness, realm, test_rules, {BootstrapMode::NoReconnect, 0, params.num_mgrs, params.num_dirs}); // Multi-message bootstrap - add employeees, remove managers and directors @@ -5332,7 +5330,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } } SECTION("Role changes for one user do not change unaffected user") { - FLXSyncTestHarness harness("flx_role_change_bootstrap", {person_schema, {"role", "firstName", "lastName"}}); + FLXSyncTestHarness harness("flx_role_change_bootstrap", {g_person_schema, {"role", "firstName", "lastName"}}); // Get the current rules so it can be updated during the test auto& app_session = harness.session().app_session(); auto default_rule = app_session.admin_api.get_default_rule(app_session.server_app_id); @@ -5348,7 +5346,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto config_2 = harness.make_test_file(); REQUIRE(config_1.sync_config->user->user_id() != config_2.sync_config->user->user_id()); - // While the default realm only allows access to the employee records + // Start with a default rule that only allows access to the employee records AppCreateConfig::ServiceRole general_role{"default"}; general_role.document_filters.read = {{"role", "employee"}}; general_role.document_filters.write = {{"role", "employee"}}; @@ -5361,7 +5359,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(!wait_for_upload(*realm_2)); set_up_realm(realm_2, num_total); - // Add the initial rule and verify the data in realm 1 and 2 (should be same) + // Add the initial rule and verify the data in realm 1 and 2 (both should just have the employees) update_perms_and_verify(harness, realm_1, rules, {BootstrapMode::Any, params.num_emps, 0, 0}); REQUIRE(!wait_for_download(*realm_2)); REQUIRE(!wait_for_upload(*realm_2)); @@ -5369,7 +5367,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); } { - // Reopen realm 2 with the hook callback + // Reopen realm 2 and add a hook callback to check for a bootstrap (which should not happen) std::mutex realm2_mutex; bool realm_2_bootstrap_detected = false; config_2.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, @@ -5393,10 +5391,10 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra std::lock_guard lock(realm2_mutex); realm_2_bootstrap_detected = false; } - // The user1 realm allows access to all records + // The first rule allows access to all records for user 1 AppCreateConfig::ServiceRole user1_role{"user 1 role"}; user1_role.apply_when = {{"%%user.id", config_1.sync_config->user->user_id()}}; - // Add a specific rule for the realm 1 user and verify the data + // Add two rules, the first applies to user 1 and the second applies to other users rules["roles"] = {transform_service_role(user1_role), transform_service_role(general_role)}; update_perms_and_verify(harness, realm_1, rules, {BootstrapMode::Any, params.num_emps, params.num_mgrs, params.num_dirs}); @@ -5408,11 +5406,11 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra } verify_records(realm_2, {BootstrapMode::None, params.num_emps, 0, 0}); - // The user1 realm will be updated to only have access to employee and managers + // The first rule will be updated to only have access to employee and managers AppCreateConfig::ServiceRole user1_role_2 = user1_role; user1_role_2.document_filters.read = {{"role", {{"$in", {"employee", "manager"}}}}}; user1_role_2.document_filters.write = {{"role", {{"$in", {"employee", "manager"}}}}}; - // Update the doc filter for the realm 1 user rules and verify the data + // Update the first rule for user 1 and verify the data after the rule is applied rules["roles"][0] = {transform_service_role(user1_role_2)}; update_perms_and_verify(harness, realm_1, rules, {BootstrapMode::Any, params.num_emps, params.num_mgrs, 0}); From 165a65180096ce9aa5cf981243ba7693efe659c4 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Tue, 4 Jun 2024 11:05:28 -0400 Subject: [PATCH 24/37] Added TODO to query 0 bootstrap detection --- src/realm/sync/noinst/client_impl_base.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 899d600c45a..51584a49bca 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2389,8 +2389,9 @@ Status Session::receive_download_message(const DownloadMessage& message) bool is_flx = m_conn.is_flx_sync_connection(); int64_t query_version = is_flx ? *message.query_version : 0; sync::DownloadBatchState batch_state = message.batch_state; + /// TODO: Remove this check after the server is updated to send the query 0 download message as a bootstrap // Handle the case for the FLX query version 0 bootstrap message, which is reported as a steady state message - if (is_flx && query_version != flx_active_version() && batch_state == sync::DownloadBatchState::SteadyState) { + if (is_flx && query_version > flx_active_version() && batch_state == sync::DownloadBatchState::SteadyState) { batch_state = sync::DownloadBatchState::LastInBatch; } From b850cacef391e36210b616fbf367c049c14a564d Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 5 Jun 2024 11:43:06 -0400 Subject: [PATCH 25/37] Updates from review; updated batch_state for schema bootstraps --- dependencies.yml | 4 +- src/realm/object-store/sync/sync_session.cpp | 19 -------- src/realm/object-store/sync/sync_session.hpp | 3 -- src/realm/sync/client.cpp | 8 ---- src/realm/sync/noinst/client_impl_base.cpp | 16 ++----- src/realm/sync/noinst/client_impl_base.hpp | 2 - test/object-store/sync/flx_sync.cpp | 50 +++++++++----------- 7 files changed, 30 insertions(+), 72 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 9527a631e23..592cbc69f7d 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -3,6 +3,6 @@ VERSION: 14.9.0 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits -# fed47fa is 2024 May 31 -BAAS_VERSION: fed47fa027f2f48c6912ee848deab0c12da2c4e7 +# b36466c is 2024 June 5 +BAAS_VERSION: b36466c6ff24539cdbfa2b62f5df3016d19cddee BAAS_VERSION_TYPE: githash diff --git a/src/realm/object-store/sync/sync_session.cpp b/src/realm/object-store/sync/sync_session.cpp index 0bea618b133..9d042801fe3 100644 --- a/src/realm/object-store/sync/sync_session.cpp +++ b/src/realm/object-store/sync/sync_session.cpp @@ -39,8 +39,6 @@ #include #include -#include - using namespace realm; using namespace realm::_impl; @@ -49,23 +47,6 @@ using SessionWaiterPointer = void (sync::Session::*)(util::UniqueFunction #include -#include #include #include #include @@ -537,8 +536,6 @@ class SyncSession : public std::enable_shared_from_this { bool m_schema_migration_in_progress GUARDED_BY(m_state_mutex) = false; }; -std::ostream& operator<<(std::ostream& out, const SyncSession::ConnectionState& val); - } // namespace realm #endif // REALM_OS_SYNC_SESSION_HPP diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 8c5f6e60703..f2c36d3c7c0 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -989,14 +989,6 @@ void SessionImpl::on_flx_sync_version_complete(int64_t version) } } -int64_t SessionImpl::flx_active_version() -{ - if (m_state == State::Active) { - return m_wrapper.m_flx_active_version; - } - return 0; -} - SyncClientHookAction SessionImpl::call_debug_hook(const SyncClientHookData& data) { // Should never be called if session is not active diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 51584a49bca..e8cd7c9cf87 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -2388,12 +2388,6 @@ Status Session::receive_download_message(const DownloadMessage& message) bool is_flx = m_conn.is_flx_sync_connection(); int64_t query_version = is_flx ? *message.query_version : 0; - sync::DownloadBatchState batch_state = message.batch_state; - /// TODO: Remove this check after the server is updated to send the query 0 download message as a bootstrap - // Handle the case for the FLX query version 0 bootstrap message, which is reported as a steady state message - if (is_flx && query_version > flx_active_version() && batch_state == sync::DownloadBatchState::SteadyState) { - batch_state = sync::DownloadBatchState::LastInBatch; - } if (!is_flx || query_version > 0) enable_progress_notifications(); @@ -2407,7 +2401,7 @@ Status Session::receive_download_message(const DownloadMessage& message) progress.download.server_version, progress.download.last_integrated_client_version, progress.latest_server_version.version, progress.latest_server_version.salt, progress.upload.client_version, progress.upload.last_integrated_server_version, - message.progress_estimate, batch_state, query_version, + message.progress_estimate, message.batch_state, query_version, message.changesets.size()); // Throws } else { @@ -2478,7 +2472,7 @@ Status Session::receive_download_message(const DownloadMessage& message) } auto hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageReceived, progress, query_version, - batch_state, message.changesets.size()); + message.batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } @@ -2487,16 +2481,16 @@ Status Session::receive_download_message(const DownloadMessage& message) if (is_flx) update_download_estimate(message.progress_estimate); - if (process_flx_bootstrap_message(progress, batch_state, query_version, message.changesets)) { + if (process_flx_bootstrap_message(progress, message.batch_state, query_version, message.changesets)) { clear_resumption_delay_state(); return Status::OK(); } uint64_t downloadable_bytes = is_flx ? 0 : message.downloadable_bytes; - initiate_integrate_changesets(downloadable_bytes, batch_state, progress, message.changesets); // Throws + initiate_integrate_changesets(downloadable_bytes, message.batch_state, progress, message.changesets); // Throws hook_action = call_debug_hook(SyncClientHookEvent::DownloadMessageIntegrated, progress, query_version, - batch_state, message.changesets.size()); + message.batch_state, message.changesets.size()); if (hook_action == SyncClientHookAction::EarlyReturn) { return Status::OK(); } diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 8016bf8aa3c..df8183c9c00 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1181,8 +1181,6 @@ class ClientImpl::Session { SyncClientHookAction call_debug_hook(SyncClientHookEvent event, const ProtocolErrorInfo* = nullptr); SyncClientHookAction call_debug_hook(const SyncClientHookData& data); - int64_t flx_active_version(); - void init_progress_handler(); void enable_progress_notifications(); void notify_upload_progress(); diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 730958a759c..58daf21ebc2 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4967,30 +4967,26 @@ TEST_CASE("flx: nested collections in mixed", "[sync][flx][baas]") { CHECK(nested_list.get_any(1) == "foo"); } -const Schema g_person_schema{{"Person", - {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, - {"role", PropertyType::String}, - {"firstName", PropertyType::String}, - {"lastName", PropertyType::String}}}}; +TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { + const Schema g_person_schema{{"Person", + {{"_id", PropertyType::ObjectId, Property::IsPrimary{true}}, + {"role", PropertyType::String}, + {"firstName", PropertyType::String}, + {"lastName", PropertyType::String}}}}; -auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { - CppContext c(realm); - for (size_t i = 0; i < count; ++i) { - // Recreate the context (transaction) for every 100 entries - if (count % 100 == 0) { - c = CppContext(realm); + auto fill_person_schema = [](SharedRealm realm, std::string role, size_t count) { + CppContext c(realm); + for (size_t i = 0; i < count; ++i) { + auto obj = Object::create(c, realm, "Person", + std::any(AnyDict{ + {"_id", ObjectId::gen()}, + {"role", role}, + {"firstName", util::format("%1-%2", role, i)}, + {"lastName", util::format("last-name-%1", i)}, + })); } - auto obj = Object::create(c, realm, "Person", - std::any(AnyDict{ - {"_id", ObjectId::gen()}, - {"role", role}, - {"firstName", util::format("%1-%2", role, i)}, - {"lastName", util::format("last-name-%1", i)}, - })); - } -}; + }; -TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstrap]") { enum BootstrapMode { NoReconnect, None, SingleMessage, SingleMessageMulti, MultiMessage, Any }; struct TestParams { @@ -5109,12 +5105,10 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra case Event::BootstrapMessageProcessed: { REQUIRE(data.batch_state != BatchState::SteadyState); REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); - if (data.batch_state != BatchState::SteadyState) { - std::lock_guard lock(callback_mutex); - ++bootstrap_msg_count; - if (data.query_version == query_version) { - role_change_bootstrap = true; - } + std::lock_guard lock(callback_mutex); + ++bootstrap_msg_count; + if (data.query_version == query_version) { + role_change_bootstrap = true; } return std::nullopt; } @@ -5317,6 +5311,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra /// TODO: Update this test to use the PAUSE_DOWNLOAD_BUILDER test command /// once it can be sent prior to the IDENT message // 500 emps, 500 mgrs, 125 dirs + // num_objects_before_bootstrap_flush: 100 + // download_loop_sleep_millis: 2000 TestParams params{500, 500, 125, 100, 2000}; update_role(test_rules, {{"role", "employee"}}); auto realm = setup_test(harness, params, test_rules, params.num_emps); From 9c7b7afb6c34f508ead047952186a7562120f1a3 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 5 Jun 2024 13:56:51 -0400 Subject: [PATCH 26/37] Removed extra mutex in favor of state machine's mutex --- test/object-store/sync/flx_sync.cpp | 96 +++++++++++++---------------- 1 file changed, 43 insertions(+), 53 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 58daf21ebc2..85016cc0b87 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5006,7 +5006,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; TestingStateMachine state_machina(TestState::not_ready); - std::mutex callback_mutex; int64_t query_version = 0; bool did_client_reset = false; BootstrapMode bootstrap_mode = BootstrapMode::None; @@ -5072,7 +5071,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return TestState::reconnect_received; case Event::DownloadMessageReceived: { - std::lock_guard lock(callback_mutex); // multi-message bootstrap in progress.. ++download_msg_count; if (cur_state != TestState::reconnect_received && cur_state != TestState::downloading) @@ -5105,7 +5103,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra case Event::BootstrapMessageProcessed: { REQUIRE(data.batch_state != BatchState::SteadyState); REQUIRE((cur_state == TestState::downloading || cur_state == TestState::downloaded)); - std::lock_guard lock(callback_mutex); ++bootstrap_msg_count; if (data.query_version == query_version) { role_change_bootstrap = true; @@ -5126,7 +5123,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Add client reset callback to verify a client reset doesn't happen config.sync_config->notify_before_client_reset = [&](std::shared_ptr) { - std::lock_guard lock(callback_mutex); did_client_reset = true; }; }; @@ -5173,18 +5169,14 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto update_perms_and_verify = [&](FLXSyncTestHarness& harness, SharedRealm check_realm, nlohmann::json new_rules, ExpectedResults expected) { - { - // Reset the test state - std::lock_guard lock(callback_mutex); + // Reset the state machine + state_machina.transition_with([&](TestState cur_state) { + REQUIRE(cur_state == TestState::not_ready); did_client_reset = false; bootstrap_msg_count = 0; download_msg_count = 0; role_change_bootstrap = false; query_version = check_realm->get_active_subscription_set().version(); - } - // Reset the state machine - state_machina.transition_with([&](TestState cur_state) { - REQUIRE(cur_state == TestState::not_ready); return TestState::start; }); @@ -5208,48 +5200,46 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Now that the server initiated bootstrap should be complete, verify the operation // performed matched what was expected. - auto cur_state = state_machina.get(); // grab before locking callback_mutex - switch (expected.bootstrap) { - case BootstrapMode::NoReconnect: { - // Confirm that the session did receive an error and a bootstrap did not occur - std::lock_guard lock(callback_mutex); - REQUIRE(cur_state == TestState::start); - REQUIRE_FALSE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); - } break; - case BootstrapMode::None: { - // Confirm that a bootstrap nor a client reset did not occur - std::lock_guard lock(callback_mutex); - REQUIRE(cur_state == TestState::reconnect_received); - REQUIRE_FALSE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); - } break; - case BootstrapMode::Any: { - // Doesn't matter which one, just that a bootstrap occurred and not a client reset - std::lock_guard lock(callback_mutex); - REQUIRE(cur_state == TestState::complete); - REQUIRE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); - } break; - default: { - // By the time the MARK response is received and wait_for_download() - // returns, the bootstrap should have already been applied. - std::lock_guard lock(callback_mutex); - REQUIRE(expected.bootstrap == bootstrap_mode); - REQUIRE(role_change_bootstrap); - REQUIRE(cur_state == TestState::complete); - if (expected.bootstrap == BootstrapMode::SingleMessageMulti || - expected.bootstrap == BootstrapMode::SingleMessage) { - REQUIRE(bootstrap_msg_count == 1); - } - else if (expected.bootstrap == BootstrapMode::MultiMessage) { - REQUIRE(bootstrap_msg_count > 1); - } - // Make sure a client reset did not occur while waiting for the role change to - // be applied - REQUIRE_FALSE(did_client_reset); - } break; - } + state_machina.transition_with([&](TestState cur_state) { + switch (expected.bootstrap) { + case BootstrapMode::NoReconnect: + // Confirm that the session did receive an error and a bootstrap did not occur + REQUIRE(cur_state == TestState::start); + REQUIRE_FALSE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + break; + case BootstrapMode::None: + // Confirm that a bootstrap nor a client reset did not occur + REQUIRE(cur_state == TestState::reconnect_received); + REQUIRE_FALSE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + break; + case BootstrapMode::Any: + // Doesn't matter which one, just that a bootstrap occurred and not a client reset + REQUIRE(cur_state == TestState::complete); + REQUIRE(role_change_bootstrap); + REQUIRE_FALSE(did_client_reset); + break; + default: + // By the time the MARK response is received and wait_for_download() + // returns, the bootstrap should have already been applied. + REQUIRE(expected.bootstrap == bootstrap_mode); + REQUIRE(role_change_bootstrap); + REQUIRE(cur_state == TestState::complete); + if (expected.bootstrap == BootstrapMode::SingleMessageMulti || + expected.bootstrap == BootstrapMode::SingleMessage) { + REQUIRE(bootstrap_msg_count == 1); + } + else if (expected.bootstrap == BootstrapMode::MultiMessage) { + REQUIRE(bootstrap_msg_count > 1); + } + // Make sure a client reset did not occur while waiting for the role change to + // be applied + REQUIRE_FALSE(did_client_reset); + break; + } + return std::nullopt; // Don't transition + }); // Validate the expected number of entries for each role type after the role change wait_for_advance(*check_realm); From 2fcdbeccc68272371a0abffa1f0d3641b71ad037 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 5 Jun 2024 16:14:47 -0400 Subject: [PATCH 27/37] Increased timeout when waiting for app initial sync to complete --- test/object-store/util/sync/baas_admin_api.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/object-store/util/sync/baas_admin_api.cpp b/test/object-store/util/sync/baas_admin_api.cpp index c16581af5a9..e0538111b00 100644 --- a/test/object-store/util/sync/baas_admin_api.cpp +++ b/test/object-store/util/sync/baas_admin_api.cpp @@ -1656,11 +1656,12 @@ AppSession create_app(const AppCreateConfig& config) return object_schema.table_type == ObjectSchema::ObjectType::TopLevel; }); if (any_sync_types) { + // Increasing timeout due to occasional slow startup of the translator on baasaas timed_sleeping_wait_for( [&] { return session.is_initial_sync_complete(app_id); }, - std::chrono::seconds(30), std::chrono::seconds(1)); + std::chrono::seconds(60), std::chrono::seconds(1)); } return {client_app_id, app_id, session, config}; From 83d2a5cf8687d0288323d1642c1bc51de04c751c Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 6 Jun 2024 11:25:01 -0400 Subject: [PATCH 28/37] Updated role change test to use test commands --- dependencies.yml | 4 +- src/realm/sync/config.hpp | 2 + src/realm/sync/noinst/client_impl_base.cpp | 1 + src/realm/sync/noinst/client_impl_base.hpp | 4 + test/object-store/sync/flx_sync.cpp | 143 +++++++++++++-------- 5 files changed, 100 insertions(+), 54 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 592cbc69f7d..7a4ca9f92d2 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -3,6 +3,6 @@ VERSION: 14.9.0 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits -# b36466c is 2024 June 5 -BAAS_VERSION: b36466c6ff24539cdbfa2b62f5df3016d19cddee +# 010c03e is 2024 June 5 +BAAS_VERSION: 010c03e0c200c59d36df0b7925bfcb1a7df23721 BAAS_VERSION_TYPE: githash diff --git a/src/realm/sync/config.hpp b/src/realm/sync/config.hpp index adf659d60dd..479a5db08b8 100644 --- a/src/realm/sync/config.hpp +++ b/src/realm/sync/config.hpp @@ -131,7 +131,9 @@ enum class SyncClientHookEvent { ErrorMessageReceived, SessionActivating, SessionSuspended, + SessionResumed, BindMessageSent, + IdentMessageSent, ClientResetMergeComplete, BootstrapBatchAboutToProcess, }; diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index e8cd7c9cf87..b8bbc8ed4fb 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -1992,6 +1992,7 @@ void Session::send_ident_message() m_conn.initiate_write_message(out, this); // Throws m_ident_message_sent = true; + call_debug_hook(SyncClientHookEvent::IdentMessageSent); // Other messages may be waiting to be sent enlist_to_send(); // Throws diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index df8183c9c00..76576580421 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1536,6 +1536,10 @@ inline void ClientImpl::Session::initiate_rebind() reset_protocol_state(); + // Call SessionResumed before sending the BIND Message to + // allow adding a test command between BIND and IDENT messages + call_debug_hook(SyncClientHookEvent::SessionResumed); + // Ready to send BIND message enlist_to_send(); // Throws } diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 85016cc0b87..a29bca6a2c1 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -4994,7 +4994,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t num_mgrs = 10; size_t num_dirs = 5; std::optional num_objects = std::nullopt; - std::optional sleep_millis = std::nullopt; }; struct ExpectedResults { @@ -5004,7 +5003,16 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t dirs; }; - enum TestState { not_ready, start, reconnect_received, downloading, downloaded, complete }; + enum TestState { + not_ready, + start, + reconnect_received, + session_resumed, + ident_message, + downloading, + downloaded, + complete + }; TestingStateMachine state_machina(TestState::not_ready); int64_t query_version = 0; bool did_client_reset = false; @@ -5012,6 +5020,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t download_msg_count = 0; size_t bootstrap_msg_count = 0; bool role_change_bootstrap = false; + bool send_test_command = false; + std::vector> test_command_futures(0); auto logger = util::Logger::get_default_logger(); auto setup_harness = [&](FLXSyncTestHarness& harness, TestParams params) { @@ -5027,10 +5037,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra app_session.server_app_id, {{"sync", {{"num_objects_before_bootstrap_flush", *params.num_objects}}}})); } - if (params.sleep_millis) { - REQUIRE(app_session.admin_api.patch_app_settings( - app_session.server_app_id, {{"sync", {{"download_loop_sleep_millis", *params.sleep_millis}}}})); - } // Initialize the realm with some data harness.load_initial_data([&](SharedRealm realm) { @@ -5040,19 +5046,32 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }; -#if 0 /// TODO: Add back once the server supports sending test commands before the IDENT message being sent auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : "RESUME_DOWNLOAD_BUILDER"}}; return SyncSession::OnlyForTesting::send_test_command(session, test_command.dump()); }; -#endif + + auto wait_for_test_command = [&](TestState wait_state) { + std::optional> cur_test_command; + REQUIRE(state_machina.wait_for(wait_state)); + state_machina.transition_with([&](TestState) { + REQUIRE(send_test_command); + REQUIRE(test_command_futures.size() > 0); + cur_test_command = std::move(test_command_futures.front()); + test_command_futures.erase(test_command_futures.begin()); + return std::nullopt; + }); + auto result = cur_test_command->get_no_throw(); + REQUIRE(result.is_ok()); + REQUIRE(result.get_value() == "{}"); + }; auto setup_config_callbacks = [&](SyncTestFile& config) { // Use the sync client event hook to check for the error received and for tracking // download messages and bootstraps - config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr, + config.sync_config->on_sync_client_event_hook = [&](std::weak_ptr weak_session, const SyncClientHookData& data) { state_machina.transition_with([&](TestState cur_state) -> std::optional { if (cur_state == TestState::not_ready || cur_state == TestState::complete) @@ -5070,12 +5089,33 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE_FALSE(data.error_info->is_fatal); return TestState::reconnect_received; + case Event::SessionResumed: + REQUIRE(cur_state == TestState::reconnect_received); + if (send_test_command) { + if (auto session = weak_session.lock()) { + logger->trace("ROLE CHANGE: sending PAUSE test command after resumed"); + test_command_futures.push_back(pause_download_builder(*session, true)); + } + } + return TestState::session_resumed; + + case Event::IdentMessageSent: + REQUIRE(cur_state == TestState::session_resumed); + if (send_test_command) { + if (auto session = weak_session.lock()) { + logger->trace("ROLE CHANGE: sending RESUME test command after idient message sent"); + test_command_futures.push_back(pause_download_builder(*session, false)); + } + } + return TestState::ident_message; + case Event::DownloadMessageReceived: { - // multi-message bootstrap in progress.. - ++download_msg_count; - if (cur_state != TestState::reconnect_received && cur_state != TestState::downloading) + // Skip unexpected download messages + if (cur_state != TestState::ident_message && cur_state != TestState::downloading) { return std::nullopt; - + } + ++download_msg_count; + // A multi-message bootstrap is in progress.. if (data.batch_state == BatchState::MoreToCome) { // More than 1 bootstrap message, always a multi-message bootstrap_mode = BootstrapMode::MultiMessage; @@ -5177,6 +5217,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra download_msg_count = 0; role_change_bootstrap = false; query_version = check_realm->get_active_subscription_set().version(); + if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { + send_test_command = true; + } return TestState::start; }); @@ -5192,6 +5235,16 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(state_machina.wait_for(TestState::reconnect_received)); } + if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { + // Wait for the session to be resumed and the "pause" test command to be sent + logger->trace("ROLE CHANGE: waiting for PAUSE test command to complete"); + wait_for_test_command(TestState::session_resumed); + // Wait for the session to send the IDENT message and "unpause" test command + logger->trace("ROLE CHANGE: waiting for RESUME test command to complete"); + wait_for_test_command(TestState::ident_message); + logger->trace("ROLE CHANGE: test commands complete"); + } + // Assuming the session disconnects and reconnects, the server initiated role change // bootstrap download will take place when the session is re-established and will // complete before the server sends the initial MARK response. @@ -5275,45 +5328,31 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto& app_session = harness.session().app_session(); auto test_rules = app_session.admin_api.get_default_rule(app_session.server_app_id); - SECTION("Basic bootstraps") { - // Single message bootstrap - remove employees, keep mgrs/dirs - // 5000 emps, 100 mgrs, 25 dirs - // num_objects_before_bootstrap_flush: 10 - TestParams params{5000, 100, 25, 10}; - auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; - auto realm = setup_test(harness, params, {}, num_total); - - // Single message bootstrap - remove employees, keep mgrs/dirs - logger->trace("ROLE CHANGE: Updating rules to remove employees"); - update_role(test_rules, {{"role", {{"$in", {"manager", "director"}}}}}); - update_perms_and_verify(harness, realm, test_rules, - {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); - // Write the same rules again - the client should not receive the reconnect (200) error - logger->trace("ROLE CHANGE: Updating same rules again and verify reconnect doesn't happen"); - update_perms_and_verify(harness, realm, test_rules, - {BootstrapMode::NoReconnect, 0, params.num_mgrs, params.num_dirs}); - // Multi-message bootstrap - add employeees, remove managers and directors - logger->trace("ROLE CHANGE: Updating rules to add back the employees and remove mgrs/dirs"); - update_role(test_rules, {{"role", "employee"}}); - update_perms_and_verify(harness, realm, test_rules, {BootstrapMode::MultiMessage, params.num_emps, 0, 0}); - } - SECTION("Single-message/Multi-changeset bootstrap") { - /// TODO: Update this test to use the PAUSE_DOWNLOAD_BUILDER test command - /// once it can be sent prior to the IDENT message - // 500 emps, 500 mgrs, 125 dirs - // num_objects_before_bootstrap_flush: 100 - // download_loop_sleep_millis: 2000 - TestParams params{500, 500, 125, 100, 2000}; - update_role(test_rules, {{"role", "employee"}}); - auto realm = setup_test(harness, params, test_rules, params.num_emps); - - // Single message/multi-changeset bootstrap - add back managers and directors - logger->trace("ROLE CHANGE: Updating rules to all records"); - update_role(test_rules, true); - update_perms_and_verify( - harness, realm, test_rules, - {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, params.num_dirs}); - } + // 5000 emps, 100 mgrs, 25 dirs + // num_objects_before_bootstrap_flush: 10 + TestParams params{5000, 100, 25, 10}; + auto num_total = params.num_emps + params.num_mgrs + params.num_dirs; + auto realm = setup_test(harness, params, {}, num_total); + + // Single message bootstrap - remove employees, keep mgrs/dirs + logger->trace("ROLE CHANGE: Updating rules to remove employees"); + update_role(test_rules, {{"role", {{"$in", {"manager", "director"}}}}}); + update_perms_and_verify(harness, realm, test_rules, + {BootstrapMode::SingleMessage, 0, params.num_mgrs, params.num_dirs}); + // Write the same rules again - the client should not receive the reconnect (200) error + logger->trace("ROLE CHANGE: Updating same rules again and verify reconnect doesn't happen"); + update_perms_and_verify(harness, realm, test_rules, + {BootstrapMode::NoReconnect, 0, params.num_mgrs, params.num_dirs}); + // Multi-message bootstrap - add employeees, remove managers and directors + logger->trace("ROLE CHANGE: Updating rules to add back the employees and remove mgrs/dirs"); + update_role(test_rules, {{"role", "employee"}}); + update_perms_and_verify(harness, realm, test_rules, {BootstrapMode::MultiMessage, params.num_emps, 0, 0}); + // Single message/multi-changeset bootstrap - add back the managers and directors + logger->trace("ROLE CHANGE: Updating rules to allow all records"); + update_role(test_rules, true); + update_perms_and_verify( + harness, realm, test_rules, + {BootstrapMode::SingleMessageMulti, params.num_emps, params.num_mgrs, params.num_dirs}); } SECTION("Role changes for one user do not change unaffected user") { FLXSyncTestHarness harness("flx_role_change_bootstrap", {g_person_schema, {"role", "firstName", "lastName"}}); From 51b54eb4d6624c8037f3f0a33737efcf413dc68d Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 6 Jun 2024 11:29:48 -0400 Subject: [PATCH 29/37] Fixed lint warning --- test/object-store/sync/flx_sync.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index a29bca6a2c1..29b1ed85c4b 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5048,9 +5048,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra /// TODO: Add back once the server supports sending test commands before the IDENT message being sent auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { - nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : - "RESUME_DOWNLOAD_BUILDER"}}; return SyncSession::OnlyForTesting::send_test_command(session, - test_command.dump()); + nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : "RESUME_DOWNLOAD_BUILDER"}}; + return SyncSession::OnlyForTesting::send_test_command(session, test_command.dump()); }; auto wait_for_test_command = [&](TestState wait_state) { From 93386d429a0f148e2ee03e7c62bd284117e08062 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 6 Jun 2024 12:30:53 -0400 Subject: [PATCH 30/37] Update resume and ident message handling --- test/object-store/sync/flx_sync.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 29b1ed85c4b..4bc8dea5645 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5089,8 +5089,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return TestState::reconnect_received; case Event::SessionResumed: - REQUIRE(cur_state == TestState::reconnect_received); if (send_test_command) { + REQUIRE(cur_state == TestState::reconnect_received); if (auto session = weak_session.lock()) { logger->trace("ROLE CHANGE: sending PAUSE test command after resumed"); test_command_futures.push_back(pause_download_builder(*session, true)); @@ -5099,8 +5099,8 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra return TestState::session_resumed; case Event::IdentMessageSent: - REQUIRE(cur_state == TestState::session_resumed); if (send_test_command) { + REQUIRE(cur_state == TestState::session_resumed); if (auto session = weak_session.lock()) { logger->trace("ROLE CHANGE: sending RESUME test command after idient message sent"); test_command_futures.push_back(pause_download_builder(*session, false)); From 7e2c5a772d8b91ed90e801a7ba138ebce423af7d Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Thu, 6 Jun 2024 17:26:54 -0400 Subject: [PATCH 31/37] Updated future waits for the pause/resume test command --- test/object-store/sync/flx_sync.cpp | 52 ++++++++--------------------- 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 4bc8dea5645..6426d3a40a8 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5021,7 +5021,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra size_t bootstrap_msg_count = 0; bool role_change_bootstrap = false; bool send_test_command = false; - std::vector> test_command_futures(0); auto logger = util::Logger::get_default_logger(); auto setup_harness = [&](FLXSyncTestHarness& harness, TestParams params) { @@ -5047,24 +5046,15 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }; /// TODO: Add back once the server supports sending test commands before the IDENT message being sent - auto pause_download_builder = [](SyncSession& session, bool pause) -> util::Future { - nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : "RESUME_DOWNLOAD_BUILDER"}}; - return SyncSession::OnlyForTesting::send_test_command(session, test_command.dump()); - }; - - auto wait_for_test_command = [&](TestState wait_state) { - std::optional> cur_test_command; - REQUIRE(state_machina.wait_for(wait_state)); - state_machina.transition_with([&](TestState) { - REQUIRE(send_test_command); - REQUIRE(test_command_futures.size() > 0); - cur_test_command = std::move(test_command_futures.front()); - test_command_futures.erase(test_command_futures.begin()); - return std::nullopt; - }); - auto result = cur_test_command->get_no_throw(); - REQUIRE(result.is_ok()); - REQUIRE(result.get_value() == "{}"); + auto pause_download_builder = [](std::weak_ptr weak_session, bool pause) { + if (auto session = weak_session.lock()) { + nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : "RESUME_DOWNLOAD_BUILDER"}}; + SyncSession::OnlyForTesting::send_test_command(*session, test_command.dump()) + .get_async([](StatusWith result) { + REQUIRE(result.is_ok()); // Future completed successfully + REQUIRE(result.get_value() == "{}"); // Command completed successfully + }); + } }; auto setup_config_callbacks = [&](SyncTestFile& config) { @@ -5091,20 +5081,16 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra case Event::SessionResumed: if (send_test_command) { REQUIRE(cur_state == TestState::reconnect_received); - if (auto session = weak_session.lock()) { - logger->trace("ROLE CHANGE: sending PAUSE test command after resumed"); - test_command_futures.push_back(pause_download_builder(*session, true)); - } + logger->trace("ROLE CHANGE: sending PAUSE test command after resumed"); + pause_download_builder(weak_session, true); } return TestState::session_resumed; case Event::IdentMessageSent: if (send_test_command) { REQUIRE(cur_state == TestState::session_resumed); - if (auto session = weak_session.lock()) { - logger->trace("ROLE CHANGE: sending RESUME test command after idient message sent"); - test_command_futures.push_back(pause_download_builder(*session, false)); - } + logger->trace("ROLE CHANGE: sending RESUME test command after ident message sent"); + pause_download_builder(weak_session, false); } return TestState::ident_message; @@ -5225,7 +5211,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Update the permissions on the server - should send an error to the client to force // it to reconnect auto& app_session = harness.session().app_session(); - logger->trace("ROLE CHANGE: New rule definitions: %1", new_rules); + logger->debug("Updating rule definitions: %1", new_rules); app_session.admin_api.update_default_rule(app_session.server_app_id, new_rules); if (expected.bootstrap != BootstrapMode::NoReconnect) { @@ -5234,16 +5220,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE(state_machina.wait_for(TestState::reconnect_received)); } - if (expected.bootstrap == BootstrapMode::SingleMessageMulti) { - // Wait for the session to be resumed and the "pause" test command to be sent - logger->trace("ROLE CHANGE: waiting for PAUSE test command to complete"); - wait_for_test_command(TestState::session_resumed); - // Wait for the session to send the IDENT message and "unpause" test command - logger->trace("ROLE CHANGE: waiting for RESUME test command to complete"); - wait_for_test_command(TestState::ident_message); - logger->trace("ROLE CHANGE: test commands complete"); - } - // Assuming the session disconnects and reconnects, the server initiated role change // bootstrap download will take place when the session is re-established and will // complete before the server sends the initial MARK response. From 239d162532eaadaad8563c7c50b5bdaa8408aedb Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 00:22:26 -0400 Subject: [PATCH 32/37] Added session connected event for when session multiplexing is disabled --- src/realm/sync/config.hpp | 1 + src/realm/sync/noinst/client_impl_base.hpp | 4 ++++ test/object-store/sync/flx_sync.cpp | 3 +++ 3 files changed, 8 insertions(+) diff --git a/src/realm/sync/config.hpp b/src/realm/sync/config.hpp index 479a5db08b8..1c9b2122508 100644 --- a/src/realm/sync/config.hpp +++ b/src/realm/sync/config.hpp @@ -131,6 +131,7 @@ enum class SyncClientHookEvent { ErrorMessageReceived, SessionActivating, SessionSuspended, + SessionConnected, SessionResumed, BindMessageSent, IdentMessageSent, diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index e227cf37b69..394b05bed60 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1467,6 +1467,10 @@ inline void ClientImpl::Session::connection_established(bool fast_reconnect) ++m_target_download_mark; } + // Call SessionResumed before sending the BIND Message to + // allow adding a test command between BIND and IDENT messages + call_debug_hook(SyncClientHookEvent::SessionConnected); + if (!m_suspended) { // Ready to send BIND message enlist_to_send(); // Throws diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 6426d3a40a8..1dc22e976de 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5078,6 +5078,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra REQUIRE_FALSE(data.error_info->is_fatal); return TestState::reconnect_received; + case Event::SessionConnected: + // Handle the reconnect if session multiplexing is disabled + [[fallthrough]]; case Event::SessionResumed: if (send_test_command) { REQUIRE(cur_state == TestState::reconnect_received); From 0f00b92fe36209b4c1b4dd4565be44de1eede407 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 10:14:04 -0400 Subject: [PATCH 33/37] Updates from review; updated baas commit to include timing fix --- dependencies.yml | 4 ++-- src/realm/sync/noinst/client_impl_base.hpp | 8 ++++---- src/realm/sync/protocol.hpp | 6 +++--- test/object-store/sync/flx_sync.cpp | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dependencies.yml b/dependencies.yml index 7a4ca9f92d2..154f99a7d5f 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -3,6 +3,6 @@ VERSION: 14.9.0 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits -# 010c03e is 2024 June 5 -BAAS_VERSION: 010c03e0c200c59d36df0b7925bfcb1a7df23721 +# 30c10fd is 2024 June 6 +BAAS_VERSION: 30c10fd8e9400fc77e594340422d8b75c210e18d BAAS_VERSION_TYPE: githash diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 394b05bed60..81cf47b8771 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -1467,8 +1467,8 @@ inline void ClientImpl::Session::connection_established(bool fast_reconnect) ++m_target_download_mark; } - // Call SessionResumed before sending the BIND Message to - // allow adding a test command between BIND and IDENT messages + // Notify the debug hook of the SessionConnected event before sending + // the bind messsage call_debug_hook(SyncClientHookEvent::SessionConnected); if (!m_suspended) { @@ -1541,8 +1541,8 @@ inline void ClientImpl::Session::initiate_rebind() reset_protocol_state(); - // Call SessionResumed before sending the BIND Message to - // allow adding a test command between BIND and IDENT messages + // Notify the debug hook of the SessionResumed event before sending + // the bind messsage call_debug_hook(SyncClientHookEvent::SessionResumed); // Ready to send BIND message diff --git a/src/realm/sync/protocol.hpp b/src/realm/sync/protocol.hpp index bcd2b1707a0..cba383b1075 100644 --- a/src/realm/sync/protocol.hpp +++ b/src/realm/sync/protocol.hpp @@ -68,9 +68,9 @@ namespace sync { // constexpr int get_current_protocol_version() noexcept { - // Also update the current protocol version test in flx_sync.cpp when - // updating this value - return 13; + // Also update the "flx: verify websocket protocol number and prefixes" test + // in flx_sync.cpp when updating this value + return 14; } constexpr std::string_view get_pbs_websocket_protocol_prefix() noexcept diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 1dc22e976de..24eb7ffc953 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -2642,7 +2642,7 @@ TEST_CASE("flx: writes work without waiting for sync", "[sync][flx][baas]") { TEST_CASE("flx: verify websocket protocol number and prefixes", "[sync][protocol]") { // Update the expected value whenever the protocol version is updated - this ensures // that the current protocol version does not change unexpectedly. - REQUIRE(13 == sync::get_current_protocol_version()); + REQUIRE(14 == sync::get_current_protocol_version()); // This was updated in Protocol V8 to use '#' instead of '/' to support the Web SDK REQUIRE("com.mongodb.realm-sync#" == sync::get_pbs_websocket_protocol_prefix()); REQUIRE("com.mongodb.realm-query-sync#" == sync::get_flx_websocket_protocol_prefix()); From 04624e935144f174525a2b505e2f8a16737dc536 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 12:29:14 -0400 Subject: [PATCH 34/37] Removed todo comment --- test/object-store/sync/flx_sync.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 24eb7ffc953..112df4cce0a 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5045,7 +5045,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }); }; - /// TODO: Add back once the server supports sending test commands before the IDENT message being sent auto pause_download_builder = [](std::weak_ptr weak_session, bool pause) { if (auto session = weak_session.lock()) { nlohmann::json test_command = {{"command", pause ? "PAUSE_DOWNLOAD_BUILDER" : "RESUME_DOWNLOAD_BUILDER"}}; From 84a4c238b472e163967cc657bc62a042b4cd4c17 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 17:21:55 -0400 Subject: [PATCH 35/37] Added wait_until() to state machine to wait for callback; updated role change test --- test/object-store/sync/flx_sync.cpp | 6 +++++- test/object-store/util/test_utils.hpp | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 112df4cce0a..1d0c3f4095a 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5219,7 +5219,11 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra if (expected.bootstrap != BootstrapMode::NoReconnect) { // After updating the permissions (if they are different), the server should send an // error that will disconnect/reconnect the session - verify the reconnect occurs. - REQUIRE(state_machina.wait_for(TestState::reconnect_received)); + // Make sure at least the reconnect state (or later) has been reached + auto state_reached = state_machina.wait_until([](TestState cur_state) { + return static_cast(cur_state) >= static_cast(TestState::reconnect_received); + }); + REQUIRE(state_reached); } // Assuming the session disconnects and reconnects, the server initiated role change diff --git a/test/object-store/util/test_utils.hpp b/test/object-store/util/test_utils.hpp index 517463494bd..c413daf6190 100644 --- a/test/object-store/util/test_utils.hpp +++ b/test/object-store/util/test_utils.hpp @@ -68,14 +68,26 @@ class TestingStateMachine { m_cv.notify_one(); } - bool wait_for(E target, std::chrono::milliseconds period = std::chrono::seconds(15)) + // Wait for the provided callback to return true + bool wait_until(util::UniqueFunction pred, + std::chrono::milliseconds period = std::chrono::seconds(15)) { std::unique_lock lock{m_mutex}; return m_cv.wait_for(lock, period, [&] { - return m_cur_state == target; + return pred(m_cur_state); }); } + // Wait for the state machine to reach a specific state + bool wait_for(E target, std::chrono::milliseconds period = std::chrono::seconds(15)) + { + return wait_until( + [&target](E cur_state) { + return cur_state == target; + }, + period); + } + private: std::mutex m_mutex; std::condition_variable m_cv; From c4e6a9e07969c34fd1a4d03cddd5eb07bc366341 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 18:13:26 -0400 Subject: [PATCH 36/37] Updates from review --- test/object-store/sync/flx_sync.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 1d0c3f4095a..bf6662fae75 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -5015,7 +5015,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra }; TestingStateMachine state_machina(TestState::not_ready); int64_t query_version = 0; - bool did_client_reset = false; BootstrapMode bootstrap_mode = BootstrapMode::None; size_t download_msg_count = 0; size_t bootstrap_msg_count = 0; @@ -5025,7 +5024,7 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra auto setup_harness = [&](FLXSyncTestHarness& harness, TestParams params) { auto& app_session = harness.session().app_session(); - /** TODO: Remove when switching to use Protocol version in RCORE-1972 */ + /** TODO: Remove once the server has been updated to use the protocol version */ // Enable the role change bootstraps REQUIRE( app_session.admin_api.set_feature_flag(app_session.server_app_id, "allow_permissions_bootstrap", true)); @@ -5150,7 +5149,9 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Add client reset callback to verify a client reset doesn't happen config.sync_config->notify_before_client_reset = [&](std::shared_ptr) { - did_client_reset = true; + // Make sure a client reset did not occur while waiting for the role change to + // be applied + FAIL("Client reset is not expected when the role/rules/permissions are changed"); }; }; @@ -5199,7 +5200,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Reset the state machine state_machina.transition_with([&](TestState cur_state) { REQUIRE(cur_state == TestState::not_ready); - did_client_reset = false; bootstrap_msg_count = 0; download_msg_count = 0; role_change_bootstrap = false; @@ -5240,19 +5240,16 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra // Confirm that the session did receive an error and a bootstrap did not occur REQUIRE(cur_state == TestState::start); REQUIRE_FALSE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); break; case BootstrapMode::None: // Confirm that a bootstrap nor a client reset did not occur REQUIRE(cur_state == TestState::reconnect_received); REQUIRE_FALSE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); break; case BootstrapMode::Any: // Doesn't matter which one, just that a bootstrap occurred and not a client reset REQUIRE(cur_state == TestState::complete); REQUIRE(role_change_bootstrap); - REQUIRE_FALSE(did_client_reset); break; default: // By the time the MARK response is received and wait_for_download() @@ -5267,9 +5264,6 @@ TEST_CASE("flx: role change bootstrap", "[sync][flx][baas][role_change][bootstra else if (expected.bootstrap == BootstrapMode::MultiMessage) { REQUIRE(bootstrap_msg_count > 1); } - // Make sure a client reset did not occur while waiting for the role change to - // be applied - REQUIRE_FALSE(did_client_reset); break; } return std::nullopt; // Don't transition From aede51bb39da4b08202e019637d8354844f85e1b Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 7 Jun 2024 18:29:49 -0400 Subject: [PATCH 37/37] Updated changelog after release --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cd432207d3..b01f097d54f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) -* None. +* Add support for server initiated bootstraps. ([PR #7440](https://github.com/realm/realm-core/pull/7440)) ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) @@ -26,7 +26,6 @@ ### Enhancements * Include the originating client reset error in AutoClientResetFailure errors. ([#7761](https://github.com/realm/realm-core/pull/7761)) * Reduce the size of the local transaction log produced by creating objects, improving the performance of insertion-heavy transactions ([PR #7734](https://github.com/realm/realm-core/pull/7734)). -* Add support for server initiated bootstraps. ([PR #7440](https://github.com/realm/realm-core/pull/7440)) ### Fixed * Fix some client resets (such as migrating to flexible sync) potentially failing with AutoClientResetFailed if a new client reset condition (such as rolling back a flexible sync migration) occurred before the first one completed. ([PR #7542](https://github.com/realm/realm-core/pull/7542), since v13.11.0)