Remove ClientHistory::fix_up_client_file_ident_in_stored_changesets() and friends

Author: jedelbo

src/realm/sync/client_base.hpp

@@ -209,14 +209,6 @@ struct ClientConfig {
     ///
     /// Testing/debugging feature. Should never be enabled in production.
     bool disable_sync_to_disk = false;
-
-    /// The sync client supports tables without primary keys by synthesizing a
-    /// pk using the client file ident, which means that all changesets waiting
-    /// to be uploaded need to be rewritten with the correct ident the first time
-    /// we connect to the server. The modern server doesn't support this and
-    /// requires pks for all tables, so this is now only applicable to old sync
-    /// tests and so is disabled by default.
-    bool fix_up_object_ids = false;
 };
 
 /// \brief Information about an error causing a session to be temporarily

src/realm/sync/noinst/client_history_impl.cpp

@@ -270,7 +270,7 @@ void ClientHistory::get_status(version_type& current_client_version, SaltedFileI
 }
 
 
-void ClientHistory::set_client_file_ident(SaltedFileIdent client_file_ident, bool fix_up_object_ids)
+void ClientHistory::set_client_file_ident(SaltedFileIdent client_file_ident)
 {
     REALM_ASSERT(client_file_ident.ident != 0);
 
@@ -287,10 +287,6 @@ void ClientHistory::set_client_file_ident(SaltedFileIdent client_file_ident, boo
     root.set(s_progress_download_client_version_iip, RefOrTagged::make_tagged(0));
     root.set(s_progress_upload_client_version_iip, RefOrTagged::make_tagged(0));
 
-    if (fix_up_object_ids) {
-        fix_up_client_file_ident_in_stored_changesets(*wt, client_file_ident.ident); // Throws
-    }
-
     // Note: This transaction produces an empty changeset. Empty changesets are
     // not uploaded to the server.
     wt->commit(); // Throws
@@ -1024,98 +1020,6 @@ void ClientHistory::do_trim_sync_history(std::size_t n)
     }
 }
 
