Unintuitive application of visitors by ElkUtil.applyVisitors(...)

[le-cds]

While fixing #562, I stumbled upon an interesting (possible) problem with ElkUtil.applyVisitors(ElkNode, IGraphElementVisitor...). That particular issue only ever occurred when layout was done through the DiagramLayoutEngine. The engine applies the following three visitors:

1. org.eclipse.elk.core.LayoutConfigurator
2. org.eclipse.elk.core.data.LayoutAlgorithmResolver
3. org.eclipse.elk.core.validation.LayoutOptionValidator

It does so by iterating over all graph elements. For each element, it applies all visitors before moving on to the next.

Issue #562 was about a problem with inside self loops. The layout algorithm resolver needs to resolve a layout algorithm for all nodes that meet at least one of two criteria:

1. It has children.
2. It has inside self loops.

My first instinct was thus to check for exactly these properties to find out whether or not to try and resolve a layout algorithm for a given node. For the second property to be true, the following two properties need to hold:

1. Inside self loops are active for the node.
2. There is at least one self loop marked as an inside self loop.

Even with the test graph in #562, I found the second property to be false even though there should have been one such self loop. The problem was that the layout configurator was applied to the edge after the algorithm resolver was applied to the node. Thus, to the resolver it seemed that there was no inside self loop, only because the configurator had not gotten around to apply the appropriate option to the edge yet.

This is a subtle source for bugs that are not obvious to find.

My proposal, thus, is this: we change ElkUtil.applyVisitors(...) to apply one visitor to every graph element before applying the next visitor. While theoretically equally complex, this might make things run slower in the real world.

Thoughts?

[soerendomroes]

@Eddykasp