Skip to content

Commit bdeb7f0

Browse files
authored
Merge pull request #596 from modelix/fix/add-new-child-target-concept
MODELIX-814 Newly created child nodes aren't properly persisted in MPS
2 parents 65720e7 + 9866c24 commit bdeb7f0

File tree

5 files changed

+57
-33
lines changed

5 files changed

+57
-33
lines changed

bulk-model-sync-gradle-test/src/test/kotlin/org/modelix/model/sync/bulk/gradle/test/ChangeApplier.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.modelix.model.sync.bulk.gradle.test
1818

19+
import GraphLang.C_Node
1920
import GraphLang.L_GraphLang
2021
import GraphLang.N_Edge
2122
import GraphLang.N_Graph
@@ -27,6 +28,7 @@ import jetbrains.mps.lang.core.L_jetbrains_mps_lang_core
2728
import kotlinx.coroutines.runBlocking
2829
import org.modelix.metamodel.TypedLanguagesRegistry
2930
import org.modelix.metamodel.typed
31+
import org.modelix.metamodel.untyped
3032
import org.modelix.model.ModelFacade
3133
import org.modelix.model.api.BuiltinLanguages
3234
import org.modelix.model.api.ConceptReference
@@ -63,6 +65,11 @@ class ChangeApplier {
6365
graphNodes[1].name = "Y"
6466
graphNodes[2].name = "Z"
6567

68+
val graph = graphNodes[0].untyped().parent?.typed<N_Graph>() ?: error("parent not found")
69+
graph.nodes.addNew(3, C_Node).apply {
70+
name = "NewNode"
71+
}
72+
6673
val edges = rootNode
6774
.getDescendants(false)
6875
.filter { it.getConceptReference() == ConceptReference(_C_UntypedImpl_Edge.getUID()) }

bulk-model-sync-gradle-test/src/test/kotlin/org/modelix/model/sync/bulk/gradle/test/PullTest.kt

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import org.junit.jupiter.api.TestInstance
2121
import org.xmlunit.builder.Input
2222
import org.xmlunit.xpath.JAXPXPathEngine
2323
import java.io.File
24+
import kotlin.test.assertContains
2425
import kotlin.test.assertContentEquals
2526
import kotlin.test.assertEquals
2627

@@ -46,11 +47,23 @@ class PullTest {
4647
.selectNodes("model/node/node[@concept='1DmExO']/property", solution1Xml)
4748

4849
val actual = properties.map { it.attributes.getNamedItem("value").nodeValue }
49-
val expected = listOf("X", "Y", "Z", "D", "E")
50+
val expected = listOf("X", "Y", "Z", "NewNode", "D", "E")
5051

5152
assertContentEquals(expected, actual)
5253
}
5354

55+
@Test
56+
fun `added child was synced to local`() {
57+
val xpath = JAXPXPathEngine()
58+
val nodes = xpath.selectNodes("model/node/node[@concept='1DmExO']", solution1Xml)
59+
60+
val nodeNames = nodes.flatMap { xpath.selectNodes("property", it) }
61+
.map { it.attributes.getNamedItem("value").nodeValue }
62+
63+
assertEquals(6, nodes.count())
64+
assertContains(nodeNames, "NewNode")
65+
}
66+
5467
@Test
5568
fun `references were synced to local`() {
5669
val references = JAXPXPathEngine()

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,10 @@ class ModelImporter(
185185
.forEach { (newChild, expected) ->
186186
val expectedId = expected.originalId()
187187
checkNotNull(expectedId) { "Specified node '$expected' has no ID." }
188-
newChild.setPropertyValue(NodeData.idPropertyKey, expectedId)
189-
originalIdToExisting[expectedId] = newChild.reference
188+
if (newChild.originalId() == null) {
189+
newChild.setPropertyValue(NodeData.ID_PROPERTY_KEY, expectedId)
190+
}
191+
originalIdToExisting[newChild.originalId() ?: expectedId] = newChild.reference
190192
syncNode(newChild, expected, progressReporter)
191193
}
192194
continue
@@ -201,6 +203,8 @@ class ModelImporter(
201203

202204
val isOrdered = existingParent.isChildRoleOrdered(role)
203205

206+
val newlyCreatedIds = mutableSetOf<String>()
207+
204208
expectedNodes.forEachIndexed { indexInImport, expected ->
205209
val existingChildren = existingParent.getChildren(role).toList()
206210
val expectedId = checkNotNull(expected.originalId()) { "Specified node '$expected' has no id" }
@@ -224,7 +228,10 @@ class ModelImporter(
224228
val existingNodeReference = originalIdToExisting[expectedId]
225229
if (existingNodeReference == null) {
226230
val newChild = existingParent.addNewChild(role, newIndex, expectedConcept)
227-
newChild.setPropertyValue(NodeData.idPropertyKey, expectedId)
231+
if (newChild.originalId() == null) {
232+
newChild.setPropertyValue(NodeData.idPropertyKey, expectedId)
233+
}
234+
newChild.originalId()?.let { newlyCreatedIds.add(it) }
228235
originalIdToExisting[expectedId] = newChild.reference
229236
newChild
230237
} else {
@@ -253,7 +260,10 @@ class ModelImporter(
253260
val expectedNodesIds = expectedNodes.map(NodeData::originalId).toSet()
254261
// Do not use existingNodes, but call node.getChildren(role) because
255262
// the recursive synchronization in the meantime already removed some nodes from node.getChildren(role).
256-
nodesToRemove += existingParent.getChildren(role).filterNot { existingNode -> expectedNodesIds.contains(existingNode.originalId()) }
263+
nodesToRemove += existingParent.getChildren(role).filterNot { existingNode ->
264+
val id = existingNode.originalId()
265+
expectedNodesIds.contains(id) || newlyCreatedIds.contains(id)
266+
}
257267
}
258268
}
259269

bulk-model-sync-mps/src/main/kotlin/org/modelix/mps/model/sync/bulk/MPSBulkSynchronizer.kt

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -95,45 +95,42 @@ object MPSBulkSynchronizer {
9595

9696
println("Found ${jsonFiles.size} modules to be imported")
9797
val access = repository.modelAccess
98-
access.runWriteInEDT {
98+
access.executeCommandInEDT {
9999
val allModules = repository.modules
100100
val includedModules: Iterable<SModule> = allModules.filter {
101101
isModuleIncluded(it.moduleName!!, includedModuleNames, includedModulePrefixes)
102102
}
103103
val numIncludedModules = includedModules.count()
104-
access.executeCommand {
105-
val repoAsNode = MPSRepositoryAsNode(repository)
104+
val repoAsNode = MPSRepositoryAsNode(repository)
105+
println("Importing modules...")
106+
try {
106107
println("Importing modules...")
107-
try {
108-
println("Importing modules...")
109-
// `modulesToImport` lazily produces modules to import
110-
// so that loaded model data can be garbage collected.
111-
val modulesToImport = includedModules.asSequence().flatMapIndexed { index, module ->
112-
println("Importing module ${index + 1} of $numIncludedModules: '${module.moduleName}'")
113-
val fileName = inputPath + File.separator + module.moduleName + ".json"
114-
val moduleFile = File(fileName)
115-
if (moduleFile.exists()) {
116-
val expectedData: ModelData = moduleFile.inputStream().use(Json::decodeFromStream)
117-
sequenceOf(ExistingAndExpectedNode(MPSModuleAsNode(module), expectedData))
118-
} else {
119-
println("Skip importing ${module.moduleName}} because $fileName does not exist.")
120-
sequenceOf()
121-
}
108+
// `modulesToImport` lazily produces modules to import
109+
// so that loaded model data can be garbage collected.
110+
val modulesToImport = includedModules.asSequence().flatMapIndexed { index, module ->
111+
println("Importing module ${index + 1} of $numIncludedModules: '${module.moduleName}'")
112+
val fileName = inputPath + File.separator + module.moduleName + ".json"
113+
val moduleFile = File(fileName)
114+
if (moduleFile.exists()) {
115+
val expectedData: ModelData = moduleFile.inputStream().use(Json::decodeFromStream)
116+
sequenceOf(ExistingAndExpectedNode(MPSModuleAsNode(module), expectedData))
117+
} else {
118+
println("Skip importing ${module.moduleName}} because $fileName does not exist.")
119+
sequenceOf()
122120
}
123-
ModelImporter(repoAsNode, continueOnError).importIntoNodes(modulesToImport)
124-
println("Import finished.")
125-
} catch (ex: Exception) {
126-
// Exceptions are only visible in the MPS log file by default
127-
ex.printStackTrace()
128121
}
129-
122+
ModelImporter(repoAsNode, continueOnError).importIntoNodes(modulesToImport)
130123
println("Import finished.")
124+
} catch (ex: Exception) {
125+
// Exceptions are only visible in the MPS log file by default
126+
ex.printStackTrace()
131127
}
128+
println("Import finished.")
132129
}
133130

134131
ApplicationManager.getApplication().invokeAndWait {
135132
println("Persisting changes...")
136-
repository.modelAccess.runWriteAction {
133+
access.executeCommandInEDT {
137134
enableWorkaroundForFilePerRootPersistence(repository)
138135
repository.saveAll()
139136
}

mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSLanguageRepository.kt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import jetbrains.mps.smodel.adapter.ids.SConceptId
2020
import jetbrains.mps.smodel.adapter.structure.MetaAdapterFactory
2121
import jetbrains.mps.smodel.language.ConceptRegistry
2222
import jetbrains.mps.smodel.language.LanguageRegistry
23-
import jetbrains.mps.smodel.runtime.illegal.IllegalConceptDescriptor
2423
import org.jetbrains.mps.openapi.language.SAbstractConcept
2524
import org.jetbrains.mps.openapi.module.SRepository
2625
import org.modelix.model.api.IConcept
@@ -39,8 +38,6 @@ data class MPSLanguageRepository(private val repository: SRepository) : ILanguag
3938

4039
val conceptDescriptor = ConceptRegistry.getInstance().getConceptDescriptor(conceptId)
4140

42-
if (conceptDescriptor is IllegalConceptDescriptor) return null
43-
4441
return MPSConcept(MetaAdapterFactory.getAbstractConcept(conceptDescriptor))
4542
}
4643

0 commit comments

Comments
 (0)