Skip to content

Commit

Permalink
Revert "Throw if a table being written to has no subscriptions and FL…
Browse files Browse the repository at this point in the history
…X sync is enabled (#5488)"

This reverts commit 02adb14.
  • Loading branch information
jedelbo committed Jun 2, 2022
1 parent 91c5ac9 commit 26a34fa
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 271 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
### Enhancements
* The sync client will gracefully handle compensating write error messages from the server and pass detailed info to the SDK's sync error handler about which objects caused the compensating write to occur. ([#5528](https://github.com/realm/realm-core/pull/5528))
* Support for asymmetric sync. Tables can be marked as Asymmetric when opening the realm. Upon creation, asymmetric objects are synched unidirectionally. ([#5505](https://github.com/realm/realm-core/pull/5505))
* Creating an object for a class that has no subscriptions opened for it will now throw a NoSubscriptionForWrite exception ([#5488](https://github.com/realm/realm-core/pull/5488)).

### Fixed
* Added better comparator for `realm_user_t` and `realm_flx_sync_subscription_t` when using `realm_equals`.(Issue [#5522])(https://github.com/realm/realm-core/issues/5522).
Expand Down
10 changes: 0 additions & 10 deletions src/realm/exceptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,6 @@ class DuplicatePrimaryKeyValueException : public std::logic_error {
std::string m_property;
};

class NoSubscriptionForWrite : public std::runtime_error {
public:
NoSubscriptionForWrite(const std::string& msg);
};


/// The \c LogicError exception class is intended to be thrown only when
/// applications (or bindings) violate rules that are stated (or ought to have
Expand Down Expand Up @@ -377,11 +372,6 @@ inline InvalidPathError::InvalidPathError(const std::string& msg)
{
}

inline NoSubscriptionForWrite::NoSubscriptionForWrite(const std::string& msg)
: std::runtime_error(msg)
{
}

inline LogicError::LogicError(LogicError::ErrorKind k)
: m_kind(k)
{
Expand Down
5 changes: 2 additions & 3 deletions src/realm/group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,9 @@ class Group : public ArrayParent {
bool table_is_public(TableKey key) const;
static StringData table_name_to_class_name(StringData table_name)
{
REALM_ASSERT(table_name.begins_with(StringData(g_class_name_prefix, g_class_name_prefix_len)));
REALM_ASSERT(table_name.begins_with(g_class_name_prefix));
return table_name.substr(g_class_name_prefix_len);
}

using TableNameBuffer = std::array<char, max_table_name_length>;
static StringData class_name_to_table_name(StringData class_name, TableNameBuffer& buffer)
{
Expand Down Expand Up @@ -926,7 +925,7 @@ inline StringData Group::get_table_name(TableKey key) const

inline bool Group::table_is_public(TableKey key) const
{
return get_table_name(key).begins_with(StringData(g_class_name_prefix, g_class_name_prefix_len));
return get_table_name(key).begins_with(g_class_name_prefix);
}

inline bool Group::has_table(StringData name) const noexcept
Expand Down
25 changes: 0 additions & 25 deletions src/realm/object-store/impl/realm_coordinator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include <realm/object-store/sync/sync_user.hpp>
#include <realm/sync/history.hpp>
#include <realm/sync/noinst/client_history_impl.hpp>
#include "realm/sync/subscriptions.hpp"
#endif

#include <realm/db.hpp>
Expand Down Expand Up @@ -103,30 +102,6 @@ void RealmCoordinator::create_sync_session()
return;

m_sync_session = m_config.sync_config->user->sync_manager()->get_session(m_db, *m_config.sync_config);
if (m_config.sync_config && m_config.sync_config->flx_sync_requested) {
std::weak_ptr<sync::SubscriptionStore> weak_sub_mgr(m_sync_session->get_flx_subscription_store());
auto& history = static_cast<sync::ClientReplication&>(*m_db->get_replication());
history.set_write_validator_factory(
[weak_sub_mgr](Transaction& tr) -> util::UniqueFunction<sync::SyncReplication::WriteValidator> {
auto sub_mgr = weak_sub_mgr.lock();
if (!sub_mgr) {
throw std::runtime_error("Subscription store was destroyed while user writes were on-going");
}

auto latest_sub_tables = sub_mgr->get_tables_for_latest(tr);
return [tables = std::move(latest_sub_tables)](const Table& table) {
if (table.get_table_type() != Table::Type::TopLevel) {
return;
}
auto object_class_name = Group::table_name_to_class_name(table.get_name());
if (tables.find(object_class_name) == tables.end()) {
throw NoSubscriptionForWrite(util::format(
"Cannot write to class %1 when no flexible sync subscription has been created.",
object_class_name));
}
};
});
}

std::weak_ptr<RealmCoordinator> weak_self = shared_from_this();
SyncSession::Internal::set_sync_transact_callback(*m_sync_session, [weak_self](VersionID, VersionID) {
Expand Down
4 changes: 2 additions & 2 deletions src/realm/object-store/sync/sync_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1033,9 +1033,9 @@ std::string const& SyncSession::path() const
return m_db->get_path();
}

const std::shared_ptr<sync::SubscriptionStore>& SyncSession::get_flx_subscription_store()
sync::SubscriptionStore* SyncSession::get_flx_subscription_store()
{
return m_flx_subscription_store;
return m_flx_subscription_store.get();
}

void SyncSession::update_configuration(SyncConfig new_config)
Expand Down
2 changes: 1 addition & 1 deletion src/realm/object-store/sync/sync_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class SyncSession : public std::enable_shared_from_this<SyncSession> {
return m_server_url;
}

const std::shared_ptr<sync::SubscriptionStore>& get_flx_subscription_store();
sync::SubscriptionStore* get_flx_subscription_store();

// Create an external reference to this session. The sync session attempts to remain active
// as long as an external reference to the session exists.
Expand Down
30 changes: 13 additions & 17 deletions src/realm/sync/instruction_replication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ void SyncReplication::do_initiate_transact(Group& group, version_type current_ve
{
Replication::do_initiate_transact(group, current_version, history_updated);
m_transaction = dynamic_cast<Transaction*>(&group); // FIXME: Is this safe?
m_write_validator = make_write_validator(*m_transaction);
reset();
}

Expand Down Expand Up @@ -182,7 +181,7 @@ void SyncReplication::add_class(TableKey tk, StringData name, Table::Type table_
{
Replication::add_class(tk, name, table_type);

bool is_class = m_transaction->table_is_public(tk);
bool is_class = name.begins_with("class_");

if (is_class && !m_short_circuit) {
Instruction::AddTable instr;
Expand Down Expand Up @@ -210,7 +209,7 @@ void SyncReplication::add_class_with_primary_key(TableKey tk, StringData name, D
{
Replication::add_class_with_primary_key(tk, name, pk_type, pk_field, nullable, table_type);

bool is_class = m_transaction->table_is_public(tk);
bool is_class = name.begins_with("class_");

if (is_class && !m_short_circuit) {
Instruction::AddTable instr;
Expand Down Expand Up @@ -278,10 +277,6 @@ void SyncReplication::create_object_with_primary_key(const Table* table, ObjKey

Replication::create_object_with_primary_key(table, oid, value);
if (select_table(*table)) {
if (m_write_validator) {
m_write_validator(*table);
}

auto col = table->get_primary_key_column();
if (col && ((value.is_null() && col.is_nullable()) || DataType(col.get_type()) == value.get_type())) {
Instruction::CreateObject instr;
Expand Down Expand Up @@ -310,7 +305,7 @@ void SyncReplication::erase_class(TableKey table_key, size_t num_tables)

StringData table_name = m_transaction->get_table_name(table_key);

bool is_class = m_transaction->table_is_public(table_key);
bool is_class = table_name.begins_with("class_");

if (is_class) {
REALM_ASSERT(table_key == m_table_being_erased);
Expand Down Expand Up @@ -714,17 +709,18 @@ bool SyncReplication::select_table(const Table& table)
if (&table == m_last_table) {
return true;
}

if (!m_transaction->table_is_public(table.get_key())) {
else {
StringData name = table.get_name();
if (name.begins_with("class_")) {
m_last_class_name = emit_class_name(table);
m_last_table = &table;
m_last_field = ColKey{};
m_last_object = ObjKey{};
m_last_primary_key.reset();
return true;
}
return false;
}

m_last_class_name = emit_class_name(table);
m_last_table = &table;
m_last_field = ColKey{};
m_last_object = ObjKey{};
m_last_primary_key.reset();
return true;
}

bool SyncReplication::select_collection(const CollectionBase& view)
Expand Down
12 changes: 0 additions & 12 deletions src/realm/sync/instruction_replication.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ namespace sync {

class SyncReplication : public Replication {
public:
// This will be called for any instruction that mutates an object (instead of instructions that mutates
// schema) with the class name (without the "class_" prefix) of the object being modified. If The
// validator needs to reject the write, it should throw an exception.
using WriteValidator = void(const Table&);

void set_short_circuit(bool) noexcept;
bool is_short_circuited() const noexcept;

Expand All @@ -50,7 +45,6 @@ class SyncReplication : public Replication {
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 prepare_erase_class(TableKey tk) final;
void erase_class(TableKey table_key, size_t num_tables) final;
void rename_class(TableKey table_key, StringData new_name) final;
Expand Down Expand Up @@ -94,11 +88,6 @@ class SyncReplication : public Replication {
// Replication interface:
void do_initiate_transact(Group& group, version_type current_version, bool history_updated) override;

virtual util::UniqueFunction<WriteValidator> make_write_validator(Transaction&)
{
return {};
}

private:
bool m_short_circuit = false;

Expand Down Expand Up @@ -144,7 +133,6 @@ class SyncReplication : public Replication {
InternString m_last_class_name;
util::Optional<Instruction::PrimaryKey> m_last_primary_key;
InternString m_last_field_name;
util::UniqueFunction<WriteValidator> m_write_validator;
};

inline void SyncReplication::set_short_circuit(bool b) noexcept
Expand Down
9 changes: 0 additions & 9 deletions src/realm/sync/noinst/client_history_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,6 @@ void ClientReplication::finalize_changeset() noexcept
m_history.m_changeset_from_server = util::none;
}

util::UniqueFunction<SyncReplication::WriteValidator> ClientReplication::make_write_validator(Transaction& tr)
{
if (!m_write_validator_factory) {
return {};
}

return m_write_validator_factory(tr);
}

void ClientHistory::get_status(version_type& current_client_version, SaltedFileIdent& client_file_ident,
SyncProgress& progress) const
{
Expand Down
47 changes: 17 additions & 30 deletions src/realm/sync/noinst/client_history_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,9 @@ class ClientHistory final : public _impl::History, public TransformHistory {
std::uint_fast64_t&, std::uint_fast64_t&);

// Overriding member functions in realm::TransformHistory
version_type find_history_entry(version_type, version_type, HistoryEntry&) const noexcept override;
ChunkedBinaryData get_reciprocal_transform(version_type, bool&) const override;
void set_reciprocal_transform(version_type, BinaryData) override;
version_type find_history_entry(version_type, version_type, HistoryEntry&) const noexcept override final;
ChunkedBinaryData get_reciprocal_transform(version_type, bool&) const override final;
void set_reciprocal_transform(version_type, BinaryData) override final;

public: // Stuff in this section is only used by CLI tools.
/// set_local_origin_timestamp_override() allows you to override the origin timestamp of new changesets
Expand Down Expand Up @@ -430,13 +430,13 @@ class ClientHistory final : public _impl::History, public TransformHistory {
}

// Overriding member functions in realm::_impl::History
void set_group(Group* group, bool updated = false) override;
void update_from_ref_and_version(ref_type ref, version_type version) override;
void update_from_parent(version_type current_version) override;
void get_changesets(version_type, version_type, BinaryIterator*) const noexcept override;
void set_oldest_bound_version(version_type) override;
void verify() const override;
bool no_pending_local_changes(version_type version) const override;
void set_group(Group* group, bool updated = false) override final;
void update_from_ref_and_version(ref_type ref, version_type version) override final;
void update_from_parent(version_type current_version) override final;
void get_changesets(version_type, version_type, BinaryIterator*) const noexcept override final;
void set_oldest_bound_version(version_type) override final;
void verify() const override final;
bool no_pending_local_changes(version_type version) const final;
};

class ClientReplication final : public SyncReplication {
Expand All @@ -447,21 +447,12 @@ class ClientReplication final : public SyncReplication {
{
}

// A write validator factory takes a write transaction and returns a UniqueFunction containing a
// SyncReplication::WriteValidator. The factory will get called at the start of a write transaction
// and the WriteValidator it returns will be re-used for all mutations within the transaction.
using WriteValidatorFactory = util::UniqueFunction<WriteValidator>(Transaction&);
void set_write_validator_factory(util::UniqueFunction<WriteValidatorFactory> validator_factory)
{
m_write_validator_factory = std::move(validator_factory);
}

// Overriding member functions in realm::Replication
void initialize(DB& sg) override;
HistoryType get_history_type() const noexcept override;
int get_history_schema_version() const noexcept override;
bool is_upgradable_history_schema(int) const noexcept override;
void upgrade_history_schema(int) override;
void initialize(DB& sg) override final;
HistoryType get_history_type() const noexcept override final;
int get_history_schema_version() const noexcept override final;
bool is_upgradable_history_schema(int) const noexcept override final;
void upgrade_history_schema(int) override final;

_impl::History* _get_history_write() override
{
Expand All @@ -475,8 +466,8 @@ class ClientReplication final : public SyncReplication {
}

// Overriding member functions in realm::Replication
version_type prepare_changeset(const char*, size_t, version_type) override;
void finalize_changeset() noexcept override;
version_type prepare_changeset(const char*, size_t, version_type) override final;
void finalize_changeset() noexcept override final;

ClientHistory& get_history() noexcept
{
Expand All @@ -493,13 +484,9 @@ class ClientReplication final : public SyncReplication {
return m_apply_server_changes;
}

protected:
util::UniqueFunction<WriteValidator> make_write_validator(Transaction& tr) override;

private:
ClientHistory m_history;
const bool m_apply_server_changes;
util::UniqueFunction<WriteValidatorFactory> m_write_validator_factory;
};


Expand Down
Loading

0 comments on commit 26a34fa

Please sign in to comment.