Remove ability to synchronize objects without primary key

src/realm/cluster_tree.cpp

@@ -994,7 +994,6 @@ void ClusterTree::erase(ObjKey k, CascadeState& state)
             }
         }
     }
-    m_owner->free_local_id_after_hash_collision(k);
     m_owner->erase_from_search_indexes(k);
 
     size_t root_size = m_root->erase(k, state);

src/realm/obj.cpp

@@ -114,11 +114,6 @@ Obj::Obj(TableRef table, MemRef mem, ObjKey key, size_t row_ndx)
     m_storage_version = get_alloc().get_storage_version();
 }
 
-GlobalKey Obj::get_object_id() const
-{
-    return m_table->get_object_id(m_key);
-}
-
 ObjLink Obj::get_link() const
 {
     return ObjLink(m_table->get_key(), m_key);

src/realm/obj.hpp

@@ -34,7 +34,6 @@ class ClusterTree;
 class TableView;
 class CascadeState;
 class ObjList;
-struct GlobalKey;
 
 template <class>
 class Lst;
@@ -114,7 +113,6 @@ class Obj : public CollectionParent {
     {
         return m_key;
     }
-    GlobalKey get_object_id() const;
     ObjLink get_link() const;
 
     /// Check if the object is still alive

src/realm/replication.cpp

@@ -142,13 +142,13 @@ void Replication::erase_column(const Table* t, ColKey col_key)
     m_encoder.erase_column(col_key); // Throws
 }
 
-void Replication::create_object(const Table* t, GlobalKey id)
+void Replication::create_object(const Table* t, ObjKey key)
 {
     if (auto logger = get_logger()) {
         logger->log(LogCategory::object, util::Logger::Level::debug, "Create object '%1'", t->get_class_name());
     }
     select_table(t);                              // Throws
-    m_encoder.create_object(id.get_local_key(0)); // Throws
+    m_encoder.create_object(key);                 // Throws
 }
 
 void Replication::create_object_with_primary_key(const Table* t, ObjKey key, Mixed pk)

src/realm/replication.hpp

@@ -74,7 +74,7 @@ class Replication {
     virtual void dictionary_erase(const CollectionBase& dict, size_t dict_ndx, Mixed key);
     virtual void dictionary_clear(const CollectionBase& dict);
 
-    virtual void create_object(const Table*, GlobalKey);
+    virtual void create_object(const Table*, ObjKey);
     virtual void create_object_with_primary_key(const Table*, ObjKey, Mixed);
     virtual void remove_object(const Table*, ObjKey);
 

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/instruction_applier.cpp

@@ -226,11 +226,8 @@ void InstructionApplier::operator()(const Instruction::CreateObject& instr)
                 }
                 m_last_object = table->create_object_with_primary_key(id);
             },
-            [&](GlobalKey key) {
-                if (pk_col) {
-                    bad_transaction_log("CreateObject(GlobalKey) on table with a primary key");
-                }
-                m_last_object = table->create_object(key);
+            [&](GlobalKey) {
+                bad_transaction_log("CreateObject(GlobalKey) not supported");
             },
         },
         instr.object);
@@ -1684,14 +1681,9 @@ ObjKey InstructionApplier::get_object_key(Table& table, const Instruction::Prima
                 ObjKey key = table.get_objkey_from_primary_key(pk);
                 return key;
             },
-            [&](GlobalKey id) {
-                if (pk_col) {
-                    bad_transaction_log(
-                        "%1 instruction without primary key, but table '%2' has a primary key column of type %3",
-                        name, table_name, pk_type);
-                }
-                ObjKey key = table.get_objkey_from_global_key(id);
-                return key;
+            [&](GlobalKey) {
+                bad_transaction_log("%1 instruction without primary key not supported", name);
+                return ObjKey();
             },
             [&](ObjectId pk) {
                 if (!pk_col) {

src/realm/sync/instruction_replication.cpp

@@ -242,26 +242,6 @@ void SyncReplication::add_class_with_primary_key(TableKey tk, StringData name, D
     }
 }
 
-void SyncReplication::create_object(const Table* table, GlobalKey oid)
-{
-    if (table->is_embedded()) {
-        unsupported_instruction(); // FIXME: TODO
-    }
-
-    Replication::create_object(table, oid);
-    if (select_table(*table)) {
-        if (table->get_primary_key_column()) {
-            // Trying to create object without a primary key in a table that
-            // has a primary key column.
-            unsupported_instruction();
-        }
-        Instruction::CreateObject instr;
-        instr.table = m_last_class_name;
-        instr.object = oid;
-        emit(instr);
-    }
-}
-
 Instruction::PrimaryKey SyncReplication::as_primary_key(Mixed value)
 {
     if (value.is_null()) {
@@ -749,13 +729,7 @@ Instruction::PrimaryKey SyncReplication::primary_key_for_object(const Table& tab
 {
     bool should_emit = select_table(table);
     REALM_ASSERT(should_emit);
-
-    if (table.get_primary_key_column()) {
-        return as_primary_key(table.get_primary_key(key));
-    }
-
-    GlobalKey global_key = table.get_object_id(key);
-    return global_key;
+    return as_primary_key(table.get_primary_key(key));
 }
 
 void SyncReplication::populate_path_instr(Instruction::PathInstruction& instr, const Table& table, ObjKey key,

src/realm/sync/instruction_replication.hpp

@@ -48,7 +48,6 @@ class SyncReplication : public Replication {
     void add_class(TableKey tk, StringData table_name, Table::Type table_type = Table::Type::TopLevel) final;
     void add_class_with_primary_key(TableKey tk, StringData table_name, DataType pk_type, StringData pk_field,
                                     bool nullable, Table::Type table_type) final;
-    void create_object(const Table*, GlobalKey) final;
     void create_object_with_primary_key(const Table*, ObjKey, Mixed) final;
 
     void erase_class(TableKey table_key, StringData table_name, size_t num_tables) final;

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
@@ -1020,98 +1016,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

@@ -142,16 +142,11 @@ class ClientHistory final : public _impl::History, public TransformHistory {
     /// identical identifiers for two client files if they are associated with
     /// different server Realms.
     ///
-    /// \param fix_up_object_ids The object ids that depend on client file ident
-    /// will be fixed in both state and history if this parameter is true. If
-    /// it is known that there are no objects to fix, it can be set to false to
-    /// achieve higher performance.
-    ///
     /// The client is required to obtain the file identifier before engaging in
     /// 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
@@ -414,7 +409,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

@@ -148,7 +148,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
@@ -2329,8 +2328,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;
@@ -1044,8 +1043,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;
@@ -1446,7 +1443,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

@@ -316,7 +316,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);
     }