introduce a transaction reconciliation framework for model listeners

[dalbrecht1]

Motivation

Model Listeners sometimes are in a bit of a tricky situation. They can be the most ideal way to define certain kinds of automation, but operate on a quite low level of abstraction and interfere with other routines. In regards to the level of abstraction, I'd compare them to using keymaps in your editors instead of action maps to define user interactions. In regards to the interference, I'd like to point to Modelix, that I had a lot of trouble with at first when applying it to existing projects and workflows recently.

But to be more concrete let me present you with two examples:

1. If a node is moved within the child list of the same parent or even across different parents, then technically this is split into a child-removal and a separate child-addition event when it comes to what model listeners will see. If you now want to or have to perform certain actions on removal to keep your model consistent / error free, then a moved child will erroneously trigger reactions, leaving your model in an altered state after the node-relocation. In most use cases when it comes to child-removal, the followup automation might be to remove additional nodes (like those referencing the now gone node). Now, with a move, these "cleaned up nodes" from the child-removed listener would in the end still be valid and desirable, given that the "referred to" node also still exists, just in a different location. For that, the model listener framework - no matter the power it already has - lacks the options needed to model real developer intent on a higher level of abstraction. Aka, it will not be able to consider the "net result" of a change.
2. Modelix is a powerful and already quite refined way to collaborate on the same project with multiple RCPs (or an RCP and web clients). And while Modelix in its current state even differentiates local edits from remote changes (in that, by triggering an undo you will only revert your own changes, not the ones from colleagues) and can suppress certain kinds of MPS automation in the sync process (e.g. constructors and node factories won't be triggered when the sync creates new nodes on your end), it cannot fundamentally block all kinds of automation from reacting to sync events. Huge thanks to @slisson for his help in understanding and dealing with Modelix intricacies when applying Modelix to our project. That taught me a lot I was able to utilize for this PR.

So, while in both/all cases you should first do your best in optimizing/refactoring model listeners to (a) not be necessary at all, (b) not alter the model in unnecessary ways, and/or (c) be idempotent/re-runnable at least, there are these cases where even the ideal model listener will struggle. For these cases, this PR introduces a utility model, which wants to give more power to the developer in expressing intent in the current model listener framework, by means of simply wrapping your previous listener body in a single, minimalistic "debouncing"-command. Ideally, very little else has to change in the actual code.

Proposal

Most if not all user interactions are encapsuled in a command or an undo-transparent action (I'll use "command" as the shorthand going forward, but remember that this means these two types of transactions). The same is true for any operation Modelix takes while fetching and integrating external changes into your local project. Hence, the MPS platform already "knows", that there are more atomic changes to come before the whole transaction finalizes. Therefore, within the model listener you are dealing with an intermediate state, but the ultimate state after the whole transaction can look significantly different. Hence, model listeners only see a snapshot at the exact times certain operations are performed, but do not consider a change in the big picture of the whole command. And while you certainly could just make use of "invokeLater"-calls or spin up your own boilerplate solution to apply in all model listeners, the strength of this proposal is a complete "action reconciliation runtime" that comes at almost no cost to you as the developer or the runtime (with no proof for that claim so far).

Whenever a model listener hook is executed, and the hook is executed in the context of a transaction that can have "user intent", you are now able to not run your code right away, but have it scheduled to only run at the end of the current transaction, when this delayed execution (the debouncing principle) will allow your code to consider the entirety of the operations being included in the command.

Usage Example

Imagine the context of a sort of truth table like use case. But one where there isn't just "true" and "false", but where the user can specify any terminals they like. But based on these terminals the user also should be able to model a binary operator in the form terminal op terminal => terminal. Here, if the table editor is done right, you do not have to take any actions when a new terminal is added. However, you might have to remove any "truth table entries" that feature a specific terminal, if the user wants to delete this particular terminal.

Hence, you might have a model listener like:

child removed in role terminals (instance, child)->void {
	findTruthTable().tableEntries.removeWhere({~it => it.lhs :eq: child || it.rhs :eq: child || it.result :eq: child; });
}

But if a terminal is just manually reordered, then the table will still drop these entries regardless. With this new framework, you can instead do the following:

child added in role terminals (instance, child)->void {
	TransactionReconciler.getInstance(instance).cancelByOrigin(child, "truthTable");
}
child removed in role terminals (instance, child)->void {
	final SNodeId childId = child/.getNodeId();
	TransactionReconciler.getInstance(instance).schedule(child, "truthTable", {node<> node =>
		findTruthTable().tableEntries.removeWhere({~it => 
			it/.getReference(link/TruthTableEntry : lhs).getTargetNodeId() :eq: childId ||
			it/.getReference(link/TruthTableEntry : rhs).getTargetNodeId() :eq: childId ||
			it/.getReference(link/TruthTableEntry : result).getTargetNodeId() :eq: childId; });
	});
}

As you see, you receive one benefit at the cost of putting yourself in the timeline where the node is already gone for good, hence having to shift to checking the underlying node-ids. Which you could circumvent by pre-querying the entries-to-be-removed prior to scheduling the removal and within that callback to simply detach them all. I'll show an example of that in the following code snippet.

Or, using another feature of this framework, you can make use of a "strategy" to already express intent of "only run on actual removal", like that:

// note, that you no longer need a 'child added' listener here to cancel
child removed in role terminals (instance, child)->void {
	final list<node<ThruthTableEntry>> toBeRemoved = findTruthTable().tableEntries.where({~it => it.lhs :eq: child || it.rhs :eq: child || it.result :eq: child; }).toList();
	TransactionReconciler.getInstance(instance).schedule(child, "truthTable", IReconciliationStrategy.IF_ABSENT, {node<> node =>
		toBeRemoved.forEach({~it => it.detach; });
	});
}

And the final gap between the original listener and the last one could even be bridged by defining specific callback types that capture even more intent (the one used here is already included in the framework, but they are just implementations of a given (functional) interface). Then it really looks very much the same as before.

child removed in role terminals (instance, child)->void {
	TransactionReconciler.getInstance(instance).schedule(child, "truthTable", IReconciliationStrategy.IF_ABSENT, new ReconciliationAction.DetachNodesAction(
		findTruthTable().tableEntries.where({~it => it.lhs :eq: child || it.rhs :eq: child || it.result :eq: child; }).toArray
	));
}

If at any point this new delaying engine isn't able to retrieve a fitting instance or finds itself outside a command - aka in a situation where there is no broader "user intent" to consider - the schedule method will simply immediately execute the provided hook, should the strategy call for it. And if you want to perform specific actions on move- and remove-only, you are free to include both definitions in the same model listener. Then, either on command finalization, or right away, only the "correct" one is executed. This ensures that the utility is safe to use in unit tests or headless scripts as well where a Command context might not be present, maintaining the 'no callback lost' guarantee.

Further Work

This PR for now only proposes a "code/runtime only" solution which expects the developer to call the right classes and methods to let their callbacks be managed by my new system. Functionality-wise this might already be as refined as it has to be, though I'm open for any comments and discussions. But what this proposal still lacks in my eyes is a nice syntax that highlights the intent better. See, I spent some time optimizing this such that the intent of the written listener code is better captured at runtime, but at the cost of "silly static calls" that are meaningless to the uninitiated.

And while it might not be much work to elevate the principles to an intent-focused syntax/DSL, I just lacked the time right now and first wanted to let you give feedback on the principle behind my concept. But in the end, I'd wish for a syntax that is closer to the "command / read action / write action with repository" statements or the "with _ models in scope do" statement. Which should be a somewhat trivial translation task, I assume.

One example, where such a DSL could still improve on the current state, is to create meaningful aliases for the strategies that adapt to the type of listener. While technically the "IF_ABSENT" strategy perfectly captures the state of the node, within a "child-removed" listener and to better communicate intent, the same states could be made available as "node moved" vs "node removed" strategies within the DSL.

[dalbrecht1]

Also, I presume that you'd like tests for this piece of functionality, but so far I was only able to "play-test" this interactively. I'd appreciate some help / input on creating a fully-automated test that incorporates concept declarations, model listeners, transactions, concurrency, all within the framework of an MPS test case. I can certainly come up with ideas on my own, but this is bit too deep into "dark arts" right now that I'd feel comfortable with such tests or be able to write them quickly.

[alexanderpann]

They can be the most ideal way to define certain kinds of automation, but operate on a quite low level of abstraction and interfere with other routines

IMO, model listeners are misused outside the MPS codebase most of the time (the async update mechanism of feature models of IETS3 is a good example for me). They often lead to unpredictable changes and often deal with MPS models on a level that is too low (people then sometimes even write MPS threading code to fix certain issues). Sometimes they are even used to interact with the open editor (e.g., to update the selection), which is also not a good idea because they know nothing about editors. People try to solve issues with them that could be solved in other ways:

• Certain modifications to the model can be made explicit (refactorings, MPS actions, intentions buttons...) so that you don't have to react to model changes.
• Property/reference changes can be detected in the property constraints and reference handlers or through transactional properties.
• Other changes can be detected sometimes through node factories, in substitute menus, or similar places.
• There is also the option to check the model with a checking rule, and as a last resort, apply an automatic quickfix.
• Tools that show additional information about the selected node should update themselves. The inspector is a good example of that.
• If you are dealing with changing IDs of nodes because they are cut/copy/pasted, you can use UUIDs instead (see MPS example folder).
• If you want to calculate a hashcode for your model to see if it changed, I would also argue that this could be done with a checking rule or if some code has to be executed as part of a facet.

[dalbrecht1]

They can be the most ideal way to define certain kinds of automation, but operate on a quite low level of abstraction and interfere with other routines

IMO, model listeners are misused outside the MPS codebase most of the time (the async update mechanism of feature models of IETS3 is a good example for me). They often lead to unpredictable changes and often deal with MPS models on a level that is too low (people then sometimes even write MPS threading code to fix certain issues). Sometimes they are even used to interact with the open editor (e.g., to update the selection), which is also not a good idea because they know nothing about editors. People try to solve issues with them that could be solved in other ways:

I'm totally with you on most if not all of these points, still - considering the user base of some tools - there are few alternatives in some scenarios. And yes, given Sascha Lißon already (rightly) blamed some strange side effects of syncing model data using Modelix on model listeners, I'm definitely looking into replacing them as often as I can. But just a few thoughts on some of your alternatives.

• Certain modifications to the model can be made explicit (refactorings, MPS actions, intentions buttons...) so that you don't have to react to model changes.

At least in regards to the intentions I have to say, we have a hard time teaching our domain-focused users how and when to use intentions. As a developer, they quickly became second nature for me. But our users just don't seem to on the same wave length with intentions at all most of the time.

• Property/reference changes can be detected in the property constraints and reference handlers or through transactional properties.

Definitely my go-to if the data and control work flow works out! But here again, Modelix first syncs all children and properties of (new) nodes, and then updates/sets all references in a second pass. That still leaves issues if you want to react to property changes. These can happen before references are configured which your followup-logic might want to check. Sure, I can place my debouncing-logic in the property-constraint then instead of the model listener, but that doesn't solve the issue that I might still need debouncing anyway.

• Other changes can be detected sometimes through node factories, in substitute menus, or similar places.

As I included, node factories aren't triggered by Modelix. That can be a good or a bad thing based on your exact context / use case. And tools like substitute menus or action maps or such only trigger when the user explicitly takes action in these places (and you don't forget to configure them in other places where the user can interact with the same instances). So, we offload the "automation" to the developer who has to be very meticulous when it comes to touching the language down the line. How quickly can that be forgotten in a PR review for example...

• There is also the option to check the model with a checking rule, and as a last resort, apply an automatic quickfix.

That leaves follow-up actions in a sort of limbo, where once you open the chunk, they apply, but in the meantime the model might report errors. Especially in the context of Modelix and web clients to your model, this is usually undesirable. The web UIs are happy to only render consistent data, where MPS is more tolerant towards broken stuff.

• Tools that show additional information about the selected node should update themselves. The inspector is a good example of that.

I guess that goes roughly in the direction of query-lists and "dynamic domain-link resolution"? Sure, but not ideal in a tool with already sub-par performance. But I guess that should then be the primary focus of any maintenance efforts...

• If you are dealing with changing IDs of nodes because they are cut/copy/pasted, you can use UUIDs instead (see MPS example folder).

• If you want to calculate a hashcode for your model to see if it changed, I would also argue that this could be done with a checking rule or if some code has to be executed as part of a facet.

That so far was both out of my focus, but definitely an interesting situation to think about.

Again, I agree to remove them when possible in favor of other ways, sometimes keeping the "automatic magic" aspect, sometimes converting them to "additional and explicit user actions". But that only helps so far, if your users have their own minds and assumptions of how something (is supposed to) work(s). And sometimes a sprinkle of magic is just what it takes to make a tool feel welcoming/lively, even if that has to be accomplished with model listeners.

[sergej-koscejev]

Without looking at the code yet, MPS provides a batch event listener (RepositoryChangeTracker) which you can use to react to all events that happened in a command. Are you aware of it? Would it help in your case or do you need to react to events during a command? See UsageModelTracker for a (trivial and only) usage example.

[dalbrecht1]

Without looking at the code yet, MPS provides a batch event listener (RepositoryChangeTracker) which you can use to react to all events that happened in a command. Are you aware of it? Would it help in your case or do you need to react to events during a command? See UsageModelTracker for a (trivial and only) usage example.

No, I wasn't aware of that type. And even looking at that type/usage for a while now doesn't really shed a lot of light on how to make this work. Am I correct that this would serve as a complete replacement of the model listeners and I would spin my own "reaction engine" in base language?

While the initial description of this PR was already complex, here are some further thoughts that guided this development:

• Any changes that I want to make in reaction to the change that triggered the model listener are done after the fact, when the command is "about to be finalized". Hence, when I have full context of the whole change set.
• The changes I make are still part of the command. The org.jetbrains.mps.openapi.repository.CommandListener for example in contrary to its Javadoc, only seems to inform me AFTER the command finalized.
• The changes are therefore undoable TOGETHER with the triggering changes. So, if property x changes and my listener were to also alter property y (either immediately, or even when using my debouncing), then this creates a single entry in the undo-chain and executing an undo will revert back both changes AT ONCE.
• The recorded changes are replayed chronologically (not a major requirement, but still one I wanted to ensure is present)
• I wanted to avoid that in all model listeners I have to have the same boiler-plate code to re-schedule my actions.
• And I explicitly wanted to come closer to "intent-encoding" and not just "here's a listener (of some sort) that will trigger under mysterious circumstances and then do some magical steps". (Again, not highest prio, but once I started this util, this was a guiding principle for the terminology and applied software patterns.)

If in the end it turns out that my tool is redundant then I'm definitely not mad/sad. It was a interesting exercise to experience transaction scopes, concurrency, undo-processes, AI-brainstorming/-conceptualization and the like. But (a) I hadn't found anything that served my use case, (b) I already found a few work flows that would violate any of the stated principles, (c) even now seeing this alternative doesn't really bring any ideas to mind how that is used / superior (other than, it exists and is already proven to work).

[sergej-koscejev]

If you want to pursue this further, I do want to see some tests or examples of this functionality, to understand it better and to keep it from breaking during migrations or other kind of maintenance.

Also, I would appreciate it if this PR was based on one of the actively maintained versions, i.e. from 2024.1 onwards.

[dalbrecht1]

If you want to pursue this further, I do want to see some tests or examples of this functionality, to understand it better and to keep it from breaking during migrations or other kind of maintenance.

Understandable. Though, I haven't had the time to delve into the realm of automated testing for this particular idea. But yes, in general I'd really want to follow up on my initial proposal and at least get some feedback here.

Also, I would appreciate it if this PR was based on one of the actively maintained versions, i.e. from 2024.1 onwards.

Given that we would want to integrate an enriched release version of the extensions into a project that (currently) is based on this exact MPS version, there is very little I can do to get this more up-to-date / more recent / in a more acceptable state.

In fact, to give some more context, I was not trying to utilize this framework as a way to promote more usage of model listeners! If you read my description carefully, you'll realize that I'm fully aware of the drawbacks in more ways than one. Instead we are in the situation that we already have some (small finite number of) model listeners which we don't see a way to replace reasonably with anything else. But which also break existing workflows in combination with Modelix sync processes that we and our users start to face more and more often. These NEED debouncing!

I wasn't trying to get you to maintain my new framework by offloading it to the extensions project. But the circumstances are the way that we will need this or something like this, and rather sooner than later. And putting my proposed solution for discussion here first and foremost was meant as a way to gather feedback. This idea apparently didn't work out as intended, as you are busy yourself... That's mostly on me for not providing examples/tests that would have simplified the review process by giving a good starting point.

[sergej-koscejev]

Ok I will wait for the tests or an example (a sandbox), then I might be able to give some feedback.

[sergej-koscejev]

Closing this for now due to no activity, please reopen or recreate when you have an example or tests.