Skip to content

Commit 38e2735

Browse files
mhuster23mhuster23
committed
fix(model-sync-lib): replaced import stats by OTBranch
1 parent 931f52a commit 38e2735

File tree

4 files changed

+85
-183
lines changed

4 files changed

+85
-183
lines changed

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

Lines changed: 0 additions & 81 deletions
This file was deleted.

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

Lines changed: 16 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import org.modelix.model.data.ModelData
55
import org.modelix.model.data.NodeData
66
import java.io.File
77

8-
class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
8+
class ModelImporter(private val root: INode) {
99

1010
private val originalIdToExisting: MutableMap<String, INode> = mutableMapOf()
1111
private val originalIdToSpec: MutableMap<String, NodeData> = mutableMapOf()
@@ -19,7 +19,6 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
1919
}
2020

2121
fun import(data: ModelData) {
22-
stats?.reset()
2322
originalIdToExisting.clear()
2423
originalIdToSpec.clear()
2524

@@ -68,14 +67,14 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
6867

6968
nodeData.properties.forEach {
7069
if (node.getPropertyValue(it.key) != it.value) {
71-
node.setPropertyValueWithStats(it.key, it.value)
70+
node.setPropertyValue(it.key, it.value)
7271
}
7372
}
7473

7574
val toBeRemoved = node.getPropertyRoles().toSet()
7675
.subtract(nodeData.properties.keys)
7776
.filter { it != NodeData.idPropertyKey }
78-
toBeRemoved.forEach { node.setPropertyValueWithStats(it, null) }
77+
toBeRemoved.forEach { node.setPropertyValue(it, null) }
7978
}
8079

8180
private fun syncAllReferences(root: INode) {
@@ -87,11 +86,14 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
8786
private fun syncReferences(node: INode, nodeData: NodeData) {
8887
nodeData.references.forEach {
8988
if (node.getReferenceTargetRef(it.key) != originalIdToExisting[it.value]?.reference) {
90-
node.setReferenceTargetWithStats(it.key, originalIdToExisting[it.value]?.reference)
89+
node.setReferenceTarget(it.key, originalIdToExisting[it.value]?.reference)
9190
}
9291
}
9392
val toBeRemoved = node.getReferenceRoles().toSet().subtract(nodeData.references.keys)
94-
toBeRemoved.forEach { node.setReferenceTargetWithStats(it, null) }
93+
toBeRemoved.forEach {
94+
val nullReference: INodeReference? = null
95+
node.setReferenceTarget(it, nullReference)
96+
}
9597
}
9698

9799
private fun addAllMissingChildren(node: INode): List<INode> {
@@ -123,7 +125,11 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
123125
!existingIds.contains(it.originalId()) // node will be moved here
124126
}
125127
val index = (baseIndex + offset).coerceIn(0..childrenInRole.size)
126-
node.addNewChildWithStats(nodeToBeAdded, index)
128+
val concept = nodeToBeAdded.concept?.let { s -> ConceptReference(s) }
129+
130+
node.addNewChild(nodeToBeAdded.role, index, concept).apply {
131+
setPropertyValue(NodeData.idPropertyKey, nodeToBeAdded.originalId())
132+
}
127133
}
128134
}
129135

@@ -162,7 +168,7 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
162168
val targetRole = originalIdToSpec[child.originalId()]?.role
163169
val targetIndex = targetIndices[child.originalId()]
164170
if (targetIndex != null && (targetIndex != currentIndex || child.roleInParent != targetRole)) {
165-
node.moveChildWithStats(targetRole, targetIndex, child)
171+
node.moveChild(targetRole, targetIndex, child)
166172
}
167173
}
168174
}
@@ -207,7 +213,7 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
207213
}
208214
val targetIndex = (baseTargetIndex + offset).coerceIn(0..childrenInRole.size)
209215

210-
node.moveChildWithStats(spec.role, targetIndex, toBeMovedHere)
216+
node.moveChild(spec.role, targetIndex, toBeMovedHere)
211217

212218
}
213219