-void ClientHistory::fix_up_client_file_ident_in_stored_changesets(Transaction& group,
-                                                                  file_ident_type client_file_ident)
-{
-    // Must be in write transaction!
-
-    REALM_ASSERT(client_file_ident != 0);
-    auto promote_global_key = [client_file_ident](GlobalKey* oid) {
-        if (oid->hi() == 0) {
-            // client_file_ident == 0
-            *oid = GlobalKey{uint64_t(client_file_ident), oid->lo()};
-            return true;
-        }
-        return false;
-    };
-
-    Group::TableNameBuffer buffer;
-    auto get_table_for_class = [&](StringData class_name) -> ConstTableRef {
-        REALM_ASSERT(class_name.size() < Group::max_table_name_length - 6);
-        return group.get_table(Group::class_name_to_table_name(class_name, buffer));
-    };
-
-    util::compression::CompressMemoryArena arena;
-    util::AppendBuffer<char> compressed;
-
-    // Fix up changesets.
-    Array& root = m_arrays->root;
-    uint64_t uploadable_bytes = root.get_as_ref_or_tagged(s_progress_uploadable_bytes_iip).get_as_int();
-    for (size_t i = 0; i < sync_history_size(); ++i) {
-        // We could have opened a pre-provisioned realm file. In this case we can skip the entries downloaded
-        // from the server.
-        if (m_arrays->origin_file_idents.get(i) != 0)
-            continue;
-
-        ChunkedBinaryData changeset{m_arrays->changesets, i};
-        ChunkedBinaryInputStream is{changeset};
-        size_t decompressed_size;
-        auto decompressed = util::compression::decompress_nonportable_input_stream(is, decompressed_size);
-        if (!decompressed)
-            continue;
-        Changeset log;
-        parse_changeset(*decompressed, log);
-
-        bool did_modify = false;
-        auto last_class_name = InternString::npos;
-        ConstTableRef selected_table;
-        for (auto instr : log) {
-            if (!instr)
-                continue;
-
-            if (auto obj_instr = instr->get_if<Instruction::ObjectInstruction>()) {
-                // Cache the TableRef
-                if (obj_instr->table != last_class_name) {
-                    StringData class_name = log.get_string(obj_instr->table);
-                    last_class_name = obj_instr->table;
-                    selected_table = get_table_for_class(class_name);
-                }
-
-                // Fix up instructions using GlobalKey to identify objects.
-                if (auto global_key = mpark::get_if<GlobalKey>(&obj_instr->object)) {
-                    did_modify = promote_global_key(global_key);
-                }
-
-                // Fix up the payload for Set and ArrayInsert.
-                Instruction::Payload* payload = nullptr;
-                if (auto set_instr = instr->get_if<Instruction::Update>()) {
-                    payload = &set_instr->value;
-                }
-                else if (auto list_insert_instr = instr->get_if<Instruction::ArrayInsert>()) {
-                    payload = &list_insert_instr->value;
-                }
-
-                if (payload && payload->type == Instruction::Payload::Type::Link) {
-                    if (auto global_key = mpark::get_if<GlobalKey>(&payload->data.link.target)) {
-                        did_modify = promote_global_key(global_key);
-                    }
-                }
-            }
-        }
-
-        if (did_modify) {
-            ChangesetEncoder::Buffer modified;
-            encode_changeset(log, modified);
-            util::compression::allocate_and_compress_nonportable(arena, modified, compressed);
-            m_arrays->changesets.set(i, BinaryData{compressed.data(), compressed.size()}); // Throws
-
-            uploadable_bytes += modified.size() - decompressed_size;
-        }
-    }
-
-    root.set(s_progress_uploadable_bytes_iip, RefOrTagged::make_tagged(uploadable_bytes));
-}
-
 void ClientHistory::set_group(Group* group, bool updated)
 {
     _impl::History::set_group(group, updated);

src/realm/sync/noinst/client_history_impl.hpp

@@ -160,7 +160,7 @@ class ClientHistory final : public _impl::History, public TransformHistory {
     /// synchronization proper, and it must store the identifier and use it to
     /// reestablish the connection between the client file and the server file
     /// when engaging in future synchronization sessions.
-    void set_client_file_ident(SaltedFileIdent client_file_ident, bool fix_up_object_ids);
+    void set_client_file_ident(SaltedFileIdent client_file_ident);
 
     /// Stores the synchronization progress in the associated Realm file in a
     /// way that makes it available via get_status() during future
@@ -428,7 +428,6 @@ class ClientHistory final : public _impl::History, public TransformHistory {
     void do_trim_sync_history(std::size_t n);
     void clamp_sync_version_range(version_type& begin, version_type& end) const noexcept;
     Transformer& get_transformer();
-    void fix_up_client_file_ident_in_stored_changesets(Transaction&, file_ident_type);
     void record_current_schema_version();
     static void record_current_schema_version(Array& schema_versions, version_type snapshot_version);
     void compress_stored_changesets();

src/realm/sync/noinst/client_impl_base.cpp

@@ -149,7 +149,6 @@ ClientImpl::ClientImpl(ClientConfig config)
     , m_dry_run{config.dry_run}
     , m_enable_default_port_hack{config.enable_default_port_hack}
     , m_disable_upload_compaction{config.disable_upload_compaction}
-    , m_fix_up_object_ids{config.fix_up_object_ids}
     , m_roundtrip_time_handler{std::move(config.roundtrip_time_handler)}
     , m_socket_provider{std::move(config.socket_provider)}
     , m_client_protocol{} // Throws
@@ -2333,8 +2332,7 @@ Status Session::receive_ident_message(SaltedFileIdent client_file_ident)
         return Status::OK();
     }
     if (!did_client_reset) {
-        repl.get_history().set_client_file_ident(client_file_ident,
-                                                 m_fix_up_object_ids); // Throws
+        repl.get_history().set_client_file_ident(client_file_ident); // Throws
         m_progress.download.last_integrated_client_version = 0;
         m_progress.upload.client_version = 0;
         m_last_version_selected_for_upload = 0;

src/realm/sync/noinst/client_impl_base.hpp

@@ -250,7 +250,6 @@ class ClientImpl {
     const bool m_dry_run; // For testing purposes only
     const bool m_enable_default_port_hack;
     const bool m_disable_upload_compaction;
-    const bool m_fix_up_object_ids;
     const std::function<RoundtripTimeHandler> m_roundtrip_time_handler;
     const std::string m_user_agent_string;
     std::shared_ptr<SyncSocketProvider> m_socket_provider;
@@ -1050,8 +1049,6 @@ class ClientImpl::Session {
 
     bool m_is_flx_sync_session = false;
 
-    bool m_fix_up_object_ids = false;
-
     // These are reset when the session is activated, and again whenever the
     // connection is lost or the rebinding process is initiated.
     bool m_enlisted_to_send;
@@ -1464,7 +1461,6 @@ inline ClientImpl::Session::Session(SessionWrapper& wrapper, Connection& conn, s
     , m_ident{ident}
     , m_try_again_delay_info(conn.get_client().m_reconnect_backoff_info, conn.get_client().get_random())
     , m_is_flx_sync_session(conn.is_flx_sync_connection())
-    , m_fix_up_object_ids(get_client().m_fix_up_object_ids)
     , m_wrapper{wrapper}
 {
     if (get_client().m_disable_upload_activation_delay)

src/realm/sync/tools/apply_to_state_command.cpp

@@ -314,7 +314,7 @@ int main(int argc, const char** argv)
                              }
                          },
                          [&](const ServerIdentMessage& ident_message) {
-                             history.set_client_file_ident(ident_message.file_ident, true);
+                             history.set_client_file_ident(ident_message.file_ident);
                          }},
                      message);
     }

test/object-store/util/test_file.hpp

@@ -102,7 +102,7 @@ void on_change_but_no_notify(realm::Realm& realm);
 #if TEST_ENABLE_LOGGING
 #define TEST_LOGGING_LEVEL all
 #else
-#define TEST_LOGGING_LEVEL off
+#define TEST_LOGGING_LEVEL debug
 #endif // TEST_ENABLE_LOGGING
 #endif // TEST_LOGGING_LEVEL
 

test/sync_fixtures.hpp

@@ -550,7 +550,6 @@ class MultiClientServerFixture {
             config_2.disable_upload_compaction = config.disable_upload_compaction;
             config_2.one_connection_per_session = config.one_connection_per_session;
             config_2.disable_upload_activation_delay = config.disable_upload_activation_delay;
-            config_2.fix_up_object_ids = true;
             m_clients[i] = std::make_unique<Client>(std::move(config_2));
         }
 

test/test_instruction_replication.cpp

@@ -31,9 +31,7 @@ struct Fixture {
         , sg_1(DB::create(*history_1, path_1))
         , sg_2(DB::create(*history_2, path_2))
     {
-        // This is to ensure that peer IDs in Object IDs are populated.
-        bool fix_up_object_ids = true;
-        history_1->get_history().set_client_file_ident({1, 123}, fix_up_object_ids);
+        history_1->get_history().set_client_file_ident({1, 123});
     }
 
     void replay_transactions()

test/test_sync.cpp

@@ -6578,7 +6578,7 @@ TEST(Sync_DanglingLinksCountInPriorSize)
     ClientReplication repl;
     auto local_db = realm::DB::create(repl, path);
     auto& history = repl.get_history();
-    history.set_client_file_ident(sync::SaltedFileIdent{1, 123456}, true);
+    history.set_client_file_ident(sync::SaltedFileIdent{1, 123456});
 
     version_type last_version, last_version_observed = 0;
     auto dump_uploadable = [&] {
@@ -6853,7 +6853,7 @@ TEST(Sync_NonIncreasingServerVersions)
     TEST_CLIENT_DB(db);
 
     auto& history = get_history(db);
-    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678}, false);
+    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678});
     timestamp_type timestamp{1};
     history.set_local_origin_timestamp_source([&] {
         return ++timestamp;
@@ -6926,7 +6926,7 @@ TEST(Sync_InvalidChangesetFromServer)
     TEST_CLIENT_DB(db);
 
     auto& history = get_history(db);
-    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678}, false);
+    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678});
 
     instr::CreateObject bad_instr;
     bad_instr.object = InternString{1};
@@ -7004,7 +7004,7 @@ TEST(Sync_SetAndGetEmptyReciprocalChangeset)
     TEST_CLIENT_DB(db);
 
     auto& history = get_history(db);
-    history.set_client_file_ident(SaltedFileIdent{1, 0x1234567812345678}, false);
+    history.set_client_file_ident(SaltedFileIdent{1, 0x1234567812345678});
     timestamp_type timestamp{1};
     history.set_local_origin_timestamp_source([&] {
         return ++timestamp;
@@ -7173,7 +7173,7 @@ TEST(Sync_ServerVersionsSkippedFromDownloadCursor)
     TEST_CLIENT_DB(db);
 
     auto& history = get_history(db);
-    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678}, false);
+    history.set_client_file_ident(SaltedFileIdent{2, 0x1234567812345678});
     timestamp_type timestamp{1};
     history.set_local_origin_timestamp_source([&] {
         return ++timestamp;