From ef632804ef282e8dc7d0ac5ea7ad7143e6613229 Mon Sep 17 00:00:00 2001
From: Thomas Goyne <tg@realm.io>
Date: Mon, 21 Mar 2016 13:03:10 -0700
Subject: [PATCH] Clean up stale moves for linkviews even without a merge

---
 src/collection_notifications.cpp    | 26 +++++++++++++++-----------
 src/collection_notifications.hpp    |  1 +
 src/impl/list_notifier.cpp          |  1 -
 src/impl/transact_log_handler.cpp   |  8 ++++++++
 tests/collection_change_indices.cpp |  7 +++++++
 5 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/collection_notifications.cpp b/src/collection_notifications.cpp
index cec46d73..59c835ad 100644
--- a/src/collection_notifications.cpp
+++ b/src/collection_notifications.cpp
@@ -146,17 +146,7 @@ void CollectionChangeBuilder::merge(CollectionChangeBuilder&& c)
     insertions.erase_at(c.deletions);
     insertions.insert_at(c.insertions);
 
-    // Look for moves which are now no-ops, and remove them plus the associated
-    // insert+delete. Note that this isn't just checking for from == to due to
-    // that rows can also be shifted by other inserts and deletes
-    IndexSet to_remove;
-    moves.erase(remove_if(begin(moves), end(moves), [&](auto const& move) {
-        if (move.from - deletions.count(0, move.from) != move.to - insertions.count(0, move.to))
-            return false;
-        deletions.remove(move.from);
-        insertions.remove(move.to);
-        return true;
-    }), end(moves));
+    clean_up_stale_moves();
 
     modifications.erase_at(c.deletions);
     modifications.shift_for_insert_at(c.insertions);
@@ -166,6 +156,20 @@ void CollectionChangeBuilder::merge(CollectionChangeBuilder&& c)
     verify();
 }
 
+void CollectionChangeBuilder::clean_up_stale_moves()
+{
+    // Look for moves which are now no-ops, and remove them plus the associated
+    // insert+delete. Note that this isn't just checking for from == to due to
+    // that rows can also be shifted by other inserts and deletes
+    moves.erase(remove_if(begin(moves), end(moves), [&](auto const& move) {
+        if (move.from - deletions.count(0, move.from) != move.to - insertions.count(0, move.to))
+            return false;
+        deletions.remove(move.from);
+        insertions.remove(move.to);
+        return true;
+    }), end(moves));
+}
+
 void CollectionChangeBuilder::modify(size_t ndx)
 {
     modifications.add(ndx);
diff --git a/src/collection_notifications.hpp b/src/collection_notifications.hpp
index 95c1fe04..019ee907 100644
--- a/src/collection_notifications.hpp
+++ b/src/collection_notifications.hpp
@@ -86,6 +86,7 @@ public:
                                              bool sort);
 
     void merge(CollectionChangeBuilder&&);
+    void clean_up_stale_moves();
 
     void insert(size_t ndx, size_t count=1);
     void modify(size_t ndx);
diff --git a/src/impl/list_notifier.cpp b/src/impl/list_notifier.cpp
index fd6a4ffe..5d69f4e3 100644
--- a/src/impl/list_notifier.cpp
+++ b/src/impl/list_notifier.cpp
@@ -101,7 +101,6 @@ void ListNotifier::run()
                          end(m_change.moves));
 
     for (size_t i = 0; i < m_lv->size(); ++i) {
-        // FIXME: may need to mark modifications even for inserts (for moves?)
         if (m_change.insertions.contains(i) || m_change.modifications.contains(i))
             continue;
         if (m_info->row_did_change(m_lv->get_target_table(), m_lv->get(i).get_index()))
diff --git a/src/impl/transact_log_handler.cpp b/src/impl/transact_log_handler.cpp
index 19617c14..87c18c25 100644
--- a/src/impl/transact_log_handler.cpp
+++ b/src/impl/transact_log_handler.cpp
@@ -445,6 +445,14 @@ public:
             change->modify(row);
     }
 
+    void parse_complete()
+    {
+        for (auto& list : m_info.lists)
+        {
+            list.changes->clean_up_stale_moves();
+        }
+    }
+
     bool select_link_list(size_t col, size_t row, size_t)
     {
         mark_dirty(row, col);
diff --git a/tests/collection_change_indices.cpp b/tests/collection_change_indices.cpp
index dacaefad..2fac5fa9 100644
--- a/tests/collection_change_indices.cpp
+++ b/tests/collection_change_indices.cpp
@@ -295,6 +295,13 @@ TEST_CASE("[collection_change] move()") {
         c.move(15, 10);
         REQUIRE_MOVES(c, {5, 11}, {15, 10});
     }
+
+    SECTION("collapses redundant swaps of adjacent rows to a no-op") {
+        c.move(7, 8);
+        c.move(7, 8);
+        c.clean_up_stale_moves();
+        REQUIRE(c.empty());
+    }
 }
 
 TEST_CASE("[collection_change] calculate() unsorted") {