@@ -219,7 +225,7 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
219225
}
220226
}
221227
toBeRemoved.forEach {
222-
it.parent?.removeChildWithStats(it)
228+
it.parent?.removeChild(it)
223229
}
224230
root.allChildren.forEach {
225231
deleteAllExtraChildren(it)
@@ -229,59 +235,6 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
229235
private fun NodeData.getIndexWithinRole(parent: NodeData) : Int =
230236
parent.children.filter { it.role == this.role }.indexOf(this)
231237

232-
233-
private fun INode.addNewChildWithStats(spec: NodeData, index: Int) : INode {
234-
val concept = spec.concept?.let { s -> ConceptReference(s) }
235-
236-
val createdNode = addNewChild(spec.role, index, concept)
237-
createdNode.setPropertyValue(NodeData.idPropertyKey, spec.originalId())
238-
if (this@ModelImporter.stats != null) {
239-
stats.addAddition(
240-
createdNode.originalId(),
241-
this.originalId(),
242-
createdNode.roleInParent,
243-
index
244-
)
245-
}
246-
return createdNode
247-
}
248-
249-
private fun INode.moveChildWithStats(role: String?, index: Int, child: INode) {
250-
if (this@ModelImporter.stats != null) {
251-
stats.addMove(
252-
child.originalId(),
253-
child.parent?.originalId(),
254-
child.roleInParent,
255-
child.index(),
256-
this.originalId(),
257-
role,
258-
index
259-
)
260-
}
261-
moveChild(role, index, child)
262-
}
263-
264-
private fun INode.removeChildWithStats(child: INode) {
265-
if (this@ModelImporter.stats != null) {
266-
stats.addDeletion(child.originalId(), parent?.originalId(), child.roleInParent, child.getDescendants(false).mapNotNull { it.originalId() }.toList())
267-
}
268-
removeChild(child)
269-
}
270-
271-
private fun INode.setPropertyValueWithStats(role: String, value: String?) {
272-
if (this@ModelImporter.stats != null) {
273-
this.originalId()?.let { stats.addPropertyChange(it, role) }
274-
}
275-
setPropertyValue(role, value)
276-
}
277-
278-
private fun INode.setReferenceTargetWithStats(role: String, target: INodeReference?) {
279-
if (this@ModelImporter.stats != null) {
280-
this.originalId()?.let { stats.addReferenceChange(it, role) }
281-
}
282-
setReferenceTarget(role, target)
283-
}
284-
285238
}
286239

