Skip to content

Commit f5746c8

Browse files
slissonslisson
committed
fix(mps-sync-plugin): initial import and continuous sync of MPS-extensions successful
1 parent cd92e05 commit f5746c8

File tree

9 files changed

+141
-19
lines changed

9 files changed

+141
-19
lines changed

‎bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelSynchronizer.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ class ModelSynchronizer(
215215
}
216216
}
217217

218-
val isOrdered = targetParent.isOrdered(role)
218+
val isOrdered = targetParent.isOrdered(role) && sourceParent.isOrdered(role)
219219

220220
// optimization for when there is no change in the child list
221221
if (associatedChildren.all { it.alreadyMatches(isOrdered) }) {

‎datastructures/src/commonMain/kotlin/org/modelix/datastructures/objects/ObjectReferenceImpl.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class ObjectReferenceImpl<E : IObjectData> private constructor(
3535
return if (oldRef == null) {
3636
resolve().flatMap { it.getDescendantsAndSelf() }
3737
} else if (getHash() == oldRef.getHash()) {
38-
IStream.Companion.empty()
38+
IStream.empty()
3939
} else {
4040
resolve().zipWith(oldRef.resolve()) { newObj, oldObj ->
4141
newObj.objectDiff(oldObj)

‎datastructures/src/commonMain/kotlin/org/modelix/datastructures/patricia/PatriciaNode.kt‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ data class PatriciaNode<K, V : Any>(
126126
return if (ownPrefix.length == prefix.length) {
127127
IStream.of(this.copy(ownPrefix = ""))
128128
} else {
129-
split(prefix).children.single().resolveData()
129+
split(prefix, config.graph).children.single().resolveData()
130130
}
131131
}
132132
if (!prefix.startsWith(ownPrefix)) return IStream.empty()
@@ -184,7 +184,7 @@ data class PatriciaNode<K, V : Any>(
184184
}
185185
} else {
186186
// key is shorter -> need to split into a node with a shorter prefix
187-
split(commonPrefix).updateSubtree(newKey, updater)
187+
split(commonPrefix, config.graph).updateSubtree(newKey, updater)
188188
}
189189
).flatMapZeroOrOne { it.tryMerge() }
190190
}
@@ -204,7 +204,7 @@ data class PatriciaNode<K, V : Any>(
204204
/**
205205
* Shortens the prefix of this node to the given one as a preparation for inserting children or setting a value.
206206
*/
207-
private fun split(commonPrefix: CharSequence): PatriciaNode<K, V> {
207+
private fun split(commonPrefix: CharSequence, graph: IObjectGraph): PatriciaNode<K, V> {
208208
require(ownPrefix.startsWith(commonPrefix))
209209
if (ownPrefix.length == commonPrefix.length) return this
210210
val remainingPrefix = this.ownPrefix.drop(commonPrefix.length)
@@ -214,7 +214,7 @@ data class PatriciaNode<K, V : Any>(
214214
value = null,
215215
firstChars = remainingPrefix.take(1),
216216
children = listOf(
217-
config.graph.fromCreated(
217+
graph.fromCreated(
218218
PatriciaNode<K, V>(
219219
config = config,
220220
ownPrefix = remainingPrefix,
@@ -313,7 +313,7 @@ data class PatriciaNode<K, V : Any>(
313313
ownChange + changesFromChildren
314314
} else {
315315
val commonPrefix = ownPrefix.commonPrefixWith(oldNode.ownPrefix)
316-
split(commonPrefix).getChanges(path, oldNode.split(commonPrefix), changesOnly)
316+
split(commonPrefix, IObjectGraph.FREE_FLOATING).getChanges(path, oldNode.split(commonPrefix, IObjectGraph.FREE_FLOATING), changesOnly)
317317
}
318318
}
319319

@@ -345,7 +345,12 @@ data class PatriciaNode<K, V : Any>(
345345
if (oldChildRef == null) {
346346
newChildRef.resolve().flatMap { it.getDescendantsAndSelf() }
347347
} else {
348-
if (newChildRef.getHash() == oldChildRef.getHash()) {
348+
// If they are part of the FREE_FLOATING graph it means they are the result of a split
349+
// and aren't actually available at the other replica.
350+
if (newChildRef.getHash() == oldChildRef.getHash() &&
351+
newChildRef.graph != IObjectGraph.FREE_FLOATING &&
352+
oldChildRef.graph != IObjectGraph.FREE_FLOATING
353+
) {
349354
IStream.empty()
350355
} else {
351356
newChildRef.resolve().zipWith(oldChildRef.resolve()) { newChild, oldChild ->
@@ -356,17 +361,34 @@ data class PatriciaNode<K, V : Any>(
356361
}
357362
}
358363

359-
IStream.of(self) + changesFromChildren
364+
val newValueReferences = value?.let { config.valueConfig.getContainmentReferences(it) } ?: emptyList()
365+
val oldValueReferences = oldNode.value?.let { config.valueConfig.getContainmentReferences(it) } ?: emptyList()
366+
val changesFromValue = when (newValueReferences.size) {
367+
0 -> IStream.empty()
368+
1 -> when (oldValueReferences.size) {
369+
0 -> newValueReferences.single().diff(null)
370+
1 -> newValueReferences.single().diff(oldValueReferences.single())
371+
else -> TODO()
372+
}
373+
else -> TODO()
374+
}
375+
376+
IStream.of(self) + changesFromValue + changesFromChildren
360377
} else {
361378
val commonPrefix = ownPrefix.commonPrefixWith(oldNode.ownPrefix)
362-
self.split(commonPrefix).let {
363-
it.data.objectDiff(it, oldObject.split(commonPrefix), path).filter { it.graph == config.graph }
379+
(
380+
IStream.of(self) + self.split(commonPrefix).let {
381+
it.data.objectDiff(it, oldObject.split(commonPrefix), path).filter { it.graph == config.graph }
382+
}
383+
).filter {
384+
// filter out the split result, which isn't part of the real object graph
385+
it.graph == config.graph
364386
}
365387
}
366388
}
367389

368390
private fun <K, V : Any> Object<PatriciaNode<K, V>>.split(commonPrefix: CharSequence): Object<PatriciaNode<K, V>> {
369-
val splitted = data.split(commonPrefix)
391+
val splitted = data.split(commonPrefix, IObjectGraph.FREE_FLOATING)
370392
if (splitted === data) return this
371393
return splitted.asObject(IObjectGraph.FREE_FLOATING)
372394
}

‎datastructures/src/commonTest/kotlin/org/modelix/datastructures/PatriciaTrieTest.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class PatriciaTrieTest {
1313
fun `same content produces same tree`() {
1414
val alphabet = "abcdefghijklmnopqrstuvwxyz"
1515
val rand = Random(5465)
16-
fun randomString(length: Int) = (0 until length).joinToString("") { alphabet.random().toString() }
16+
fun randomString(length: Int) = (0 until length).joinToString("") { alphabet.random(rand).toString() }
1717

1818
val initialTree = PatriciaTrie.withStrings(IObjectGraph.FREE_FLOATING)
1919

@@ -36,13 +36,13 @@ class PatriciaTrieTest {
3636

3737
repeat(1_000) {
3838
if (removedEntries.size > addedEntries.size) {
39-
val key = removedEntries.random()
39+
val key = removedEntries.random(rand)
4040
removedEntries.remove(key)
4141
addedEntries.add(key)
4242
tree = tree.put(key, values[key]!!).getBlocking(tree)
4343
assertTree()
4444
} else {
45-
val key = addedEntries.random()
45+
val key = addedEntries.random(rand)
4646
removedEntries.add(key)
4747
addedEntries.remove(key)
4848
tree = tree.remove(key).getBlocking(tree)

‎datastructures/src/commonTest/kotlin/org/modelix/datastructures/TreeDiffTest.kt‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package org.modelix.datastructures
2+
3+
import org.modelix.datastructures.objects.FullyLoadedObjectGraph
4+
import org.modelix.datastructures.objects.StringDataTypeConfiguration
5+
import org.modelix.datastructures.objects.getDescendantsAndSelf
6+
import org.modelix.datastructures.patricia.PatriciaNode
7+
import org.modelix.datastructures.patricia.PatriciaTrie
8+
import org.modelix.datastructures.patricia.PatriciaTrieConfig
9+
import org.modelix.streams.getBlocking
10+
import kotlin.random.Random
11+
import kotlin.test.Test
12+
import kotlin.test.assertEquals
13+
14+
class TreeDiffTest {
15+
16+
@Test
17+
fun `tree can be restored from diff`() {
18+
val graph = FullyLoadedObjectGraph()
19+
var tree = PatriciaTrie.withStrings(graph)
20+
21+
val alphabet = "abcdefghijklmnopqrstuvwxyz"
22+
val rand = Random(5465)
23+
fun randomString(length: Int) = (0 until length).joinToString("") { alphabet.random(rand).toString() }
24+
25+
val removedEntries = (1..100).map { randomString(rand.nextInt(0, 6)) }.toMutableSet()
26+
val values = removedEntries.associateWith { "value_of_$it" }
27+
val addedEntries = mutableSetOf<String>()
28+
29+
val restoringGraph = FullyLoadedObjectGraph()
30+
val initialDiff = tree.asObject().getDescendantsAndSelf()
31+
.map { it.getHash() to it.data.serialize() }
32+
.toList().getBlocking(tree).toMap()
33+
val restoringConfig = PatriciaTrieConfig(
34+
restoringGraph,
35+
StringDataTypeConfiguration(),
36+
StringDataTypeConfiguration(),
37+
)
38+
val restoringDeserializer = PatriciaNode.Deserializer { restoringConfig }
39+
var restoredTree = PatriciaTrie(restoringGraph.loadObjects(tree.asObject().getHash(), restoringDeserializer, initialDiff))
40+
fun assertTree() {
41+
val diff = tree.asObject().objectDiff(restoredTree.asObject())
42+
.map { it.getHash() to it.data.serialize() }
43+
.toList().getBlocking(tree).toMap()
44+
restoredTree = PatriciaTrie(restoringGraph.loadObjects(tree.asObject().getHash(), restoringDeserializer, diff))
45+
46+
val expectedEntries = addedEntries.associateWith { values[it]!! }
47+
val actualEntries = restoredTree.getAll().toList().getBlocking(tree).toMap()
48+
assertEquals(expectedEntries, actualEntries)
49+
}
50+
51+
repeat(1_000) {
52+
if (removedEntries.size > addedEntries.size) {
53+
val key = removedEntries.random(rand)
54+
removedEntries.remove(key)
55+
addedEntries.add(key)
56+
tree = tree.put(key, values[key]!!).getBlocking(tree)
57+
assertTree()
58+
} else {
59+
val key = addedEntries.random(rand)
60+
removedEntries.add(key)
61+
addedEntries.remove(key)
62+
tree = tree.remove(key).getBlocking(tree)
63+
assertTree()
64+
}
65+
}
66+
67+
assertTree()
68+
}
69+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class VersionMerger(private val idGenerator: IIdGenerator) {
119119
val v0 = filtered[i]
120120
val v1 = filtered.getOrNull(i + 1) ?: continue
121121

122-
if (v1.numberOfOperations == 1 && (v1.operations.single() as? UndoOp)?.versionHash?.getHash() == v0.getObjectHash()) {
122+
if (v1.numberOfOperations == 1 && (v1.operations.singleOrNull() as? UndoOp)?.versionHash?.getHash() == v0.getObjectHash()) {
123123
filtered.removeAt(i)
124124
filtered.removeAt(i)
125125
}

‎model-datastructure/src/commonMain/kotlin/org/modelix/model/persistent/CPTree.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import org.modelix.model.TreeId
2424
import org.modelix.model.api.INodeReference
2525
import org.modelix.model.api.ITree
2626
import org.modelix.streams.IStream
27+
import org.modelix.streams.flatten
2728
import org.modelix.streams.plus
2829

2930
class CPTree(
@@ -78,7 +79,9 @@ class CPTree(
7879
val oldData = oldObject?.data
7980
return when (oldData) {
8081
is CPTree -> {
81-
IStream.of(self) + getTreeReference().diff(oldData.int64Hamt)
82+
IStream.of(self) + getTreeReference().resolve().zipWith(oldData.getTreeReference().resolve()) { newTree, oldTree ->
83+
newTree.objectDiff(oldTree)
84+
}.flatten()
8285
}
8386
else -> self.getDescendantsAndSelf()
8487
}

‎model-server/src/main/kotlin/org/modelix/model/server/handlers/RepositoriesManager.kt‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,20 @@ class RepositoriesManager(val stores: StoreManager) : IRepositoriesManager {
302302
}
303303

304304
private fun validateVersion(newVersion: CLVersion, oldVersion: CLVersion?) {
305-
// ensure there are no missing objects
306-
newVersion.graph.getStreamExecutor().iterate({ newVersion.fullDiff(oldVersion) }) { }
305+
if (newVersion.getObjectHash() == oldVersion?.getObjectHash()) return
307306

307+
// Deleting the root node isn't allowed
308308
val mainTree = newVersion.getModelTree()
309309
check(mainTree.containsNode(mainTree.getRootNodeId()).getBlocking(mainTree))
310310

311+
// ensure there are no missing objects
312+
newVersion.graph.getStreamExecutor().iterate({ newVersion.fullDiff(oldVersion) }) { }
313+
if (oldVersion != null) {
314+
// If the object diff is buggy, client and server will skip over the same objects.
315+
// The model diff should also iterate over all new objects and is used for additional validation.
316+
newVersion.getModelTree().getChanges(oldVersion.getModelTree(), false).iterateBlocking(newVersion.getModelTree()) { }
317+
}
318+
311319
// TODO check invariants of the model (consistent parent-child relations, single root, containment cycles)
312320

313321
// check that the operations actually reproduce the model

‎mps-sync-plugin3/src/test/kotlin/org/modelix/mps/sync3/ProjectSyncTest.kt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,26 @@ class ProjectSyncTest : MPSTestBase() {
524524
assertEquals(expectedSnapshot, project.captureSnapshot())
525525
}
526526

527+
fun `test no change at startup doesn't create a new version`(): Unit = runWithModelServer { port ->
528+
// A client was previously in sync with the server ...
529+
val branchRef = RepositoryId("sync-test").getBranchReference()
530+
val version1 = syncProjectToServer("initial", port, branchRef)
531+
val expectedSnapshot = lastSnapshotBeforeSync
532+
533+
// ... and then MPS is restart. No change happened.
534+
openTestProject("initial")
535+
val binding = IModelSyncService.getInstance(mpsProject)
536+
.addServer("http://localhost:$port")
537+
.bind(branchRef, version1.getContentHash())
538+
val version2 = binding.flush()
539+
540+
// The client shouldn't push any changes ...
541+
assertEquals(version1.getContentHash(), version2.getContentHash())
542+
543+
// ... and it shouldn't pull any changes.
544+
assertEquals(expectedSnapshot, project.captureSnapshot())
545+
}
546+
527547
private fun runWithModelServer(body: suspend (port: Int) -> Unit) = runBlocking {
528548
@OptIn(ExperimentalTime::class)
529549
withTimeout(5.minutes) {

0 commit comments

Comments
 (0)