fix(model-datastructure): UndoOp reverted unrelated changes

If a branch did some changes and then directly reverted them, you would
expect that merging it has no effect on the resulting model, but it
actually reverted conflicting changes from the other branch.

There was already a test for that, but the number of changes it applied
was to small to create such conflicts.

Author: slisson

model-datastructure/src/commonMain/kotlin/org/modelix/model/VersionMerger.kt

@@ -24,6 +24,7 @@ import org.modelix.model.lazy.CLVersion
 import org.modelix.model.lazy.IDeserializingKeyValueStore
 import org.modelix.model.operations.IOperation
 import org.modelix.model.operations.IOperationIntend
+import org.modelix.model.operations.UndoOp
 import org.modelix.model.persistent.CPVersion
 
 class VersionMerger(private val storeCache: IDeserializingKeyValueStore, private val idGenerator: IIdGenerator) {
@@ -70,7 +71,7 @@ class VersionMerger(private val storeCache: IDeserializingKeyValueStore, private
             return if (leftVersion.id < rightVersion.id) leftVersion else rightVersion
         }
 
-        val versionsToApply = LinearHistory(commonBase?.hash).load(leftVersion, rightVersion)
+        val versionsToApply = filterUndo(LinearHistory(commonBase?.hash).load(leftVersion, rightVersion))
 
         val operationsToApply = versionsToApply.flatMap { captureIntend(it) }
         var mergedVersion: CLVersion? = null
@@ -116,6 +117,24 @@ class VersionMerger(private val storeCache: IDeserializingKeyValueStore, private
         return mergedVersion!!
     }
 
+    /**
+     * Instead of computing and applying the inverse operation we can optimize for the case when an undo follows
+     * directly after the version to be undone and just drop the version.
+     */
+    private fun filterUndo(versions: List<CLVersion>): List<CLVersion> {
+        val filtered = versions.toMutableList()
+        for (i in (0 until filtered.lastIndex).reversed()) {
+            val v0 = filtered[i]
+            val v1 = filtered.getOrNull(i + 1) ?: continue
+
+            if (v1.numberOfOperations == 1 && (v1.operations.single() as? UndoOp)?.versionHash?.getHash() == v0.getContentHash()) {
+                filtered.removeAt(i)
+                filtered.removeAt(i)
+            }
+        }
+        return filtered
+    }
+
     private fun captureIntend(version: CLVersion): List<IOperationIntend> {
         val operations = version.operations.toList()
         if (operations.isEmpty()) return listOf()

model-datastructure/src/commonTest/kotlin/UndoTest.kt

@@ -67,14 +67,14 @@ class UndoTest {
         val baseBranch = OTBranch(PBranch(CLTree(store), idGenerator), idGenerator, store)
         val rand = Random(347663)
 
-        randomChanges(baseBranch, 5, idGenerator, rand)
+        randomChanges(baseBranch, 50, idGenerator, rand)
         val baseVersion = createVersion(baseBranch.operationsAndTree, null, versionIdGenerator, store)
 
         val maxIndex = 2
         val branches = (0..maxIndex).map { OTBranch(PBranch(baseVersion.tree, idGenerator), idGenerator, store) }.toList()
         for (i in 0..maxIndex) {
             branches[i].runWrite {
-                randomChanges(branches[i], 5, idGenerator, rand)
+                randomChanges(branches[i], 50, idGenerator, rand)
             }
         }
         val versions = branches.map { branch ->
@@ -108,14 +108,14 @@ class UndoTest {
         val baseBranch = OTBranch(PBranch(CLTree(store), idGenerator), idGenerator, store)
         val rand = Random(347663)
 
-        randomChanges(baseBranch, 5, idGenerator, rand)
+        randomChanges(baseBranch, 50, idGenerator, rand)
         val baseVersion = createVersion(baseBranch.operationsAndTree, null, versionIdGenerator, store)
 
         val maxIndex = 2
         val branches = (0..maxIndex).map { OTBranch(PBranch(baseVersion.tree, idGenerator), idGenerator, store) }.toList()
         for (i in 0..maxIndex) {
             branches[i].runWrite {
-                randomChanges(branches[i], 5, idGenerator, rand)
+                randomChanges(branches[i], 50, idGenerator, rand)
             }
         }
         val versions = branches.map { branch ->
@@ -162,9 +162,13 @@ class UndoTest {
     private fun randomChanges(baseBranch: OTBranch, numChanges: Int, idGenerator: IIdGenerator, rand: Random) {
         baseBranch.runWrite {
             val changeGenerator = RandomTreeChangeGenerator(idGenerator, rand).growingOperationsOnly()
-            for (i in 0 until numChanges) {
+            for (i in 0 until (numChanges / 2)) {
                 changeGenerator.applyRandomChange(baseBranch, null)
             }
+            val changeGenerator2 = RandomTreeChangeGenerator(idGenerator, rand)
+            for (i in 0 until (numChanges / 2)) {
+                changeGenerator2.applyRandomChange(baseBranch, null)
+            }
         }
     }