287240
internal fun INode.originalId(): String? {

‎model-sync-lib/src/jvmTest/kotlin/org/modelix/model/sync/ModelImporterTest.kt‎

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package org.modelix.model.sync
33
import org.junit.jupiter.api.BeforeAll
44
import org.junit.jupiter.api.Test
55
import org.junit.jupiter.api.assertDoesNotThrow
6-
import org.modelix.model.api.IBranch
76
import org.modelix.model.api.PBranch
87
import org.modelix.model.api.getRootNode
98
import org.modelix.model.api.serialize
@@ -12,19 +11,19 @@ import org.modelix.model.data.ModelData
1211
import org.modelix.model.data.NodeData
1312
import org.modelix.model.lazy.CLTree
1413
import org.modelix.model.lazy.ObjectStoreCache
14+
import org.modelix.model.operations.*
1515
import org.modelix.model.persistent.MapBaseStore
1616
import org.modelix.model.test.RandomModelChangeGenerator
1717
import java.io.File
1818
import kotlin.random.Random
1919
import kotlin.test.assertEquals
20-
import kotlin.test.fail
2120

2221
class ModelImporterTest {
2322

2423
companion object {
2524
private lateinit var model: ModelData
2625
private lateinit var newModel: ModelData
27-
private lateinit var branch: IBranch
26+
private lateinit var branch: OTBranch
2827
private lateinit var importer: ModelImporter
2928

3029
@JvmStatic
@@ -34,14 +33,20 @@ class ModelImporterTest {
3433
val newModelFile = File("src/jvmTest/resources/newmodel.json")
3534
newModel = ModelData.fromJson(newModelFile.readText())
3635

37-
val tree = CLTree(ObjectStoreCache(MapBaseStore()))
38-
branch = PBranch(tree, IdGenerator.getInstance(1))
36+
val store = ObjectStoreCache(MapBaseStore())
37+
val tree = CLTree(store)
38+
val idGenerator = IdGenerator.getInstance(1)
39+
val pBranch = PBranch(tree, idGenerator)
40+
41+
pBranch.runWrite {
42+
model.load(pBranch)
43+
}
44+
branch = OTBranch(pBranch, idGenerator, store)
3945

4046
branch.runWrite {
41-
model.load(branch)
4247
// println("PRE-SPEC ${model.toJson()}")
4348
// println("PRE-LOADED ${branch.getRootNode().toJson()}")
44-
importer = ModelImporter(branch.getRootNode(), ImportStats()).apply { import(newModelFile) }
49+
importer = ModelImporter(branch.getRootNode()).apply { import(newModelFile) }
4550
// println("POST-SPEC ${newModel.root.toJson()}")
4651
// println("POST-LOADED ${branch.getRootNode().toJson()}")
4752
}
@@ -91,17 +96,22 @@ class ModelImporterTest {
9196

9297
@Test
9398
fun `uses minimal amount of operations`() {
94-
val stats = importer.stats ?: fail("No import stats found.")
95-
assertEquals(1, stats.additions.size)
96-
assertEquals(1, stats.deletions.size)
97-
assertEquals(5, stats.moves.size)
98-
assertEquals(4, stats.propertyChanges.size)
99-
assertEquals(3, stats.referenceChanges.size)
99+
val operations = branch.operationsAndTree.first
100+
101+
val numOps = operations.numOpsByType()
102+
val numPropertyChangesIgnoringOriginalId =
103+
numOps[AddNewChildOp::class]?.let { numOps[SetPropertyOp::class]?.minus(it) } ?: 0
104+
105+
assertEquals(1, numOps[AddNewChildOp::class])
106+
assertEquals(1, numOps[DeleteNodeOp::class])
107+
assertEquals(5, numOps[MoveNodeOp::class])
108+
assertEquals(4, numPropertyChangesIgnoringOriginalId)
109+
assertEquals(3, numOps[SetReferenceOp::class])
100110
}
101111

102112
@Test
103113
fun `operations do not overlap`() {
104-
assertNoOverlappingOperations(importer.stats)
114+
assertNoOverlappingOperations(branch.operationsAndTree.first)
105115
}
106116

107117
@Test
@@ -125,13 +135,13 @@ class ModelImporterTest {
125135
println("Seed for random change test: $seed")
126136
lateinit var initialState: NodeData
127137
lateinit var specification: NodeData
128-
val numChanges = 50
138+
val numChanges = 200
129139

130140
branch0.runWrite {
131141
val rootNode = branch0.getRootNode()
132142
rootNode.setPropertyValue(NodeData.idPropertyKey, rootNode.reference.serialize())
133143
val grower = RandomModelChangeGenerator(rootNode, Random(seed)).growingOperationsOnly()
134-
for (i in 1..20) {
144+
for (i in 1..1000) {
135145
grower.applyRandomChange()
136146
}
137147
initialState = rootNode.asExported()
@@ -143,28 +153,31 @@ class ModelImporterTest {
143153
specification = rootNode.asExported()
144154
}
145155

146-
val tree1 = CLTree(ObjectStoreCache(MapBaseStore()))
147-
val branch1 = PBranch(tree1, IdGenerator.getInstance(1))
156+
val store = ObjectStoreCache(MapBaseStore())
157+
val tree1 = CLTree(store)
158+
val idGenerator = IdGenerator.getInstance(1)
159+
val branch1 = PBranch(tree1, idGenerator)
148160

149161
branch1.runWrite {
150-
val importer = ModelImporter(branch1.getRootNode(), ImportStats())
162+
val importer = ModelImporter(branch1.getRootNode())
151163
importer.import(ModelData(root = initialState))
164+
}
165+
val otBranch = OTBranch(branch1, idGenerator, store)
166+
167+
otBranch.runWrite {
168+
val importer = ModelImporter(otBranch.getRootNode())
152169
importer.import(ModelData(root = specification))
153170

154-
// println("INITIAL")
155-
// println(initialState.toJson())
156-
//
157-
// println("SPEC")
158-
// println(specification.toJson())
159-
//
160-
// println("ACTUAL")
161-
// println(branch1.getRootNode().toJson())
162-
163-
assertAllNodesConformToSpec(specification, branch1.getRootNode())
164-
assertNoOverlappingOperations(importer.stats)
165-
assert(importer.stats!!.getTotal() <= numChanges) {
166-
"expected operations: <= $numChanges, actual: ${importer.stats!!.getTotal()}"
167-
}
171+
assertAllNodesConformToSpec(specification, otBranch.getRootNode())
172+
}
173+
val operations = otBranch.operationsAndTree.first
174+
assertNoOverlappingOperations(operations)
175+
176+
val numSetOriginalIdOps = specification.countNodes()
177+
val expectedNumOps = numChanges + numSetOriginalIdOps
178+
179+
assert(operations.size <= expectedNumOps ) {
180+
"expected operations: <= $expectedNumOps, actual: ${operations.size}"
168181
}
169182
}
170183
}

0 commit comments

Comments
 (0)