(WF4) Rolling your own Undo Units in Workflow Designer .net 4.0

A long time go in a galaxy very, very close (so close it could be coincidental or indeed the same) I remember writing a couple things which touched upon Undo and Redo in workflow designer. Since someone invented hyperlinks, I should probably use this neat feature of the interwebs and point you at them.

“Undoable Layout for the Freeform Canvas Activity Designer” - This article contains exploration around the idea of building a Custom Flowchart Designer. In the article I found that it is hard to get undo and redo working just right – but it is possible. Mainly by using Undoable ViewState.

“Some Gory details of Workflow Designer Undo Redo” – as we will see, this one was not nearly gory and detailed enough! The reason being it failed to explore UndoUnit!

At some point I also very briefly mentioned that instead of calling UndoEngine directly, you can juts invoke the undo/redo designer commands from your rehosted app, which does pretty much the same thing, but going via WPF commanding pattern.

Anyhow. As usual in software, version 1.0 of this information isn’t fully featured.  And today I am out to some shortcomings of the previous articles.

Undo, Starting from the top – UndoEngine

The most general API that System.Activities.Presentation has for implementing Undo/Redo functionality (which I never yet explained) is called UndoEngine. It may be an accident, but it has a public constructor API, and it is not sealed, so it’s entirely possible for you to construct as many of these as you wish or even subclass it. (Why? Shrug. So far I have only thought of bad reasons to do this.) When you construct it, you would need to pass in the EditingContext it is associated with.

Normally there is no need for you to create an UndoEngine. The default UndoEngine instance of the workflow designer which you can use is this one:

(Thought – is it possible to replace the designer’s UndoEngine with one of your own? As far as I can see… no. The ServiceManager would complain about adding duplicate service.)

The most interesting 3 operations that UndoEngine presents us with are the following:

• AddUndoUnit(UndoUnit unit)
• bool Undo() – returns false if it fails. If it ever fails. Assuming it hasn’t already thrown.
• bool Redo() – returns false if it fails.

UndoEngine is in fact the workflow designer’s ‘top level’ undo stack, and these APIs control that stack, and are used in direct fashion to implement the Undo/Redo commands.

As well as these stack-modifying APIs, there are a couple getter methods, GetUndoActions() and GetRedoActions() which return IEnumerable<string> . The point of these is just to return descriptions of the actions on the undo stack or the redo stack, for displaying in a user interface or whatever.

There’s one last interesting operation. Actually it’s a property.

• IsUndoRedoInProgress - a boolean value indicating whether an undo or redo operation is in progress

Later we will see that this is a useful property.

Still up near the top: UndoUnit

UndoUnit is also public, abstract, and with a protected constructor. So we can subclass it too - and it turns out that we might even think of some good reasons to do this, great! Say we want to implement a custom UndoUnit, which we’ll add on the designer’s undo stack using UndoEngine.AddUndoUnit. What do we need to do? The minimum implementation required is that you support an Undo action and a Redo action.

I think the first big question which arises from this API is now how to use it, but “When do I need to create my own custom UndoUnit?” Let’s think about when it’s appropriate.

• It’s not necessary for ModelItem changes. Any model tree change causes UndoUnits to be generated and added to the tree automatically, via the internal implementation details of System.Activities.Presentation.
• It’s not necessary for ViewState changes. You can make your ViewState undoable just by using the ViewStateService.StoreViewStateWithUndo() API.
• It only makes sense to use this Undo/Redo infrastructure if you want undo/redo sequenced with the workflow designer undo operations.

That last one was a little hard to explain, the only way I can really explain it is by a negative example. Say you were tasked with integrating Workflow Designer with Visual Studio, and also with implementing the C# code editor for Visual Studio. Would it make sense for you to use the Workflow Designer’s Undo/Redo stack to track edits to your C# file? NO! This would have terrible consequences such as every time you want to undo work in your C# edit buffer, you also end up undoing all your edits in workflow designer simultaneously. Separate editing buffers need separate undo stack instances.

On the other hand, the answer can change if our requirements change. For instance, say we are building a rehosted app where we want to maintain a Rule Editor window whose state is linked to the workflow which is currently open in the workflow designer. Any edit to the workflow designer can cause our rule editor window to change. And any edit to the rule editor can cause the workflow to change, and it should be impossible to undo in one without undoing in the other. This scenario is the kind of scenario where we could consider using custom UndoUnits to implement a solution.

Until today I never tried out to see how workable this is, having multiple viewing or editing windows tied together via the workflow designer’s undo stack. But here is some code I wrote to test this out.

I decided that what I would create is a quick rehosted application with Undo/Redo buttons, a toolbox, and a special list window. The list window keeps track of the activities which you have added to your workflow, by listening to model changed events from the ModelService.

In order to ‘drive’ the list view, we need to bind it to some INotifyCollectionChanged collection. In order for our list window’s collection to be updated correctly by Undo/Redo, we also need to have UndoUnit which implements undo of adding something to the collection.

The prototype I came up with has a custom Collection class with Add () logic looking something like this:

