diff --git a/.github/workflows/lock-threads.yml b/.github/workflows/lock-threads.yml new file mode 100644 index 00000000000..dc1d483a8e1 --- /dev/null +++ b/.github/workflows/lock-threads.yml @@ -0,0 +1,24 @@ +name: 'Lock Threads' + +on: + schedule: + - cron: '0 * * * *' + workflow_dispatch: + +permissions: + issues: write + pull-requests: write + discussions: write + +concurrency: + group: lock-threads + +jobs: + action: + runs-on: ubuntu-latest + steps: + - uses: dessant/lock-threads@v5 + with: + issue-inactive-days: 30 + pr-inactive-days: 30 + log-output: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 9de6e50571b..827fd806b22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,16 +2,14 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) -* Nested path included in 'OutOfBoundsø error message ([#7438](https://github.com/realm/realm-core/issues/7438)) -* Improve file compaction performance on platforms with page sizes greater than 4k (for example arm64 Apple platforms) for files less than 256 pages in size ([PR #7492](https://github.com/realm/realm-core/pull/7492)). +* None. ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* Modifying nested collections left the accessor used to make the modification in a stale state, resulting in some unneccesary work being done when making multiple modifications via one accessor ([PR #7470](https://github.com/realm/realm-core/pull/7470), since v14.0.0). -* Fix depth level for nested collection in debug mode, set it to the same level as release ([#7484](https://github.com/realm/realm-core/issues/7484), since v14.0.0). +* None. ### Breaking changes -* Update C-API log callback signature to include the log category, and `realm_set_log_callback` to not take a `realm_log_level_e`. ([PR #7494](https://github.com/realm/realm-core/pull/7494) +* None. ### Compatibility * Fileformat: Generates files with format v24. Reads and automatically upgrade from fileformat v10. If you want to upgrade from an earlier file format version you will have to use RealmCore v13.x.y or earlier. @@ -23,6 +21,26 @@ ---------------------------------------------- +# 14.4.0 Release notes + +### Enhancements +* Nested path included in 'OutOfBoundsø error message ([#7438](https://github.com/realm/realm-core/issues/7438)) +* Improve file compaction performance on platforms with page sizes greater than 4k (for example arm64 Apple platforms) for files less than 256 pages in size ([PR #7492](https://github.com/realm/realm-core/pull/7492)). + +### Fixed +* Modifying nested collections left the accessor used to make the modification in a stale state, resulting in some unneccesary work being done when making multiple modifications via one accessor ([PR #7470](https://github.com/realm/realm-core/pull/7470), since v14.0.0). +* Fix depth level for nested collection in debug mode, set it to the same level as release ([#7484](https://github.com/realm/realm-core/issues/7484), since v14.0.0). +* Fix opening realm with cached user while offline results in fatal error and session does not retry connection. ([#7349](https://github.com/realm/realm-core/issues/7349), since v13.26.0) +* Fix disallow Sets in ArrayMixed. ([#7502](https://github.com/realm/realm-core/pull/7502), since v14.0.0) + +### Breaking changes +* Update C-API log callback signature to include the log category, and `realm_set_log_callback` to not take a `realm_log_level_e`. ([PR #7494](https://github.com/realm/realm-core/pull/7494) + +### Compatibility +* Fileformat: Generates files with format v24. Reads and automatically upgrade from fileformat v10. If you want to upgrade from an earlier file format version you will have to use RealmCore v13.x.y or earlier. + +---------------------------------------------- + # 14.3.0 Release notes ### Enhancements diff --git a/Package.swift b/Package.swift index f5fc4f92030..97b7d26dcd0 100644 --- a/Package.swift +++ b/Package.swift @@ -3,7 +3,7 @@ import PackageDescription import Foundation -let versionStr = "14.3.0" +let versionStr = "14.4.0" let versionPieces = versionStr.split(separator: "-") let versionCompontents = versionPieces[0].split(separator: ".") let versionExtra = versionPieces.count > 1 ? versionPieces[1] : "" diff --git a/dependencies.yml b/dependencies.yml index 87f50995eea..4842354ee1c 100644 --- a/dependencies.yml +++ b/dependencies.yml @@ -1,5 +1,5 @@ PACKAGE_NAME: realm-core -VERSION: 14.3.0 +VERSION: 14.4.0 OPENSSL_VERSION: 3.2.0 ZLIB_VERSION: 1.2.13 # https://github.com/10gen/baas/commits diff --git a/external/catch b/external/catch index 05e10dfccc2..3f0283de7a9 160000 --- a/external/catch +++ b/external/catch @@ -1 +1 @@ -Subproject commit 05e10dfccc28c7f973727c54f850237d07d5e10f +Subproject commit 3f0283de7a9c43200033da996ff9093be3ac84dc diff --git a/src/realm.h b/src/realm.h index bfbcc50c984..e310db2faa6 100644 --- a/src/realm.h +++ b/src/realm.h @@ -88,6 +88,7 @@ typedef bool (*realm_migration_func_t)(realm_userdata_t userdata, realm_t* old_r typedef bool (*realm_data_initialization_func_t)(realm_userdata_t userdata, realm_t* realm); typedef bool (*realm_should_compact_on_launch_func_t)(realm_userdata_t userdata, uint64_t total_bytes, uint64_t used_bytes); + typedef enum realm_schema_mode { RLM_SCHEMA_MODE_AUTOMATIC, RLM_SCHEMA_MODE_IMMUTABLE, @@ -2977,6 +2978,7 @@ RLM_API realm_auth_provider_e realm_auth_credentials_get_provider(realm_app_cred RLM_API realm_app_config_t* realm_app_config_new(const char* app_id, const realm_http_transport_t* http_transport) RLM_API_NOEXCEPT; +RLM_API const char* realm_app_get_default_base_url(void) RLM_API_NOEXCEPT; RLM_API void realm_app_config_set_base_url(realm_app_config_t*, const char*) RLM_API_NOEXCEPT; RLM_API void realm_app_config_set_default_request_timeout(realm_app_config_t*, uint64_t ms) RLM_API_NOEXCEPT; RLM_API void realm_app_config_set_platform_version(realm_app_config_t*, const char*) RLM_API_NOEXCEPT; diff --git a/src/realm/array_mixed.cpp b/src/realm/array_mixed.cpp index b0813f736c5..dddc6b8fbd7 100644 --- a/src/realm/array_mixed.cpp +++ b/src/realm/array_mixed.cpp @@ -550,7 +550,7 @@ int64_t ArrayMixed::store(const Mixed& value) break; } default: - REALM_ASSERT(type == type_List || type == type_Dictionary || type == type_Set); + REALM_ASSERT(type == type_List || type == type_Dictionary); ensure_ref_array(); size_t ndx = m_refs.size(); m_refs.add(value.get_ref()); diff --git a/src/realm/cluster.cpp b/src/realm/cluster.cpp index d940e870cd0..20268b8580a 100644 --- a/src/realm/cluster.cpp +++ b/src/realm/cluster.cpp @@ -1653,50 +1653,74 @@ ref_type Cluster::typed_write(ref_type ref, _impl::ArrayWriterBase& out, const T else if (col_type == col_type_Mixed) { const auto sz = leaf.size(); REALM_ASSERT(sz == 6); - for (size_t i = 0; i < sz; ++i) { - auto rot = leaf.get_as_ref_or_tagged(i); - if (rot.is_ref() && rot.get_as_ref()) { - - /* - In order to know how Mixed stores things, we need to take in consideration this enum - enum { - payload_idx_type, - payload_idx_int, - payload_idx_pair, - payload_idx_str, - payload_idx_ref, - payload_idx_key, - payload_idx_size - }; - Note: - 1. First 3 entries can be compressed (they are integers) - 2. entry number 4 is for strings (no compression for now) - 3. entry number 5 is actually storing refs to collections (List or Dictionaries). - They can only be BPlusTree or BPlusTree. - 4. Other entries should be skipped, we do not compress them. - */ - - const auto do_compress = (i < 3) ? true : false; - if (i == 4) { - // TODO this is not working... - bool compress = true; - auto bptree_rot = leaf.get_as_ref_or_tagged(i); - if (bptree_rot.is_ref() && bptree_rot.get_as_ref()) { - written_leaf.set_as_ref(i, BPlusTreeBase::typed_write(bptree_rot.get_as_ref(), out, - m_alloc, col_type, deep, - only_modified, compress)); - } - else { - written_leaf.set(i, bptree_rot); - } + + /* + In order to know how Mixed stores things, we need to take in consideration this enum + enum { + payload_idx_type, + payload_idx_int, + payload_idx_pair, + payload_idx_str, + payload_idx_ref, + payload_idx_key, + payload_idx_size + }; + Note: + 1. entry at index 0 is the parent pointer + 2. entries at indices 1 and 2 can be compressed (they are integers) + 3. entry at index 3 is for strings (no compression for now) + 4. entry at index 4 is actually storing refs to all the stored types, + including to collections (List or Dictionaries). + They can only be BPlusTree or BPlusTree. + 5. Is the key array, marks whether the composite array at position i is a collection or not + */ + auto rot_parent = leaf.get_as_ref_or_tagged(0); + auto rot_int = leaf.get_as_ref_or_tagged(1); + auto rot_pair_int = leaf.get_as_ref_or_tagged(2); + auto rot_string = leaf.get_as_ref_or_tagged(3); + auto rot_composite = leaf.get_as_ref_or_tagged(4); + auto rot_key = leaf.get_as_ref_or_tagged(5); + + if (rot_parent.get_as_ref()) + written_leaf.set(0, Array::write(rot_parent.get_as_ref(), m_alloc, out, only_modified, false)); + else + written_leaf.set(0, rot_parent); + + if (rot_int.get_as_ref()) + written_leaf.set_as_ref(1, Array::write(rot_int.get_as_ref(), m_alloc, out, only_modified, true)); + else + written_leaf.set(1, rot_int); + + if (rot_pair_int.get_as_ref()) + written_leaf.set_as_ref( + 2, Array::write(rot_pair_int.get_as_ref(), m_alloc, out, only_modified, true)); + else + written_leaf.set(2, rot_pair_int); + + written_leaf.set(3, rot_string); // no compression for strings now. + + if (rot_composite.get_as_ref() && rot_key.get_as_ref()) { + Array composite(Allocator::get_default()); + Array keys(Allocator::get_default()); + composite.init_from_ref(rot_composite.get_as_ref()); + keys.init_from_ref(rot_key.get_as_ref()); + + for (size_t i = 0; i < composite.size(); ++i) { + if (i < keys.size() && keys.get(i)) { + // collection. + auto rot = composite.get_as_ref_or_tagged(i); + REALM_ASSERT_DEBUG(rot.is_ref() && rot.get_as_ref()); + const auto new_ref = BPlusTreeBase::typed_write(rot.get_as_ref(), out, m_alloc, col_type, + deep, only_modified, true); + composite.set_as_ref(i, new_ref); } - // compress only mixed arrays that are integers. skip all the rest for now. - written_leaf.set_as_ref( - i, Array::write(rot.get_as_ref(), m_alloc, out, only_modified, do_compress)); - } - else { - written_leaf.set(i, rot); } + written_leaf.set(4, rot_composite); + written_leaf.set(5, rot_key); + } + else { + written_leaf.set(4, rot_composite); + written_leaf.set(5, rot_key); } } else { diff --git a/src/realm/object-store/c_api/app.cpp b/src/realm/object-store/c_api/app.cpp index f43bef96bb3..959e5efa2de 100644 --- a/src/realm/object-store/c_api/app.cpp +++ b/src/realm/object-store/c_api/app.cpp @@ -125,6 +125,11 @@ static inline bson::BsonArray parse_ejson_array(const char* serialized) } } +RLM_API const char* realm_app_get_default_base_url(void) noexcept +{ + return app::App::default_base_url.data(); +} + RLM_API realm_app_credentials_t* realm_app_credentials_new_anonymous(bool reuse_credentials) noexcept { return new realm_app_credentials_t(AppCredentials::anonymous(reuse_credentials)); diff --git a/src/realm/object-store/sync/app.cpp b/src/realm/object-store/sync/app.cpp index a0f28084abc..f89a8e79ddd 100644 --- a/src/realm/object-store/sync/app.cpp +++ b/src/realm/object-store/sync/app.cpp @@ -167,7 +167,6 @@ UniqueFunction handle_default_response(UniqueFunction lock(s_apps_mutex); - auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(s_default_base_url))]; + auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(default_base_url))]; if (!app) { app = std::make_shared(Private(), config); app->configure(sync_client_config); @@ -261,7 +262,7 @@ void App::close_all_sync_sessions() App::App(Private, const Config& config) : m_config(config) - , m_base_url(m_config.base_url.value_or(std::string(s_default_base_url))) + , m_base_url(m_config.base_url.value_or(std::string(default_base_url))) , m_location_updated(false) , m_request_timeout_ms(m_config.default_request_timeout_ms.value_or(s_default_timeout_ms)) { @@ -301,16 +302,21 @@ App::~App() {} void App::configure(const SyncClientConfig& sync_client_config) { + std::string ws_route; { util::CheckedLockGuard guard(m_route_mutex); // Make sure to request the location when the app is configured m_location_updated = false; + // Create a tentative sync route using the generated ws_host_url + REALM_ASSERT(!m_ws_host_url.empty()); + ws_route = make_sync_route(); } - // Start with an empty sync route in the sync manager. It will ensure the - // location has been updated at least once when the first sync session is - // started by requesting a new access token. - m_sync_manager = SyncManager::create(shared_from_this(), {}, sync_client_config, config().app_id); + // When App starts, the ws_host_url will be populated with the generated value based on + // the provided host_url value and the sync route will be created using this. If this is + // is incorrect, the websocket connection will fail and the SyncSession will request a + // new access token, which will update the location if it has not already. + m_sync_manager = SyncManager::create(shared_from_this(), ws_route, sync_client_config, config().app_id); } bool App::init_logger() @@ -384,23 +390,58 @@ void App::configure_route(const std::string& host_url, const std::optionallength() > 0) { m_ws_host_url = *ws_host_url; } - // Otherwise, convert the host url to a websocket host url (http[s]:// -> ws[s]://) + // Otherwise, convert the host url to a websocket host url else { - m_ws_host_url = m_host_url; - if (m_ws_host_url.find("http") == 0) { - m_ws_host_url.replace(0, 4, "ws"); - } + m_ws_host_url = App::create_ws_host_url(m_host_url); } // host_url is the url to the server: e.g., https://realm.mongodb.com or https://localhost:9090 - // base_route is the baseline client api path: e.g. https://realm.mongodb.com/api/client/v2.0 + // base_route is the baseline client api path: e.g. /api/client/v2.0 m_base_route = util::format("%1%2", m_host_url, s_base_path); - // app_route is the cloud app URL: https://realm.mongodb.com/api/client/v2.0/app/ + // app_route is the cloud app URL: /api/client/v2.0/app/ m_app_route = util::format("%1%2/%3", m_base_route, s_app_path, m_config.app_id); - // auth_route is cloud app auth URL: https://realm.mongodb.com/api/client/v2.0/app//auth + // auth_route is cloud app auth URL: /api/client/v2.0/app//auth m_auth_route = util::format("%1%2", m_app_route, s_auth_path); } +// Create a temporary websocket URL domain using the given host URL +// This updates the URL based on the following assumptions: +// If the URL doesn't start with 'http' => +// http[s]://[region-prefix]realm.mongodb.com => ws[s]://ws.[region-prefix]realm.mongodb.com +// http[s]://[region-prefix]services.cloud.mongodb.com => ws[s]://[region-prefix].ws.services.cloud.mongodb.com +// All others => http[s]:// => ws[s]:// +std::string App::create_ws_host_url(const std::string_view host_url) +{ + constexpr static std::string_view orig_base_domain = "realm.mongodb.com"; + constexpr static std::string_view new_base_domain = "services.cloud.mongodb.com"; + const size_t base_len = std::char_traits::length("http://"); + + // Doesn't contain 7 or more characters (length of 'http://') or start with http, + // just return provided string + if (host_url.length() < base_len || host_url.substr(0, 4) != "http") { + return std::string(host_url); + } + // If it starts with 'https' then ws url will start with 'wss' + bool https = host_url[4] == 's'; + size_t prefix_len = base_len + (https ? 1 : 0); + std::string_view prefix = https ? "wss://" : "ws://"; + + // http[s]://[]realm.mongodb.com[/] => + // ws[s]://ws.[]realm.mongodb.com[/] + if (host_url.find(orig_base_domain) != std::string_view::npos) { + return util::format("%1ws.%2", prefix, host_url.substr(prefix_len)); + } + // http[s]://[]services.cloud.mongodb.com[/] => + // ws[s]://[].ws.services.cloud.mongodb.com[/] + if (auto start = host_url.find(new_base_domain); start != std::string_view::npos) { + return util::format("%1%2ws.%3", prefix, host_url.substr(prefix_len, start - prefix_len), + host_url.substr(start)); + } + + // All others => http[s]://[/] => ws[s]://[/] + return util::format("ws%1", host_url.substr(4)); +} + void App::update_hostname(const std::string& host_url, const std::optional& ws_host_url, const std::optional& new_base_url) { @@ -612,11 +653,11 @@ std::string App::get_base_url() const void App::update_base_url(std::optional base_url, UniqueFunction)>&& completion) { - std::string new_base_url = base_url.value_or(std::string(s_default_base_url)); + std::string new_base_url = base_url.value_or(std::string(default_base_url)); if (new_base_url.empty()) { // Treat an empty string the same as requesting the default base url - new_base_url = s_default_base_url; + new_base_url = default_base_url; log_debug("App::update_base_url: empty => %1", new_base_url); } else { @@ -1023,8 +1064,6 @@ std::optional App::update_location(const Response& response, const std return error; } - REALM_ASSERT(m_sync_manager); // Need a valid sync manager - // Update the location info with the data from the response try { auto json = parse(response.body); @@ -1038,8 +1077,10 @@ std::optional App::update_location(const Response& response, const std // Update the local hostname and path information update_hostname(hostname, ws_hostname, base_url); m_location_updated = true; - // Provide the Device Sync websocket route to the SyncManager - m_sync_manager->set_sync_route(make_sync_route()); + if (m_sync_manager) { + // Provide the Device Sync websocket route to the SyncManager + m_sync_manager->set_sync_route(make_sync_route()); + } } } catch (const AppError& ex) { diff --git a/src/realm/object-store/sync/app.hpp b/src/realm/object-store/sync/app.hpp index f9371f66015..03aeb81e51c 100644 --- a/src/realm/object-store/sync/app.hpp +++ b/src/realm/object-store/sync/app.hpp @@ -90,6 +90,8 @@ class App : public std::enable_shared_from_this, DeviceInfo device_info; }; + static std::string_view default_base_url; + // `enable_shared_from_this` is unsafe with public constructors; // use `App::get_app()` instead explicit App(Private, const Config& config); @@ -432,6 +434,8 @@ class App : public std::enable_shared_from_this, // Return the base url path used for Sync Session Websocket requests std::string get_ws_host_url() REQUIRES(!m_route_mutex); + static std::string create_ws_host_url(const std::string_view host_url); + private: const Config m_config; diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index 4a4b6726c4c..bf86816606d 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -43,13 +43,13 @@ SyncClientTimeouts::SyncClientTimeouts() { } -std::shared_ptr SyncManager::create(std::shared_ptr app, std::optional sync_route, +std::shared_ptr SyncManager::create(std::shared_ptr app, std::string sync_route, const SyncClientConfig& config, const std::string& app_id) { - return std::make_shared(Private(), std::move(app), std::move(sync_route), config, app_id); + return std::make_shared(Private(), std::move(app), sync_route, config, app_id); } -SyncManager::SyncManager(Private, std::shared_ptr app, std::optional sync_route, +SyncManager::SyncManager(Private, std::shared_ptr app, std::string sync_route, const SyncClientConfig& config, const std::string& app_id) : m_config(config) , m_file_manager(std::make_unique(m_config.base_file_path, app_id)) @@ -150,7 +150,6 @@ void SyncManager::tear_down_for_testing() // Destroy the client now that we have no remaining sessions. m_sync_client = nullptr; m_logger_ptr.reset(); - m_sync_route.reset(); } { @@ -666,6 +665,25 @@ void SyncManager::close_all_sessions() get_sync_client().wait_for_session_terminations(); } +void SyncManager::set_sync_route(std::string sync_route, bool verified) +{ + REALM_ASSERT(!sync_route.empty()); // Cannot be set to empty string + { + util::CheckedLockGuard lock(m_mutex); + m_sync_route = sync_route; + m_sync_route_verified = verified; + } +} + +void SyncManager::restart_all_sessions() +{ + // Restart the sessions that are currently active + auto sessions = get_all_sessions(); + for (auto& session : sessions) { + session->restart_session(); + } +} + void SyncManager::OnlyForTesting::voluntary_disconnect_all_connections(SyncManager& mgr) { mgr.get_sync_client().voluntary_disconnect_all_connections(); diff --git a/src/realm/object-store/sync/sync_manager.hpp b/src/realm/object-store/sync/sync_manager.hpp index aec300209a7..ff683e9f4c4 100644 --- a/src/realm/object-store/sync/sync_manager.hpp +++ b/src/realm/object-store/sync/sync_manager.hpp @@ -225,23 +225,20 @@ class SyncManager : public std::enable_shared_from_this { // Immediately closes any open sync sessions for this sync manager void close_all_sessions() REQUIRES(!m_mutex, !m_session_mutex); + // Force all the active sessions to restart + void restart_all_sessions() REQUIRES(!m_mutex, !m_session_mutex); + // Used by App to update the sync route any time the location info has been refreshed. - // m_sync_route starts out as unset when the SyncManager is created or configured. - // It will be updated to a valid value upon the first App AppServices HTTP request or - // the access token will be refreshed (forcing a location update) when a SyncSession - // is activated and it is still unset. This value is not allowed to be reset to - // nullopt once it has a valid value. - void set_sync_route(std::string sync_route) REQUIRES(!m_mutex) - { - REALM_ASSERT(!sync_route.empty()); - util::CheckedLockGuard lock(m_mutex); - m_sync_route = std::move(sync_route); - } + // m_sync_route starts out as a generated value based on the configured base_url when + // the SyncManager is created by App. If this is incorrect, the websocket connection + // will fail, resulting in an update to the access token (and the location, if it hasn't + // been updated yet). + void set_sync_route(std::string sync_route, bool verified = true) REQUIRES(!m_mutex); - const std::optional sync_route() const REQUIRES(!m_mutex) + std::pair sync_route() REQUIRES(!m_mutex) { util::CheckedLockGuard lock(m_mutex); - return m_sync_route; + return std::make_pair(m_sync_route, m_sync_route_verified); } std::weak_ptr app() const REQUIRES(!m_mutex) @@ -270,10 +267,10 @@ class SyncManager : public std::enable_shared_from_this { static void voluntary_disconnect_all_connections(SyncManager&); }; - static std::shared_ptr create(std::shared_ptr app, std::optional sync_route, + static std::shared_ptr create(std::shared_ptr app, std::string sync_route, const SyncClientConfig& config, const std::string& app_id); - SyncManager(Private, std::shared_ptr app, std::optional sync_route, - const SyncClientConfig& config, const std::string& app_id); + SyncManager(Private, std::shared_ptr app, std::string sync_route, const SyncClientConfig& config, + const std::string& app_id); private: friend class app::App; @@ -331,9 +328,11 @@ class SyncManager : public std::enable_shared_from_this { // Callers of this method should hold the `m_session_mutex` themselves. bool do_has_existing_sessions() REQUIRES(m_session_mutex); - // The sync route URL to connect to the server. This can be initially empty, but should not - // be cleared once it has been set to a value, except by `tear_down_for_testing()`. - std::optional m_sync_route GUARDED_BY(m_mutex); + // The sync route URL for the sync connection to the server. + std::string m_sync_route GUARDED_BY(m_mutex); + // If true, then the sync route has been verified by querying the location info or successfully + // connecting to the server. + bool m_sync_route_verified GUARDED_BY(m_mutex) = false; std::weak_ptr m_app GUARDED_BY(m_mutex); const std::string m_app_id; diff --git a/src/realm/object-store/sync/sync_session.cpp b/src/realm/object-store/sync/sync_session.cpp index c20ec940633..5560a7c61ce 100644 --- a/src/realm/object-store/sync/sync_session.cpp +++ b/src/realm/object-store/sync/sync_session.cpp @@ -124,12 +124,6 @@ void SyncSession::become_active() } } -void SyncSession::restart_session() -{ - util::CheckedUniqueLock lock(m_state_mutex); - do_restart_session(std::move(lock)); -} - void SyncSession::become_dying(util::CheckedUniqueLock lock) { REALM_ASSERT(m_state != State::Dying); @@ -176,13 +170,23 @@ void SyncSession::become_paused(util::CheckedUniqueLock lock) do_become_inactive(std::move(lock), Status::OK(), true); } -void SyncSession::do_restart_session(util::CheckedUniqueLock) +void SyncSession::restart_session() { - // Nothing to do if the sync session is currently paused - // It will be resumed when resume() is called - if (m_state == State::Paused) - return; + util::CheckedUniqueLock lock(m_state_mutex); + switch (m_state) { + case State::Active: + do_restart_session(std::move(lock)); + break; + case State::WaitingForAccessToken: + case State::Paused: + case State::Dying: + case State::Inactive: + return; + } +} +void SyncSession::do_restart_session(util::CheckedUniqueLock) +{ // Go straight to inactive so the progress completion waiters will // continue to wait until the session restarts and completes the // upload/download sync @@ -244,23 +248,6 @@ void SyncSession::become_waiting_for_access_token() m_state = State::WaitingForAccessToken; } -void SyncSession::handle_location_update_failed(Status status) -{ - // TODO: ideally this would write to the logs as well in case users didn't set up their error handler. - { - util::CheckedUniqueLock lock(m_state_mutex); - // Should only be called while waiting to update the access token - REALM_ASSERT(m_state == State::WaitingForAccessToken); - // Close the session, since there's nothing more to do at this point, it can be resumed - // after resolving the location update failure - become_inactive(std::move(lock), status); - } - if (auto error_handler = config(&SyncConfig::error_handler)) { - auto user_facing_error = SyncError({ErrorCodes::SyncConnectFailed, status.reason()}, true); - error_handler(shared_from_this(), std::move(user_facing_error)); - } -} - void SyncSession::handle_bad_auth(const std::shared_ptr& user, Status status) { // TODO: ideally this would write to the logs as well in case users didn't set up their error handler. @@ -318,17 +305,6 @@ SyncSession::handle_refresh(const std::shared_ptr& session, bool re if (error->code() == ErrorCodes::ClientAppDeallocated) { return; // this response came in after the app shut down, ignore it } - else if (!session->get_sync_route()) { - // If the sync route is empty at this point, it means the forced location update - // failed while trying to start a sync session with a cached user and no other - // AppServices HTTP requests have been performed since the App was created. - // Since a valid websocket host url is not available, fail the SyncSession start - // and pass the error to the user. - // This function will not log out the user, since it is not known at this point - // whether or not the user is valid. - session->handle_location_update_failed( - {error->code(), util::format("Unable to reach the server: %1", error->reason())}); - } else if (ErrorCodes::error_categories(error->code()).test(ErrorCategory::client_error)) { // any other client errors other than app_deallocated are considered fatal because // there was a problem locally before even sending the request to the server @@ -952,18 +928,22 @@ void SyncSession::create_sync_session() } { - // At this point, the sync_route should be valid, since the location should have been updated by - // an AppServices http request or by updating the access token before starting the sync session. - auto sync_route = m_sync_manager->sync_route(); - REALM_ASSERT_EX(sync_route && !sync_route->empty(), "Location was not updated prior to sync session start"); - - if (!m_client.decompose_server_url(*sync_route, session_config.protocol_envelope, + // At this point the sync route was either updated when the first App request was performed, or + // was populated by a generated value that will be used for first contact. If the generated sync + // route is not correct, either a redirection will be received or the connection will fail, + // resulting in an update to both the access token and the location. + auto [sync_route, verified] = m_sync_manager->sync_route(); + REALM_ASSERT_EX(!sync_route.empty(), "Server URL cannot be empty"); + + if (!m_client.decompose_server_url(sync_route, session_config.protocol_envelope, session_config.server_address, session_config.server_port, session_config.service_identifier)) { - throw sync::BadServerUrl(*sync_route); + throw sync::BadServerUrl(sync_route); } + session_config.server_verified = verified; - m_server_url = *sync_route; + m_server_url = sync_route; + m_server_url_verified = verified; } if (sync_config.authorization_header_name) { @@ -1002,12 +982,6 @@ void SyncSession::create_sync_session() // the connection state. m_session->set_connection_state_change_listener( [weak_self](sync::ConnectionState state, util::Optional error) { - // If the OS SyncSession object is destroyed, we ignore any events from the underlying Session as there is - // nothing useful we can do with them. - auto self = weak_self.lock(); - if (!self) { - return; - } using cs = sync::ConnectionState; ConnectionState new_state = [&] { switch (state) { @@ -1020,21 +994,37 @@ void SyncSession::create_sync_session() } REALM_UNREACHABLE(); }(); - util::CheckedUniqueLock lock(self->m_connection_state_mutex); - auto old_state = self->m_connection_state; - self->m_connection_state = new_state; - lock.unlock(); - - if (old_state != new_state) { - self->m_connection_change_notifier.invoke_callbacks(old_state, new_state); - } - - if (error) { - self->handle_error(std::move(*error)); + // If the OS SyncSession object is destroyed, we ignore any events from the underlying Session as there is + // nothing useful we can do with them. + if (auto self = weak_self.lock()) { + self->update_connection_state(new_state); + if (error) { + self->handle_error(std::move(*error)); + } } }); } +void SyncSession::update_connection_state(ConnectionState new_state) +{ + if (new_state == ConnectionState::Connected) { + util::CheckedLockGuard lock(m_config_mutex); + m_server_url_verified = true; + } + + ConnectionState old_state; + { + util::CheckedLockGuard lock(m_connection_state_mutex); + old_state = m_connection_state; + m_connection_state = new_state; + } + + // Notify any registered connection callbacks of the state transition + if (old_state != new_state) { + m_connection_change_notifier.invoke_callbacks(old_state, new_state); + } +} + void SyncSession::nonsync_transact_notify(sync::version_type version) { m_progress_notifier.set_local_version(version); @@ -1134,7 +1124,7 @@ void SyncSession::do_revive(util::CheckedUniqueLock&& lock) auto u = user(); // If the sync manager has a valid route and the user and it's access token // are valid, then revive the session. - if (m_sync_manager->sync_route() && (!u || !u->access_token_refresh_required())) { + if (!u || !u->access_token_refresh_required()) { become_active(); m_state_mutex.unlock(lock); return; @@ -1351,15 +1341,6 @@ std::string SyncSession::get_appservices_connection_id() const return m_session->get_appservices_connection_id(); } -std::optional SyncSession::get_sync_route() const -{ - util::CheckedLockGuard lk(m_state_mutex); - if (!m_sync_manager) { - return std::nullopt; - } - return m_sync_manager->sync_route(); -} - void SyncSession::update_configuration(SyncConfig new_config) { while (true) { diff --git a/src/realm/object-store/sync/sync_session.hpp b/src/realm/object-store/sync/sync_session.hpp index ceed4dac480..ac564b06ab1 100644 --- a/src/realm/object-store/sync/sync_session.hpp +++ b/src/realm/object-store/sync/sync_session.hpp @@ -261,14 +261,35 @@ class SyncSession : public std::enable_shared_from_this { return *m_config.sync_config; } - // If the `SyncSession` has been configured, the full remote URL of the Realm - // this `SyncSession` represents. - util::Optional full_realm_url() const REQUIRES(!m_config_mutex) + // When App is created, it will provide a generated websocket sync route when the SyncManager is + // created. When the next AppServices HTTP request is made via the App object, the location info + // will be requested and a verified websocket sync route will be provided to the SyncManager. If + // the SyncSession is started with a cached user, the websocket connection will be initially + // established using the generated sync route. If that connection is successful, then the sync + // route will be marked verified and used from that point on. If that connection fails, the + // SyncSession will update the location via an access token update to retrieve the verified + // sync route from the server's location info. The SyncSessio will then be restarted so it + // uses the updated sync route value. + + // If the SyncSession is active, this function returns the sync route that is being used + // by the current underlying session to connect to the server. + std::string full_realm_url() const REQUIRES(!m_config_mutex) { util::CheckedLockGuard lock(m_config_mutex); return m_server_url; } + // If the sync route value was returned by querying the location information, or the + // SyncSession has successfully connected to the server using the configured sync route, + // then the sync route will be marked "verified". Otherwise, the location information will + // be requested from the server if the connection fails when trying to open a websocket + // to the server. + bool realm_url_verified() const REQUIRES(!m_config_mutex) + { + util::CheckedLockGuard lock(m_config_mutex); + return m_server_url_verified; + } + std::shared_ptr get_flx_subscription_store() REQUIRES(!m_state_mutex); // Create an external reference to this session. The sync session attempts to remain active @@ -427,7 +448,6 @@ class SyncSession : public std::enable_shared_from_this { REQUIRES(m_state_mutex); std::string get_appservices_connection_id() const REQUIRES(!m_state_mutex); - std::optional get_sync_route() const REQUIRES(!m_state_mutex); util::Future send_test_command(std::string body) REQUIRES(!m_state_mutex); @@ -445,6 +465,10 @@ class SyncSession : public std::enable_shared_from_this { // Return the subscription_store_base - to be used only for testing std::shared_ptr get_subscription_store_base() REQUIRES(!m_state_mutex); + // Updates the connection state for this SyncSession and notify any registered callbacks if changed. + // Also ensures server_url_verified is set if the connection state is updated to connected. + void update_connection_state(ConnectionState new_state) REQUIRES(!m_config_mutex, !m_connection_state_mutex); + util::CheckedMutex m_state_mutex; util::CheckedMutex m_connection_state_mutex; @@ -486,7 +510,8 @@ class SyncSession : public std::enable_shared_from_this { std::unique_ptr m_session GUARDED_BY(m_state_mutex); // The fully-resolved URL of this Realm, including the server and the path. - util::Optional m_server_url GUARDED_BY(m_config_mutex); + std::string m_server_url GUARDED_BY(m_config_mutex); + bool m_server_url_verified GUARDED_BY(m_config_mutex) = false; _impl::SyncProgressNotifier m_progress_notifier; ConnectionChangeNotifier m_connection_change_notifier; diff --git a/src/realm/sync/client.cpp b/src/realm/sync/client.cpp index 703808c87f1..2a35c746aab 100644 --- a/src/realm/sync/client.cpp +++ b/src/realm/sync/client.cpp @@ -147,6 +147,7 @@ class SessionWrapper final : public util::AtomicRefCountBase, DB::CommitListener const ProtocolEnvelope m_protocol_envelope; const std::string m_server_address; const port_type m_server_port; + const bool m_server_verified; const std::string m_user_id; const SyncServerMode m_sync_mode; const std::string m_authorization_header_name; @@ -1150,6 +1151,7 @@ SessionWrapper::SessionWrapper(ClientImpl& client, DBRef db, std::shared_ptradd_commit_listener(this); } diff --git a/src/realm/sync/client.hpp b/src/realm/sync/client.hpp index f9e024efbde..15667eeb11f 100644 --- a/src/realm/sync/client.hpp +++ b/src/realm/sync/client.hpp @@ -192,6 +192,11 @@ class Session { /// to file system paths, and thus, these restrictions do not apply. std::string realm_identifier = ""; + // If the client has successfully contacted the server, then this will be + // set to true, otherwise it is false and the sync sessions will attempt + // to update the location info if the connection fails. + bool server_verified = false; + /// The user id of the logged in user for this sync session. This will be used /// along with the server_address/server_port/protocol_envelope to determine /// which connection to the server this session will use. diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 1441536dfa4..1d73f893368 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -525,6 +525,11 @@ bool Connection::websocket_closed_handler(bool was_clean, WebSocketError error_c case WebSocketError::websocket_connection_failed: { SessionErrorInfo error_info( {ErrorCodes::SyncConnectFailed, util::format("Failed to connect to sync: %1", msg)}, IsFatal{false}); + // If the connection fails/times out and the server has not been contacted yet, refresh the location + // to make sure the websocket URL is correct + if (!m_server_endpoint.is_verified) { + error_info.server_requests_action = ProtocolErrorInfo::Action::RefreshLocation; + } involuntary_disconnect(std::move(error_info), ConnectionTerminationReason::connect_operation_failed); break; } @@ -579,8 +584,19 @@ bool Connection::websocket_closed_handler(bool was_clean, WebSocketError error_c break; } case WebSocketError::websocket_fatal_error: { - involuntary_disconnect(SessionErrorInfo({ErrorCodes::ConnectionClosed, msg}, IsFatal{true}), - ConnectionTerminationReason::http_response_says_fatal_error); + // Error is fatal if the sync_route has already been verified - if the sync_route has not + // been verified, then use a non-fatal error and try to perform a location update. + SessionErrorInfo error_info( + {ErrorCodes::SyncConnectFailed, util::format("Failed to connect to sync: %1", msg)}, + IsFatal{m_server_endpoint.is_verified}); + ConnectionTerminationReason reason = ConnectionTerminationReason::http_response_says_fatal_error; + // If the connection fails/times out and the server has not been contacted yet, refresh the location + // to make sure the websocket URL is correct + if (!m_server_endpoint.is_verified) { + error_info.server_requests_action = ProtocolErrorInfo::Action::RefreshLocation; + reason = ConnectionTerminationReason::connect_operation_failed; + } + involuntary_disconnect(std::move(error_info), reason); break; } case WebSocketError::websocket_forbidden: { @@ -806,10 +822,14 @@ void Connection::handle_connect_wait(Status status) REALM_ASSERT_EX(m_state == ConnectionState::connecting, m_state); logger.info("Connect timeout"); // Throws - involuntary_disconnect( - SessionErrorInfo{Status{ErrorCodes::SyncConnectTimeout, "Sync connection was not fully established in time"}, - IsFatal{false}}, - ConnectionTerminationReason::sync_connect_timeout); // Throws + SessionErrorInfo error_info({ErrorCodes::SyncConnectTimeout, "Sync connection was not fully established in time"}, + IsFatal{false}); + // If the connection fails/times out and the server has not been contacted yet, refresh the location + // to make sure the websocket URL is correct + if (!m_server_endpoint.is_verified) { + error_info.server_requests_action = ProtocolErrorInfo::Action::RefreshLocation; + } + involuntary_disconnect(std::move(error_info), ConnectionTerminationReason::sync_connect_timeout); // Throws } @@ -819,6 +839,7 @@ void Connection::handle_connection_established() m_connect_timer.reset(); m_state = ConnectionState::connected; + m_server_endpoint.is_verified = true; // sync route is valid since connection is successful milliseconds_type now = monotonic_clock_now(); m_pong_wait_started_at = now; // Initially, no time was spent waiting for a PONG message diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index 13b0abf38ba..a354e74761a 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -42,6 +42,7 @@ struct ServerEndpoint { network::Endpoint::port_type port; std::string user_id; SyncServerMode server_mode = SyncServerMode::PBS; + bool is_verified = false; private: auto to_tuple() const @@ -691,7 +692,7 @@ class ClientImpl::Connection { OutputBuffer m_output_buffer; const connection_ident_type m_ident; - const ServerEndpoint m_server_endpoint; + ServerEndpoint m_server_endpoint; std::string m_appservices_coid; /// DEPRECATED - These will be removed in a future release diff --git a/test/object-store/c_api/c_api.cpp b/test/object-store/c_api/c_api.cpp index 28ab82bf9e4..41f0731d5f2 100644 --- a/test/object-store/c_api/c_api.cpp +++ b/test/object-store/c_api/c_api.cpp @@ -584,7 +584,6 @@ TEST_CASE("C API (non-database)", "[c_api]") { const uint64_t request_timeout = 2500; std::string base_url = "https://path/to/app"; std::string base_url2 = "https://some/other/path"; - std::string default_base_url = "https://realm.mongodb.com"; auto transport = std::make_shared(request_timeout); transport->set_expected_options({{"device", {{"appId", "app_id_123"}, @@ -606,6 +605,8 @@ TEST_CASE("C API (non-database)", "[c_api]") { CHECK(app_config->app_id == "app_id_123"); CHECK(app_config->transport == transport); + CHECK(realm_app_get_default_base_url() == app::App::default_base_url); + CHECK(!app_config->base_url); realm_app_config_set_base_url(app_config.get(), base_url.c_str()); CHECK(app_config->base_url == base_url); @@ -665,14 +666,14 @@ TEST_CASE("C API (non-database)", "[c_api]") { auto token = realm_sync_user_on_state_change_register_callback(sync_user, user_state, nullptr, user_data_free); - auto check_base_url = [&](const std::string& expected) { + auto check_base_url = [&](const std::string_view expected) { CHECK(transport->get_location_called()); auto app_base_url = realm_app_get_base_url(test_app.get()); CHECK(app_base_url == expected); realm_free(app_base_url); }; - auto update_and_check_base_url = [&](const char* new_base_url, const std::string& expected) { + auto update_and_check_base_url = [&](const char* new_base_url, const std::string_view expected) { transport->set_base_url(expected); realm_app_update_base_url( test_app.get(), new_base_url, @@ -694,13 +695,13 @@ TEST_CASE("C API (non-database)", "[c_api]") { check_base_url(base_url); // Reset to the default base url using nullptr - update_and_check_base_url(nullptr, default_base_url); + update_and_check_base_url(nullptr, app::App::default_base_url); // Set to some other base url update_and_check_base_url(base_url2.c_str(), base_url2); // Reset to default base url using empty string - update_and_check_base_url("", default_base_url); + update_and_check_base_url("", app::App::default_base_url); realm_release(sync_user); realm_release(token); diff --git a/test/object-store/sync/app.cpp b/test/object-store/sync/app.cpp index 92e13b89a2d..3ee45661600 100644 --- a/test/object-store/sync/app.cpp +++ b/test/object-store/sync/app.cpp @@ -2585,50 +2585,6 @@ TEST_CASE("app: make distributable client file", "[sync][pbs][app][baas]") { } } -struct HookedSocketProvider : public sync::websocket::DefaultSocketProvider { - HookedSocketProvider(const std::shared_ptr& logger, const std::string user_agent, - AutoStart auto_start = AutoStart{true}) - : DefaultSocketProvider(logger, user_agent, nullptr, auto_start) - { - } - - std::unique_ptr connect(std::unique_ptr observer, - sync::WebSocketEndpoint&& endpoint) override - { - using WebSocketError = sync::websocket::WebSocketError; - - int status_code = 101; - bool was_clean = true; - WebSocketError ws_error = WebSocketError::websocket_ok; - std::string body; - - sync::WebSocketEndpoint ep{std::move(endpoint)}; - if (endpoint_verify_func) { - endpoint_verify_func(ep); - } - - if (force_failure_func && force_failure_func(was_clean, ws_error, body)) { - observer->websocket_error_handler(); - observer->websocket_closed_handler(was_clean, ws_error, body); - return nullptr; - } - - bool use_simulated_response = websocket_connect_func && websocket_connect_func(status_code, body); - auto websocket = DefaultSocketProvider::connect(std::move(observer), std::move(ep)); - if (use_simulated_response) { - auto default_websocket = static_cast(websocket.get()); - if (default_websocket) - default_websocket->force_handshake_response_for_testing(status_code, body); - } - return websocket; - } - - std::function endpoint_verify_func; - std::function - force_failure_func; - std::function websocket_connect_func; -}; - TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { auto logger = util::Logger::get_default_logger(); @@ -2728,63 +2684,7 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { REQUIRE(dogs.get(0).get("name") == "fido"); } } - - class HookedTransport : public SynchronousTestTransport { - public: - void send_request_to_server(const Request& request, - util::UniqueFunction&& completion) override - { - if (request_hook) { - if (auto simulated_response = request_hook(request)) { - return completion(*simulated_response); - } - } - SynchronousTestTransport::send_request_to_server(request, [&](const Response& response) mutable { - if (response_hook) { - response_hook(request, response); - } - completion(response); - }); - } - // Optional handler for the request and response before it is returned to completion - std::function response_hook; - // Optional handler for the request before it is sent to the server - std::function(const Request&)> request_hook; - }; - - struct HookedSocketProvider : public sync::websocket::DefaultSocketProvider { - HookedSocketProvider(const std::shared_ptr& logger, const std::string user_agent, - AutoStart auto_start = AutoStart{true}) - : DefaultSocketProvider(logger, user_agent, nullptr, auto_start) - { - } - - std::unique_ptr connect(std::unique_ptr observer, - sync::WebSocketEndpoint&& endpoint) override - { - auto simulated_response = websocket_connect_simulated_response_func - ? websocket_connect_simulated_response_func() - : std::nullopt; - - if (websocket_endpoint_resolver) { - endpoint = websocket_endpoint_resolver(std::move(endpoint)); - } - std::unique_ptr websocket = - DefaultSocketProvider::connect(std::move(observer), std::move(endpoint)); - if (simulated_response) { - auto default_websocket = static_cast(websocket.get()); - if (default_websocket) - default_websocket->force_handshake_response_for_testing(*simulated_response, ""); - } - return websocket; - } - - std::function()> websocket_connect_simulated_response_func; - std::function websocket_endpoint_resolver; - }; - { - std::unique_ptr app_session; auto redir_transport = std::make_shared(); AutoVerifiedEmailCredentials creds; @@ -2952,7 +2852,6 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { } } SECTION("Test app redirect with no metadata") { - std::unique_ptr app_session; auto redir_transport = std::make_shared(); AutoVerifiedEmailCredentials creds, creds2; @@ -3006,8 +2905,9 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { creds.email, creds.password, [&](util::Optional error) { REQUIRE(!error); }); - REQUIRE(redir_app->sync_manager()->sync_route()); - REQUIRE(redir_app->sync_manager()->sync_route()->find(original_ws_host) != std::string::npos); + auto [sync_route, verified] = app->sync_manager()->sync_route(); + REQUIRE(sync_route.find(original_ws_host) != std::string::npos); + REQUIRE(verified); // Register another email address and verify location data isn't requested again request_count = 0; @@ -3082,13 +2982,17 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { auto sync_manager = test_session.sync_manager(); auto sync_session = sync_manager->get_existing_session(r->config().path); sync_session->pause(); + SyncManager::OnlyForTesting::voluntary_disconnect_all_connections(*sync_manager); int connect_count = 0; - redir_provider->websocket_connect_simulated_response_func = [&connect_count]() -> std::optional { - if (connect_count++ > 0) - return std::nullopt; - - return static_cast(sync::HTTPStatus::PermanentRedirect); + redir_provider->websocket_connect_func = [&logger, + &connect_count]() -> std::optional { + logger->trace("websocket connect (%1)", ++connect_count); + if (connect_count == 1) + return SocketProviderError(sync::HTTPStatus::PermanentRedirect); + if (connect_count == 2) + return SocketProviderError(sync::websocket::WebSocketError::websocket_moved_permanently); + return std::nullopt; }; redir_provider->websocket_endpoint_resolver = [&](sync::WebSocketEndpoint&& ep) { if (connect_count < 2) { @@ -3134,16 +3038,16 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { } return std::nullopt; }; - - SyncManager::OnlyForTesting::voluntary_disconnect_all_connections(*sync_manager); sync_session->resume(); REQUIRE(!wait_for_download(*r)); REQUIRE(user1->is_logged_in()); // Verify session is using the updated server url from the redirect auto server_url = sync_session->full_realm_url(); - logger->trace("FULL_REALM_URL: %1", server_url); - REQUIRE((server_url && server_url->find(redirect_host) != std::string::npos)); + auto verified = sync_session->realm_url_verified(); + logger->trace("FULL_REALM_URL: %1 (%2)", server_url, verified ? "verified" : "not verified"); + REQUIRE((server_url.find(redirect_host) != std::string::npos)); + REQUIRE(verified); } SECTION("Websocket redirect logs out user") { auto sync_manager = test_session.sync_manager(); @@ -3151,11 +3055,11 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { sync_session->pause(); int connect_count = 0; - redir_provider->websocket_connect_simulated_response_func = [&connect_count]() -> std::optional { + redir_provider->websocket_connect_func = [&connect_count]() -> std::optional { if (connect_count++ > 0) return std::nullopt; - return static_cast(sync::HTTPStatus::MovedPermanently); + return SocketProviderError(sync::HTTPStatus::MovedPermanently); }; int request_count = 0; redir_transport->request_hook = [&](const Request& request) -> std::optional { @@ -3205,11 +3109,11 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { sync_session->pause(); int connect_count = 0; - redir_provider->websocket_connect_simulated_response_func = [&connect_count]() -> std::optional { + redir_provider->websocket_connect_func = [&connect_count]() -> std::optional { if (connect_count++ > 0) return std::nullopt; - return static_cast(sync::HTTPStatus::MovedPermanently); + return SocketProviderError(sync::HTTPStatus::MovedPermanently); }; int request_count = 0; const int max_http_redirects = 20; // from app.cpp in object-store @@ -3247,6 +3151,107 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") { } } + SECTION("Test websocket location update with invalid ws host url") { + std::string configured_app_url = get_base_url(); + std::string original_host = configured_app_url.substr(configured_app_url.find("://") + 3); + original_host = original_host.substr(0, original_host.find("/")); + std::string original_address = original_host; + uint16_t original_port = 443; + if (auto port_pos = original_host.find(":"); port_pos != std::string::npos) { + auto original_port_str = original_host.substr(port_pos + 1); + + original_port = strtol(original_port_str.c_str(), nullptr, 10); + original_address = original_host.substr(0, port_pos); + } + + auto redir_transport = std::make_shared(); + auto redir_provider = std::make_shared(logger, ""); + redir_provider->websocket_endpoint_resolver = [&](sync::WebSocketEndpoint&& ep) { + ep.address = original_address; + ep.port = original_port; + return ep; + }; + + // Create App and User and log in + auto server_app_config = minimal_app_config("websocket_location_update", schema); + TestAppSession test_session(create_app(server_app_config), redir_transport, DeleteApp{true}, + realm::ReconnectMode::normal, redir_provider); + auto partition = random_string(100); + + { + // Open the realm with the current user + auto user = test_session.app()->current_user(); + REQUIRE(user); + SyncTestFile r_config(user, partition, schema); + auto r = Realm::get_shared_realm(r_config); + REQUIRE(!wait_for_download(*r)); + } + + // Close the app + test_session.close(); + + // Set the base URL to an invalid value + std::string fake_host = "http://fakerealm.example.com:1234"; + test_session.app_config.base_url = fake_host; + test_session.sc_config.multiplex_sessions = GENERATE(true, false); + + // Set up the socket provider and transport to report the correct server + int connect_count = 0; + redir_provider->websocket_endpoint_resolver = [&connect_count](sync::WebSocketEndpoint&& ep) { + // Verify the first websocket attempt uses the iniital fake URL + ++connect_count; + if (connect_count == 1) { + REQUIRE(ep.address == "fakerealm.example.com"); + REQUIRE(ep.port == 1234); + } + return ep; + }; + + redir_provider->websocket_connect_func = [&connect_count]() -> std::optional { + if (connect_count == 1) { + return SocketProviderError(sync::websocket::WebSocketError::websocket_fatal_error); + } + return std::nullopt; + }; + + int request_count = 0; + redir_transport->request_hook = [&](const Request& request) -> std::optional { + logger->trace("request.url (%1): %2", request_count, request.url); + if (request_count++ == 0) { + // First request should be a location request against the original URL + REQUIRE(request.url.find(fake_host) != std::string::npos); + REQUIRE(request.url.find("/location") != std::string::npos); + return Response{static_cast(sync::HTTPStatus::PermanentRedirect), + 0, + {{"Location", configured_app_url}, {"Content-Type", "application/json"}}, + "Some body data"}; + } + return std::nullopt; + }; + + // Reopen the app, but don't log in at this point + test_session.reopen(false); + + // Open the realm with the cached user + int num_realms = GENERATE(1, 3); + auto user = test_session.app()->current_user(); + REQUIRE(user); + // Verify no transport calls have been made at this point since the + // app was re-opened. + REQUIRE(request_count == 0); + SyncTestFile r_config(user, partition, schema); + + std::vector realms; + for (int i = 0; i < num_realms; i++) { + SyncTestFile r_config(user, partition, schema); + realms.push_back(Realm::get_shared_realm(r_config)); + } + // wait for download + for (auto& realm : realms) { + REQUIRE(!wait_for_download(*realm)); + } + } + SECTION("Fast clock on client") { { SyncTestFile config(app, partition, schema); @@ -3861,13 +3866,13 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { struct BaseUrlTransport : GenericNetworkTransport { std::string expected_url; - std::optional redirect_url; + std::optional redirect_url; bool location_requested = false; bool location_returns_error = false; - void reset(std::string expect_url, std::optional redir_url = std::nullopt) + void reset(std::string_view expect_url, std::optional redir_url = std::nullopt) { - expected_url = expect_url; + expected_url = std::string(expect_url); redirect_url = redir_url; location_requested = false; location_returns_error = false; @@ -3899,7 +3904,7 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { } if (redirect_url) { // Update the expected url to be the redirect url - expected_url = *redirect_url; + expected_url = std::string(*redirect_url); redirect_url.reset(); completion(app::Response{static_cast(sync::HTTPStatus::PermanentRedirect), @@ -3908,8 +3913,7 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { "308 permanent redirect"}); return; } - auto ws_url = expected_url; - ws_url.replace(0, 4, "ws"); + auto ws_url = App::create_ws_host_url(expected_url); completion( app::Response{static_cast(sync::HTTPStatus::Ok), 0, @@ -3945,23 +3949,62 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { }); }; + SECTION("Test App::create_ws_host_url") { + auto result = App::create_ws_host_url("blah"); + CHECK(result == "blah"); + result = App::create_ws_host_url("http://localhost:9090"); + CHECK(result == "ws://localhost:9090"); + result = App::create_ws_host_url("https://localhost:9090"); + CHECK(result == "wss://localhost:9090"); + result = App::create_ws_host_url("https://localhost:9090/some/extra/stuff"); + CHECK(result == "wss://localhost:9090/some/extra/stuff"); + result = App::create_ws_host_url("http://172.0.0.1:9090"); + CHECK(result == "ws://172.0.0.1:9090"); + result = App::create_ws_host_url("https://172.0.0.1:9090"); + CHECK(result == "wss://172.0.0.1:9090"); + result = App::create_ws_host_url("http://realm.mongodb.com"); + CHECK(result == "ws://ws.realm.mongodb.com"); + result = App::create_ws_host_url("https://realm.mongodb.com"); + CHECK(result == "wss://ws.realm.mongodb.com"); + result = App::create_ws_host_url("https://realm.mongodb.com/some/extra/stuff"); + CHECK(result == "wss://ws.realm.mongodb.com/some/extra/stuff"); + result = App::create_ws_host_url("https://us-east-1.aws.realm.mongodb.com"); + CHECK(result == "wss://ws.us-east-1.aws.realm.mongodb.com"); + result = App::create_ws_host_url("https://us-east-1.aws.realm.mongodb.com"); + CHECK(result == "wss://ws.us-east-1.aws.realm.mongodb.com"); + result = App::create_ws_host_url("https://us-east-1.aws.realm.mongodb.com/some/extra/stuff"); + CHECK(result == "wss://ws.us-east-1.aws.realm.mongodb.com/some/extra/stuff"); + result = App::create_ws_host_url("http://services.cloud.mongodb.com"); + CHECK(result == "ws://ws.services.cloud.mongodb.com"); + result = App::create_ws_host_url("https://services.cloud.mongodb.com"); + CHECK(result == "wss://ws.services.cloud.mongodb.com"); + result = App::create_ws_host_url("https://services.cloud.mongodb.com/some/extra/stuff"); + CHECK(result == "wss://ws.services.cloud.mongodb.com/some/extra/stuff"); + result = App::create_ws_host_url("http://us-east-1.aws.services.cloud.mongodb.com"); + CHECK(result == "ws://us-east-1.aws.ws.services.cloud.mongodb.com"); + result = App::create_ws_host_url("https://us-east-1.aws.services.cloud.mongodb.com"); + CHECK(result == "wss://us-east-1.aws.ws.services.cloud.mongodb.com"); + result = App::create_ws_host_url("https://us-east-1.aws.services.cloud.mongodb.com/some/extra/stuff"); + CHECK(result == "wss://us-east-1.aws.ws.services.cloud.mongodb.com/some/extra/stuff"); + } + SECTION("Test app config baseurl") { { - redir_transport->reset("https://realm.mongodb.com"); + redir_transport->reset(App::default_base_url); // First time through, base_url is empty; https://realm.mongodb.com is expected auto app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); // Location is not requested until first app services request CHECK(!redir_transport->location_requested); // Initial hostname and ws hostname use base url, but aren't used until location is updated - CHECK(app->get_host_url() == "https://realm.mongodb.com"); - CHECK(app->get_ws_host_url() == "wss://realm.mongodb.com"); + CHECK(app->get_host_url() == App::default_base_url); + CHECK(app->get_ws_host_url() == App::create_ws_host_url(App::default_base_url)); do_login(app); CHECK(redir_transport->location_requested); - CHECK(app->get_base_url() == "https://realm.mongodb.com"); - CHECK(app->get_host_url() == "https://realm.mongodb.com"); - CHECK(app->get_ws_host_url() == "wss://realm.mongodb.com"); + CHECK(app->get_base_url() == App::default_base_url); + CHECK(app->get_host_url() == App::default_base_url); + CHECK(app->get_ws_host_url() == App::create_ws_host_url(App::default_base_url)); } { // Second time through, base_url is set to https://alternate.someurl.fake is expected @@ -3985,8 +4028,8 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { // Third time through, base_url is not set, expect https://realm.mongodb.com, since metadata // is no longer used app_config.base_url = util::none; - std::string expected_url = "https://realm.mongodb.com"; - std::string expected_wsurl = "wss://realm.mongodb.com"; + std::string expected_url = std::string(App::default_base_url); + std::string expected_wsurl = App::create_ws_host_url(App::default_base_url); redir_transport->reset(expected_url); auto app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); @@ -4039,17 +4082,17 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { CHECK(app->get_host_url() == "https://alternate.someurl.fake"); CHECK(app->get_ws_host_url() == "wss://alternate.someurl.fake"); - redir_transport->reset("https://realm.mongodb.com"); + redir_transport->reset(App::default_base_url); // Revert the base URL to the default URL value using std::nullopt app->update_base_url(std::nullopt, [](util::Optional error) { CHECK(!error); }); CHECK(redir_transport->location_requested); - CHECK(app->get_base_url() == "https://realm.mongodb.com"); - CHECK(app->get_host_url() == "https://realm.mongodb.com"); - CHECK(app->get_ws_host_url() == "wss://realm.mongodb.com"); - // Expected URL is still "https://realm.mongodb.com" + CHECK(app->get_base_url() == App::default_base_url); + CHECK(app->get_host_url() == App::default_base_url); + CHECK(app->get_ws_host_url() == App::create_ws_host_url(App::default_base_url)); + // Expected URL is still App::default_base_url do_login(app); redir_transport->reset("http://some-other.url.fake"); @@ -4063,17 +4106,17 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { // Expected URL is still "http://some-other.url.fake" do_login(app); - redir_transport->reset("https://realm.mongodb.com"); + redir_transport->reset(App::default_base_url); // Revert the base URL to the default URL value using the empty string app->update_base_url("", [](util::Optional error) { CHECK(!error); }); CHECK(redir_transport->location_requested); - CHECK(app->get_base_url() == "https://realm.mongodb.com"); - CHECK(app->get_host_url() == "https://realm.mongodb.com"); - CHECK(app->get_ws_host_url() == "wss://realm.mongodb.com"); - // Expected URL is still "https://realm.mongodb.com" + CHECK(app->get_base_url() == App::default_base_url); + CHECK(app->get_host_url() == App::default_base_url); + CHECK(app->get_ws_host_url() == App::create_ws_host_url(App::default_base_url)); + // Expected URL is still App::default_base_url do_login(app); } } @@ -4136,29 +4179,21 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { } } - // Verify new sync session updates location after app created with cached user + // Verify new sync session updates location when created with cached user SECTION("Verify new sync session updates location") { bool use_ssl = GENERATE(true, false); + std::string initial_host = "alternate.someurl.fake"; + unsigned initial_port = use_ssl ? 443 : 80; std::string expected_host = "redirect.someurl.fake"; unsigned expected_port = 8081; - std::string init_url = util::format("http%1://alternate.someurl.fake", use_ssl ? "s" : ""); - std::string init_wsurl = util::format("ws%1://alternate.someurl.fake", use_ssl ? "s" : ""); + std::string init_url = util::format("http%1://%2", use_ssl ? "s" : "", initial_host); + std::string init_wsurl = util::format("ws%1://%2", use_ssl ? "s" : "", initial_host); std::string redir_url = util::format("http%1://%2:%3", use_ssl ? "s" : "", expected_host, expected_port); std::string redir_wsurl = util::format("ws%1://%2:%3", use_ssl ? "s" : "", expected_host, expected_port); auto socket_provider = std::make_shared(logger, "some user agent"); - socket_provider->endpoint_verify_func = [&use_ssl, &expected_host, - &expected_port](sync::WebSocketEndpoint& ep) { - CHECK(ep.address == expected_host); - CHECK(ep.port == expected_port); - CHECK(ep.is_ssl == use_ssl); - }; - socket_provider->force_failure_func = [](bool& was_clean, sync::websocket::WebSocketError& error_code, - std::string& message) { - was_clean = false; - error_code = sync::websocket::WebSocketError::websocket_connection_failed; - message = "404 not found"; - return true; + socket_provider->websocket_connect_func = []() -> std::optional { + return SocketProviderError(sync::websocket::WebSocketError::websocket_connection_failed, "404 not found"); }; sc_config.socket_provider = socket_provider; @@ -4170,44 +4205,57 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { redir_transport->reset(init_url); auto app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); - // At this point, the sync route is not set - CHECK(!app->sync_manager()->sync_route()); + + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(init_url)) != std::string::npos); + CHECK_FALSE(verified); + } do_login(app); CHECK(redir_transport->location_requested); CHECK(app->get_base_url() == init_url); CHECK(app->get_host_url() == init_url); CHECK(app->get_ws_host_url() == init_wsurl); - CHECK(app->sync_manager()->sync_route()); - CHECK(app->sync_manager()->sync_route()->find(init_wsurl) != std::string::npos); + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(init_url)) != std::string::npos); + CHECK(verified); + } } - // Recreate the app using the cached user and start a sync session, which will is set to fail on connect - SECTION("Sync Session fails on connect") { + // Recreate the app using the cached user and start a sync session, which is set to fail on connect + SECTION("Sync Session fails on connect after updating location") { enum class TestState { start, session_started }; TestingStateMachine state(TestState::start); redir_transport->reset(init_url, redir_url); auto app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); - // At this point, the sync route is not set - CHECK(!app->sync_manager()->sync_route()); + // Verify the default sync route, which has not been verified + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(init_url)) != std::string::npos); + CHECK_FALSE(verified); + } + + socket_provider->endpoint_verify_func = [&use_ssl, &expected_host, + &expected_port](const sync::WebSocketEndpoint& ep) { + CHECK(ep.address == expected_host); + CHECK(ep.port == expected_port); + CHECK(ep.is_ssl == use_ssl); + }; RealmConfig r_config; r_config.path = sc_config.base_file_path + "/fakerealm.realm"; r_config.sync_config = std::make_shared(app->current_user(), SyncConfig::FLXSyncEnabled{}); r_config.sync_config->error_handler = [&state, &logger](std::shared_ptr, SyncError error) mutable { - // Expect an error due to 404 response when creating websocket - state.transition_with([&error, &logger](TestState cur_state) -> std::optional { - if (cur_state == TestState::start) { - // The session will start, but the connection is rejected on purpose - logger->debug("Expected error: %1", error.status); - CHECK(!error.status.is_ok()); - CHECK(error.status.code() == ErrorCodes::SyncConnectFailed); - return TestState::session_started; - } - return std::nullopt; - }); + // Websocket is forcing a 404 failure so it won't actually start + logger->debug("Received expected error: %1", error.status); + CHECK(!error.status.is_ok()); + CHECK(error.status.code() == ErrorCodes::SyncConnectFailed); + CHECK(!error.is_fatal); + state.transition_to(TestState::session_started); }; auto realm = Realm::get_shared_realm(r_config); state.wait_for(TestState::session_started); @@ -4216,69 +4264,104 @@ TEST_CASE("app: base_url", "[sync][app][base_url]") { CHECK(app->get_base_url() == init_url); CHECK(app->get_host_url() == redir_url); CHECK(app->get_ws_host_url() == redir_wsurl); - CHECK(app->sync_manager()->sync_route()); - CHECK(app->sync_manager()->sync_route()->find(redir_wsurl) != std::string::npos); + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(redir_url)) != std::string::npos); + CHECK(verified); + } } - // Recreate the app using the cached user and start a sync session, which will fail during location update - SECTION("Location update fails prior to sync session connect") { - enum class TestState { start, location_failed, waiting_for_session, session_started }; + + SECTION("Sync Session retries after initial location failure") { + enum class TestState { start, location_failed, session_started }; TestingStateMachine state(TestState::start); + int retry_count = GENERATE(1, 3); - redir_transport->reset(init_url, redir_url); + redir_transport->reset(init_url); redir_transport->location_returns_error = true; auto app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); - // At this point, the sync route is not set - CHECK(!app->sync_manager()->sync_route()); + // Verify the default sync route, which has not been verified + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(init_url)) != std::string::npos); + CHECK_FALSE(verified); + } - RealmConfig r_config; - r_config.path = sc_config.base_file_path + "/fakerealm.realm"; - r_config.sync_config = std::make_shared(app->current_user(), SyncConfig::FLXSyncEnabled{}); - r_config.sync_config->error_handler = [&state, &logger](std::shared_ptr, - SyncError error) mutable { - // Expect an error due to location failed or 404 response when creating websocket - state.transition_with([&error, &logger](TestState cur_state) -> std::optional { - if (cur_state == TestState::start || cur_state == TestState::waiting_for_session) { - logger->debug("Expected error: %1: %2", error.status.code_string(), error.status.reason()); - CHECK(!error.status.is_ok()); - CHECK(error.status.code() == ErrorCodes::SyncConnectFailed); + socket_provider->endpoint_verify_func = [&use_ssl, &initial_host, + &initial_port](const sync::WebSocketEndpoint& ep) { + CHECK(ep.address == initial_host); + CHECK(ep.port == initial_port); + CHECK(ep.is_ssl == use_ssl); + }; + + socket_provider->websocket_connect_func = [&]() -> std::optional { + // Check these items prior to holding the lock in transition_with() + if (state.get() == TestState::start) { + logger->debug("State: start"); + // Verify the location update failed + CHECK(redir_transport->location_requested); + CHECK(app->get_base_url() == init_url); + CHECK(app->get_host_url() == init_url); + CHECK(app->get_ws_host_url() == init_wsurl); + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(init_url)) != std::string::npos); + CHECK_FALSE(verified); } + } + + state.transition_with([&](TestState cur_state) -> std::optional { if (cur_state == TestState::start) { - // The first time through, the location update fails - return TestState::location_failed; + // After number of location verify attempts has passed, let the location succeed + if (--retry_count <= 0) { + redir_transport->reset(init_url, redir_url); + socket_provider->endpoint_verify_func = + [&use_ssl, &expected_host, &expected_port](const sync::WebSocketEndpoint& ep) { + CHECK(ep.address == expected_host); + CHECK(ep.port == expected_port); + CHECK(ep.is_ssl == use_ssl); + }; + return TestState::location_failed; + } + redir_transport->location_requested = false; } - else if (cur_state == TestState::waiting_for_session) { - // The second time through, the session start, but the connection is rejected on purpose + return std::nullopt; + }); + + return SocketProviderError(sync::websocket::WebSocketError::websocket_connection_failed, + "404 not found"); + }; + + RealmConfig r_config; + r_config.path = sc_config.base_file_path + "/fakerealm.realm"; + r_config.sync_config = std::make_shared(app->current_user(), SyncConfig::FLXSyncEnabled{}); + r_config.sync_config->error_handler = [&](std::shared_ptr, SyncError error) mutable { + // An error will only be reported if the websocket fails after updating the location and access token + logger->debug("Received expected error: %1", error.status); + CHECK(!error.status.is_ok()); + CHECK(error.status.code() == ErrorCodes::SyncConnectFailed); + CHECK(!error.is_fatal); + state.transition_with([&](TestState cur_state) -> std::optional { + if (cur_state == TestState::location_failed) { + // This time, the session was being started, and the location was successful + // Websocket is forcing a 404 failure so it won't actually start return TestState::session_started; } return std::nullopt; }); }; auto realm = Realm::get_shared_realm(r_config); - state.wait_for(TestState::location_failed); - - CHECK(redir_transport->location_requested); - CHECK(app->get_base_url() == init_url); - // Location was never updated - CHECK(app->get_host_url() == init_url); - CHECK(app->get_ws_host_url() == init_wsurl); - CHECK(!app->sync_manager()->sync_route()); - - // Location request will pass this time, try to reconnect - // expecting 404 when websocket connects - redir_transport->reset(init_url, redir_url); - state.transition_to(TestState::waiting_for_session); - auto session = app->sync_manager()->get_existing_session(r_config.path); - CHECK(session); - session->resume(); state.wait_for(TestState::session_started); CHECK(redir_transport->location_requested); CHECK(app->get_base_url() == init_url); CHECK(app->get_host_url() == redir_url); CHECK(app->get_ws_host_url() == redir_wsurl); - CHECK(app->sync_manager()->sync_route()); - CHECK(app->sync_manager()->sync_route()->find(redir_wsurl) != std::string::npos); + { + auto [sync_route, verified] = app->sync_manager()->sync_route(); + CHECK(sync_route.find(app::App::create_ws_host_url(redir_url)) != std::string::npos); + CHECK(verified); + } } } } @@ -5721,7 +5804,7 @@ TEST_CASE("app: shared instances", "[sync][app]") { auto config2 = base_config; config2.app_id = "app1"; - config2.base_url = "https://realm.mongodb.com"; // equivalent to default_base_url + config2.base_url = std::string(App::default_base_url); auto config3 = base_config; config3.app_id = "app2"; diff --git a/test/object-store/sync/flx_sync.cpp b/test/object-store/sync/flx_sync.cpp index 3ac457c8e3e..ab1fadde4ed 100644 --- a/test/object-store/sync/flx_sync.cpp +++ b/test/object-store/sync/flx_sync.cpp @@ -2678,19 +2678,6 @@ TEST_CASE("flx: connect to PBS as FLX returns an error", "[sync][flx][protocol][ } TEST_CASE("flx: commit subscription while refreshing the access token", "[sync][flx][token][baas]") { - class HookedTransport : public SynchronousTestTransport { - public: - void send_request_to_server(const Request& request, - util::UniqueFunction&& completion) override - { - if (request_hook) { - request_hook(request); - } - SynchronousTestTransport::send_request_to_server(request, std::move(completion)); - } - util::UniqueFunction request_hook; - }; - auto transport = std::make_shared(); FLXSyncTestHarness harness("flx_wait_access_token2", FLXSyncTestHarness::default_server_schema(), transport); auto app = harness.app(); @@ -2721,6 +2708,7 @@ TEST_CASE("flx: commit subscription while refreshing the access token", "[sync][ mut_subs.commit(); } } + return std::nullopt; }; SyncTestFile config(harness.app()->current_user(), harness.schema(), SyncConfig::FLXSyncEnabled{}); // This triggers the token refresh. diff --git a/test/object-store/util/sync/baas_admin_api.hpp b/test/object-store/util/sync/baas_admin_api.hpp index faa6cd51623..c45a150bb45 100644 --- a/test/object-store/util/sync/baas_admin_api.hpp +++ b/test/object-store/util/sync/baas_admin_api.hpp @@ -36,7 +36,6 @@ #include namespace realm { -app::Response do_http_request(const app::Request& request); class AdminAPIEndpoint { public: @@ -260,30 +259,6 @@ struct AppSession { }; AppSession create_app(const AppCreateConfig& config); -class SynchronousTestTransport : public app::GenericNetworkTransport { -public: - void send_request_to_server(const app::Request& request, - util::UniqueFunction&& completion) override - { - { - std::lock_guard barrier(m_mutex); - } - completion(do_http_request(request)); - } - - void block() - { - m_mutex.lock(); - } - void unblock() - { - m_mutex.unlock(); - } - -private: - std::mutex m_mutex; -}; - // This will create a new test app in the baas server - base_url and admin_url // are automatically set AppSession get_runtime_app_session(); diff --git a/test/object-store/util/sync/sync_test_utils.cpp b/test/object-store/util/sync/sync_test_utils.cpp index ddcea52d4bd..5bec44aea65 100644 --- a/test/object-store/util/sync/sync_test_utils.cpp +++ b/test/object-store/util/sync/sync_test_utils.cpp @@ -260,12 +260,18 @@ AutoVerifiedEmailCredentials create_user_and_log_in(app::SharedApp app) creds.email, creds.password, [&](util::Optional error) { REQUIRE(!error); }); - app->log_in_with_credentials(realm::app::AppCredentials::username_password(creds.email, creds.password), + log_in_user(app, creds); + return creds; +} + +void log_in_user(app::SharedApp app, app::AppCredentials creds) +{ + REQUIRE(app); + app->log_in_with_credentials(creds, [&](std::shared_ptr user, util::Optional error) { REQUIRE(user); REQUIRE(!error); }); - return creds; } void wait_for_advance(Realm& realm) diff --git a/test/object-store/util/sync/sync_test_utils.hpp b/test/object-store/util/sync/sync_test_utils.hpp index c58c58de52d..89046211345 100644 --- a/test/object-store/util/sync/sync_test_utils.hpp +++ b/test/object-store/util/sync/sync_test_utils.hpp @@ -28,6 +28,9 @@ #include #include #include +#include +#include +#include #include #include @@ -155,12 +158,139 @@ struct AutoVerifiedEmailCredentials : app::AppCredentials { }; AutoVerifiedEmailCredentials create_user_and_log_in(app::SharedApp app); +// Log in the user again using the AutoVerifiedEmailCredentials returned +// when calling create_user_and_log_in() +void log_in_user(app::SharedApp app, app::AppCredentials creds); void wait_for_advance(Realm& realm); void async_open_realm(const Realm::Config& config, util::UniqueFunction finish); +app::Response do_http_request(const app::Request& request); + +class SynchronousTestTransport : public app::GenericNetworkTransport { +public: + void send_request_to_server(const app::Request& request, + util::UniqueFunction&& completion) override + { + { + std::lock_guard barrier(m_mutex); + } + completion(do_http_request(request)); + } + + void block() + { + m_mutex.lock(); + } + void unblock() + { + m_mutex.unlock(); + } + +private: + std::mutex m_mutex; +}; + + +class HookedTransport : public SynchronousTestTransport { +public: + void send_request_to_server(const app::Request& request, + util::UniqueFunction&& completion) override + { + if (request_hook) { + if (auto simulated_response = request_hook(request)) { + return completion(*simulated_response); + } + } + SynchronousTestTransport::send_request_to_server(request, [&](const app::Response& response) mutable { + if (response_hook) { + response_hook(request, response); + } + completion(response); + }); + } + + // Optional handler for the request and response before it is returned to completion + std::function response_hook; + // Optional handler for the request before it is sent to the server + std::function(const app::Request&)> request_hook; +}; + + +struct SocketProviderError { + SocketProviderError(sync::HTTPStatus code, std::string message = "") + : SocketProviderError(static_cast(code), message) + { + } + + SocketProviderError(int code, std::string message = "") + : status_code(code) + , was_clean(code == 101) + , body(message) + { + } + + using WebSocketError = sync::websocket::WebSocketError; + SocketProviderError(WebSocketError error, std::string message = "") + : was_clean(false) + , ws_error(error) + , body(message) + { + } + + int status_code = 0; + bool was_clean = true; + WebSocketError ws_error = WebSocketError::websocket_ok; + std::string body; +}; + + +struct HookedSocketProvider : public sync::websocket::DefaultSocketProvider { + HookedSocketProvider(const std::shared_ptr& logger, const std::string user_agent, + AutoStart auto_start = AutoStart{true}) + : DefaultSocketProvider(logger, user_agent, nullptr, auto_start) + { + } + + std::unique_ptr connect(std::unique_ptr observer, + sync::WebSocketEndpoint&& endpoint) override + { + std::optional error; + if (endpoint_verify_func) { + endpoint_verify_func(endpoint); + } + + if (websocket_endpoint_resolver) { + endpoint = websocket_endpoint_resolver(std::move(endpoint)); + } + + if (websocket_connect_func) { + error = websocket_connect_func(); + } + + if (error && error->ws_error != sync::websocket::WebSocketError::websocket_ok) { + observer->websocket_error_handler(); + observer->websocket_closed_handler(error->was_clean, error->ws_error, error->body); + return nullptr; + } + + std::unique_ptr websocket = + DefaultSocketProvider::connect(std::move(observer), std::move(endpoint)); + if (error && error->status_code > 0) { + auto default_websocket = dynamic_cast(websocket.get()); + if (default_websocket) + default_websocket->force_handshake_response_for_testing(error->status_code, error->body); + } + return websocket; + } + + std::function websocket_endpoint_resolver; + std::function endpoint_verify_func; + std::function()> websocket_connect_func; +}; + #endif // REALM_ENABLE_SYNC namespace reset_utils { diff --git a/test/object-store/util/test_file.cpp b/test/object-store/util/test_file.cpp index 048130cf40a..e4365668f07 100644 --- a/test/object-store/util/test_file.cpp +++ b/test/object-store/util/test_file.cpp @@ -337,11 +337,10 @@ TestAppSession::TestAppSession(AppSession session, { if (!m_transport) m_transport = instance_of; - auto app_config = get_config(m_transport, *m_app_session); + app_config = get_config(m_transport, *m_app_session); set_app_config_defaults(app_config, m_transport); util::try_make_dir(m_base_file_path); - SyncClientConfig sc_config; sc_config.base_file_path = m_base_file_path; sc_config.metadata_mode = realm::SyncManager::MetadataMode::NoEncryption; sc_config.reconnect_mode = reconnect_mode; @@ -355,23 +354,59 @@ TestAppSession::TestAppSession(AppSession session, // initialize sync client m_app->sync_manager()->get_sync_client(); - create_user_and_log_in(m_app); + user_creds = create_user_and_log_in(m_app); } TestAppSession::~TestAppSession() { - if (util::File::exists(m_base_file_path)) { - try { + close(true); + if (m_delete_app) { + m_app_session->admin_api.delete_app(m_app_session->server_app_id); + } +} + +void TestAppSession::close(bool tear_down) +{ + try { + if (tear_down) { + // If tearing down, make sure there's an app to work with + if (!m_app) { + reopen(false); + } + REALM_ASSERT(m_app); + // Clean up the app data m_app->sync_manager()->tear_down_for_testing(); - util::try_remove_dir_recursive(m_base_file_path); } - catch (const std::exception& ex) { - std::cerr << ex.what() << "\n"; + else if (m_app) { + // Otherwise, make sure all the session are closed + m_app->sync_manager()->close_all_sessions(); + } + m_app.reset(); + + // If tearing down, clean up the test file directory + if (tear_down && !m_base_file_path.empty() && util::File::exists(m_base_file_path)) { + util::try_remove_dir_recursive(m_base_file_path); + m_base_file_path.clear(); } - app::App::clear_cached_apps(); } - if (m_delete_app) { - m_app_session->admin_api.delete_app(m_app_session->server_app_id); + catch (const std::exception& ex) { + std::cerr << "Error tearing down TestAppSession: " << ex.what() << "\n"; + } + // Ensure all cached apps are cleared + app::App::clear_cached_apps(); +} + +void TestAppSession::reopen(bool log_in) +{ + // These are REALM_ASSERTs so the test crashes if this object is in a bad state + REALM_ASSERT(!m_base_file_path.empty()); + REALM_ASSERT(!m_app); + m_app = app::App::get_app(app::App::CacheMode::Disabled, app_config, sc_config); + + // initialize sync client + m_app->sync_manager()->get_sync_client(); + if (log_in) { + log_in_user(m_app, user_creds); } } diff --git a/test/object-store/util/test_file.hpp b/test/object-store/util/test_file.hpp index d3b50438e25..560d45d3dfb 100644 --- a/test/object-store/util/test_file.hpp +++ b/test/object-store/util/test_file.hpp @@ -270,18 +270,36 @@ class TestAppSession { } const std::shared_ptr& sync_manager() const { + REALM_ASSERT(m_app); return m_app->sync_manager(); } + void close() + { + close(false); + } + + // Re-open the app without deleting the dir contents - if close() has not been called + // the App will be closed first before recreating the object. + // If log_in is true, user will be logged in again once the App instance is created + void reopen(bool log_in); + + realm::app::App::Config app_config; + realm::SyncClientConfig sc_config; + std::vector get_documents(realm::SyncUser& user, const std::string& object_type, size_t expected_count) const; private: + // Close the app and, if tear_down, remove the app data and base_file_path directory + void close(bool tear_down); + std::shared_ptr m_app; std::unique_ptr m_app_session; std::string m_base_file_path; bool m_delete_app = true; std::shared_ptr m_transport; + realm::app::AppCredentials user_creds; }; #endif diff --git a/test/object-store/util/unit_test_transport.hpp b/test/object-store/util/unit_test_transport.hpp index 509e4187275..cbee53d1941 100644 --- a/test/object-store/util/unit_test_transport.hpp +++ b/test/object-store/util/unit_test_transport.hpp @@ -45,9 +45,9 @@ class UnitTestTransport : public realm::app::GenericNetworkTransport { static const std::string identity_0_id; static const std::string identity_1_id; - void set_base_url(const std::string& base_url) + void set_base_url(const std::string_view base_url) { - m_base_url = base_url; + m_base_url = std::string(base_url); m_location_called = false; } diff --git a/test/test_list.cpp b/test/test_list.cpp index ecd1d36ed49..7d1213afd94 100644 --- a/test/test_list.cpp +++ b/test/test_list.cpp @@ -633,6 +633,30 @@ TEST(List_AggOps) test_lists_numeric_agg(test_context, sg, type_Decimal, Decimal128(realm::null()), true); } +ONLY(Test_Write_List_Nested_InMixed) +{ + SHARED_GROUP_TEST_PATH(path); + std::string message; + DBOptions options; + options.logger = test_context.logger; + DBRef db = DB::create(make_in_realm_history(), path, options); + auto tr = db->start_write(); + auto table = tr->add_table_with_primary_key("table", type_Int, "id"); + auto col_any = table->add_column(type_Mixed, "something"); + + Obj obj = table->create_object_with_primary_key(1); + obj.set_any(col_any, Mixed{20}); + tr->verify(); + tr->commit_and_continue_writing(); // commit simple mixed + + obj.set_collection(col_any, CollectionType::List); + auto list = obj.get_list_ptr(col_any); + list->add(Mixed{10}); + list->add(Mixed{11}); + tr->verify(); + tr->commit(); // commit nested list in mixed +} + TEST(List_Nested_InMixed) { SHARED_GROUP_TEST_PATH(path);