Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not completely synchronize read-only modules and models #126

Closed
wants to merge 1 commit into from

Conversation

benedekh
Copy link
Contributor

@benedekh benedekh commented Jul 1, 2024

The idea is to synchronize only those parts of the read-only Modules and Models that are used in MPS:

  • Modules and all their Models, because it makes creating Model Imports for read-only Models easier,
  • Models,
  • Module Dependencies for read-only Modules but not between the read-only Modules,
  • Model Imports for read-only Models but not between the read-only Models,
  • Nodes that are referred to in our MPS models. All such Nodes are added as rootNode to the read-only Models. Although it may make the uploaded read-only Models structurally invalid, but we do not read those Models via their original, generated language so it's fine.

I've developed a new language for the read-only Modules, Models, etc that should be safe to use as meta-model to read the aforementioned modelix nodes.

Such feature comes in handy, when we have a DevKit in a plugin that imports a lot of languages and Modules that we do not want to sync to the cloud, but we also do not want to lose the references for them (and the nodes used from them).

Bug (!!!): test the code if it works when we have a DevKit and a module that is synchronized to modelix and we add a new model import / node referenece / module dependency, etc. to a model/module/node that is readonly, but is not synchronized to modelix yet. Do we synchronize the references and the targets in this case?

@benedekh benedekh self-assigned this Jul 1, 2024
@benedekh benedekh force-pushed the feature/readonly-modules-and-models-2 branch 3 times, most recently from 52ce802 to 4055a73 Compare July 1, 2024 21:21
@benedekh benedekh force-pushed the feature/readonly-modules-and-models-2 branch 2 times, most recently from 1d14987 to b50b192 Compare July 1, 2024 22:39
@benedekh benedekh marked this pull request as ready for review July 1, 2024 22:57
@benedekh benedekh requested a review from odzhychko July 1, 2024 22:57
@benedekh benedekh changed the title feat(mps-sync-plugin-lib): do not synchronize read-only modules and models Do not completely synchronize read-only modules and models Jul 2, 2024
@odzhychko
Copy link
Contributor

odzhychko commented Jul 22, 2024

Nodes that are referred to in our MPS models. All such Nodes are added as rootNode to the read-only Models. Although it may make the uploaded read-only Models structurally invalid, but we do not read those Models via their original, generated language so it's fine.

I think we do not need to sync the referenced nodes at all.
The references to nodes that are not in synced modules can be made an `MPSNodeReference.
(Its serialized string starts with "mps:")

@benedekh
Copy link
Contributor Author

benedekh commented Jul 22, 2024

All such Nodes are added as rootNode to the read-only Models. Although it may make the uploaded read-only Models structurally invalid, but we do not read those Models via their original, generated language so it's fine.

TODO (1): As suggested by Oleks, instead of this we could use the MPSNodeReference and save only the INodeReference instead of the INode itself (in the MPS -> modelix sync direction).

TODO (2): remove the code that creates a ReadonlyModelNode and remove the ReadonlyModelNode as Concept.

TODO (3): When we want to read the target of this INodeReference (in the modelix -> MPS sync direction), then do the following in the ModelixTreeChangeVisitor.referenceChanged:

Instead of

val targetINode = iReferenceLink?.let { iNode.getReferenceTarget(it) }
val targetSNode = targetINode?.let { nodeMap.getNode(it.nodeIdAsLong()) }

we resolve the targetSNode as follows:

val targetSNode = iReferenceLink?.let {
	val targetRef = iNode.getReferenceTargetRef(it)
	if(targetRef == null) {
		null
	} else {
		val serializedRef = targetRef.serialize()
		val pNodeRef = PNodeReference.tryDeserialize(serializedRef)
		if (pNodeRef == null) {
			val area = MPSArea(mpsRepository)
			val mpsNode = area.resolveNode(targetRef) as MPSNode
			mpsNode.node
		} else {
			val targetNode = iNode.getReferenceTarget(it) ?: throw IllegalArgumentException("PNodeReference exists, but PNode cannot be resolved: '$serializedRef'")
			nodeMap.getNode(targetNode.nodeIdAsLong())
		}
	}
}

TODO (4): refactor the whole PR and do the same principle for the Model, Module, Language imports / dependencies as well. The benefit will be that we do not store accidentally outdated (meta)data (e.g. Module version, Language version) on model server, but only the reference to these elements.

@benedekh
Copy link
Contributor Author

As suggested in the previous comment, #154 will replace this PR.

@benedekh benedekh closed this Sep 27, 2024
@benedekh benedekh deleted the feature/readonly-modules-and-models-2 branch October 29, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants