Throw if you mutate a committed MutableSubscriptionSet (#5162)

CHANGELOG.md

@@ -17,7 +17,7 @@
 -----------
 
 ### Internals
-* None.
+* Mutating a committed MutableSubscriptionSet will throw a LogicError. ([#5162](https://github.com/realm/realm-core/pull/5162))
 
 ----------------------------------------------
 

src/realm/sync/subscriptions.cpp

@@ -216,6 +216,13 @@ MutableSubscriptionSet::MutableSubscriptionSet(std::weak_ptr<const SubscriptionS
 {
 }
 
+void MutableSubscriptionSet::check_is_mutable() const
+{
+    if (m_tr->get_transact_stage() != DB::transact_Writing) {
+        throw LogicError(LogicError::wrong_transact_state);
+    }
+}
+
 MutableSubscriptionSet::iterator MutableSubscriptionSet::begin()
 {
     return m_subs.begin();
@@ -228,23 +235,27 @@ MutableSubscriptionSet::iterator MutableSubscriptionSet::end()
 
 MutableSubscriptionSet::iterator MutableSubscriptionSet::erase(const_iterator it)
 {
+    check_is_mutable();
     return m_subs.erase(it);
 }
 
 void MutableSubscriptionSet::clear()
 {
+    check_is_mutable();
     m_subs.clear();
 }
 
 void MutableSubscriptionSet::insert_sub(const Subscription& sub)
 {
+    check_is_mutable();
     m_subs.push_back(sub);
 }
 
 std::pair<SubscriptionSet::iterator, bool>
 MutableSubscriptionSet::insert_or_assign_impl(iterator it, util::Optional<std::string> name,
                                               std::string object_class_name, std::string query_str)
 {
+    check_is_mutable();
     if (it != end()) {
         it->m_object_class_name = std::move(object_class_name);
         it->m_query_string = std::move(query_str);
@@ -283,6 +294,7 @@ std::pair<SubscriptionSet::iterator, bool> MutableSubscriptionSet::insert_or_ass
 
 void MutableSubscriptionSet::update_state(State new_state, util::Optional<std::string_view> error_str)
 {
+    check_is_mutable();
     auto old_state = state();
     switch (new_state) {
         case State::Uncommitted:

src/realm/sync/subscriptions.hpp

@@ -266,6 +266,8 @@ class MutableSubscriptionSet : public SubscriptionSet {
 
     std::pair<iterator, bool> insert_or_assign_impl(iterator it, util::Optional<std::string> name,
                                                     std::string object_class_name, std::string query_str);
+    // Throws is m_tr is in the wrong state.
+    void check_is_mutable() const;
 
     void insert_sub_impl(ObjectId id, Timestamp created_at, Timestamp updated_at, StringData name,
                          StringData object_class_name, StringData query_str);

test/test_sync_subscriptions.cpp

@@ -1,3 +1,4 @@
+#include "realm/exceptions.hpp"
 #include "realm/sync/subscriptions.hpp"
 #include "realm/sync/noinst/client_history_impl.hpp"
 
@@ -130,10 +131,16 @@ TEST(Sync_SubscriptionStoreStateUpdates)
     // Clone the completed set and update it to have a new query.
     {
         auto new_set = store->get_latest().make_mutable_copy();
+        auto new_set_copy = new_set;
         CHECK_EQUAL(new_set.version(), 2);
         new_set.clear();
         new_set.insert_or_assign("b sub", query_b);
         std::move(new_set).commit();
+
+        // Mutating a MutableSubscriptionSet that's already been committed should throw a LogicError
+        CHECK_THROW(new_set_copy.clear(), LogicError);
+        CHECK_THROW(new_set_copy.erase(new_set_copy.begin()), LogicError);
+        CHECK_THROW(new_set_copy.insert_or_assign(query_b), LogicError);
     }
 
     // There should now be two subscription sets, version 1 is complete with query a and version 2 is pending with