Skip to content

Commit fbfb803

Browse files
slissonslisson
committed
test(model-datastructure): new test case that reveals a bug in LinearHistory
1 parent c2e8488 commit fbfb803

File tree

3 files changed

+178
-25
lines changed

3 files changed

+178
-25
lines changed

‎model-datastructure/src/commonMain/kotlin/org/modelix/model/lazy/CLVersion.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,14 @@ import org.modelix.model.IKeyListener
2323
import org.modelix.model.IKeyValueStore
2424
import org.modelix.model.IVersion
2525
import org.modelix.model.LinearHistory
26+
import org.modelix.model.api.IIdGenerator
2627
import org.modelix.model.api.INodeReference
28+
import org.modelix.model.api.IWriteTransaction
2729
import org.modelix.model.api.LocalPNodeReference
2830
import org.modelix.model.api.PNodeReference
31+
import org.modelix.model.api.TreePointer
2932
import org.modelix.model.operations.IOperation
33+
import org.modelix.model.operations.OTBranch
3034
import org.modelix.model.operations.SetReferenceOp
3135
import org.modelix.model.persistent.CPHamtNode
3236
import org.modelix.model.persistent.CPNode
@@ -432,3 +436,16 @@ private class AccessTrackingStore(val store: IKeyValueStore) : IKeyValueStore {
432436
TODO("Not yet implemented")
433437
}
434438
}
439+
440+
fun CLVersion.runWrite(idGenerator: IIdGenerator, author: String?, body: (IWriteTransaction) -> Unit): CLVersion {
441+
val branch = OTBranch(TreePointer(getTree(), idGenerator), idGenerator, store)
442+
branch.computeWriteT(body)
443+
val (ops, newTree) = branch.getPendingChanges()
444+
return CLVersion.createRegularVersion(
445+
id = idGenerator.generate(),
446+
author = author,
447+
tree = newTree as CLTree,
448+
baseVersion = this,
449+
operations = ops.map { it.getOriginalOp() }.toTypedArray(),
450+
)
451+
}

‎model-datastructure/src/commonTest/kotlin/ConflictResolutionTest.kt‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -815,35 +815,35 @@ class ConflictResolutionTest : TreeTestBase() {
815815
operations = opsAndTree.first.map { it.getOriginalOp() }.toTypedArray(),
816816
)
817817
}
818+
}
818819

819-
fun assertSameTree(tree1: ITree, tree2: ITree) {
820-
tree2.visitChanges(
821-
tree1,
822-
object : ITreeChangeVisitorEx {
823-
override fun containmentChanged(nodeId: Long) {
824-
fail("containmentChanged ${nodeId.toString(16)}")
825-
}
820+
fun assertSameTree(tree1: ITree, tree2: ITree) {
821+
tree2.visitChanges(
822+
tree1,
823+
object : ITreeChangeVisitorEx {
824+
override fun containmentChanged(nodeId: Long) {
825+
fail("containmentChanged ${nodeId.toString(16)}")
826+
}
826827

827-
override fun childrenChanged(nodeId: Long, role: String?) {
828-
fail("childrenChanged ${nodeId.toString(16)}, $role")
829-
}
828+
override fun childrenChanged(nodeId: Long, role: String?) {
829+
fail("childrenChanged ${nodeId.toString(16)}, $role")
830+
}
830831

831-
override fun referenceChanged(nodeId: Long, role: String) {
832-
fail("referenceChanged ${nodeId.toString(16)}, $role")
833-
}
832+
override fun referenceChanged(nodeId: Long, role: String) {
833+
fail("referenceChanged ${nodeId.toString(16)}, $role")
834+
}
834835

835-
override fun propertyChanged(nodeId: Long, role: String) {
836-
fail("propertyChanged ${nodeId.toString(16)}, $role")
837-
}
836+
override fun propertyChanged(nodeId: Long, role: String) {
837+
fail("propertyChanged ${nodeId.toString(16)}, $role")
838+
}
838839

839-
override fun nodeRemoved(nodeId: Long) {
840-
fail("nodeRemoved ${nodeId.toString(16)}")
841-
}
840+
override fun nodeRemoved(nodeId: Long) {
841+
fail("nodeRemoved ${nodeId.toString(16)}")
842+
}
842843

843-
override fun nodeAdded(nodeId: Long) {
844-
fail("nodeAdded nodeId")
845-
}
846-
},
847-
)
848-
}
844+
override fun nodeAdded(nodeId: Long) {
845+
fail("nodeAdded nodeId")
846+
}
847+
},
848+
)
849849
}

