Don't perform a large number of writes when opening a sync metadata Realm

Initializing sync users read from the metadata Realm used the same code path as
initializing newly logged in users, resulting in it performing three no-op
write transactions per user to write the data which was just read back to the
Realm.

Author: tgoyne

CHANGELOG.md

@@ -12,6 +12,7 @@
   - Deleting the server-side user via one of the SyncUsers left the other SyncUsers in an invalid state.
   - A SyncUser which was originally created via anonymous login and then linked to an identity would still be treated as an anonymous users and removed entirely on logout.
   (since v10.0.0)
+* Reading existing logged-in users on app startup from the sync metadata Realm performed three no-op writes per user on the metadata Realm.
 
 ### Breaking changes
 * SyncUser::provider_type() and realm_user_get_auth_provider() have been removed. Users don't have provider types; identities do.

src/realm/object-store/sync/app.cpp

@@ -595,9 +595,8 @@ void App::get_profile(const std::shared_ptr<SyncUser>& sync_user,
                         SyncUserIdentity(get<std::string>(doc, "id"), get<std::string>(doc, "provider_type")));
                 }
 
-                sync_user->update_identities(identities);
-                sync_user->update_user_profile(SyncUserProfile(get<BsonDocument>(profile_json, "data")));
-                sync_user->set_state(SyncUser::State::LoggedIn);
+                sync_user->update_user_profile(std::move(identities),
+                                               SyncUserProfile(get<BsonDocument>(profile_json, "data")));
                 self->m_sync_manager->set_current_user(sync_user->identity());
                 self->emit_change_to_subscribers(*self);
             }

src/realm/object-store/sync/sync_manager.cpp

