fix(mps-legacy-sync-plugin): fix initial synchronization of reference…

…s in models Previously, a reference was not synced as PNodeReference if the referenced node not was already seen. The fix is done for the initial sync in the legacy API. The new API is not used because it does not support callbacks for progress monitor. Extending the new API is not reasonable because the plugin is currently rewritten. The same still exists for references between nodes in different models and in different modules. But it is harder to fix.
modelix · Mar 1, 2024 · 180e5b0 · 180e5b0
1 parent 1be0fb6
commit 180e5b0
Show file tree

Hide file tree

Showing 13 changed files with 298 additions and 113 deletions.
diff --git a/mps-legacy-sync-plugin/src/main/java/org/modelix/model/mpsplugin/INodeUtils.kt b/mps-legacy-sync-plugin/src/main/java/org/modelix/model/mpsplugin/INodeUtils.kt
@@ -1,9 +1,6 @@
 package org.modelix.model.mpsplugin
 
-import jetbrains.mps.baseLanguage.tuples.runtime.MultiTuple
-import jetbrains.mps.baseLanguage.tuples.runtime.Tuples
 import jetbrains.mps.internal.collections.runtime.ListSequence
-import jetbrains.mps.internal.collections.runtime.MapSequence
 import jetbrains.mps.internal.collections.runtime.Sequence
 import jetbrains.mps.smodel.adapter.structure.MetaAdapterFactory
 import org.jetbrains.mps.openapi.language.SConcept
@@ -14,7 +11,6 @@ import org.modelix.model.api.INode
 import org.modelix.model.api.IProperty
 import org.modelix.model.api.IReferenceLink
 import org.modelix.model.api.PNodeAdapter
-import org.modelix.model.api.PNodeAdapter.Companion.wrap
 import org.modelix.model.mpsadapters.mps.SConceptAdapter
 import java.util.LinkedList
 import java.util.Objects
@@ -115,77 +111,84 @@ object INodeUtils {
         }
     }
 