The custom UndoUnit is the CollectionUndoInsert class. (Note - I can’t use any existing public UndoUnit class for this – there aren’t any.) And it has simple logic.

Everything’s going great, right? Oh. Wait. Uh oh. There’s a bug. Can you spot it?

Undo and Redo – never quite simple enough!

The bug, ah the glorious bug. I actually managed to overlook it for a little while. Consider this sequence of events.

1. Add an activity
2. Add another activity
3. Undo the second add

Performing just these actions everything appears to work*. But in fact things are happening on the undo/redo stack that shouldn’t be happening. The problem is that Undo() calls into our collection’s RemoveAt() method. And RemoveAt() will… add another undo unit. Which wipes out anything we have on the redo stack. D’oh! And puts an ‘Undo Remove’ on top of the Undo stack, so that doing another Undo will actually… re-add the activity?!? Which appears more like a Redo.

OK, not a big deal, fix it, fix it. I can just modify the internal owner.inner collection directly instead of going through the regular owner APIs. Hmm... no, wait, something still seems wrong…

[*If you ever need to test an Undo/Redo feature some day, you’re probably going to need to spend a lot of time on testing pluralities of undo operations, not single operations.]

This next issues, are still basically of my own making, and a little trickier, as the problem is really in the way our collection class got called – in response to ModelChanged events.

It turns out that ModelChangedEvents are 1) not raised simultaneously with updating the UndoEngine stack and 2) raised when we Undo and Redo.

We can see what is happening a little more clearly if we also add a listener to UndoEngine’s UndoRedoBufferChanged event. Which shows us some interesting behavior of how Undo works involving the ModelEditingScope.

1. Add an activity
2. This triggers ModelChanged event
3. We receive the event, update our list. This updates the UndoEngine stack. This triggers UndoRedoBufferChanged.
4. We receive UndoRedoBufferChanged, and print some debug trace info showing what is on the undo stack. We see our own UndoUnit is on the stack.
5. The ModelEditingScope’s  UndoUnit is added to the UndoEngine stack. This triggers UndoRedoBufferChanged.
6. We received UndoRedoBufferChanged, and examining the stack, we see that our change is now underneath the model edit UndoUnit (even though the model edit started before we started doing our operation).

Now let’s say that as operation number 7 we do this:

     7. Cut the flowchart from the workflow. (We don’t update the special list window).

This puts yet another model edit change on the undo stack.

If we want to undo the cut operation, what happens? (counting from 1 instead of 8…)

1. Click Undo. A model editing change is on top of the undo stack. It’s Undo() method is executed.
2. This triggers ModelChanged event, for adding the cut activity back.
3. We receive ModelChanged event, and since it appears that an activity has been added, we will add it into our collection (this is kinda weird, but just assume this is really what we are doing for now).
4. We update our collection, which will add an undo unit on top of the undo stack…?
   Uh oh. We haven’t even removed the first undo unit yet have we? At least we hadn’t received any event notifying us that UndoRedoBufferChanged.
5. We receive UndoRedoBufferChanged in response to us adding an undo unit… probably allowing us to wipe out the redo stack, in the middle of undo or redo.
   Oh dear.
6. We receive UndoRedoBufferChanged again in response to the original undo unit finishing being moved onto the redo stack.

Probably, what should have happened at point 4. is that UndoEngine.AddUndoUnit() should have thrown. It is in the middle of executing an undo unit, and it really doesn’t make sense to allow adding any new undo units at this point in time, and it is still only half-way through updating the undo/redo stacks to compensate, so it could just refuse us. And this could have actually helped me with debugging, if it had happened. But it didn’t. Oh well, if it’s a bug, at least it isn’t blocking us from fixing our implementation. (I think it’s a bug – but since someone, somewhere might now be depending on the non-throwing behavior, it’s not too likely this will be fixed.)

What we should do in practice is always check whether UndoEngine.IsUndoRedoInProgress before we make a call to UndoEngine.AddUndoUnit().

So let’s revert the CollectionUndoInsert class back to its original (above) implementation version, and then we can fix it inside an AddUndoUnit helper function:

The Missing Feature?

There’s one more thing which we I really wanted for my task above, which is the ability to group my undo units with the ModelItem undo units, and make them happen as a single action. Because what I have right now is that I have to do two undos or redos for for every one ‘do’ that I do through the Workflow Designer user interface.

One feature that might let me do this thing I want to do does appear to exist… but it is internal. (D’oh.) Internally the designer uses UndoEngine.CreateBookmark() for bunching multiple edits performed by a user inside a dialog into a single logical change. Which can then be undone or redone as a group. The overall effect of UndoEngine.CreateBookmark is to set up a scope where UndoUnits added are captured and added to a special list. Once the scope is exited, the units become grouped inside a single compound UndoUnit. But until then they can be undone or redone individually. Groovy.

There is also one other feature that we could look at who groups multiple changes into single undo actions. ModelEditingScope. This ModelEditingScope guy is a bit like a rebel who didn’t want to live in harmony with the UndoUnit system – instead he lives for Change objects , which are yet another abstract class encapsulating very much the same idea as UndoUnit… (Undo, Redo) just with a different object model. An UndoUnit knows how to undo and redo itself. A Change object, on the other hand, can either do itself, or point you at its evil twin, another Change object who knows how to do exactly the opposite.

