Allow registering notifiers in write transactions prior to the first …

…change (#6560)

This fixes one of the common problems resulting from
realm/realm-swift#4818, as it makes it so that adding
new callbacks from within a notification callback will _usually_ work even if
the notification was triggered by beginning a write transaction. It still will
not work if a previously invoked callback made any changes (which would be a
very strange thing to do, but possible).

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
-* None.
+* New notifiers can now be registered in write transactions until changes have actually been made in the write transaction. This makes it so that new notifications can be registered inside change notifications triggered by beginning a write transaction (unless a previous callback performed writes).
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

src/realm/object-store/shared_realm.cpp

@@ -620,10 +620,16 @@ bool Realm::verify_notifications_available(bool throw_on_error) const
             throw WrongTransactionState("Cannot create asynchronous query for immutable Realms");
         return false;
     }
-    if (is_in_transaction()) {
-        if (throw_on_error)
-            throw WrongTransactionState("Cannot create asynchronous query while in a write transaction");
-        return false;
+    if (throw_on_error) {
+        if (m_transaction && m_transaction->get_commit_size() > 0)
+            throw WrongTransactionState(
+                "Cannot create asynchronous query after making changes in a write transaction.");
+    }
+    else {
+        // Don't create implicit notifiers inside write transactions even if
+        // we could as it wouldn't actually be used
+        if (is_in_transaction())
+            return false;
     }
 
     return true;
@@ -1310,7 +1316,7 @@ uint64_t Realm::get_schema_version(const Realm::Config& config)
 bool Realm::is_frozen() const
 {
     bool result = bool(m_frozen_version);
-    REALM_ASSERT_DEBUG((result && m_transaction) ? m_transaction->is_frozen() : true);
+    REALM_ASSERT_DEBUG(!result || !m_transaction || m_transaction->is_frozen());
     return result;
 }
 

src/realm/transaction.cpp

@@ -322,12 +322,20 @@ TransactionRef Transaction::freeze()
 TransactionRef Transaction::duplicate()
 {
     auto version = VersionID(m_read_lock.m_version, m_read_lock.m_reader_idx);
-    if (m_transact_stage == DB::transact_Reading)
-        return db->start_read(version);
-    if (m_transact_stage == DB::transact_Frozen)
-        return db->start_frozen(version);
-
-    throw WrongTransactionState("Can only duplicate a read/frozen transaction");
+    switch (m_transact_stage) {
+        case DB::transact_Ready:
+            throw WrongTransactionState("Cannot duplicate a transaction which does not have a read lock.");
+        case DB::transact_Reading:
+            return db->start_read(version);
+        case DB::transact_Frozen:
+            return db->start_frozen(version);
+        case DB::transact_Writing:
+            if (get_commit_size() != 0)
+                throw WrongTransactionState(
+                    "Can only duplicate a write transaction before any changes have been made.");
+            return db->start_read(version);
+    }
+    REALM_UNREACHABLE();
 }
 
 void Transaction::copy_to(TransactionRef dest) const

test/object-store/results.cpp

@@ -403,16 +403,49 @@ TEST_CASE("notifications: async delivery") {
 
         SECTION("Results which already has callbacks") {
             SECTION("notifier before active") {
-                token4 = results2.add_notification_callback([&](CollectionChangeSet) {});
+                token4 = results2.add_notification_callback([](CollectionChangeSet) {});
                 check(results3, results2);
             }
             SECTION("notifier after active") {
-                token4 = results2.add_notification_callback([&](CollectionChangeSet) {});
+                token4 = results2.add_notification_callback([](CollectionChangeSet) {});
                 check(results, results2);
             }
         }
     }
 
+    SECTION("callbacks can be added from within callbacks triggered by beginning a write transaction") {
+        auto results2 = results;
+        auto results3 = results;
+
+        bool called = false;
+        NotificationToken token2, token3;
+        token2 = results2.add_notification_callback([&](CollectionChangeSet) {
+            token2 = {};
+            token3 = results3.add_notification_callback([&](CollectionChangeSet) {
+                called = true;
+            });
+        });
+        r->begin_transaction();
+        REQUIRE_FALSE(called);
+        r->cancel_transaction();
+        r->begin_transaction();
+        REQUIRE(called);
+        r->cancel_transaction();
+    }
+
+    SECTION("callbacks can be added inside write transactions but only before any changes have been made") {
+        auto results2 = results;
+        auto results3 = results;
+        r->begin_transaction();
+        REQUIRE_NOTHROW(results2.add_notification_callback([](CollectionChangeSet) {}));
+        table->begin()->remove();
+        // Works because it already has a notifier
+        REQUIRE_NOTHROW(results2.add_notification_callback([](CollectionChangeSet) {}));
+        // Fails because we're in a state where we can't create a notifier
+        REQUIRE_EXCEPTION(results3.add_notification_callback([](CollectionChangeSet) {}), WrongTransactionState,
+                          "Cannot create asynchronous query after making changes in a write transaction.");
+    }
+
     SECTION("remote changes made before adding a callback from within a callback are not reported") {
         NotificationToken token2, token3;
         bool called = false;