From e75b0a88c01c259f3fc4283e21758370e604ed7e Mon Sep 17 00:00:00 2001 From: Sascha Lisson Date: Fri, 14 Jun 2024 14:34:19 +0200 Subject: [PATCH] fix(mps-model-adapters): Module.allChildren didn't return all children It only returned models, but not nodes in other roles. --- .../modelix/model/sync/bulk/ModelImporter.kt | 9 +++- ...lChildrenActuallyReturnsAllChildrenTest.kt | 51 +++++++++++++++++++ .../DescriptorModelIsFilteredTest.kt | 35 +++++++++++++ .../model/mpsadapters/MPSModelAsNode.kt | 37 ++++++-------- .../model/mpsadapters/MPSModelImportAsNode.kt | 2 +- .../model/mpsadapters/MPSModuleAsNode.kt | 22 ++++---- .../model/mpsadapters/MPSRepositoryAsNode.kt | 28 +++++----- 7 files changed, 137 insertions(+), 47 deletions(-) create mode 100644 mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/AllChildrenActuallyReturnsAllChildrenTest.kt create mode 100644 mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/DescriptorModelIsFilteredTest.kt diff --git a/bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelImporter.kt b/bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelImporter.kt index 13bfa77e65..9f353b79b8 100644 --- a/bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelImporter.kt +++ b/bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelImporter.kt @@ -32,6 +32,8 @@ import org.modelix.model.data.ModelData import org.modelix.model.data.NodeData import kotlin.jvm.JvmName +private val LOG = mu.KotlinLogging.logger { } + /** * A ModelImporter updates an existing [INode] and its subtree based on a [ModelData] specification. * @@ -136,7 +138,12 @@ class ModelImporter( nodesToRemove.forEach { doAndPotentiallyContinueOnErrors { if (it.isValid) { // if it's invalid then it's already removed - it.remove() + try { + it.remove() + } catch (ex: UnsupportedOperationException) { + // it might be read-only + LOG.warn(ex) { "Cannot delete node $it" } + } } } } diff --git a/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/AllChildrenActuallyReturnsAllChildrenTest.kt b/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/AllChildrenActuallyReturnsAllChildrenTest.kt new file mode 100644 index 0000000000..448e075866 --- /dev/null +++ b/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/AllChildrenActuallyReturnsAllChildrenTest.kt @@ -0,0 +1,51 @@ +package org.modelix.model.mpsadapters + +import org.modelix.model.api.INode + +/* + * Copyright (c) 2023. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class AllChildrenActuallyReturnsAllChildrenTest : MpsAdaptersTestBase("SimpleProject") { + + fun `test repository adapter consistency`() { + readAction { + checkAdapterConsistence(MPSRepositoryAsNode(mpsProject.repository)) + } + } + + fun `test module adapter consistency`() { + readAction { + for (module in mpsProject.repository.modules.map { MPSModuleAsNode(it) }) { + checkAdapterConsistence(module) + } + } + } + + fun `test model adapter consistency`() { + readAction { + for (model in mpsProject.repository.modules.flatMap { it.models }.map { MPSModelAsNode(it) }) { + checkAdapterConsistence(model) + } + } + } + + private fun checkAdapterConsistence(adapter: INode) { + val concept = checkNotNull(adapter.concept) + val expected = concept.getAllChildLinks().flatMap { adapter.getChildren(it) }.toSet() + val actual = adapter.allChildren.toSet() + assertEquals(expected, actual) + } +} diff --git a/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/DescriptorModelIsFilteredTest.kt b/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/DescriptorModelIsFilteredTest.kt new file mode 100644 index 0000000000..774d12f656 --- /dev/null +++ b/mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/DescriptorModelIsFilteredTest.kt @@ -0,0 +1,35 @@ +package org.modelix.model.mpsadapters + +import junit.framework.TestCase +import org.modelix.model.api.BuiltinLanguages +import org.modelix.model.api.IChildLink +import org.modelix.model.api.INode + +/* + * Copyright (c) 2023. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class DescriptorModelIsFilteredTest : MpsAdaptersTestBase("SimpleProject") { + + fun `test descriptor model is filtered by adapter`() { + readAction { + val module = checkNotNull(mpsProject.projectModules.find { it.moduleName == "Solution1" }) + assertEquals(2, module.models.count()) + assertEquals(1, module.models.count { it.name.stereotype == "descriptor" }) + + assertEquals(1, MPSModuleAsNode(module).getChildren(BuiltinLanguages.MPSRepositoryConcepts.Module.models).count()) + } + } +} diff --git a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelAsNode.kt b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelAsNode.kt index e15ae4a527..06c1652cfe 100644 --- a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelAsNode.kt +++ b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelAsNode.kt @@ -29,6 +29,17 @@ import org.modelix.model.area.IArea data class MPSModelAsNode(val model: SModel) : IDefaultNodeAdapter { + private val childrenAccessors: Map Iterable> = mapOf( + BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes to { model.rootNodes.map { MPSNode(it) } }, + BuiltinLanguages.MPSRepositoryConcepts.Model.modelImports to { + ModelImports(model).importedModels.mapNotNull { modelRef -> + val target = modelRef.resolve(model.repository) + target?.let { MPSModelImportAsNode(it, model) } + } + }, + BuiltinLanguages.MPSRepositoryConcepts.Model.usedLanguages to { getImportedLanguagesAndDevKits() }, + ) + override fun getArea(): IArea { return MPSArea(model.repository) } @@ -41,14 +52,7 @@ data class MPSModelAsNode(val model: SModel) : IDefaultNodeAdapter { get() = MPSModuleAsNode(model.module) override val allChildren: Iterable - get() { - val childLinks = listOf( - BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes, - BuiltinLanguages.MPSRepositoryConcepts.Model.modelImports, - BuiltinLanguages.MPSRepositoryConcepts.Model.usedLanguages, - ) - return childLinks.flatMap { getChildren(it) } - } + get() = childrenAccessors.values.flatMap { it() } override fun removeChild(child: INode) { val link = child.getContainmentLink() ?: error("ContainmentLink not found for node $child") @@ -71,20 +75,11 @@ data class MPSModelAsNode(val model: SModel) : IDefaultNodeAdapter { } override fun getChildren(link: IChildLink): Iterable { - return if (link is NullChildLink) { - emptyList() - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes)) { - model.rootNodes.map { MPSNode(it) } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Model.modelImports)) { - ModelImports(model).importedModels.mapNotNull { modelRef -> - val target = modelRef.resolve(model.repository) - target?.let { MPSModelImportAsNode(it, model) } - } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Model.usedLanguages)) { - getImportedLanguagesAndDevKits() - } else { - emptyList() + if (link is NullChildLink) return emptyList() + for (childrenAccessor in childrenAccessors) { + if (link.conformsTo(childrenAccessor.key)) return childrenAccessor.value() } + return emptyList() } private fun getImportedLanguagesAndDevKits(): List { diff --git a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelImportAsNode.kt b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelImportAsNode.kt index a6ad62266e..0a0214e35f 100644 --- a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelImportAsNode.kt +++ b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModelImportAsNode.kt @@ -26,7 +26,7 @@ import org.modelix.model.api.IProperty import org.modelix.model.api.IReferenceLink import org.modelix.model.area.IArea -class MPSModelImportAsNode(val importedModel: SModel, val importingModel: SModel) : IDefaultNodeAdapter { +data class MPSModelImportAsNode(val importedModel: SModel, val importingModel: SModel) : IDefaultNodeAdapter { override fun getArea(): IArea = MPSArea(importingModel.repository) diff --git a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModuleAsNode.kt b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModuleAsNode.kt index d24e09cdea..51a4fdd046 100644 --- a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModuleAsNode.kt +++ b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSModuleAsNode.kt @@ -37,6 +37,13 @@ data class MPSModuleAsNode(val module: SModule) : IDefaultNodeAdapter { private val logger = mu.KotlinLogging.logger { } } + private val childrenAccessors: Map Iterable> = mapOf( + BuiltinLanguages.MPSRepositoryConcepts.Module.models to { module.models.map { MPSModelAsNode(it) } }, + BuiltinLanguages.MPSRepositoryConcepts.Module.facets to { module.facets.filterIsInstance().map { MPSJavaModuleFacetAsNode(it) } }, + BuiltinLanguages.MPSRepositoryConcepts.Module.dependencies to { getDependencies() }, + BuiltinLanguages.MPSRepositoryConcepts.Module.languageDependencies to { getLanguageDependencies() }, + ) + override fun getArea(): IArea { return MPSArea(module.repository ?: MPSModuleRepository.getInstance()) } @@ -49,24 +56,17 @@ data class MPSModuleAsNode(val module: SModule) : IDefaultNodeAdapter { get() = module.repository?.let { MPSRepositoryAsNode(it) } override val allChildren: Iterable - get() = module.models.map { MPSModelAsNode(it) } + get() = childrenAccessors.values.flatMap { it() } override fun getContainmentLink(): IChildLink { return BuiltinLanguages.MPSRepositoryConcepts.Repository.modules } override fun getChildren(link: IChildLink): Iterable { - return if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Module.models)) { - module.models.map { MPSModelAsNode(it) } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Module.facets)) { - module.facets.filterIsInstance().map { MPSJavaModuleFacetAsNode(it) } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Module.dependencies)) { - getDependencies() - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Module.languageDependencies)) { - getLanguageDependencies() - } else { - emptyList() + for (childrenAccessor in childrenAccessors) { + if (link.conformsTo(childrenAccessor.key)) return childrenAccessor.value() } + return emptyList() } private fun getDependencies(): Iterable { diff --git a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSRepositoryAsNode.kt b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSRepositoryAsNode.kt index 1803282970..d8a8df29ba 100644 --- a/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSRepositoryAsNode.kt +++ b/mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSRepositoryAsNode.kt @@ -29,6 +29,16 @@ import org.modelix.model.area.IArea data class MPSRepositoryAsNode(val repository: SRepository) : IDefaultNodeAdapter { + private val childrenAccessors: Map Iterable> = mapOf( + BuiltinLanguages.MPSRepositoryConcepts.Repository.modules to { repository.modules.filter { !it.isTempModule() }.map { MPSModuleAsNode(it) } }, + BuiltinLanguages.MPSRepositoryConcepts.Repository.projects to { + ProjectManager.getInstance().openedProjects + .filterIsInstance() + .map { MPSProjectAsNode(it) } + }, + BuiltinLanguages.MPSRepositoryConcepts.Repository.tempModules to { repository.modules.filter { it.isTempModule() }.map { MPSModuleAsNode(it) } }, + ) + override fun getArea(): IArea { return MPSArea(repository) } @@ -41,26 +51,18 @@ data class MPSRepositoryAsNode(val repository: SRepository) : IDefaultNodeAdapte get() = null override val allChildren: Iterable - get() = repository.modules.map { MPSModuleAsNode(it) } + get() = childrenAccessors.values.flatMap { it() } override fun getContainmentLink(): IChildLink? { return null } override fun getChildren(link: IChildLink): Iterable { - return if (link is NullChildLink) { - return emptyList() - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Repository.modules)) { - repository.modules.filter { !it.isTempModule() }.map { MPSModuleAsNode(it) } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Repository.projects)) { - ProjectManager.getInstance().openedProjects - .filterIsInstance() - .map { MPSProjectAsNode(it) } - } else if (link.conformsTo(BuiltinLanguages.MPSRepositoryConcepts.Repository.tempModules)) { - repository.modules.filter { it.isTempModule() }.map { MPSModuleAsNode(it) } - } else { - emptyList() + if (link is NullChildLink) return emptyList() + for (childrenAccessor in childrenAccessors) { + if (link.conformsTo(childrenAccessor.key)) return childrenAccessor.value() } + return emptyList() } }