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

For unresolvable references we store the serialized expected reference, as we assume the node is not part of the model but the reference might still be useful for dynamic resolution on access.
If the target node became part of the model later on, we did not update it because the originalIds still matched.
Now we check, if it was previously unresolvable and update the reference to the INode accordingly.

Author: mhuster23

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

@@ -88,12 +88,15 @@ class ModelImporter(
 
     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.
             node.setReferenceTarget(role, SerializedNodeReference(expectedTargetId))
-        } else {
-            node.setReferenceTarget(role, expectedRefTarget)
         }
     }
 
@@ -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`() {