diff --git a/src/realm/cluster_tree.cpp b/src/realm/cluster_tree.cpp index de71c7f4bef..5144f50b8e9 100644 --- a/src/realm/cluster_tree.cpp +++ b/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); diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 89ac317e26c..fa050d60744 100644 --- a/src/realm/obj.cpp +++ b/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); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 00a9586a328..17d23cb6614 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -34,7 +34,6 @@ class ClusterTree; class TableView; class CascadeState; class ObjList; -struct GlobalKey; template 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 diff --git a/src/realm/replication.cpp b/src/realm/replication.cpp index c6f76c0d315..9d361b9adb2 100644 --- a/src/realm/replication.cpp +++ b/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) diff --git a/src/realm/replication.hpp b/src/realm/replication.hpp index 437d6654901..8e315ab14b9 100644 --- a/src/realm/replication.hpp +++ b/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); diff --git a/src/realm/sync/client_base.hpp b/src/realm/sync/client_base.hpp index 71b9722ea18..92302edac82 100644 --- a/src/realm/sync/client_base.hpp +++ b/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 diff --git a/src/realm/sync/instruction_applier.cpp b/src/realm/sync/instruction_applier.cpp index ab2cdc8f6c9..f6557b06e7a 100644 --- a/src/realm/sync/instruction_applier.cpp +++ b/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) { diff --git a/src/realm/sync/instruction_replication.cpp b/src/realm/sync/instruction_replication.cpp index ef3c17c1701..00d61537ad3 100644 --- a/src/realm/sync/instruction_replication.cpp +++ b/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, diff --git a/src/realm/sync/instruction_replication.hpp b/src/realm/sync/instruction_replication.hpp index b9008b9609b..146a57e774d 100644 --- a/src/realm/sync/instruction_replication.hpp +++ b/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; diff --git a/src/realm/sync/noinst/client_history_impl.cpp b/src/realm/sync/noinst/client_history_impl.cpp index 1b12f39763d..9add26a1ca9 100644 --- a/src/realm/sync/noinst/client_history_impl.cpp +++ b/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 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()) { - // 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(&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()) { - payload = &set_instr->value; - } - else if (auto list_insert_instr = instr->get_if()) { - payload = &list_insert_instr->value; - } - - if (payload && payload->type == Instruction::Payload::Type::Link) { - if (auto global_key = mpark::get_if(&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); diff --git a/src/realm/sync/noinst/client_history_impl.hpp b/src/realm/sync/noinst/client_history_impl.hpp index 78ff4bd91da..57265e00cc9 100644 --- a/src/realm/sync/noinst/client_history_impl.hpp +++ b/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(); diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index a326dfcf8d9..5dc9a513d1c 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/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; diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index fc008314cd5..74bb1497fe4 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/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 m_roundtrip_time_handler; const std::string m_user_agent_string; std::shared_ptr 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) diff --git a/src/realm/sync/tools/apply_to_state_command.cpp b/src/realm/sync/tools/apply_to_state_command.cpp index 99c40e61436..ba4096fde11 100644 --- a/src/realm/sync/tools/apply_to_state_command.cpp +++ b/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); } diff --git a/src/realm/table.cpp b/src/realm/table.cpp index 533c1c2eb29..805b2da9d1b 100644 --- a/src/realm/table.cpp +++ b/src/realm/table.cpp @@ -1489,7 +1489,6 @@ void Table::clear() { CascadeState state(CascadeState::Mode::Strong, get_parent_group()); m_clusters.clear(state); - free_collision_table(); } @@ -1535,11 +1534,6 @@ void Table::set_sequence_number(uint64_t seq) m_top.set(top_position_for_sequence_number, RefOrTagged::make_tagged(seq)); } -void Table::set_collision_map(ref_type ref) -{ - m_top.set(top_position_for_collision_map, RefOrTagged::make_ref(ref)); -} - TableRef Table::get_link_target(ColKey col_key) noexcept { return get_opposite_table(col_key); @@ -2182,17 +2176,9 @@ Obj Table::create_object(ObjKey key, const FieldValues& values) if (m_primary_key_col) throw IllegalOperation(util::format("Table has primary key: %1", get_name())); if (key == null_key) { - GlobalKey object_id = allocate_object_id_squeezed(); - key = object_id.get_local_key(get_sync_file_id()); - // Check if this key collides with an already existing object - // This could happen if objects were at some point created with primary keys, - // but later primary key property was removed from the schema. - while (m_clusters.is_valid(key)) { - object_id = allocate_object_id_squeezed(); - key = object_id.get_local_key(get_sync_file_id()); - } + key = get_next_valid_key(); if (auto repl = get_repl()) - repl->create_object(this, object_id); + repl->create_object(this, key); } REALM_ASSERT(key.value >= 0); @@ -2206,49 +2192,12 @@ Obj Table::create_linked_object() { REALM_ASSERT(is_embedded()); - GlobalKey object_id = allocate_object_id_squeezed(); - ObjKey key = object_id.get_local_key(get_sync_file_id()); - - REALM_ASSERT(key.value >= 0); - + ObjKey key = get_next_valid_key(); Obj obj = m_clusters.insert(key, {}); return obj; } -Obj Table::create_object(GlobalKey object_id, const FieldValues& values) -{ - if (is_embedded()) - throw IllegalOperation(util::format("Explicit creation of embedded object not allowed in: %1", get_name())); - if (m_primary_key_col) - throw IllegalOperation(util::format("Table has primary key: %1", get_name())); - ObjKey key = object_id.get_local_key(get_sync_file_id()); - - if (auto repl = get_repl()) - repl->create_object(this, object_id); - - try { - Obj obj = m_clusters.insert(key, values); - // Check if tombstone exists - if (m_tombstones && m_tombstones->is_valid(key.get_unresolved())) { - auto unres_key = key.get_unresolved(); - // Copy links over - auto tombstone = m_tombstones->get(unres_key); - obj.assign_pk_and_backlinks(tombstone); - // If tombstones had no links to it, it may still be alive - if (m_tombstones->is_valid(unres_key)) { - CascadeState state(CascadeState::Mode::None); - m_tombstones->erase(unres_key, state); - } - } - - return obj; - } - catch (const KeyAlreadyUsed&) { - return m_clusters.get(key); - } -} - Obj Table::create_object_with_primary_key(const Mixed& primary_key, FieldValues&& field_values, UpdateMode mode, bool* did_create) { @@ -2292,17 +2241,16 @@ Obj Table::create_object_with_primary_key(const Mixed& primary_key, FieldValues& ObjKey unres_key; if (m_tombstones) { - // Check for potential tombstone - GlobalKey object_id{primary_key}; - ObjKey object_key = global_to_local_object_id_hashed(object_id); - - ObjKey key = object_key.get_unresolved(); - if (auto obj = m_tombstones->try_get_obj(key)) { - auto existing_pk_value = obj.get_any(primary_key_col); - - // If the primary key is the same, the object should be resurrected below - if (existing_pk_value == primary_key) { - unres_key = key; + if (auto sz = m_tombstones->size()) { + // Check for potential tombstone + Iterator end(*m_tombstones, sz); + for (Iterator it(*m_tombstones, 0); it != end; ++it) { + auto existing_pk_value = it->get_any(primary_key_col); + // If the primary key is the same, the object should be resurrected below + if (existing_pk_value == primary_key) { + unres_key = it->get_key(); + break; + } } } } @@ -2343,25 +2291,9 @@ ObjKey Table::find_primary_key(Mixed primary_key) const DataType type = DataType(primary_key_col.get_type()); REALM_ASSERT((primary_key.is_null() && primary_key_col.get_attrs().test(col_attr_Nullable)) || primary_key.get_type() == type); + REALM_ASSERT(m_index_accessors[primary_key_col.get_index().val]); - if (auto&& index = m_index_accessors[primary_key_col.get_index().val]) { - return index->find_first(primary_key); - } - - // This must be file format 11, 20 or 21 as those are the ones we can open in read-only mode - // so try the old algorithm - GlobalKey object_id{primary_key}; - ObjKey object_key = global_to_local_object_id_hashed(object_id); - - // Check if existing - if (auto obj = m_clusters.try_get_obj(object_key)) { - auto existing_pk_value = obj.get_any(primary_key_col); - - if (existing_pk_value == primary_key) { - return object_key; - } - } - return {}; + return m_index_accessors[primary_key_col.get_index().val]->find_first(primary_key); } ObjKey Table::get_objkey_from_primary_key(const Mixed& primary_key) @@ -2371,51 +2303,7 @@ ObjKey Table::get_objkey_from_primary_key(const Mixed& primary_key) return key; } - // Object does not exist - create tombstone - GlobalKey object_id{primary_key}; - ObjKey object_key = global_to_local_object_id_hashed(object_id); - return get_or_create_tombstone(object_key, m_primary_key_col, primary_key).get_key(); -} - -ObjKey Table::get_objkey_from_global_key(GlobalKey global_key) -{ - REALM_ASSERT(!m_primary_key_col); - auto object_key = global_key.get_local_key(get_sync_file_id()); - - // Check if existing - if (m_clusters.is_valid(object_key)) { - return object_key; - } - - return get_or_create_tombstone(object_key, {}, {}).get_key(); -} - -ObjKey Table::get_objkey(GlobalKey global_key) const -{ - ObjKey key; - REALM_ASSERT(!m_primary_key_col); - uint32_t max = std::numeric_limits::max(); - if (global_key.hi() <= max && global_key.lo() <= max) { - key = global_key.get_local_key(get_sync_file_id()); - } - if (key && !is_valid(key)) { - key = realm::null_key; - } - return key; -} - -GlobalKey Table::get_object_id(ObjKey key) const -{ - auto col = get_primary_key_column(); - if (col) { - const Obj obj = get_object(key); - auto val = obj.get_any(col); - return {val}; - } - else { - return {key, get_sync_file_id()}; - } - return {}; + return get_or_create_tombstone(ObjKey{}, m_primary_key_col, primary_key).get_key(); } Obj Table::get_object_with_primary_key(Mixed primary_key) const @@ -2442,193 +2330,28 @@ Mixed Table::get_primary_key(ObjKey key) const } } -GlobalKey Table::allocate_object_id_squeezed() -{ - // m_client_file_ident will be zero if we haven't been in contact with - // the server yet. - auto peer_id = get_sync_file_id(); - auto sequence = allocate_sequence_number(); - return GlobalKey{peer_id, sequence}; -} - -namespace { - -/// Calculate optimistic local ID that may collide with others. It is up to -/// the caller to ensure that collisions are detected and that -/// allocate_local_id_after_collision() is called to obtain a non-colliding -/// ID. -inline ObjKey get_optimistic_local_id_hashed(GlobalKey global_id) -{ -#if REALM_EXERCISE_OBJECT_ID_COLLISION - const uint64_t optimistic_mask = 0xff; -#else - const uint64_t optimistic_mask = 0x3fffffffffffffff; -#endif - static_assert(!(optimistic_mask >> 62), "optimistic Object ID mask must leave the 63rd and 64th bit zero"); - return ObjKey{int64_t(global_id.lo() & optimistic_mask)}; -} - -inline ObjKey make_tagged_local_id_after_hash_collision(uint64_t sequence_number) -{ - REALM_ASSERT(!(sequence_number >> 62)); - return ObjKey{int64_t(0x4000000000000000 | sequence_number)}; -} - -} // namespace - -ObjKey Table::global_to_local_object_id_hashed(GlobalKey object_id) const -{ - ObjKey optimistic = get_optimistic_local_id_hashed(object_id); - - if (ref_type collision_map_ref = to_ref(m_top.get(top_position_for_collision_map))) { - Allocator& alloc = m_top.get_alloc(); - Array collision_map{alloc}; - collision_map.init_from_ref(collision_map_ref); // Throws - - Array hi{alloc}; - hi.init_from_ref(to_ref(collision_map.get(s_collision_map_hi))); // Throws - - // Entries are ordered by hi,lo - size_t found = hi.find_first(object_id.hi()); - if (found != npos && uint64_t(hi.get(found)) == object_id.hi()) { - Array lo{alloc}; - lo.init_from_ref(to_ref(collision_map.get(s_collision_map_lo))); // Throws - size_t candidate = lo.find_first(object_id.lo(), found); - if (candidate != npos && uint64_t(hi.get(candidate)) == object_id.hi()) { - Array local_id{alloc}; - local_id.init_from_ref(to_ref(collision_map.get(s_collision_map_local_id))); // Throws - return ObjKey{local_id.get(candidate)}; - } - } - } - - return optimistic; -} - -ObjKey Table::allocate_local_id_after_hash_collision(GlobalKey incoming_id, GlobalKey colliding_id, - ObjKey colliding_local_id) -{ - // Possible optimization: Cache these accessors - Allocator& alloc = m_top.get_alloc(); - Array collision_map{alloc}; - Array hi{alloc}; - Array lo{alloc}; - Array local_id{alloc}; - - collision_map.set_parent(&m_top, top_position_for_collision_map); - hi.set_parent(&collision_map, s_collision_map_hi); - lo.set_parent(&collision_map, s_collision_map_lo); - local_id.set_parent(&collision_map, s_collision_map_local_id); - - ref_type collision_map_ref = to_ref(m_top.get(top_position_for_collision_map)); - if (collision_map_ref) { - collision_map.init_from_parent(); // Throws - } - else { - MemRef mem = Array::create_empty_array(Array::type_HasRefs, false, alloc); // Throws - collision_map.init_from_mem(mem); // Throws - collision_map.update_parent(); - - ref_type lo_ref = Array::create_array(Array::type_Normal, false, 0, 0, alloc).get_ref(); // Throws - ref_type hi_ref = Array::create_array(Array::type_Normal, false, 0, 0, alloc).get_ref(); // Throws - ref_type local_id_ref = Array::create_array(Array::type_Normal, false, 0, 0, alloc).get_ref(); // Throws - collision_map.add(lo_ref); // Throws - collision_map.add(hi_ref); // Throws - collision_map.add(local_id_ref); // Throws - } - - hi.init_from_parent(); // Throws - lo.init_from_parent(); // Throws - local_id.init_from_parent(); // Throws - - size_t num_entries = hi.size(); - REALM_ASSERT(lo.size() == num_entries); - REALM_ASSERT(local_id.size() == num_entries); - - auto lower_bound_object_id = [&](GlobalKey object_id) -> size_t { - size_t i = hi.lower_bound_int(int64_t(object_id.hi())); - while (i < num_entries && uint64_t(hi.get(i)) == object_id.hi() && uint64_t(lo.get(i)) < object_id.lo()) - ++i; - return i; - }; - - auto insert_collision = [&](GlobalKey object_id, ObjKey new_local_id) { - size_t i = lower_bound_object_id(object_id); - if (i != num_entries) { - GlobalKey existing{uint64_t(hi.get(i)), uint64_t(lo.get(i))}; - if (existing == object_id) { - REALM_ASSERT(new_local_id.value == local_id.get(i)); - return; - } - } - hi.insert(i, int64_t(object_id.hi())); - lo.insert(i, int64_t(object_id.lo())); - local_id.insert(i, new_local_id.value); - ++num_entries; - }; - - auto sequence_number_for_local_id = allocate_sequence_number(); - ObjKey new_local_id = make_tagged_local_id_after_hash_collision(sequence_number_for_local_id); - insert_collision(incoming_id, new_local_id); - insert_collision(colliding_id, colliding_local_id); - - return new_local_id; -} - Obj Table::get_or_create_tombstone(ObjKey key, ColKey pk_col, Mixed pk_val) { - auto unres_key = key.get_unresolved(); - ensure_graveyard(); - auto tombstone = m_tombstones->try_get_obj(unres_key); - if (tombstone) { - if (pk_col) { - auto existing_pk_value = tombstone.get_any(pk_col); - // It may just be the same object - if (existing_pk_value != pk_val) { - // We have a collision - create new ObjKey - key = allocate_local_id_after_hash_collision({pk_val}, {existing_pk_value}, key); - return get_or_create_tombstone(key, pk_col, pk_val); + if (auto sz = m_tombstones->size()) { + // Check for existing tombstone + Iterator end(*m_tombstones, sz); + for (Iterator it(*m_tombstones, 0); it != end; ++it) { + auto existing_pk_value = it->get_any(m_primary_key_col); + // If the primary key is the same, the object should be resurrected below + if (existing_pk_value == pk_val) { + return *it; } } - return tombstone; } - return m_tombstones->insert(unres_key, {{pk_col, pk_val}}); -} -void Table::free_local_id_after_hash_collision(ObjKey key) -{ - if (ref_type collision_map_ref = to_ref(m_top.get(top_position_for_collision_map))) { - if (key.is_unresolved()) { - // Keys will always be inserted as resolved - key = key.get_unresolved(); - } - // Possible optimization: Cache these accessors - Array collision_map{m_alloc}; - Array local_id{m_alloc}; - - collision_map.set_parent(&m_top, top_position_for_collision_map); - local_id.set_parent(&collision_map, s_collision_map_local_id); - collision_map.init_from_ref(collision_map_ref); - local_id.init_from_parent(); - auto ndx = local_id.find_first(key.value); - if (ndx != realm::npos) { - Array hi{m_alloc}; - Array lo{m_alloc}; - - hi.set_parent(&collision_map, s_collision_map_hi); - lo.set_parent(&collision_map, s_collision_map_lo); - hi.init_from_parent(); - lo.init_from_parent(); - - hi.erase(ndx); - lo.erase(ndx); - local_id.erase(ndx); - if (hi.size() == 0) { - free_collision_table(); - } - } + // Create new tombstone + if (!key) { + key = get_next_valid_key(); } + auto unres_key = key.get_unresolved(); + REALM_ASSERT(!m_tombstones->is_valid(unres_key)); + return m_tombstones->insert(unres_key, {{pk_col, pk_val}}); } void Table::free_collision_table() @@ -2690,15 +2413,10 @@ ObjKey Table::invalidate_object(ObjKey key) if (obj.has_backlinks(false)) { // If the object has backlinks, we should make a tombstone // and make inward links point to it, - if (auto primary_key_col = get_primary_key_column()) { - auto pk = obj.get_any(primary_key_col); - GlobalKey object_id{pk}; - auto unres_key = global_to_local_object_id_hashed(object_id); - tombstone = get_or_create_tombstone(unres_key, primary_key_col, pk); - } - else { - tombstone = get_or_create_tombstone(key, {}, {}); - } + auto primary_key_col = get_primary_key_column(); + REALM_ASSERT(primary_key_col); + auto pk = obj.get_any(primary_key_col); + tombstone = get_or_create_tombstone(key, primary_key_col, pk); tombstone.assign_pk_and_backlinks(obj); } @@ -2934,7 +2652,7 @@ ObjKey Table::get_next_valid_key() ObjKey key; do { key = ObjKey(allocate_sequence_number()); - } while (m_clusters.is_valid(key)); + } while (key.value < m_clusters.get_last_key_value() && m_clusters.is_valid(key)); return key; } diff --git a/src/realm/table.hpp b/src/realm/table.hpp index 7d5f9fa39ec..10200a210d6 100644 --- a/src/realm/table.hpp +++ b/src/realm/table.hpp @@ -35,7 +35,6 @@ #include #include #include -#include // Only set this to one when testing the code paths that exercise object ID // hash collisions. It artificially limits the "optimistic" local ID to use @@ -293,9 +292,6 @@ class Table { // Create an object with key. If the key is omitted, a key will be generated by the system Obj create_object(ObjKey key = {}, const FieldValues& = {}); - // Create an object with specific GlobalKey - or return already existing object - // Potential tombstone will be resurrected - Obj create_object(GlobalKey object_id, const FieldValues& = {}); // Create an object with primary key. If an object with the given primary key already exists, it // will be returned and did_create (if supplied) will be set to false. // Potential tombstone will be resurrected @@ -307,18 +303,10 @@ class Table { } // Return key for existing object or return null key. ObjKey find_primary_key(Mixed value) const; - // Return ObjKey for object identified by id. If objects does not exist, return null key - // Important: This function must not be called for tables with primary keys. - ObjKey get_objkey(GlobalKey id) const; // Return key for existing object or return unresolved key. // Important: This is to be used ONLY by the Sync client. SDKs should NEVER // observe an unresolved key. Ever. ObjKey get_objkey_from_primary_key(const Mixed& primary_key); - // Return key for existing object or return unresolved key. - // Important: This is to be used ONLY by the Sync client. SDKs should NEVER - // observe an unresolved key. Ever. - // Important (2): This function must not be called for tables with primary keys. - ObjKey get_objkey_from_global_key(GlobalKey key); /// Create a number of objects and add corresponding keys to a vector void create_objects(size_t number, std::vector& keys); /// Create a number of objects with keys supplied @@ -328,7 +316,6 @@ class Table { { return key && m_clusters.is_valid(key); } - GlobalKey get_object_id(ObjKey key) const; Obj get_object(ObjKey key) const { REALM_ASSERT(!key.is_unresolved()); @@ -417,7 +404,6 @@ class Table { uint64_t allocate_sequence_number(); // Used by upgrade void set_sequence_number(uint64_t seq); - void set_collision_map(ref_type ref); // Get the key of this table directly, without needing a Table accessor. static TableKey get_key_direct(Allocator& alloc, ref_type top_ref); @@ -788,26 +774,9 @@ class Table { void validate_column_is_unique(ColKey col_key) const; ObjKey get_next_valid_key(); - /// Some Object IDs are generated as a tuple of the client_file_ident and a - /// local sequence number. This function takes the next number in the - /// sequence for the given table and returns an appropriate globally unique - /// GlobalKey. - GlobalKey allocate_object_id_squeezed(); - - /// Find the local 64-bit object ID for the provided global 128-bit ID. - ObjKey global_to_local_object_id_hashed(GlobalKey global_id) const; - - /// After a local ObjKey collision has been detected, this function may be - /// called to obtain a non-colliding local ObjKey in such a way that subsequent - /// calls to global_to_local_object_id() will return the correct local ObjKey - /// for both \a incoming_id and \a colliding_id. - ObjKey allocate_local_id_after_hash_collision(GlobalKey incoming_id, GlobalKey colliding_id, - ObjKey colliding_local_id); /// Create a placeholder for a not yet existing object and return key to it Obj get_or_create_tombstone(ObjKey key, ColKey pk_col, Mixed pk_val); - /// Should be called when an object is deleted - void free_local_id_after_hash_collision(ObjKey key); - /// Should be called when last entry is removed - or when table is cleared + /// Should be called from upgrade function void free_collision_table(); /// Called in the context of Group::commit() to ensure that @@ -1427,10 +1396,6 @@ class _impl::TableFriend { { table.batch_erase_rows(keys); // Throws } - static ObjKey global_to_local_object_id_hashed(const Table& table, GlobalKey global_id) - { - return table.global_to_local_object_id_hashed(global_id); - } }; } // namespace realm diff --git a/src/realm/transaction.cpp b/src/realm/transaction.cpp index f427836c8f4..65a2a1f683f 100644 --- a/src/realm/transaction.cpp +++ b/src/realm/transaction.cpp @@ -570,6 +570,7 @@ void Transaction::upgrade_file_format(int target_file_format_version) if (current_file_format_version < 24) { for (auto k : table_keys) { auto t = get_table(k); + t->free_collision_table(); t->migrate_set_orderings(); // rewrite sets to use the new string/binary order // Although StringIndex sort order has been changed in this format, we choose to // avoid upgrading them because it affects a small niche case. Instead, there is a diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 27b084d0ad8..ff96d397e48 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -46,7 +46,6 @@ set(CORE_TEST_SOURCES test_dictionary.cpp test_file.cpp test_file_locks.cpp - test_global_key.cpp test_group.cpp test_impl_simulated_failure.cpp test_index_string.cpp @@ -189,7 +188,6 @@ if(REALM_ENABLE_SYNC) test_instruction_replication.cpp test_lang_bind_helper_sync.cpp test_noinst_server_dir.cpp - test_stable_ids.cpp test_sync.cpp test_sync_auth.cpp test_sync_history_migration.cpp diff --git a/test/object-store/transaction_log_parsing.cpp b/test/object-store/transaction_log_parsing.cpp index 3de2cfc6e1a..cc534fe89c9 100644 --- a/test/object-store/transaction_log_parsing.cpp +++ b/test/object-store/transaction_log_parsing.cpp @@ -1592,7 +1592,11 @@ TEMPLATE_TEST_CASE("DeepChangeChecker collections", "[notifications]", cf::ListO auto r = Realm::get_shared_realm(config); r->update_schema({ {"table", - {{"int", PropertyType::Int}, + {{ + "int", + PropertyType::Int, + Property::IsPrimary{true}, + }, {"link1", PropertyType::Object | PropertyType::Nullable, "table"}, {"link2", PropertyType::Object | PropertyType::Nullable, "table"}, test_type.property()}}, @@ -1603,7 +1607,7 @@ TEMPLATE_TEST_CASE("DeepChangeChecker collections", "[notifications]", cf::ListO std::vector objects; r->begin_transaction(); for (int i = 0; i < 10; ++i) - objects.push_back(table->create_object().set_all(i)); + objects.push_back(table->create_object_with_primary_key(i)); r->commit_transaction(); auto track_changes = [&](auto&& f) { @@ -1835,7 +1839,7 @@ TEST_CASE("DeepChangeChecker singular links", "[notifications]") { r->update_schema({{ "table", { - {"int", PropertyType::Int}, + {"int", PropertyType::Int, Property::IsPrimary{true}}, {"link1", PropertyType::Object | PropertyType::Nullable, "table"}, {"link2", PropertyType::Object | PropertyType::Nullable, "table"}, {"mixed_link", PropertyType::Mixed | PropertyType::Nullable}, @@ -1847,7 +1851,7 @@ TEST_CASE("DeepChangeChecker singular links", "[notifications]") { std::vector objects; r->begin_transaction(); for (int i = 0; i < 10; ++i) - objects.push_back(table->create_object().set_all(i)); + objects.push_back(table->create_object_with_primary_key(i)); r->commit_transaction(); auto track_changes = [&](auto&& f) { @@ -2124,7 +2128,7 @@ TEST_CASE("DeepChangeChecker singular links", "[notifications]") { table->clear(); objects.clear(); for (int i = 0; i < 20; ++i) - objects.push_back(table->create_object().set_all(i)); + objects.push_back(table->create_object_with_primary_key(i)); for (int i = 0; i < 19; ++i) set_link(objects[i], link_column, ObjLink{dst_table_key, objects[i + 1].get_key()}); r->commit_transaction(); diff --git a/test/sync_fixtures.hpp b/test/sync_fixtures.hpp index 720e15d608b..0c8ea3cb084 100644 --- a/test/sync_fixtures.hpp +++ b/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(std::move(config_2)); } diff --git a/test/test_global_key.cpp b/test/test_global_key.cpp deleted file mode 100644 index e504d53c2ab..00000000000 --- a/test/test_global_key.cpp +++ /dev/null @@ -1,77 +0,0 @@ -/************************************************************************* - * - * Copyright 2016 Realm Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - **************************************************************************/ - -#include - -#include "test.hpp" - -#include -#include - -using namespace realm; - -TEST(GlobalKey_ToString) -{ - CHECK_EQUAL(GlobalKey(0xabc, 0xdef).to_string(), "{0abc-0def}"); - CHECK_EQUAL(GlobalKey(0x11abc, 0x999def).to_string(), "{11abc-999def}"); - CHECK_EQUAL(GlobalKey(0, 0).to_string(), "{0000-0000}"); -} - -TEST(GlobalKey_FromString) -{ - CHECK_EQUAL(GlobalKey::from_string("{0-0}"), GlobalKey(0, 0)); - CHECK_EQUAL(GlobalKey::from_string("{aaaabbbbccccdddd-eeeeffff00001111}"), - GlobalKey(0xaaaabbbbccccddddULL, 0xeeeeffff00001111ULL)); - CHECK_THROW(GlobalKey::from_string(""), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("0"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("-"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("0-"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0-0"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0-0-0}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{aaaabbbbccccdddde-0}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0g-0}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0-0g}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{0-aaaabbbbccccdddde}"), InvalidArgument); - CHECK_THROW(GlobalKey::from_string("{-}"), InvalidArgument); - - // std::strtoull accepts the "0x" prefix. We don't. - CHECK_THROW(GlobalKey::from_string("{0x0-0x0}"), InvalidArgument); - { - std::istringstream istr("{1-2}"); - GlobalKey oid; - istr >> oid; - CHECK_EQUAL(oid, GlobalKey(1, 2)); - } - { - std::istringstream istr("{1-2"); - GlobalKey oid; - istr >> oid; - CHECK(istr.rdstate() & std::istream::failbit); - CHECK_EQUAL(oid, GlobalKey()); - } -} - -TEST(GlobalKey_Compare) -{ - CHECK_LESS(GlobalKey(0, 0), GlobalKey(0, 1)); - CHECK_LESS(GlobalKey(0, 0), GlobalKey(1, 0)); -} diff --git a/test/test_instruction_replication.cpp b/test/test_instruction_replication.cpp index 3c1c7239916..3b2e5dd3af8 100644 --- a/test/test_instruction_replication.cpp +++ b/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() diff --git a/test/test_mixed_null_assertions.cpp b/test/test_mixed_null_assertions.cpp index 33a5ea2984a..f5dc6db194e 100644 --- a/test/test_mixed_null_assertions.cpp +++ b/test/test_mixed_null_assertions.cpp @@ -89,10 +89,10 @@ TEST(List_Mixed_do_insert) TEST(Mixed_List_unresolved_as_null) { Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_list(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); auto list = obj.get_list("mixeds"); @@ -168,10 +168,10 @@ TEST(Mixed_List_unresolved_as_null) { Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_list(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); auto list = obj.get_list("mixeds"); list.insert(0, obj1); @@ -186,10 +186,10 @@ TEST(Mixed_List_unresolved_as_null) { Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_list(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); auto list = obj.get_list("mixeds"); list.insert(0, obj1); @@ -205,11 +205,11 @@ TEST(Mixed_Set_unresolved_links) { Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_set(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); - auto obj2 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); + auto obj2 = t->create_object_with_primary_key(3); auto set = obj.get_set("mixeds"); auto [it, success] = set.insert(Mixed{obj1}); obj1.invalidate(); @@ -261,11 +261,11 @@ TEST(Mixed_Set_unresolved_links) { // erase null but there are only unresolved links in the set Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_set(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); - auto obj2 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); + auto obj2 = t->create_object_with_primary_key(3); auto set = obj.get_set("mixeds"); set.insert(obj1); set.insert(obj2); @@ -286,11 +286,11 @@ TEST(Mixed_Set_unresolved_links) { // erase null when there are unresolved and nulls Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_set(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); - auto obj2 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); + auto obj2 = t->create_object_with_primary_key(3); auto set = obj.get_set("mixeds"); set.insert(obj1); set.insert(obj2); @@ -312,11 +312,11 @@ TEST(Mixed_Set_unresolved_links) { // assure that random access iterator does not return an unresolved link Group g; - auto t = g.add_table("foo"); + auto t = g.add_table_with_primary_key("foo", type_Int, "id"); t->add_column_set(type_Mixed, "mixeds"); - auto obj = t->create_object(); - auto obj1 = t->create_object(); - auto obj2 = t->create_object(); + auto obj = t->create_object_with_primary_key(1); + auto obj1 = t->create_object_with_primary_key(2); + auto obj2 = t->create_object_with_primary_key(3); auto set = obj.get_set("mixeds"); set.insert(obj1); set.insert(obj2); diff --git a/test/test_set.cpp b/test/test_set.cpp index 4c0ecd0d444..28120f31d66 100644 --- a/test/test_set.cpp +++ b/test/test_set.cpp @@ -241,7 +241,7 @@ TEST(Set_Links) { Group g; auto foos = g.add_table("class_Foo"); - auto bars = g.add_table("class_Bar"); + auto bars = g.add_table_with_primary_key("class_Bar", type_Int, "id"); auto cabs = g.add_table("class_Cab"); ColKey col_links = foos->add_column_set(*bars, "links"); @@ -249,10 +249,10 @@ TEST(Set_Links) auto foo = foos->create_object(); - auto bar1 = bars->create_object(); - auto bar2 = bars->create_object(); - auto bar3 = bars->create_object(); - auto bar4 = bars->create_object(); + auto bar1 = bars->create_object_with_primary_key(1); + auto bar2 = bars->create_object_with_primary_key(2); + auto bar3 = bars->create_object_with_primary_key(3); + auto bar4 = bars->create_object_with_primary_key(4); auto cab1 = cabs->create_object(); auto cab2 = cabs->create_object(); @@ -473,13 +473,13 @@ TEST(Set_LnkSetUnresolved) { Group g; auto foos = g.add_table("class_Foo"); - auto bars = g.add_table("class_Bar"); + auto bars = g.add_table_with_primary_key("class_Bar", type_Int, "id"); ColKey col_links = foos->add_column_set(*bars, "links"); auto foo = foos->create_object(); - auto bar1 = bars->create_object(); - auto bar2 = bars->create_object(); - auto bar3 = bars->create_object(); + auto bar1 = bars->create_object_with_primary_key(1); + auto bar2 = bars->create_object_with_primary_key(2); + auto bar3 = bars->create_object_with_primary_key(3); auto key_set = foo.get_set(col_links); auto link_set = foo.get_linkset(col_links); diff --git a/test/test_stable_ids.cpp b/test/test_stable_ids.cpp deleted file mode 100644 index e28a6cf7896..00000000000 --- a/test/test_stable_ids.cpp +++ /dev/null @@ -1,322 +0,0 @@ -#include "test.hpp" - -#include -#include -#include -#include - -#include -#include -#include - -using namespace realm; -using namespace realm::sync; - - -namespace { - -struct MakeClientHistory { - static std::unique_ptr make_history() - { - return realm::sync::make_client_replication(); - } -}; - -struct MakeServerHistory { - class HistoryContext : public _impl::ServerHistory::Context { - public: - std::mt19937_64& server_history_get_random() noexcept override final - { - return m_random; - } - - private: - std::mt19937_64 m_random; - }; - class WrapServerHistory : public HistoryContext, public _impl::ServerHistory { - public: - WrapServerHistory() - : _impl::ServerHistory{static_cast(*this)} - { - } - }; - - static std::unique_ptr<_impl::ServerHistory> make_history() - { - return std::make_unique(); - } -}; - -} // unnamed namespace - - -TEST_TYPES(InstructionReplication_CreateIdColumnInNewTables, MakeClientHistory, MakeServerHistory) -{ - SHARED_GROUP_TEST_PATH(test_dir); - auto history = TEST_TYPE::make_history(); - DBRef sg = DB::create(*history, test_dir); - - { - WriteTransaction wt{sg}; - wt.get_or_add_table("class_foo"); - wt.commit(); - } - - // Check that only the AddTable instruction is emitted - Changeset result; - auto buffer = history->get_instruction_encoder().release(); - util::SimpleInputStream stream{buffer}; - sync::parse_changeset(stream, result); - CHECK_EQUAL(result.size(), 1); - CHECK_EQUAL(result.begin()->type(), Instruction::Type::AddTable); - auto& instr = result.begin()->get_as(); - CHECK_EQUAL(result.get_string(instr.table), "foo"); - - auto rt = sg->start_read(); - ConstTableRef foo = rt->get_table("class_foo"); - CHECK(foo); - CHECK_EQUAL(foo->get_column_count(), 0); -} - -TEST_TYPES(InstructionReplication_PopulatesObjectIdColumn, MakeClientHistory, MakeServerHistory) -{ - SHARED_GROUP_TEST_PATH(test_dir); - auto history = TEST_TYPE::make_history(); - - DBRef sg = DB::create(*history, test_dir); - - auto client_file_ident = sg->start_read()->get_sync_file_id(); - - // Tables without primary keys: - { - WriteTransaction wt{sg}; - TableRef t0 = wt.get_or_add_table("class_t0"); - - auto obj0 = t0->create_object(); - auto obj1 = t0->create_object(); - - // Object IDs should be peerID plus a sequence number - CHECK_EQUAL(obj0.get_object_id(), GlobalKey(client_file_ident, 0)); - CHECK_EQUAL(obj1.get_object_id(), GlobalKey(client_file_ident, 1)); - } - - // Tables with integer primary keys: - { - WriteTransaction wt{sg}; - TableRef t1 = wt.get_group().add_table_with_primary_key("class_t1", type_Int, "pk"); - auto obj0 = t1->create_object_with_primary_key(123); - - GlobalKey expected_object_id(123); - CHECK_EQUAL(obj0.get_object_id(), expected_object_id); - } - - // Tables with string primary keys: - { - WriteTransaction wt{sg}; - TableRef t2 = wt.get_group().add_table_with_primary_key("class_t2", type_String, "pk"); - auto obj0 = t2->create_object_with_primary_key("foo"); - - GlobalKey expected_object_id("foo"); - CHECK_EQUAL(obj0.get_object_id(), expected_object_id); - } - - // Attempting to create a table that already exists is a no-op if the same primary key name, type and nullability - // is used. - { - WriteTransaction wt{sg}; - TableRef t1 = wt.get_group().get_or_add_table_with_primary_key("class_t1", type_Int, "pk"); - TableRef t11 = wt.get_group().get_or_add_table_with_primary_key("class_t1", type_Int, "pk"); - CHECK_EQUAL(t1, t11); - - TableRef t2 = - wt.get_group().get_or_add_table_with_primary_key("class_t2", type_Int, "pk", /* nullable */ true); - TableRef t21 = - wt.get_group().get_or_add_table_with_primary_key("class_t2", type_Int, "pk", /* nullable */ true); - CHECK_EQUAL(t2, t21); - - TableRef t3 = wt.get_group().get_or_add_table_with_primary_key("class_t3", type_String, "pk"); - TableRef t31 = wt.get_group().get_or_add_table_with_primary_key("class_t3", type_String, "pk"); - CHECK_EQUAL(t3, t31); - - TableRef t4 = - wt.get_group().get_or_add_table_with_primary_key("class_t4", type_String, "pk", /* nullable */ true); - TableRef t41 = - wt.get_group().get_or_add_table_with_primary_key("class_t4", type_String, "pk", /* nullable */ true); - CHECK_EQUAL(t4, t41); - } - - // Attempting to create a table that already exists causes an assertion failure if different primary key name, - // type, or nullability is specified. This is not currently testable. -} - -TEST(StableIDs_ChangesGlobalObjectIdWhenPeerIdReceived) -{ - SHARED_GROUP_TEST_PATH(test_dir); - auto repl = make_client_replication(); - - DBRef sg = DB::create(*repl, test_dir); - - ColKey link_col; - { - WriteTransaction wt{sg}; - TableRef t0 = wt.get_or_add_table("class_t0"); - TableRef t1 = wt.get_or_add_table("class_t1"); - link_col = t0->add_column(*t1, "link"); - - Obj t1_k1 = t1->create_object(); - Obj t0_k1 = t0->create_object().set(link_col, t1_k1.get_key()); - Obj t0_k2 = t0->create_object(); - - // Object IDs should be peerID plus a sequence number - CHECK_EQUAL(t0_k1.get_object_id(), GlobalKey(0, 0)); - CHECK_EQUAL(t0_k2.get_object_id(), GlobalKey(0, 1)); - wt.commit(); - } - - bool fix_up_object_ids = true; - auto& history = repl->get_history(); - history.set_client_file_ident({1, 123}, fix_up_object_ids); - - // Save the changeset to replay later - UploadCursor upload_cursor{0, 0}; - std::vector changesets; - version_type locked_server_version; // Dummy - history.find_uploadable_changesets(upload_cursor, 2, changesets, locked_server_version); - CHECK_GREATER_EQUAL(changesets.size(), 1); - auto& changeset = changesets[0].changeset; - ChunkedBinaryInputStream stream{changeset}; - Changeset result; - sync::parse_changeset(stream, result); - - // Check that ObjectIds gets translated correctly - { - ReadTransaction rt{sg}; - ConstTableRef t0 = rt.get_table("class_t0"); - ConstTableRef t1 = rt.get_table("class_t1"); - auto it = t0->begin(); - GlobalKey oid0 = it->get_object_id(); - ObjKey link_ndx = it->get(link_col); - ++it; - GlobalKey oid1 = it->get_object_id(); - CHECK_EQUAL(oid0, GlobalKey(1, 0)); - CHECK_EQUAL(oid1, GlobalKey(1, 1)); - GlobalKey oid2 = t1->get_object_id(link_ndx); - CHECK_EQUAL(oid2.hi(), 1); - CHECK_EQUAL(oid2, t1->begin()->get_object_id()); - } - - // Replay the transaction to see that the instructions were modified. - { - SHARED_GROUP_TEST_PATH(test_dir_2); - auto history_2 = make_client_replication(); - DBRef sg_2 = DB::create(*history_2, test_dir_2); - - WriteTransaction wt{sg_2}; - InstructionApplier applier{wt}; - applier.apply(result); - wt.commit(); - - // Check same invariants as above. - ReadTransaction rt{sg_2}; - ConstTableRef t0 = rt.get_table("class_t0"); - ConstTableRef t1 = rt.get_table("class_t1"); - auto it = t0->begin(); - GlobalKey oid0 = it->get_object_id(); - ObjKey link_ndx = it->get(link_col); - ++it; - GlobalKey oid1 = it->get_object_id(); - CHECK_EQUAL(oid0, GlobalKey(1, 0)); - CHECK_EQUAL(oid1, GlobalKey(1, 1)); - GlobalKey oid2 = t1->get_object_id(link_ndx); - CHECK_EQUAL(oid2.hi(), 1); - CHECK_EQUAL(oid2, t1->begin()->get_object_id()); - } -} - -TEST_TYPES(StableIDs_PersistPerTableSequenceNumber, MakeClientHistory, MakeServerHistory) -{ - SHARED_GROUP_TEST_PATH(test_dir); - { - auto history = TEST_TYPE::make_history(); - DBRef sg = DB::create(*history, test_dir); - WriteTransaction wt{sg}; - TableRef t0 = wt.get_or_add_table("class_t0"); - t0->create_object(); - t0->create_object(); - CHECK_EQUAL(t0->size(), 2); - wt.commit(); - } - { - auto history = TEST_TYPE::make_history(); - DBRef sg = DB::create(*history, test_dir); - WriteTransaction wt{sg}; - TableRef t0 = wt.get_or_add_table("class_t0"); - t0->create_object(); - t0->create_object(); - CHECK_EQUAL(t0->size(), 4); - wt.commit(); - } -} - -TEST_TYPES(StableIDs_CollisionMapping, MakeClientHistory, MakeServerHistory) -{ -#if REALM_EXERCISE_OBJECT_ID_COLLISION - - // This number corresponds to the mask used to calculate "optimistic" - // object IDs. See `GlobalKeyProvider::get_optimistic_local_id_hashed`. - const size_t num_objects_with_guaranteed_collision = 0xff; - - SHARED_GROUP_TEST_PATH(test_dir); - - { - auto history = TEST_TYPE::make_history(); - DBRef sg = DB::create(*history, test_dir); - { - WriteTransaction wt{sg}; - TableRef t0 = wt.get_group().add_table_with_primary_key("class_t0", type_String, "pk"); - - char buffer[12]; - for (size_t i = 0; i < num_objects_with_guaranteed_collision; ++i) { - const char* in = reinterpret_cast(&i); - size_t len = base64_encode(in, sizeof(i), buffer, sizeof(buffer)); - - sync::create_object_with_primary_key(wt, *t0, StringData{buffer, len}); - } - wt.commit(); - } - - { - ReadTransaction rt{sg}; - ConstTableRef t0 = rt.get_table("class_t0"); - // Check that at least one object exists where the 63rd bit is set. - size_t num_object_keys_with_63rd_bit_set = 0; - uint64_t bit63 = 0x4000000000000000; - for (Obj obj : *t0) { - if (obj.get_key().value & bit63) - ++num_object_keys_with_63rd_bit_set; - } - CHECK_GREATER(num_object_keys_with_63rd_bit_set, 0); - } - } - - // Check that locally allocated IDs are properly persisted - { - auto history_2 = TEST_TYPE::make_history(); - DBRef sg_2 = DB::create(*history_2, test_dir); - WriteTransaction wt{sg_2}; - TableRef t0 = wt.get_table("class_t0"); - - // Make objects with primary keys that do not already exist but are guaranteed - // to cause further collisions. - char buffer[12]; - for (size_t i = 0; i < num_objects_with_guaranteed_collision; ++i) { - size_t foo = num_objects_with_guaranteed_collision + i; - const char* in = reinterpret_cast(&foo); - size_t len = base64_encode(in, sizeof(foo), buffer, sizeof(buffer)); - - sync::create_object_with_primary_key(wt, *t0, StringData{buffer, len}); - } - } - -#endif // REALM_EXERCISE_ID_COLLISION -} diff --git a/test/test_sync.cpp b/test/test_sync.cpp index a3c7114bcc8..3c2eb9f139b 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -5604,49 +5604,6 @@ TEST(Sync_ServerSideEncryption) } -// This test calls row_for_object_id() for various object ids and tests that -// the right value is returned including that no assertions are hit. -TEST(Sync_RowForGlobalKey) -{ - TEST_CLIENT_DB(db); - - { - WriteTransaction wt(db); - TableRef table = wt.add_table("class_foo"); - table->add_column(type_Int, "i"); - wt.commit(); - } - - // Check that various object_ids are not in the table. - { - ReadTransaction rt(db); - ConstTableRef table = rt.get_table("class_foo"); - CHECK(table); - - // Default constructed GlobalKey - { - GlobalKey object_id; - auto row_ndx = table->get_objkey(object_id); - CHECK_NOT(row_ndx); - } - - // GlobalKey with small lo and hi values - { - GlobalKey object_id{12, 24}; - auto row_ndx = table->get_objkey(object_id); - CHECK_NOT(row_ndx); - } - - // GlobalKey with lo and hi values past the 32 bit limit. - { - GlobalKey object_id{uint_fast64_t(1) << 50, uint_fast64_t(1) << 52}; - auto row_ndx = table->get_objkey(object_id); - CHECK_NOT(row_ndx); - } - } -} - - TEST(Sync_LogCompaction_EraseObject_LinkList) { TEST_DIR(dir); @@ -6678,7 +6635,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 = [&] { @@ -6953,7 +6910,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; @@ -7025,7 +6982,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}; @@ -7103,7 +7060,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; @@ -7271,7 +7228,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; diff --git a/test/test_unresolved_links.cpp b/test/test_unresolved_links.cpp index db675150829..a59d86e55bb 100644 --- a/test/test_unresolved_links.cpp +++ b/test/test_unresolved_links.cpp @@ -53,7 +53,7 @@ TEST(Unresolved_Basic) col_owns = persons->add_column(*cars, "car"); auto dealers = wt->add_table_with_primary_key("Dealer", type_Int, "cvr"); col_has = dealers->add_column_list(*cars, "stock"); - auto parts = wt->add_table("Parts"); // No primary key + auto parts = wt->add_table_with_primary_key("Parts", type_String, "id"); // No primary key col_part = cars->add_column(*parts, "part"); auto finn = persons->create_object_with_primary_key("finn.schiermer-andersen@mongodb.com"); @@ -65,7 +65,7 @@ TEST(Unresolved_Basic) auto stock = joergen.get_list(col_has); auto skoda = cars->create_object_with_primary_key("Skoda Fabia").set(col_price, Decimal128("149999.5")); - auto thingamajig = parts->create_object(); + auto thingamajig = parts->create_object_with_primary_key("abc-123"); skoda.set(col_part, thingamajig.get_key()); auto new_tesla = cars->get_objkey_from_primary_key("Tesla 10"); @@ -78,7 +78,7 @@ TEST(Unresolved_Basic) stock.insert(1, skoda.get_key()); // Create a tombstone implicitly - auto doodad = parts->get_objkey_from_global_key(GlobalKey{999, 999}); + auto doodad = parts->get_objkey_from_primary_key("def-123"); CHECK(doodad.is_unresolved()); CHECK_EQUAL(parts->nb_unresolved(), 1); @@ -138,12 +138,11 @@ TEST(Unresolved_Basic) auto parts = wt->get_table("Parts"); auto tesla = wt->get_table("Car")->create_object_with_primary_key("Tesla 10"); tesla.set(col_price, Decimal128("499999.5")); - auto doodad = parts->create_object(GlobalKey{999, 999}); - auto doodad1 = parts->create_object(GlobalKey{999, 999}); // Check idempotency + auto doodad = parts->create_object_with_primary_key("def-123"); + auto doodad1 = parts->create_object_with_primary_key("def-123"); // Check idempotency CHECK_EQUAL(doodad.get_key(), doodad1.get_key()); - CHECK_EQUAL(doodad.get_object_id(), doodad1.get_object_id()); tesla.set(col_part, doodad.get_key()); - auto doodad_key = parts->get_objkey_from_global_key(GlobalKey{999, 999}); + auto doodad_key = parts->get_objkey_from_primary_key("def-123"); CHECK(!doodad_key.is_unresolved()); CHECK_EQUAL(wt->get_table("Parts")->nb_unresolved(), 0); @@ -165,13 +164,13 @@ TEST(Unresolved_InvalidateObject) auto cars = g.add_table_with_primary_key("Car", type_String, "model"); auto col_wheels = cars->add_column_list(*wheels, "wheels"); auto col_price = cars->add_column(type_Decimal, "price"); - auto dealers = g.add_table("Dealer"); + auto dealers = g.add_table_with_primary_key("Dealer", type_Int, "id"); auto col_has = dealers->add_column_list(*cars, "stock"); auto organization = g.add_table("Organization"); auto col_members = organization->add_column_list(*dealers, "members"); - auto dealer1 = dealers->create_object(); - auto dealer2 = dealers->create_object(); + auto dealer1 = dealers->create_object_with_primary_key(1); + auto dealer2 = dealers->create_object_with_primary_key(2); auto org = organization->create_object(); auto members = org.get_linklist(col_members); diff --git a/test/util/compare_groups.cpp b/test/util/compare_groups.cpp index ce8d4efc632..7cc8c86702e 100644 --- a/test/util/compare_groups.cpp +++ b/test/util/compare_groups.cpp @@ -198,8 +198,8 @@ sync::PrimaryKey primary_key_for_row(const Obj& obj) REALM_TERMINATE("Missing primary key type support"); } - GlobalKey global_key = obj.get_object_id(); - return global_key; + REALM_TERMINATE("Missing primary key"); + return {}; } sync::PrimaryKey primary_key_for_row(const Table& table, ObjKey key) @@ -261,12 +261,7 @@ ObjKey row_for_primary_key(const Table& table, sync::PrimaryKey key) REALM_TERMINATE("row_for_primary_key missing primary key type support"); } - if (auto global_key = mpark::get_if(&key)) { - return table.get_objkey(*global_key); - } - else { - REALM_TERMINATE("row_for_primary_key() with primary key, expected GlobalKey"); - } + REALM_TERMINATE("row_for_primary_key() expected primary key"); return {}; }