Merge pull request #636 from modelix/feature/bulk-sync-handle-concept-changes

MODELIX-710 bulk-model-sync-gradle fails with "unexpected concept change"

Author: mhuster23

.github/workflows/mps-compatibility.yaml

@@ -16,7 +16,6 @@ jobs:
     strategy:
       matrix:
         version:
-          - "2020.3"
           - "2021.1"
           - "2021.2"
           - "2021.3"

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

@@ -22,6 +22,7 @@ import org.modelix.model.api.ConceptReference
 import org.modelix.model.api.INode
 import org.modelix.model.api.INodeReference
 import org.modelix.model.api.INodeResolutionScope
+import org.modelix.model.api.IReplaceableNode
 import org.modelix.model.api.SerializedNodeReference
 import org.modelix.model.api.getDescendants
 import org.modelix.model.api.isChildRoleOrdered
@@ -162,9 +163,10 @@ class ModelImporter(
         currentNodeProgress += 1
         progressReporter.step(currentNodeProgress.toULong())
         doAndPotentiallyContinueOnErrors {
-            syncProperties(node, data)
-            syncChildren(node, data, progressReporter)
-            syncReferences(node, data)
+            val conceptCorrectedNode = checkAndHandleConceptChange(node, data)
+            syncProperties(conceptCorrectedNode, data)
+            syncChildren(conceptCorrectedNode, data, progressReporter)
+            syncReferences(conceptCorrectedNode, data)
         }
     }
 
@@ -200,7 +202,9 @@ class ModelImporter(
             // optimization for when there is no change in the child list
             // size check first to avoid querying the original ID
             if (expectedNodes.size == existingNodes.size && expectedNodes.map { it.originalId() } == existingNodes.map { it.originalId() }) {
-                existingNodes.zip(expectedNodes).forEach { syncNode(it.first, it.second, progressReporter) }
+                existingNodes.zip(expectedNodes).forEach {
+                    syncNode(it.first, it.second, progressReporter)
+                }
                 continue
             }
 
@@ -255,8 +259,6 @@ class ModelImporter(
                 } else {
                     nodeAtIndex
                 }
-                check(childNode.getConceptReference() == expectedConcept) { "Unexpected concept change from $expectedConcept to ${childNode.getConceptReference()}" }
-
                 syncNode(childNode, expected, progressReporter)
             }
 
@@ -270,6 +272,15 @@ class ModelImporter(
         }
     }
 