-    fun copyPropertyIfNecessary(_this: INode, original: INode, property: SProperty) {
-        if (Objects.equals(original.getPropertyValue(property.name), _this.getPropertyValue(property.name))) {
+    fun copyPropertyIfNecessary(copiedNode: INode, originalNode: INode, property: SProperty) {
+        if (Objects.equals(originalNode.getPropertyValue(property.name), copiedNode.getPropertyValue(property.name))) {
             return
         }
-        copyProperty(_this, original, property)
+        copyProperty(copiedNode, originalNode, property)
     }
 
-    fun replicateChild(_this: INode?, role: String, original: INode?): INode {
-        try {
-            val equivalenceMap: Map<INode?, INode> = MapSequence.fromMap(HashMap())
-            val postponedReferencesAssignments: List<Tuples._3<INode, String, INode>> =
-                ListSequence.fromList(LinkedList())
-            val result: INode =
-                replicateChildHelper(_this, role, original, equivalenceMap, postponedReferencesAssignments)
-            for (postponedRefAssignment: Tuples._3<INode, String, INode> in ListSequence.fromList(
-                postponedReferencesAssignments,
-            )) {
-                var target: INode? = postponedRefAssignment._2()
-                if (MapSequence.fromMap(equivalenceMap).containsKey(target)) {
-                    target = MapSequence.fromMap(equivalenceMap).get(target)
-                }
-                postponedRefAssignment._0().setReferenceTarget(postponedRefAssignment._1(), target)
+    fun replicateChild(copiedParent: INode, role: String, originalChild: INode): INode {
+        return replicateChildren(copiedParent, role, listOf(originalChild)).single()
+    }
+
+    data class PostponedReferenceAssignments(
+        val copiedNode: INode,
+        val referenceRole: String,
+        val originalTarget: INode,
+    )
+
+    private fun replicateChildren(copiedParent: INode, role: String, originalChildren: Iterable<INode>): List<INode> {
+        val equivalenceMap = mutableMapOf<INode, INode>()
+        val postponedReferencesAssignments = mutableListOf<PostponedReferenceAssignments>()
+        val resultNodes = mutableListOf<INode>()
+        for (originalChild in originalChildren) {
+            try {
+                val result = replicateChildHelper(copiedParent, role, originalChild, equivalenceMap, postponedReferencesAssignments)
+                resultNodes.add(result)
+            } catch (e: Exception) {
+                throw RuntimeException(
+                    "Unable to replicate child in role " + role + ". Original child: " + originalChild + ", Copied parent: " + copiedParent,
+                    e,
+                )
             }
-            return result
-        } catch (e: Exception) {
-            throw RuntimeException(
-                "Unable to replicate child in role " + role + ". Original: " + original + ", This: " + _this,
-                e,
-            )
         }
+        for (postponedRefAssignment in postponedReferencesAssignments) {
+            val copiedNode = postponedRefAssignment.copiedNode
+            val referenceRole = postponedRefAssignment.referenceRole
+            val copiedTarget = equivalenceMap[postponedRefAssignment.originalTarget]
+            val targetToSet = copiedTarget ?: postponedRefAssignment.originalTarget
+            copiedNode.setReferenceTarget(referenceRole, targetToSet)
+        }
+        return resultNodes
     }
 
-    fun cloneChildren(_this: INode?, original: INode?, role: String) {
-        removeAllChildrenWithRole(_this, role)
-        for (originalChild: INode? in Sequence.fromIterable(
-            original!!.getChildren(role),
-        )) {
-            replicateChild(_this, role, originalChild)
-        }
+    fun cloneChildren(copiedParent: INode, originalParent: INode, role: String) {
+        removeAllChildrenWithRole(copiedParent, role)
+        replicateChildren(copiedParent, role, originalParent.getChildren(role))
     }
 
     fun replicateChildHelper(
-        _this: INode?,
+        copiedParent: INode,
         role: String,
-        original: INode?,
-        equivalenceMap: Map<INode?, INode>?,
-        postponedReferencesAssignments: List<Tuples._3<INode, String, INode>>?,
+        original: INode,
+        equivalenceMap: MutableMap<INode, INode>,
+        postponedReferencesAssignments: MutableList<PostponedReferenceAssignments>,
     ): INode {
-        val concept: IConcept? = original!!.concept
-        var copy: INode? = null
-        try {
-            copy = _this!!.addNewChild(role, -1, concept)
+        val concept: IConcept? = original.concept
+        checkNotNull(concept)
+        val copy = try {
+            val addedChild = copiedParent.addNewChild(role, -1, concept)
+            equivalenceMap[original] = addedChild
+            addedChild
         } catch (e: Exception) {
             throw RuntimeException(
-                "Unable to add child to " + _this + " with role " + role + " and concept " + concept,
+                "Unable to add child to " + copiedParent + " with role " + role + " and concept " + concept,
                 e,
             )
         }
-        for (property: IProperty in ListSequence.fromList<IProperty>(concept!!.getAllProperties())) {
+        for (property: IProperty in concept.getAllProperties()) {
             copy.setPropertyValue(property.name, original.getPropertyValue(property.name))
         }
-        for (childLink: IChildLink in ListSequence.fromList<IChildLink>(concept.getAllChildLinks())) {
-            for (child: INode? in ListSequence.fromList<INode>(getChidlrenAsList(original, childLink.name))) {
+        for (childLink: IChildLink in concept.getAllChildLinks()) {
+            for (child: INode in getChidlrenAsList(original, childLink.name)) {
                 replicateChildHelper(copy, childLink.name, child, equivalenceMap, postponedReferencesAssignments)
             }
         }
-        for (refLink: IReferenceLink in ListSequence.fromList<IReferenceLink>(concept.getAllReferenceLinks())) {
-            val target: INode? = original.getReferenceTarget(refLink.name)
-            if (target != null) {
-                ListSequence.fromList(postponedReferencesAssignments)
-                    .addElement(MultiTuple.from(copy, refLink.name, target))
+        for (refLink: IReferenceLink in concept.getAllReferenceLinks()) {
+            val originalTarget: INode? = original.getReferenceTarget(refLink.name)
+            if (originalTarget != null) {
+                postponedReferencesAssignments.add(PostponedReferenceAssignments(copy, refLink.name, originalTarget))
             }
         }
         return copy

diff --git a/mps-legacy-sync-plugin/src/main/java/org/modelix/model/mpsplugin/ModelCloudImportUtils.kt b/mps-legacy-sync-plugin/src/main/java/org/modelix/model/mpsplugin/ModelCloudImportUtils.kt
@@ -3,7 +3,6 @@ package org.modelix.model.mpsplugin
 import jetbrains.mps.baseLanguage.closures.runtime.Wrappers._T
 import jetbrains.mps.ide.project.ProjectHelper
 import jetbrains.mps.internal.collections.runtime.ListSequence
-import jetbrains.mps.internal.collections.runtime.Sequence
 import jetbrains.mps.progress.EmptyProgressMonitor
 import jetbrains.mps.project.MPSProject
 import jetbrains.mps.project.Project
@@ -14,18 +13,14 @@ import org.jetbrains.mps.openapi.language.SContainmentLink
 import org.jetbrains.mps.openapi.language.SProperty
 import org.jetbrains.mps.openapi.model.SModel
 import org.jetbrains.mps.openapi.model.SNode
-import org.jetbrains.mps.openapi.model.SReference
 import org.jetbrains.mps.openapi.module.SModule
 import org.jetbrains.mps.openapi.util.ProgressMonitor
 import org.modelix.model.api.INode
 import org.modelix.model.api.PNodeAdapter
-import org.modelix.model.api.PNodeAdapter.Companion.wrap
 import org.modelix.model.lazy.RepositoryId
 import org.modelix.model.mpsadapters.mps.NodeToSNodeAdapter
-import org.modelix.model.mpsadapters.mps.SConceptAdapter
 import org.modelix.model.mpsadapters.mps.SModelAsNode
 import org.modelix.model.mpsadapters.mps.SModuleAsNode
-import org.modelix.model.mpsadapters.mps.SNodeToNodeAdapter
 import org.modelix.model.mpsplugin.history.CloudNodeTreeNode
 import org.modelix.model.mpsplugin.history.CloudNodeTreeNodeBinding
 import org.modelix.model.mpsplugin.plugin.PersistedBindingConfiguration
@@ -114,7 +109,7 @@ object ModelCloudImportUtils {
         progress: ProgressMonitor?,
     ): INode? {
         // First create the module
-        val cloudModuleNode: INode? = treeInRepository.createModule(module!!.moduleName)
+        val cloudModuleNode: INode = treeInRepository.createModule(module!!.moduleName)
         replicatePhysicalModule(treeInRepository, cloudModuleNode, module, null, progress)
         return cloudModuleNode
     }
@@ -172,16 +167,16 @@ object ModelCloudImportUtils {
      */
     fun replicatePhysicalModule(
         treeInRepository: CloudRepository,
-        cloudModule: INode?,
-        physicalModule: SModule?,
+        cloudModule: INode,
+        physicalModule: SModule,
         modelMappingConsumer: Consumer<PhysicalToCloudModelMapping?>?,
         progress: ProgressMonitor?,
     ) {
         val _progress: _T<ProgressMonitor?> = _T(progress)
         if (_progress.value == null) {
             _progress.value = EmptyProgressMonitor()
         }
-        val sModuleAsNode: SModuleAsNode? = SModuleAsNode.Companion.wrap(physicalModule)
+        val sModuleAsNode = SModuleAsNode(physicalModule)
         treeInRepository.runWrite(object : Consumer<PNodeAdapter?> {
             override fun accept(rootNode: PNodeAdapter?) {
                 INodeUtils.copyProperty(cloudModule, sModuleAsNode, PROPS.`name$MnvL`.name)
@@ -227,8 +222,8 @@ object ModelCloudImportUtils {
      *
      * @return the created cloud model
      */
-    fun copyPhysicalModel(treeInRepository: CloudRepository, cloudModule: INode?, physicalModel: SModel?): INode? {
-        val originalModel: INode? = SModelAsNode.Companion.wrap(physicalModel)
+    fun copyPhysicalModel(treeInRepository: CloudRepository, cloudModule: INode?, physicalModel: SModel): INode {
+        val originalModel = SModelAsNode(physicalModel)
         val cloudModel: INode = treeInRepository.createNode(
             cloudModule,
             LINKS.`models$h3QT`,
@@ -239,46 +234,13 @@ object ModelCloudImportUtils {
                     INodeUtils.copyProperty(cloudModel, originalModel, PROPS.`id$lDUo`.name)
                     INodeUtils.cloneChildren(cloudModel, originalModel, LINKS.`modelImports$8DOI`.name)
                     INodeUtils.cloneChildren(cloudModel, originalModel, LINKS.`usedLanguages$QK4E`.name)
-                    for (physicalRoot: SNode in Sequence.fromIterable<SNode>(
-                        physicalModel!!.rootNodes,
-                    )) {
-                        val cloudRoot: INode = cloudModel.addNewChild(
-                            LINKS.`rootNodes$jxXY`.name,
-                            -1,
-                            SConceptAdapter.Companion.wrap(physicalRoot.concept),
-                        )
-                        replicatePhysicalNode(cloudRoot, physicalRoot)
-                    }
+                    INodeUtils.cloneChildren(cloudModel, originalModel, LINKS.`rootNodes$jxXY`.name)
                 }
             },
         )
         return cloudModel
     }
 
-    /**
-     * This takes a cloud node already created and a physical node.
-     * It then ensures that the cloud node is exactly as the original physical node.
-     *
-     * It operates recursively on children.
-     */
-    private fun replicatePhysicalNode(cloudNode: INode, physicalNode: SNode) {
-        MPSNodeMapping.mapToMpsNode(cloudNode, physicalNode)
-        for (prop: SProperty in Sequence.fromIterable(physicalNode.properties)) {
-            cloudNode.setPropertyValue(prop.name, physicalNode.getProperty(prop))
-        }
-        for (ref: SReference in Sequence.fromIterable(physicalNode.references)) {
-            cloudNode.setReferenceTarget(ref.role, SNodeToNodeAdapter.Companion.wrap(ref.targetNode))
-        }
-        for (physicalChild: SNode in Sequence.fromIterable(physicalNode.children)) {
-            val cloudChild: INode = cloudNode.addNewChild(
-                physicalChild.containmentLink!!.name,
-                -1,
-                SConceptAdapter.Companion.wrap(physicalChild.concept),
-            )
-            replicatePhysicalNode(cloudChild, physicalChild)
-        }
-    }
-
     private object PROPS {
         /*package*/
         val `name$MnvL`: SProperty = MetaAdapterFactory.getProperty(

diff --git a/mps-legacy-sync-plugin/src/main/kotlin/org/modelix/mps/sync/ModelSyncService.kt b/mps-legacy-sync-plugin/src/main/kotlin/org/modelix/mps/sync/ModelSyncService.kt
@@ -113,7 +113,7 @@ class ModelSyncService : Disposable, ISyncService {
     private fun getRootBindings() = ModelServerConnections.instance.modelServers
         .flatMap(ModelServerConnection::getRootBindings)
 
-    private fun flushAllBindings() {
+    override fun flushAllBindings() {
         getRootBindings().forEach { it.syncQueue.flush() }
     }
 

diff --git a/mps-legacy-sync-plugin/src/main/kotlin/org/modelix/mps/sync/api/ISyncService.kt b/mps-legacy-sync-plugin/src/main/kotlin/org/modelix/mps/sync/api/ISyncService.kt
@@ -62,6 +62,15 @@ interface ISyncService {
      * To not deadlock your program, do not call this method while holding a write lock or read lock.
      */
     fun findMpsNode(cloudNodeReference: INodeReference): List<SNode>
+
+    /**
+     * Synchronize all bindings between MPS and the model server.
+     * The call blocks until all synchronizations are finished or timeout.
+     *
+     * The synchronization is always scheduled on a different thread which will try to acquire a write and read lock.
+     * To not deadlock your program, do not call this method while holding a write lock or read lock.
+     */
+    fun flushAllBindings()
 }
 
 interface IModelServerConnection : Disposable {

diff --git a/mps-legacy-sync-plugin/src/test/kotlin/FindingNodesInBindingTest.kt b/mps-legacy-sync-plugin/src/test/kotlin/FindingNodesInBindingTest.kt
@@ -15,7 +15,6 @@
  */
 
 import io.ktor.http.Url
-import kotlinx.coroutines.delay
 import org.jetbrains.mps.openapi.language.SConcept
 import org.jetbrains.mps.openapi.model.SNode
 import org.modelix.model.api.PNodeReference
@@ -80,9 +79,9 @@ class FindingNodesInBindingTest : SyncPluginTestBase("projectWithOneEmptyModel")
     }
 
     fun `test can find cloud nodes and MPS nodes when connection by legacy API`() = runTestWithSyncService { syncService ->
-        // Using ModelCloudImportUtils.copyAndSyncInModelixAsIndependentModule
-        // instead of IBranchConnection.bindModule is the legacy API
-        // The old API must be supported as it is used in actions like CopyAndSyncPhysicalModuleOnCloud_Action
+        // Using the legacy API ModelCloudImportUtils.copyAndSyncInModelixAsIndependentModule
+        // instead of IBranchConnection.bindModule.
+        // The legacy API must be supported as it is used in actions like CopyAndSyncPhysicalModuleOnCloud_Action
 
         // Arrange: Create data
         val classConcept = resolveMPSConcept("jetbrains.mps.baseLanguage.ClassConcept")
@@ -131,15 +130,4 @@ class FindingNodesInBindingTest : SyncPluginTestBase("projectWithOneEmptyModel")
         assertEquals(newRootNode, mpsNodeFromFirstSecondCloudNode)
         assertEquals(mpsNodeFromFirstCloudNode, mpsNodeFromFirstSecondCloudNode)
     }
-
-    private suspend fun delayUntil(condition: () -> Boolean) {
-        var remainingDelays = 30
-        while (!condition()) {
-            delay(1000)
-            remainingDelays--
-            if (remainingDelays == 0) {
-                throw IllegalStateException("Waited too long.")
-            }
-        }
-    }
 }