‎model-datastructure/src/commonTest/kotlin/MergeOrderTest.kt‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright (c) 2023.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import org.modelix.model.VersionMerger
18+
import org.modelix.model.api.IIdGenerator
19+
import org.modelix.model.api.ITree
20+
import org.modelix.model.api.IWriteTransaction
21+
import org.modelix.model.client.IdGenerator
22+
import org.modelix.model.lazy.CLTree
23+
import org.modelix.model.lazy.CLVersion
24+
import org.modelix.model.lazy.ObjectStoreCache
25+
import org.modelix.model.lazy.runWrite
26+
import org.modelix.model.persistent.MapBaseStore
27+
import kotlin.test.Test
28+
import kotlin.test.assertContains
29+
import kotlin.test.assertEquals
30+
31+
/**
32+
* This tests indirectly the algorithm in LinearHistory, by creating and merging versions.
33+
*/
34+
class MergeOrderTest {
35+
private var store: MapBaseStore = MapBaseStore()
36+
private var storeCache: ObjectStoreCache = ObjectStoreCache(store)
37+
private var idGenerator: IIdGenerator = IdGenerator.newInstance(3)
38+
39+
fun CLVersion.runWrite(id: Long, body: IWriteTransaction.() -> Unit): CLVersion {
40+
return nextId(id) { runWrite(idGenerator, null, { it.apply(body) }) }
41+
}
42+
fun CLVersion.runWrite(body: IWriteTransaction.() -> Unit) = runWrite(idGenerator, null, { it.apply(body) })
43+
fun merge(v1: CLVersion, v2: CLVersion) = VersionMerger(storeCache, idGenerator).mergeChange(v1, v2)
44+
fun merge(id: Long, v1: CLVersion, v2: CLVersion) = nextId(id) { VersionMerger(storeCache, idGenerator).mergeChange(v1, v2) }
45+
46+
@Test
47+
fun mergeOrderShouldNotMatter() = mergeOrderShouldNotMatter(false)
48+
49+
@Test
50+
fun mergeOrderShouldNotMatterAlternativeIds() = mergeOrderShouldNotMatter(true)
51+
52+
fun mergeOrderShouldNotMatter(alternativeIds: Boolean) {
53+
val merger = VersionMerger(storeCache, idGenerator)
54+
55+
val v0 = CLVersion.createRegularVersion(
56+
idGenerator.generate(),
57+
null,
58+
null,
59+
CLTree(storeCache),
60+
null,
61+
emptyArray(),
62+
)
63+
64+
// There are two clients working on the same version and both change the same property.
65+
// This creates a conflict.
66+
val va1 = v0.runWrite(0xa1) { setProperty(ITree.ROOT_ID, "name", "MyClassA") }
67+
assertEquals("MyClassA", va1.getTree().getProperty(ITree.ROOT_ID, "name"))
68+
val vb1 = v0.runWrite(0xb1) { setProperty(ITree.ROOT_ID, "name", "MyClassB") }
69+
assertEquals("MyClassB", vb1.getTree().getProperty(ITree.ROOT_ID, "name"))
70+
71+
// Now both clients exchange their modifications ...
72+
73+
// In the meantime, Client A keeps working on his branch and creates a new version that doesn't actually
74+
// change anything, so should not have an effect on the following merge.
75+
// It changes an unrelated part of the model that wouldn't cause a conflict even if the operations were ordered
76+
// in a completely unexpected way.
77+
val va2 = va1.runWrite(0xa2L + (if (alternativeIds) 0x100 else 0)) {
78+
setProperty(ITree.ROOT_ID, "unrelated", "Class renamed")
79+
setProperty(ITree.ROOT_ID, "unrelated", null)
80+
}
81+
assertEquals("MyClassA", va2.getTree().getProperty(ITree.ROOT_ID, "name"))
82+
83+
// Client B receives the version with the first property change from client A, but not the second empty change.
84+
val vb2 = merge(0xb2, vb1, va1)
85+
assertContains(setOf("MyClassA", "MyClassB"), vb2.getTree().getProperty(ITree.ROOT_ID, "name"))
86+
87+
// Client A receives the version with the property change from client B.
88+
val va3 = merge(0xa3, va2, vb1)
89+
assertContains(setOf("MyClassA", "MyClassB"), va3.getTree().getProperty(ITree.ROOT_ID, "name"))
90+
91+
// The history should now look like this:
92+
//
93+
// v0
94+
// / \
95+
// va1 vb1
96+
// | \ / |
97+
// va2 /\ |
98+
// | / \ |
99+
// |/ \ |
100+
// va3 vb2
101+
//
102+
// - va1 and vb2 contain the property change
103+
// - va2 didn't change anything
104+
// - va3 and vb2 are the current head versions that merged both property changes,
105+
// and are expected to contain the same resulting model.
106+
107+
// After Client B receives another update from Client A that includes the empty change va2
108+
// there shouldn't be any doubt that the merge result has to be identical ...
109+
assertEquals(
110+
va3.getTree().getProperty(ITree.ROOT_ID, "name"),
111+
merger.mergeChange(vb2, va2).getTree().getProperty(ITree.ROOT_ID, "name"),
112+
)
113+
assertSameTree(va3.getTree(), merger.mergeChange(vb2, va2).getTree())
114+
115+
// ... but even the previous merge should have an identical result.
116+
// It would be confusing for the user if the merge algorithm keeps changing its mind about the result for
117+
// no obvious reason.
118+
assertEquals(
119+
va3.getTree().getProperty(ITree.ROOT_ID, "name"),
120+
vb2.getTree().getProperty(ITree.ROOT_ID, "name"),
121+
)
122+
assertSameTree(va3.getTree(), vb2.getTree())
123+
}
124+
125+
fun <R> nextId(id: Long, body: () -> R): R {
126+
val saved = idGenerator
127+
try {
128+
idGenerator = object : IIdGenerator {
129+
override fun generate(): Long = id
130+
}
131+
return body()
132+
} finally {
133+
idGenerator = saved
134+
}
135+
}
136+
}

0 commit comments

Comments
 (0)