Replace TestAppSession close()/reopen() with scope based TestAppSession instance (#7672)

Michael Wilkerson-Barker · web-flow · commit bd17af2f682f · 2024-08-12T08:30:19.000-04:00
* Updated TestAppSession to remove close/reopen and take a config structure
* Updated changelog and minor tweaks
* AutoVerify email address matters...
* Forgot to adjust format string
* Updated REQUIRE's in TestAppSession to REALM_ASSERTs due to thread sanitizer
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,7 +17,7 @@
 -----------
 
 ### Internals
-* None.
+* Update TestAppSession to allow scope-based usage for restarting the local app resources. ([PR #7672](https://github.com/realm/realm-core/pull/7672))
 
 ----------------------------------------------
 
diff --git a/test/object-store/c_api/c_api.cpp b/test/object-store/c_api/c_api.cpp
@@ -6412,7 +6412,7 @@ TEST_CASE("C API app: link_user integration w/c_api transport", "[sync][app][c_a
     auto user_data = new TestTransportUserData();
     auto http_transport = realm_http_transport_new(send_request_to_server, user_data, user_data_free);
     auto app_session = get_runtime_app_session();
-    TestAppSession session(app_session, *http_transport, DeleteApp{false});
+    TestAppSession session(app_session, {*http_transport}, DeleteApp{false});
     realm_app app(session.app());
 
     SECTION("remove_user integration") {
diff --git a/test/object-store/realm.cpp b/test/object-store/realm.cpp
@@ -1464,16 +1464,22 @@ TEST_CASE("Synchronized realm: AutoOpen", "[sync][baas][pbs][async open]") {
     std::mutex mutex;
 
     // Create the app session and get the logged in user identity
-    auto server_app_config = minimal_app_config("autoopen-realm", schema);
-    TestAppSession session(create_app(server_app_config), transport, DeleteApp{true}, realm::ReconnectMode::normal,
-                           socket_provider);
-    auto user = session.app()->current_user();
-    std::string identity = user->user_id();
-    REQUIRE(user->is_logged_in());
-    REQUIRE(!identity.empty());
-    // Reopen the App instance and retrieve the cached user
-    session.reopen(false);
-    user = session.app()->get_existing_logged_in_user(identity);
+    auto app_session = create_app(minimal_app_config("autoopen-realm", schema));
+    std::string identity;
+    TestAppSession::Config tas_config;
+    {
+        // Keep the app and realm storage
+        TestAppSession session(app_session, {transport, realm::ReconnectMode::normal, socket_provider},
+                               DeleteApp{false}, false);
+        auto user = session.current_user();
+        REQUIRE(user);
+        REQUIRE(user->is_logged_in());
+        identity = user->user_id();
+        tas_config = session.config(); // get config with storage path and user creds populated
+    }
+    REQUIRE_FALSE(identity.empty());
+    TestAppSession session(app_session, tas_config);
+    auto user = session.app()->get_existing_logged_in_user(identity);
 
     SyncTestFile config(user, partition, schema);
     config.sync_config->cancel_waits_on_nonfatal_error = true;
diff --git a/test/object-store/sync/app.cpp b/test/object-store/sync/app.cpp
@@ -2158,7 +2158,7 @@ TEST_CASE("app: mixed lists with object links", "[sync][pbs][app][links][baas]")
         Mixed{target_id},
     };
     {
-        TestAppSession test_session(app_session, nullptr, DeleteApp{false});
+        TestAppSession test_session(app_session, {}, DeleteApp{false});
         SyncTestFile config(test_session.app()->current_user(), partition, schema);
         auto realm = Realm::get_shared_realm(config);
 
@@ -2222,7 +2222,7 @@ TEST_CASE("app: roundtrip values", "[sync][pbs][app][baas]") {
     Decimal128 large_significand = Decimal128(70) / Decimal128(1.09);
     auto obj_id = ObjectId::gen();
     {
-        TestAppSession test_session(app_session, nullptr, DeleteApp{false});
+        TestAppSession test_session(app_session, {}, DeleteApp{false});
         SyncTestFile config(test_session.app()->current_user(), partition, schema);
         auto realm = Realm::get_shared_realm(config);
 
@@ -2646,7 +2646,7 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") {
         }
 
         auto transport = std::make_shared<HookedTransport<>>();
-        TestAppSession hooked_session(session.app_session(), transport, DeleteApp{false});
+        TestAppSession hooked_session(session.app_session(), {transport}, DeleteApp{false});
         auto app = hooked_session.app();
         std::shared_ptr<User> user = app->current_user();
         REQUIRE(user);
@@ -2704,7 +2704,7 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") {
         }
 
         auto transport = std::make_shared<HookedTransport<>>();
-        TestAppSession hooked_session(session.app_session(), transport, DeleteApp{false});
+        TestAppSession hooked_session(session.app_session(), {transport}, DeleteApp{false});
         auto app = hooked_session.app();
         std::shared_ptr<User> user = app->current_user();
         REQUIRE(user);
@@ -3306,8 +3306,9 @@ TEST_CASE("app: sync logs contain baas coid", "[sync][app][baas]") {
 
     auto in_mem_logger = std::make_shared<InMemoryLogger>();
     in_mem_logger->set_level_threshold(InMemoryLogger::Level::all);
-    TestAppSession app_session(get_runtime_app_session(), nullptr, DeleteApp{false}, ReconnectMode::normal, nullptr,
-                               in_mem_logger);
+    TestAppSession::Config session_config;
+    session_config.logger = in_mem_logger;
+    TestAppSession app_session(get_runtime_app_session(), session_config, DeleteApp{false});
 
     const auto partition = random_string(100);
     SyncTestFile config(app_session.app()->current_user(), partition, util::none);
@@ -4521,7 +4522,7 @@ TEST_CASE("app: full-text compatible with sync", "[sync][app][baas]") {
     auto server_app_config = minimal_app_config("full_text", schema);
     auto app_session = create_app(server_app_config);
     const auto partition = random_string(100);
-    TestAppSession test_session(app_session, nullptr);
+    TestAppSession test_session(app_session);
     SyncTestFile config(test_session.app()->current_user(), partition, schema);
     SharedRealm realm;
     SECTION("sync open") {
diff --git a/test/object-store/util/sync/flx_sync_harness.hpp b/test/object-store/util/sync/flx_sync_harness.hpp
@@ -84,16 +84,17 @@ class FLXSyncTestHarness {
     };
 
     explicit FLXSyncTestHarness(Config&& config)
-        : m_test_session(make_app_from_server_schema(config.test_name, config.server_schema), config.transport, true,
-                         config.reconnect_mode, config.custom_socket_provider)
+        : m_test_session(make_app_from_server_schema(config.test_name, config.server_schema),
+                         {config.transport, config.reconnect_mode, config.custom_socket_provider}, DeleteApp{true})
         , m_schema(std::move(config.server_schema.schema))
     {
     }
     FLXSyncTestHarness(const std::string& test_name, ServerSchema server_schema = default_server_schema(),
                        std::shared_ptr<GenericNetworkTransport> transport = instance_of<SynchronousTestTransport>,
                        std::shared_ptr<realm::sync::SyncSocketProvider> custom_socket_provider = nullptr)
-        : m_test_session(make_app_from_server_schema(test_name, server_schema), std::move(transport), true,
-                         realm::ReconnectMode::normal, custom_socket_provider)
+        : m_test_session(make_app_from_server_schema(test_name, server_schema),
+                         {std::move(transport), realm::ReconnectMode::normal, custom_socket_provider},
+                         DeleteApp{true})
         , m_schema(std::move(server_schema.schema))
     {
     }
diff --git a/test/object-store/util/test_file.cpp b/test/object-store/util/test_file.cpp
@@ -329,36 +329,47 @@ void set_app_config_defaults(app::AppConfig& app_config,
 #if REALM_ENABLE_AUTH_TESTS
 
 TestAppSession::TestAppSession()
-    : TestAppSession(get_runtime_app_session(), nullptr, DeleteApp{false})
+    // Don't delete the global runtime app session
+    : TestAppSession(get_runtime_app_session(), {}, DeleteApp{false})
 {
 }
 
-TestAppSession::TestAppSession(AppSession session,
-                               std::shared_ptr<realm::app::GenericNetworkTransport> custom_transport,
-                               DeleteApp delete_app, ReconnectMode reconnect_mode,
-                               std::shared_ptr<realm::sync::SyncSocketProvider> custom_socket_provider,
-                               std::shared_ptr<realm::util::Logger> logger)
+TestAppSession::TestAppSession(AppSession session)
+    : TestAppSession(session, {}, DeleteApp{true})
+{
+}
+
+TestAppSession::TestAppSession(AppSession session, Config config, DeleteApp delete_app, bool delete_storage)
     : m_app_session(std::make_unique<AppSession>(session))
-    , m_base_file_path(util::make_temp_dir() + random_string(10))
+    , m_config(config)
     , m_delete_app(delete_app)
-    , m_transport(custom_transport)
+    , m_delete_storage(delete_storage)
 {
-    if (!m_transport)
-        m_transport = instance_of<SynchronousTestTransport>;
-    app_config = get_config(m_transport, *m_app_session);
-    set_app_config_defaults(app_config, m_transport);
-    app_config.base_file_path = m_base_file_path;
-    app_config.metadata_mode = realm::app::AppConfig::MetadataMode::NoEncryption;
+    if (!m_config.storage_path || m_config.storage_path->empty()) {
+        m_config.storage_path.emplace(util::make_temp_dir() + random_string(10));
+    }
+    REALM_ASSERT(m_config.storage_path);
+    util::try_make_dir(*m_config.storage_path);
 
-    util::try_make_dir(m_base_file_path);
-    app_config.sync_client_config.reconnect_mode = reconnect_mode;
-    app_config.sync_client_config.socket_provider = custom_socket_provider;
+    if (!m_config.transport) {
+        m_config.transport = instance_of<SynchronousTestTransport>;
+    }
+    realm::app::AppConfig app_config = get_config(m_config.transport, *m_app_session);
+    set_app_config_defaults(app_config, m_config.transport);
+    // If a base URL was provided, set it in the app config
+    if (m_config.base_url) {
+        app_config.base_url = *m_config.base_url;
+    }
+    app_config.base_file_path = *m_config.storage_path;
+    app_config.metadata_mode = m_config.metadata_mode;
+    app_config.sync_client_config.reconnect_mode = m_config.reconnect_mode;
     // With multiplexing enabled, the linger time controls how long a
     // connection is kept open for reuse. In tests, we want to shut
     // down sync clients immediately.
     app_config.sync_client_config.timeouts.connection_linger_time = 0;
-    if (logger) {
-        app_config.sync_client_config.logger_factory = [logger](util::Logger::Level) {
+    app_config.sync_client_config.socket_provider = m_config.socket_provider;
+    if (m_config.logger) {
+        app_config.sync_client_config.logger_factory = [logger = m_config.logger](util::Logger::Level) {
             return logger;
         };
     }
@@ -367,70 +378,80 @@ TestAppSession::TestAppSession(AppSession session,
 
     // initialize sync client
     m_app->sync_manager()->get_sync_client();
-    user_creds = create_user_and_log_in(m_app);
+    // If no user creds are supplied, then create the user and log in
+    if (!m_config.user_creds) {
+        auto result = create_user_and_log_in();
+        REALM_ASSERT(result.is_ok());
+        m_config.user_creds = result.get_value();
+    }
+    // If creds are supplied, it is up to the caller to log in separately
 }
 
 TestAppSession::~TestAppSession()
 {
-    if (util::File::exists(m_base_file_path)) {
+    if (m_app) {
+        m_app->sync_manager()->tear_down_for_testing();
+        m_app.reset();
+    }
+    app::App::clear_cached_apps();
+    // If the app session is being deleted or the config tells us to, delete the storage path
+    if ((m_delete_app || m_delete_storage) && util::File::exists(*m_config.storage_path)) {
         try {
-            m_app->sync_manager()->tear_down_for_testing();
-            util::try_remove_dir_recursive(m_base_file_path);
+            util::try_remove_dir_recursive(*m_config.storage_path);
         }
         catch (const std::exception& ex) {
-            std::cerr << ex.what() << "\n";
+            std::cerr << "Error tearing down TestAppSession(" << m_app_session->config.app_name << "): " << ex.what()
+                      << "\n";
         }
-        app::App::clear_cached_apps();
     }
-    if (m_delete_app) {
+    if (m_delete_app && m_app_session) {
         m_app_session->admin_api.delete_app(m_app_session->server_app_id);
     }
 }
 
-void TestAppSession::close(bool tear_down)
+StatusWith<realm::app::AppCredentials> TestAppSession::create_user_and_log_in()
 {
-    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);
+    AutoVerifiedEmailCredentials creds;
+    auto pf = util::make_promise_future<void>();
+    m_app->provider_client<app::App::UsernamePasswordProviderClient>().register_email(
+        creds.email, creds.password,
+        [this, &creds, promise = util::CopyablePromiseHolder<void>(std::move(pf.promise))](
+            util::Optional<app::AppError> error) mutable {
+            if (error) {
+                promise.get_promise().set_error(error->to_status());
+                return;
             }
-            REALM_ASSERT(m_app);
-            // Clean up the app data
-            m_app->sync_manager()->tear_down_for_testing();
-        }
-        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();
-        }
-    }
-    catch (const std::exception& ex) {
-        std::cerr << "Error tearing down TestAppSession: " << ex.what() << "\n";
+            auto result = log_in_user(creds);
+            if (!result.is_ok()) {
+                promise.get_promise().set_error(result.get_status());
+                return;
+            }
+            promise.get_promise().emplace_value();
+        });
+    auto result = pf.future.get_no_throw();
+    if (!result.is_ok()) {
+        return result;
     }
-    // Ensure all cached apps are cleared
-    app::App::clear_cached_apps();
+    return creds;
 }
 
-void TestAppSession::reopen(bool log_in)
+StatusWith<std::shared_ptr<realm::SyncUser>>
+TestAppSession::log_in_user(std::optional<realm::app::AppCredentials> user_creds)
 {
-    REALM_ASSERT(!m_base_file_path.empty());
-    if (m_app) {
-        close(false);
-    }
-    m_app = app::App::get_app(app::App::CacheMode::Disabled, app_config);
-
-    // initialize sync client
-    m_app->sync_manager()->get_sync_client();
-    if (log_in) {
-        log_in_user(m_app, user_creds);
-    }
+    REALM_ASSERT(m_app);
+    REALM_ASSERT((user_creds || m_config.user_creds));
+    auto pf = util::make_promise_future<std::shared_ptr<realm::SyncUser>>();
+    m_app->log_in_with_credentials(
+        *user_creds, [promise = util::CopyablePromiseHolder<std::shared_ptr<realm::SyncUser>>(std::move(pf.promise))](
+                         std::shared_ptr<realm::SyncUser> user, util::Optional<app::AppError> error) mutable {
+            if (error) {
+                promise.get_promise().set_error(error->to_status());
+                return;
+            }
+            promise.get_promise().emplace_value(user);
+        });
+    return pf.future.get_no_throw();
 }
 
 std::vector<bson::BsonDocument> TestAppSession::get_documents(app::User& user, const std::string& object_type,
diff --git a/test/object-store/util/test_file.hpp b/test/object-store/util/test_file.hpp

Original file line number	Diff line number	Diff line change
`@@ -84,16 +84,17 @@ class FLXSyncTestHarness {`
`84`	`84`	`};`
`85`	`85`
`86`	`86`	`explicit FLXSyncTestHarness(Config&& config)`
`87`		`- : m_test_session(make_app_from_server_schema(config.test_name, config.server_schema), config.transport, true,`
`88`		`- config.reconnect_mode, config.custom_socket_provider)`
	`87`	`+ : m_test_session(make_app_from_server_schema(config.test_name, config.server_schema),`
	`88`	`+ {config.transport, config.reconnect_mode, config.custom_socket_provider}, DeleteApp{true})`
`89`	`89`	`, m_schema(std::move(config.server_schema.schema))`
`90`	`90`	`{`
`91`	`91`	`}`
`92`	`92`	`FLXSyncTestHarness(const std::string& test_name, ServerSchema server_schema = default_server_schema(),`
`93`	`93`	`std::shared_ptr<GenericNetworkTransport> transport = instance_of<SynchronousTestTransport>,`
`94`	`94`	`std::shared_ptr<realm::sync::SyncSocketProvider> custom_socket_provider = nullptr)`
`95`		`- : m_test_session(make_app_from_server_schema(test_name, server_schema), std::move(transport), true,`
`96`		`- realm::ReconnectMode::normal, custom_socket_provider)`
	`95`	`+ : m_test_session(make_app_from_server_schema(test_name, server_schema),`
	`96`	`+ {std::move(transport), realm::ReconnectMode::normal, custom_socket_provider},`
	`97`	`+ DeleteApp{true})`
`97`	`98`	`, m_schema(std::move(server_schema.schema))`
`98`	`99`	`{`
`99`	`100`	`}`