+    private fun checkAndHandleConceptChange(existingNode: INode, expectedNode: NodeData): INode {
+        val newConcept = expectedNode.concept?.let { ConceptReference(it) }
+        if (existingNode.getConceptReference() == newConcept) {
+            return existingNode
+        }
+        require(existingNode is IReplaceableNode) { "Concept has changed but node is not replaceable. node=$existingNode" }
+        return existingNode.replaceNode(newConcept)
+    }
+
     private fun buildExistingIndex(): MutableMap<String, INodeReference> {
         val localOriginalIdToExisting = createMemoryEfficientMap<String, INodeReference>()
         root.getDescendants(true).forEach { node ->

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

@@ -399,6 +399,76 @@ class ModelImporterTest {
         assertEquals(expectedOperations, branch.getNumOfUsedOperationsByType())
     }
 
+    @Test
+    @JsName("can_handle_concept_changes")
+    fun `can handle concept changes`() {
+        // language=json
+        val initialData = """
+            {
+              "root": {
+                "id": "node:001",
+                "children": [
+                  {
+                    "id": "node:002",
+                    "concept": "ref:MyOldConcept",
+                    "children": [
+                      {
+                        "id": "child02",
+                        "references": {
+                          "myReference": "node:003"
+                        },
+                        "properties": {
+                          "myProperty": "myPropertyValue"
+                        }
+                      }
+                    ]
+                  },
+                  {
+                    "id": "node:003"
+                  }
+                ]
+              }
+            }
+        """.trimIndent().let { ModelData.fromJson(it) }
+
+        // language=json
+        val expectedData = """
+            {
+              "root": {
+                "id": "node:001",
+                "children": [
+                  {
+                    "id": "node:002",
+                    "concept": "ref:MyNewConcept",
+                    "children": [
+                      {
+                        "id": "child02",
+                        "references": {
+                          "myReference": "node:003"
+                        },
+                        "properties": {
+                          "myProperty": "myPropertyValue"
+                        }
+                      }
+                    ]
+                  },
+                  {
+                    "id": "node:003"
+                  }
+                ]
+              }
+            }
+        """.trimIndent().let { ModelData.fromJson(it) }
+
+        val branch = createOTBranchFromModel(initialData)
+        branch.importIncrementally(expectedData)
+
+        branch.runRead {
+            val changedNode = branch.getRootNode().allChildren.first()
+            assertEquals("ref:MyNewConcept", changedNode.getConceptReference().toString())
+        }
+    }
+
     @Test
     @JsName("can_import_complex_model")
     fun `can import complex model`() {

model-api/src/commonMain/kotlin/org/modelix/model/api/INode.kt

@@ -297,6 +297,20 @@ interface IDeprecatedNodeDefaults : INode {
     override fun getReferenceLinks(): List<IReferenceLink>
 }
 
+/**
+ *  Interface for nodes that can be replaced by a new instance.
+ */
+interface IReplaceableNode : INode {
+    /**
+     * Replaces this node with a new node of the given concept that uses the same id as this node.
+     * Properties, references and children will be the same.
+     *
+     * @param concept the concept of the new node
+     * @return replacement for this node with the new given concept
+     */
+    fun replaceNode(concept: ConceptReference?): INode
+}
+
 @Deprecated("Use .key(INode), .key(IBranch), .key(ITransaction) or .key(ITree)")
 fun IRole.key(): String = RoleAccessContext.getKey(this)
 fun IRole.key(node: INode): String = if (node.usesRoleIds()) getUID() else getSimpleName()

model-api/src/commonMain/kotlin/org/modelix/model/api/ITree.kt

@@ -254,6 +254,12 @@ interface ITree {
      */
     fun deleteNodes(nodeIds: LongArray): ITree
 
+    /**
+     * @param nodeId id of the node
+     * @param concept the concept ref of the new concept for the node, or null to remove the concept
+     */
+    fun setConcept(nodeId: Long, concept: IConceptReference?): ITree
+
     fun getAllChildrenAsFlow(parentId: Long): Flow<Long> = getAllChildren(parentId).asFlow()
     fun getDescendantsAsFlow(nodeId: Long, includeSelf: Boolean = false): Flow<Long> {
         return if (includeSelf) {

model-api/src/commonMain/kotlin/org/modelix/model/api/ITreeChangeVisitor.kt

@@ -26,6 +26,13 @@ interface ITreeChangeVisitor {
      */
     fun containmentChanged(nodeId: Long)
 
+    /**
+     * Called when the concept of a node has changed.
+     *
+     * @param nodeId id of the affected node
+     */
+    fun conceptChanged(nodeId: Long) {} // Noop default implementation to avoid breaking change
+
     /**
      * Called when the children of a node have changed.
      *

model-api/src/commonMain/kotlin/org/modelix/model/api/IWriteTransaction.kt

@@ -105,6 +105,14 @@ interface IWriteTransaction : ITransaction {
     fun addNewChildren(parentId: Long, role: String?, index: Int, concepts: Array<IConceptReference?>): LongArray
     fun addNewChildren(parentId: Long, role: String?, index: Int, childIds: LongArray, concepts: Array<IConceptReference?>)
 
+    /**
+     * Sets the concept of the node identified by the given id.
+     *
+     * @param nodeId id of the node
+     * @param concept the new concept, or null to remove the concept
+     */
+    fun setConcept(nodeId: Long, concept: IConceptReference?)
+
     /**
      * Deletes the given node.
      *

model-api/src/commonMain/kotlin/org/modelix/model/api/PNodeAdapter.kt

@@ -20,7 +20,7 @@ import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import org.modelix.model.area.PArea
 
-open class PNodeAdapter(val nodeId: Long, val branch: IBranch) : INode, INodeEx {
+open class PNodeAdapter(val nodeId: Long, val branch: IBranch) : INode, INodeEx, IReplaceableNode {
 
     init {
         require(nodeId != 0L, { "Invalid node 0" })
@@ -82,6 +82,11 @@ open class PNodeAdapter(val nodeId: Long, val branch: IBranch) : INode, INodeEx
         return PNodeAdapter(branch.writeTransaction.addNewChild(nodeId, role.key(this), index, concept), branch)
     }
 
+    override fun replaceNode(concept: ConceptReference?): INode {
+        branch.writeTransaction.setConcept(nodeId, concept)
+        return this
+    }
+
     override val allChildren: Iterable<INode>
         get() {
             notifyAccess()

model-api/src/commonMain/kotlin/org/modelix/model/api/TreePointer.kt

@@ -138,6 +138,10 @@ class TreePointer(private var tree_: ITree, val idGenerator: IIdGenerator = IdGe
         tree = tree.addNewChildren(parentId, role, index, childIds, concepts)
     }
 
+    override fun setConcept(nodeId: Long, concept: IConceptReference?) {
+        tree = tree.setConcept(nodeId, concept)
+    }
+
     override fun deleteNode(nodeId: Long) {
         tree = tree.deleteNode(nodeId)
     }

model-api/src/commonMain/kotlin/org/modelix/model/api/WriteTransaction.kt

@@ -91,6 +91,11 @@ class WriteTransaction(_tree: ITree, branch: IBranch, idGenerator: IIdGenerator)
         return newIds
     }
 
+    override fun setConcept(nodeId: Long, concept: IConceptReference?) {
+        checkNotClosed()
+        tree = tree.setConcept(nodeId, concept)
+    }
+
     override fun deleteNode(nodeId: Long) {
         checkNotClosed()
         tree.getAllChildren(nodeId).forEach { deleteNode(it) }