This sounds a lot like pointless chaos. But it turns out there is one small, interesting and maybe just possibly useful difference between the two worlds. UndoUnits, grouped by a Bookmark (from createBookmark()) can and do apply their side effects up front, even though the Bookmark scope has not been completed yet. If the bookmark scope does not complete, UndoUnits can be rolled back by calling Undo(). Changes, on the other hand are all batched, and not applied until the ModelEditingScope is committed. At which point they all try to happen in sequence.

The point in time where you receive ModelChanged event notification from ModelService is when the Change is executed (at commit time), not created.

This interesting difference makes ModelChanged suitable for bunching up programmatic changes that you really want to appear simultaneous to the user, but unsuitable for bunching up a set of gradual changes, which the user should see happen, and then combining them into a single UndoUnit. (Those Bookmark things would probably work for the latter though.)

Why the question mark?

(Not in this section title, the Missing Feature(?) one…)

OK, I’ve noticed a Bookmark feature that might solve my problem… except for some obvious dependency: someone has to decide exactly when to begin and when to end the bookmark scope in order for the correct set of changes to be scoped together. This is actually a bit tricky. I think it is ideal that an undo unit on the stack corresponds to a user action. But it is even more important that executing an UndoUnit on the stack corresponds to a ‘well defined’, consistent state.

Negative example: my implementation above. What I really want is for the action (the user adding an activity), and also my reaction (adding an item to the list which tracks added activities) to be undone simultaneously, otherwise what could happen is that, for example, it is possible for the user to undo the reaction only, and then continue their editing from there. This leaves the list in an incorrect state with respect to current workflow, since it no longer tracks one of the activities they added.

Is it actually possible to figure out safe times to call CreateBookmark(), if the API were public?

The timing of when the ModelEdit change notification occurs relative to the ModelEdit undo action being added to the undo engine is important here.

There is also another worry: multiple model changes might happen in a single ModelEditingScope. In case there are actually multiple model changes grouped together by a single editing scope, we might want to monitor ModelTreeManager.EditingScopeCompleted instead of ModelService.ModelChanged.In this way we can try to listen for a notification that happens after all of the model changes. (Instead of trying to inject our own editing changes in the middle of some series of model changes that is supposed to be atomic.)

With sufficient event handlers, the sequence that I can observe is something like this:

1. User edit
2. ModelChanged
3. EditingScopeCompleted
4. UndoUnit for the editing scope is created. (By WorkflowDesigner’s EditingScopeCompleted handler. If Undo/Redo is not in progress.)

It is tempting to modify the sequence by inserting bookmark scopes to

1. User edit
2. ModelChanged
3. Create the undo bookmark scope
4. Do our changes, add our custom undo unit
5. EditingScopeCompleted
6. UndoUnit for the editing scope is created. (By WorkflowDesigner’s EditingScopeCompleted handler. If Undo/Redo is not in progress.)
7. Finish the undo bookmark scope…

But this feels dangerous for a couple reasons…

1. If we just respond to arbitrary ModelChanged events, then the operations could be undone in the wrong order.

Especially if there are multiple model changes.

e.g.:

Original ‘do’ order: model change #1, followed by our change, followed by model change #2.
Undo order: model change #2, model change #1 (they definitely occur together, since they are grouped by ModelEditingScope), and then followed by our change.

This could lead to problems because our undo operation gets a different view of the model tree than the one it expects based on the model tree when it was created. For a safer solution we would need to do our changes either at the beginning, or the end of the model editing scope.

2. What if everyone was creating and using bookmark scopes like this?

If everyone starts creating and using bookmark scopes willy-nilly in reaction to model changed events, the scopes might not be nested properly. Also custom actions may be affected by unexpected bookmark scopes added by 3rd parties. Ugh. This class of problem does also exist when it comes to ModelEditingScopes…

Example:

My nifty activity designer starts a bookmark scope. ISV Contoso’s activity designer starts their bookmark scope. I then try to complete my bookmark scope first… argh! The bookmark scopes are overlapping! What should happen? I’m not sure.

So it really only feels safe to use bookmark scopes where we have some level of ‘containment’ where we know that a whole lot of actions happen in a well-known timespan inside that scope.

For the case of regular user actions such as dragging an activity around the canvas, or editing a text box, there is no point of ‘containment’ where we can inject our code and safely assume that all interesting changes (model changes, view state changes, whatever) resulting from the user action will occur within that scope. (And if there are asynchronous edits? We are blanked.)

Given all these issues I can maybe feel a little more satisfied with not having access to CreateBookmark. It’s tempting to dream of a world in which all the user actions automatically have a undo bookmark scope, so that everything that happened as a result of a user action is always a single undo change. But it doesn’t sound very real - since new user actions can be added in an infinite number of ways by extensions…

So this is the state of Undo in WF4 designer. And yup, I still haven’t got a really nice solution to the problem of how to group custom UndoUnits… sigh. But maybe this’ll inspire you to think of one.