@@ -46,16 +46,7 @@ SyncManager::SyncManager() = default;
 void SyncManager::configure(std::shared_ptr<app::App> app, const std::string& sync_route,
                             const SyncClientConfig& config)
 {
-    struct UserCreationData {
-        std::string identity;
-        std::string refresh_token;
-        std::string access_token;
-        std::vector<SyncUserIdentity> identities;
-        SyncUser::State state;
-        std::string device_id;
-    };
-
-    std::vector<UserCreationData> users_to_add;
+    std::vector<std::shared_ptr<SyncUser>> users_to_add;
     {
         // Locking the mutex here ensures that it is released before locking m_user_mutex
         util::CheckedLockGuard lock(m_mutex);
@@ -113,11 +104,8 @@ void SyncManager::configure(std::shared_ptr<app::App> app, const std::string& sy
                 auto user_data = users.get(i);
                 auto refresh_token = user_data.refresh_token();
                 auto access_token = user_data.access_token();
-                auto device_id = user_data.device_id();
                 if (!refresh_token.empty() && !access_token.empty()) {
-                    users_to_add.push_back(UserCreationData{user_data.identity(), std::move(refresh_token),
-                                                            std::move(access_token), user_data.identities(),
-                                                            user_data.state(), device_id});
+                    users_to_add.push_back(std::make_shared<SyncUser>(user_data, this));
                 }
             }
 
@@ -144,13 +132,7 @@ void SyncManager::configure(std::shared_ptr<app::App> app, const std::string& sy
     }
     {
         util::CheckedLockGuard lock(m_user_mutex);
-        for (auto& user_data : users_to_add) {
-            auto& identity = user_data.identity;
-            auto user = std::make_shared<SyncUser>(user_data.refresh_token, identity, user_data.access_token,
-                                                   user_data.state, user_data.device_id, this);
-            user->update_identities(user_data.identities);
-            m_users.emplace_back(std::move(user));
-        }
+        m_users.insert(m_users.end(), users_to_add.begin(), users_to_add.end());
     }
 }
 
@@ -351,17 +333,16 @@ bool SyncManager::perform_metadata_update(util::FunctionRef<void(SyncMetadataMan
     return true;
 }
 
-std::shared_ptr<SyncUser> SyncManager::get_user(const std::string& user_id, std::string refresh_token,
-                                                std::string access_token, std::string device_id)
+std::shared_ptr<SyncUser> SyncManager::get_user(const std::string& user_id, const std::string& refresh_token,
+                                                const std::string& access_token, const std::string& device_id)
 {
     util::CheckedLockGuard lock(m_user_mutex);
     auto it = std::find_if(m_users.begin(), m_users.end(), [&](const auto& user) {
         return user->identity() == user_id && user->state() != SyncUser::State::Removed;
     });
     if (it == m_users.end()) {
         // No existing user.
-        auto new_user = std::make_shared<SyncUser>(std::move(refresh_token), user_id, std::move(access_token),
-                                                   SyncUser::State::LoggedIn, device_id, this);
+        auto new_user = std::make_shared<SyncUser>(refresh_token, user_id, access_token, device_id, this);
         m_users.emplace(m_users.begin(), new_user);
         {
             util::CheckedLockGuard lock(m_file_system_mutex);

src/realm/object-store/sync/sync_manager.hpp

@@ -180,8 +180,9 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
 
     // Get a sync user for a given identity, or create one if none exists yet, and set its token.
     // If a logged-out user exists, it will marked as logged back in.
-    std::shared_ptr<SyncUser> get_user(const std::string& id, std::string refresh_token, std::string access_token,
-                                       std::string device_id) REQUIRES(!m_user_mutex, !m_file_system_mutex);
+    std::shared_ptr<SyncUser> get_user(const std::string& user_id, const std::string& refresh_token,
+                                       const std::string& access_token, const std::string& device_id)
+        REQUIRES(!m_user_mutex, !m_file_system_mutex);
 
     // Get an existing user for a given identifier, if one exists and is logged in.
     std::shared_ptr<SyncUser> get_existing_logged_in_user(const std::string& user_id) const REQUIRES(!m_user_mutex);

src/realm/object-store/sync/sync_user.cpp

@@ -84,31 +84,50 @@ SyncUserIdentity::SyncUserIdentity(const std::string& id, const std::string& pro
 SyncUserContextFactory SyncUser::s_binding_context_factory;
 std::mutex SyncUser::s_binding_context_factory_mutex;
 
-SyncUser::SyncUser(std::string refresh_token, std::string identity, std::string access_token, SyncUser::State state,
-                   std::string device_id, SyncManager* sync_manager)
-    : m_state(state)
-    , m_identity(std::move(identity))
-    , m_refresh_token(RealmJWT(std::move(refresh_token)))
-    , m_access_token(RealmJWT(std::move(access_token)))
-    , m_device_id(std::move(device_id))
+SyncUser::SyncUser(const std::string& refresh_token, const std::string& id, const std::string& access_token,
+                   const std::string& device_id, SyncManager* sync_manager)
+    : m_state(State::LoggedIn)
+    , m_identity(id)
+    , m_refresh_token(RealmJWT(refresh_token))
+    , m_access_token(RealmJWT(access_token))
+    , m_device_id(device_id)
     , m_sync_manager(sync_manager)
 {
     {
-        std::lock_guard<std::mutex> lock(s_binding_context_factory_mutex);
+        std::lock_guard lock(s_binding_context_factory_mutex);
         if (s_binding_context_factory) {
             m_binding_context = s_binding_context_factory();
         }
     }
 
     m_sync_manager->perform_metadata_update([&](const auto& manager) NO_THREAD_SAFETY_ANALYSIS {
         auto metadata = manager.get_or_make_user_metadata(m_identity);
-        metadata->set_state_and_tokens(state, m_access_token.token, m_refresh_token.token);
+        metadata->set_state_and_tokens(State::LoggedIn, m_access_token.token, m_refresh_token.token);
         metadata->set_device_id(m_device_id);
         m_legacy_identities = metadata->legacy_identities();
         this->m_user_profile = metadata->profile();
     });
 }
 
+SyncUser::SyncUser(const SyncUserMetadata& data, SyncManager* sync_manager)
+    : m_state(data.state())
+    , m_legacy_identities(data.legacy_identities())
+    , m_identity(data.identity())
+    , m_refresh_token(RealmJWT(data.refresh_token()))
+    , m_access_token(RealmJWT(data.access_token()))
+    , m_user_identities(data.identities())
+    , m_user_profile(data.profile())
+    , m_device_id(data.device_id())
+    , m_sync_manager(sync_manager)
+{
+    {
+        std::lock_guard lock(s_binding_context_factory_mutex);
+        if (s_binding_context_factory) {
+            m_binding_context = s_binding_context_factory();
+        }
+    }
+}
+
 std::shared_ptr<SyncManager> SyncUser::sync_manager() const
 {
     util::CheckedLockGuard lk(m_mutex);
@@ -218,41 +237,6 @@ std::vector<std::shared_ptr<SyncSession>> SyncUser::revive_sessions()
     return sessions_to_revive;
 }
 
-void SyncUser::update_refresh_token(std::string&& token)
-{
-    std::vector<std::shared_ptr<SyncSession>> sessions_to_revive;
-    {
-        util::CheckedLockGuard lock(m_mutex);
-        util::CheckedLockGuard lock2(m_tokens_mutex);
-        switch (m_state) {
-            case State::Removed:
-                return;
-            case State::LoggedIn:
-                m_refresh_token = RealmJWT(std::move(token));
-                break;
-            case State::LoggedOut: {
-                m_refresh_token = RealmJWT(std::move(token));
-                m_state = State::LoggedIn;
-                sessions_to_revive = revive_sessions();
-                break;
-            }
-        }
-
-        m_sync_manager->perform_metadata_update([&, raw_refresh_token = m_refresh_token.token](const auto& manager) {
-            auto metadata = manager.get_or_make_user_metadata(m_identity);
-            metadata->set_refresh_token(raw_refresh_token);
-        });
-    }
-    // (Re)activate all pending sessions.
-    // Note that we do this after releasing the lock, since the session may
-    // need to access protected User state in the process of binding itself.
-    for (auto& session : sessions_to_revive) {
-        session->revive_if_needed();
-    }
-
-    emit_change_to_subscribers(*this);
-}
-
 void SyncUser::update_access_token(std::string&& token)
 {
     std::vector<std::shared_ptr<SyncSession>> sessions_to_revive;
@@ -295,22 +279,6 @@ std::vector<SyncUserIdentity> SyncUser::identities() const
     return m_user_identities;
 }
 
-
-void SyncUser::update_identities(std::vector<SyncUserIdentity> identities)
-{
-    util::CheckedLockGuard lock(m_mutex);
-    if (m_state != SyncUser::State::LoggedIn) {
-        return;
-    }
-
-    m_user_identities = identities;
-
-    m_sync_manager->perform_metadata_update([&](const auto& manager) {
-        auto metadata = manager.get_or_make_user_metadata(m_identity);
-        metadata->set_identities(identities);
-    });
-}
-
 void SyncUser::log_out()
 {
     // We'll extend the lifetime of SyncManager while holding m_mutex so that we know it's safe to call methods on it
@@ -446,18 +414,20 @@ util::Optional<bson::BsonDocument> SyncUser::custom_data() const
     return m_access_token.user_data;
 }
 
-void SyncUser::update_user_profile(const SyncUserProfile& profile)
+void SyncUser::update_user_profile(std::vector<SyncUserIdentity> identities, SyncUserProfile profile)
 {
     util::CheckedLockGuard lock(m_mutex);
-    if (m_state != SyncUser::State::LoggedIn) {
+    if (m_state == SyncUser::State::Removed) {
         return;
     }
 
-    m_user_profile = profile;
+    m_user_identities = std::move(identities);
+    m_user_profile = std::move(profile);
 
-    m_sync_manager->perform_metadata_update([&](const auto& manager) {
+    m_sync_manager->perform_metadata_update([&](const auto& manager) NO_THREAD_SAFETY_ANALYSIS {
         auto metadata = manager.get_or_make_user_metadata(m_identity);
-        metadata->set_user_profile(profile);
+        metadata->set_identities(m_user_identities);
+        metadata->set_user_profile(m_user_profile);
     });
 }
 

src/realm/object-store/sync/sync_user.hpp

@@ -38,8 +38,9 @@ namespace app {
 struct AppError;
 class MongoClient;
 } // namespace app
-class SyncSession;
 class SyncManager;
+class SyncSession;
+class SyncUserMetadata;
 
 // A superclass that bindings can inherit from in order to store information
 // upon a `SyncUser` object.
@@ -179,10 +180,6 @@ class SyncUser : public std::enable_shared_from_this<SyncUser>, public Subscriba
         Removed,
     };
 
-    // Don't use this directly; use the `SyncManager` APIs. Public for use with `make_shared`.
-    SyncUser(std::string refresh_token, std::string id, std::string access_token, SyncUser::State state,
-             std::string device_id, SyncManager* sync_manager);
-
     // Return a list of all sessions belonging to this user.
     std::vector<std::shared_ptr<SyncSession>> all_sessions() REQUIRES(!m_mutex);
 
@@ -192,28 +189,6 @@ class SyncUser : public std::enable_shared_from_this<SyncUser>, public Subscriba
     // for testing purposes, and for bindings for consumers that are servers or tools.
     std::shared_ptr<SyncSession> session_for_on_disk_path(const std::string& path) REQUIRES(!m_mutex);
 
-    // Update the user's state and refresh/access tokens atomically in a Realm transaction.
-    // If the user is transitioning between LoggedIn and LoggedOut, then the access_token and
-    // refresh token must be empty, and likewise must not be empty if transitioning between
-    // logged out and logged in.
-    // Note that this is called by the SyncManager, and should not be directly called.
-    void update_state_and_tokens(SyncUser::State state, const std::string& access_token,
-                                 const std::string& refresh_token) REQUIRES(!m_mutex, !m_tokens_mutex);
-
-    // Update the user's refresh token. If the user is logged out, it will log itself back in.
-    // Note that this is called by the SyncManager, and should not be directly called.
-    void update_refresh_token(std::string&& token) REQUIRES(!m_mutex, !m_tokens_mutex);
-
-    // Update the user's access token. If the user is logged out, it will log itself back in.
-    // Note that this is called by the SyncManager, and should not be directly called.
-    void update_access_token(std::string&& token) REQUIRES(!m_mutex, !m_tokens_mutex);
-
-    // Update the user's profile.
-    void update_user_profile(const SyncUserProfile& profile) REQUIRES(!m_mutex);
-
-    // Update the user's identities.
-    void update_identities(std::vector<SyncUserIdentity> identities) REQUIRES(!m_mutex);
-
     // Log the user out and mark it as such. This will also close its associated Sessions.
     void log_out() REQUIRES(!m_mutex, !m_tokens_mutex);
 
@@ -234,36 +209,65 @@ class SyncUser : public std::enable_shared_from_this<SyncUser>, public Subscriba
     }
 
     std::string access_token() const REQUIRES(!m_tokens_mutex);
-
     std::string refresh_token() const REQUIRES(!m_tokens_mutex);
-
     std::string device_id() const REQUIRES(!m_mutex);
-
     bool has_device_id() const REQUIRES(!m_mutex);
-
     SyncUserProfile user_profile() const REQUIRES(!m_mutex);
-
     std::vector<SyncUserIdentity> identities() const REQUIRES(!m_mutex);
+    State state() const REQUIRES(!m_mutex);
 
     // Custom user data embedded in the access token.
     util::Optional<bson::BsonDocument> custom_data() const REQUIRES(!m_tokens_mutex);
 
-    State state() const REQUIRES(!m_mutex);
-    void set_state(SyncUser::State state) REQUIRES(!m_mutex);
-
     std::shared_ptr<SyncUserContext> binding_context() const
     {
         return m_binding_context.load();
     }
 
+    // Optionally set a context factory. If so, must be set before any sessions are created.
+    static void set_binding_context_factory(SyncUserContextFactory factory);
+
+    std::shared_ptr<SyncManager> sync_manager() const REQUIRES(!m_mutex);
+
+    /// Retrieves a general-purpose service client for the Realm Cloud service
+    /// @param service_name The name of the cluster
+    app::MongoClient mongo_client(const std::string& service_name) REQUIRES(!m_mutex);
+
+    // ------------------------------------------------------------------------
+    // All of the following are called by `SyncManager` and are public only for
+    // testing purposes. SDKs should not call these directly in non-test code
+    // or expose them in the public API.
+
+    // Don't use this directly; use the `SyncManager` APIs. Public for use with `make_shared`.
+    SyncUser(const std::string& refresh_token, const std::string& id, const std::string& access_token,
+             const std::string& device_id, SyncManager* sync_manager);
+    SyncUser(const SyncUserMetadata& data, SyncManager* sync_manager);
+
+    void set_state(SyncUser::State state) REQUIRES(!m_mutex);
+
+    // Update the user's state and refresh/access tokens atomically in a Realm transaction.
+    // If the user is transitioning between LoggedIn and LoggedOut, then the access_token and
+    // refresh token must be empty, and likewise must not be empty if transitioning between
+    // logged out and logged in.
+    // Note that this is called by the SyncManager, and should not be directly called.
+    void update_state_and_tokens(SyncUser::State state, const std::string& access_token,
+                                 const std::string& refresh_token) REQUIRES(!m_mutex, !m_tokens_mutex);
+
+    // Update the user's access token. If the user is logged out, it will log itself back in.
+    // Note that this is called by the SyncManager, and should not be directly called.
+    void update_access_token(std::string&& token) REQUIRES(!m_mutex, !m_tokens_mutex);
+
+    // Update the user's profile and identities.
+    void update_user_profile(std::vector<SyncUserIdentity> identities, SyncUserProfile profile) REQUIRES(!m_mutex);
+
     // Register a session to this user.
     // A registered session will be bound at the earliest opportunity: either
     // immediately, or upon the user becoming Active.
     // Note that this is called by the SyncManager, and should not be directly called.
     void register_session(std::shared_ptr<SyncSession>) REQUIRES(!m_mutex);
 
     /// Refreshes the custom data for this user
-    /// If update_location is true, the location metadata will be queried before the request
+    /// If `update_location` is true, the location metadata will be queried before the request
     void refresh_custom_data(bool update_location,
                              util::UniqueFunction<void(util::Optional<app::AppError>)> completion_block)
         REQUIRES(!m_mutex);
@@ -274,15 +278,7 @@ class SyncUser : public std::enable_shared_from_this<SyncUser>, public Subscriba
     /// true.
     bool access_token_refresh_required() const REQUIRES(!m_mutex, !m_tokens_mutex);
 
-    // Optionally set a context factory. If so, must be set before any sessions are created.
-    static void set_binding_context_factory(SyncUserContextFactory factory);
-
-    std::shared_ptr<SyncManager> sync_manager() const REQUIRES(!m_mutex);
-
-    /// Retrieves a general-purpose service client for the Realm Cloud service
-    /// @param service_name The name of the cluster
-    app::MongoClient mongo_client(const std::string& service_name) REQUIRES(!m_mutex);
-
+    // Hook for testing access token timeouts
     void set_seconds_to_adjust_time_for_testing(int seconds)
     {
         m_seconds_to_adjust_time_for_testing.store(seconds);