Skip to content

Commit c418222

Browse files
committed
fix(model-sync-lib): fixed moving within the same parent but across roles
1 parent 7aee76e commit c418222

File tree

4 files changed

+29
-9
lines changed

4 files changed

+29
-9
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -164,22 +164,28 @@ class ModelImporter(private val root: INode, val stats: ImportStats? = null) {
164164

165165
val targetIndices = HashMap<String?, Int>(nodeData.children.size)
166166
for (child in toBeSortedSpec) {
167-
val baseIndex = toBeSortedSpec.filter { it.role == node.roleInParent }.indexOf(child)
167+
168+
val childrenInRole = existingChildren.filter { it.roleInParent == child.role }
169+
val baseIndex = child.getIndexWithinRole(nodeData, childrenInRole.lastIndex)
168170
var offset = 0
169-
offset += existingChildren.filter { it.roleInParent == child.role }.slice(0..baseIndex).count {
171+
offset += childrenInRole.slice(0..baseIndex).count {
170172
!originalIdToSpec.containsKey(it.originalId()) // node will be deleted
171173
}
172-
offset -= specifiedChildren.filter { it.role == child.role }.slice(0..baseIndex).count {
174+
offset -= childrenInRole.slice(0..baseIndex).count {
173175
!existingIds.contains(it.originalId()) // node will be moved here
174176
}
175-
val index = baseIndex + offset
176-
targetIndices[child.originalId()] = minOf(index, existingChildren.size - 1)
177+
val index = if (childrenInRole.isEmpty()) 0 else baseIndex + offset
178+
val upperBound = if (existingChildren.isEmpty()) 0 else existingChildren.lastIndex
179+
targetIndices[child.originalId()] = minOf(index, upperBound)
177180
}
178181

179-
for ((index, child) in existingChildren.withIndex()) {
182+
existingChildren.forEach { child ->
183+
184+
val currentIndex = child.index()
185+
val targetRole = originalIdToSpec[child.originalId()]?.role
180186
val targetIndex = targetIndices[child.originalId()]
181-
if (targetIndex != null && targetIndex != index) {
182-
node.moveChildWithStats(child.roleInParent, targetIndex, child)
187+
if (targetIndex != null && (targetIndex != currentIndex || child.roleInParent != targetRole)) {
188+
node.moveChildWithStats(targetRole, targetIndex, child)
183189
}
184190
}
185191
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class ModelImporterTest {
9494
val stats = importer.stats ?: fail("No import stats found.")
9595
assertEquals(1, stats.additions.size)
9696
assertEquals(1, stats.deletions.size)
97-
assertEquals(4, stats.moves.size)
97+
assertEquals(5, stats.moves.size)
9898
assertEquals(4, stats.propertyChanges.size)
9999
assertEquals(3, stats.referenceChanges.size)
100100
}

model-sync-lib/src/test/resources/model.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@
5555
"properties": {
5656
"purpose": "move across parents"
5757
}
58+
},
59+
{
60+
"id": "node:006",
61+
"role": "roleBefore",
62+
"properties": {
63+
"purpose": "change role"
64+
}
5865
}
5966
]
6067
}

model-sync-lib/src/test/resources/newmodel.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@
5858
"properties": {
5959
"purpose": "move across parents"
6060
}
61+
},
62+
{
63+
"id": "node:006",
64+
"role": "roleAfter",
65+
"properties": {
66+
"purpose": "change role"
67+
}
6168
}
6269
]
6370
}

0 commit comments

Comments
 (0)