Merge pull request #677 from modelix/fix/bulk-sync-update-prev-unresolvable-refs

fix(bulk-model-sync): update previously unresolvable references if they can be resolved

Author: mhuster23

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

@@ -80,20 +80,23 @@ class ModelImporter(
         }
     }
 
-    data class PostponedReference(
+    private data class PostponedReference(
         val expectedTargetId: String,
-        val mpsNode: INode,
+        val node: INode,
         val role: String,
     )
 
     private fun PostponedReference.setPostponedReference() {
         val expectedRefTarget = originalIdToExisting[expectedTargetId]
-        if (expectedRefTarget == null) {
+        if (expectedRefTarget != null) {
+            node.setReferenceTarget(role, expectedRefTarget)
+            return
+        }
+        // Ensure unresolvable reference wasn't stored before
+        if (node.getReferenceTargetRef(role)?.serialize() != expectedTargetId) {
             // The target node is not part of the model. Assuming it exists in some other model we can
             // store the reference and try to resolve it dynamically on access.
-            mpsNode.setReferenceTarget(role, SerializedNodeReference(expectedTargetId))
-        } else {
-            mpsNode.setReferenceTarget(role, expectedRefTarget)
+            node.setReferenceTarget(role, SerializedNodeReference(expectedTargetId))
         }
     }
 
@@ -293,16 +296,13 @@ class ModelImporter(
     }
 
     private fun syncReferences(node: INode, nodeData: NodeData) {
-        nodeData.references.forEach {
-            val expectedTargetId = it.value
-            val actualTargetId = node.getReferenceTarget(it.key)?.originalId()
-                ?: node.getReferenceTargetRef(it.key)?.serialize()
-            if (actualTargetId != expectedTargetId) {
+        nodeData.references.forEach { (referenceRole, expectedTargetId) ->
+            if (referenceNeedsUpdate(node, referenceRole, expectedTargetId)) {
                 val expectedTarget = originalIdToExisting[expectedTargetId]
                 if (expectedTarget == null) {
-                    postponedReferences += PostponedReference(expectedTargetId, node, it.key)
+                    postponedReferences += PostponedReference(expectedTargetId, node, referenceRole)
                 } else {
-                    node.setReferenceTarget(it.key, expectedTarget)
+                    node.setReferenceTarget(referenceRole, expectedTarget)
                 }
             }
         }
@@ -311,6 +311,14 @@ class ModelImporter(
             node.setReferenceTarget(it, null as INodeReference?)
         }
     }
+
+    private fun referenceNeedsUpdate(node: INode, referenceRole: String, expectedTargetRef: String): Boolean {
+        val actualTarget = node.getReferenceTarget(referenceRole)
+            ?: return true // previously unresolvable reference might become resolvable e.g., when a referenced node was added
+        val actualTargetRef = actualTarget.originalId() ?: node.getReferenceTargetRef(referenceRole)?.serialize()
+
+        return actualTargetRef != expectedTargetRef
+    }
 }
 
 internal fun INode.originalId(): String? {

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

@@ -20,6 +20,7 @@ import org.modelix.model.api.IBranch
 import org.modelix.model.api.IProperty
 import org.modelix.model.api.IReferenceLink
 import org.modelix.model.api.PBranch
+import org.modelix.model.api.PNodeReference
 import org.modelix.model.api.getRootNode
 import org.modelix.model.client.IdGenerator
 import org.modelix.model.data.ModelData
@@ -42,6 +43,7 @@ import kotlin.random.Random
 import kotlin.reflect.KClass
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
 class ModelImporterTest {
@@ -198,6 +200,75 @@ class ModelImporterTest {
         assertEquals(expectedOperations, branch.getNumOfUsedOperationsByType())
     }
 
+    @Test
+    @JsName("can_update_previously_unresolvable_references")
+    fun `can update previously unresolvable reference`() {
+        // language=json
+        val initialData = """
+            {
+              "root": {
+                "id": "root",
+                "children": [
+                  {
+                    "id": "node:001",
+                    "references": {
+                      "myRef": "node:002",
+                      "myRef2": "node:unresolvable"
+                    }
+                  }
+                ]
+              }
+            }
+        """.trimIndent().let { ModelData.fromJson(it) }
+
+        // language=json
+        val expectedData = """
+            {
+              "root": {
+                "id": "root",
+                "children": [
+                  {
+                    "id": "node:001",
+                    "references": {
+                      "myRef": "node:002",
+                      "myRef2": "node:unresolvable"
+                    }
+                  },
+                  {
+                    "id": "node:002",
+                    "properties": {
+                      "name": "PreviouslyUnresolvableNode"
+                    }
+                  }
+                ]
+              }
+            }
+        """.trimIndent().let { ModelData.fromJson(it) }
+
+        val expectedOperations: Map<KClass<out IOperation>, Int> = mapOf(
+            SetReferenceOp::class to 1,
+            AddNewChildOp::class to 1,
+            SetPropertyOp::class to 2, // name and originalRef of new node
+        )
+
+        val branch = createOTBranchFromModel(initialData)
+        branch.importIncrementally(expectedData)
+
+        branch.runRead {
+            val root = branch.getRootNode()
+            val sourceNode = root.allChildren.first()
+            val referenceLink = IReferenceLink.fromName("myRef")
+
+            val targetReference = sourceNode.getReferenceTargetRef(referenceLink)
+            val targetNode = sourceNode.getReferenceTarget(referenceLink)
+
+            assertNotNull(targetNode)
+            assertTrue { targetReference is PNodeReference }
+            assertAllNodesConformToSpec(expectedData.root, root)
+            assertEquals(expectedOperations, branch.getNumOfUsedOperationsByType())
+        }
+    }
+
     @Test
     @JsName("can_sync_children")
     fun `can sync children`() {