From d8b8e47a6a699fccaf4b5000eebfd0a4f38b898e Mon Sep 17 00:00:00 2001 From: Sascha Lisson Date: Wed, 25 Feb 2026 10:16:05 +0100 Subject: [PATCH] fix(model-api): cross model references weren't resolved --- .../modelix/model/api/AutoTransactionsNode.kt | 37 ++++++++--- .../org/modelix/model/api/CompositeModel.kt | 6 +- .../model/api/CrossModelReferencesTest.kt | 66 +++++++++++++++++++ 3 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 model-client/src/commonTest/kotlin/org/modelix/model/api/CrossModelReferencesTest.kt diff --git a/model-api/src/commonMain/kotlin/org/modelix/model/api/AutoTransactionsNode.kt b/model-api/src/commonMain/kotlin/org/modelix/model/api/AutoTransactionsNode.kt index 76b742aa7c..914c27e186 100644 --- a/model-api/src/commonMain/kotlin/org/modelix/model/api/AutoTransactionsNode.kt +++ b/model-api/src/commonMain/kotlin/org/modelix/model/api/AutoTransactionsNode.kt @@ -1,12 +1,24 @@ package org.modelix.model.api -class AutoTransactionsNode(val node: IWritableNode) : IWritableNode { - private fun IWritableNode.wrap() = AutoTransactionsNode(this) +class AutoTransactionsNode(val node: IWritableNode, val transactionsManager: ITransactionManager) : IWritableNode { + init { + require(node !is AutoTransactionsNode) { "Double wrapping prevented: $node" } + } + + private fun IWritableNode.wrap() = wrapNode(this) + private fun wrapNode(node: IWritableNode): AutoTransactionsNode { + return if (node is AutoTransactionsNode) { + require(node.transactionsManager == transactionsManager) + node + } else { + AutoTransactionsNode(node, transactionsManager) + } + } private fun List.wrap() = map { it.wrap() } private fun IWritableNode.unwrap() = if (this is AutoTransactionsNode) this.node else this - private fun read(body: () -> R): R = node.getModel().executeRead(body) - private fun write(body: () -> R): R = node.getModel().executeWrite(body) + private fun read(body: () -> R): R = transactionsManager.executeRead(body) + private fun write(body: () -> R): R = transactionsManager.executeWrite(body) override fun getModel(): IMutableModel { return AutoTransactionsModel(node.getModel()) @@ -24,6 +36,10 @@ class AutoTransactionsNode(val node: IWritableNode) : IWritableNode { return read { node.getLocalReferenceTarget(role)?.wrap() } } + override fun getReferenceTarget(role: IReferenceLinkReference): IWritableNode? { + return read { super.getReferenceTarget(role)?.wrap() } + } + override fun getAllReferenceTargets(): List> { return read { getAllReferenceTargets().map { it.first to it.second.wrap() } } } @@ -142,15 +158,20 @@ class AutoTransactionsNode(val node: IWritableNode) : IWritableNode { } } -fun IWritableNode.withAutoTransactions() = AutoTransactionsNode(this) +fun IWritableNode.withAutoTransactions() = if (this is AutoTransactionsNode) this else withAutoTransactions(getModel()) +fun IWritableNode.withAutoTransactions(transactionsManager: ITransactionManager) = AutoTransactionsNode(this, transactionsManager) class AutoTransactionsModel(val model: IMutableModel) : IMutableModel { - override fun getRootNode(): IWritableNode = AutoTransactionsNode(model.getRootNode()) + init { + require(model !is AutoTransactionsModel) { "Double wrapping prevented: $model" } + } + + override fun getRootNode(): IWritableNode = AutoTransactionsNode(model.getRootNode(), model) - override fun getRootNodes(): List = model.getRootNodes().map { AutoTransactionsNode(it) } + override fun getRootNodes(): List = model.getRootNodes().map { AutoTransactionsNode(it, model) } override fun tryResolveNode(ref: INodeReference): IWritableNode? { - return model.tryResolveNode(ref)?.let { AutoTransactionsNode(it) } + return model.tryResolveNode(ref)?.let { AutoTransactionsNode(it, model) } } override fun executeRead(body: () -> R): R = model.executeRead(body) diff --git a/model-api/src/commonMain/kotlin/org/modelix/model/api/CompositeModel.kt b/model-api/src/commonMain/kotlin/org/modelix/model/api/CompositeModel.kt index fe8c590d91..ca1a7988db 100644 --- a/model-api/src/commonMain/kotlin/org/modelix/model/api/CompositeModel.kt +++ b/model-api/src/commonMain/kotlin/org/modelix/model/api/CompositeModel.kt @@ -1,6 +1,8 @@ package org.modelix.model.api class CompositeModel(models: List) : IMutableModel { + constructor(vararg models: IModel) : this(models.toList()) + val models: List = models.flatMap { it as IMutableModel when (it) { @@ -22,11 +24,11 @@ class CompositeModel(models: List) : IMutableModel { } override fun executeRead(body: () -> R): R { - return executeRead(models, body) + return IModel.runWith(this) { executeRead(models, body) } } override fun executeWrite(body: () -> R): R { - return executeWrite(models, body) + return IModel.runWith(this) { executeWrite(models, body) } } private fun executeRead(remainingModels: List, body: () -> R): R { diff --git a/model-client/src/commonTest/kotlin/org/modelix/model/api/CrossModelReferencesTest.kt b/model-client/src/commonTest/kotlin/org/modelix/model/api/CrossModelReferencesTest.kt new file mode 100644 index 0000000000..ceb58e235e --- /dev/null +++ b/model-client/src/commonTest/kotlin/org/modelix/model/api/CrossModelReferencesTest.kt @@ -0,0 +1,66 @@ +package org.modelix.model.api + +import org.modelix.datastructures.model.IGenericModelTree +import org.modelix.model.mutable.asModel +import org.modelix.model.mutable.asMutableThreadSafe +import kotlin.test.Test +import kotlin.test.assertEquals + +class CrossModelReferencesTest { + + @Test + fun `cross model reference resolution`() { + val modelA = IGenericModelTree.builder().withNodeReferenceIds().build().asMutableThreadSafe().asModel() + val modelB = IGenericModelTree.builder().withNodeReferenceIds().build().asMutableThreadSafe().asModel() + + val nameRole = IPropertyReference.fromName("name") + val refRole = IReferenceLinkReference.fromName("crossModelRef") + modelA.executeWrite { + modelA.getRootNode().setPropertyValue(nameRole, "nodeA") + modelA.getRootNode().setReferenceTarget(refRole, modelB.getRootNode()) + } + modelB.executeWrite { + modelB.getRootNode().setPropertyValue(nameRole, "nodeB") + } + + val model = CompositeModel(modelA, modelB) + + model.executeRead { + assertEquals(2, model.getRootNodes().size) + assertEquals("nodeA", model.getRootNodes()[0].getPropertyValue(nameRole)) + assertEquals("nodeB", model.getRootNodes()[1].getPropertyValue(nameRole)) + assertEquals(model.getRootNodes()[1], model.getRootNodes()[0].getReferenceTarget(refRole)) + assertEquals(model.getRootNodes()[1].getNodeReference(), model.getRootNodes()[0].getReferenceTarget(refRole)?.getNodeReference()) + assertEquals("nodeB", model.getRootNodes()[0].getReferenceTarget(refRole)?.getPropertyValue(nameRole)) + } + + model.executeWrite { + assertEquals(model.getRootNodes()[1], model.getRootNodes()[0].getReferenceTarget(refRole)) + } + } + + @Test + fun `cross model reference resolution with auto transactions`() { + val modelA = IGenericModelTree.builder().withNodeReferenceIds().build().asMutableThreadSafe().asModel() + val modelB = IGenericModelTree.builder().withNodeReferenceIds().build().asMutableThreadSafe().asModel() + + val nameRole = IPropertyReference.fromName("name") + val refRole = IReferenceLinkReference.fromName("crossModelRef") + modelA.executeWrite { + modelA.getRootNode().setPropertyValue(nameRole, "nodeA") + modelA.getRootNode().setReferenceTarget(refRole, modelB.getRootNode()) + } + modelB.executeWrite { + modelB.getRootNode().setPropertyValue(nameRole, "nodeB") + } + + val model = CompositeModel(modelA, modelB).withAutoTransactions() + + assertEquals(2, model.getRootNodes().size) + assertEquals("nodeA", model.getRootNodes()[0].getPropertyValue(nameRole)) + assertEquals("nodeB", model.getRootNodes()[1].getPropertyValue(nameRole)) + assertEquals(model.getRootNodes()[1], model.getRootNodes()[0].getReferenceTarget(refRole)) + assertEquals(model.getRootNodes()[1].getNodeReference(), model.getRootNodes()[0].getReferenceTarget(refRole)?.getNodeReference()) + assertEquals("nodeB", model.getRootNodes()[0].getReferenceTarget(refRole)?.getPropertyValue(nameRole)) + } +}