loadSourceModel, createGModel and ....update model...

[rsoika]

Hi,
I have a more conceptional question about the new interfaces (0.10.0-SNAPSHOT) to load a source model and create a GModel. I know this is currently work in progress and also the documentation about the 'Model Operations', which may bring me the answer to my question. But let me summarize what I understood so far:

1. Initially, the server calls the loadSourceModel from my registered custom SourceModelStorage. In my case I load a BPMN file into memory and create a dom tree. I store this full version of a BPMN model in my custom version of the ModelState object.
2. Next the server calls the method createGModel() from my custom GModelFactory. Here I traverse over the dom tree of my BPMN model and generate a GModel from the parts to be visualized. This part of the concept is brilliant! Because this allows me to extract only a subset of my source model. This is an important concept in BPMN that the user can drill down into sub-parts of the model.
3. When the user/client changes the model (e.g. moving an element) an update operation is send (this is now a missing part in my implementation).
4. After the update action, the server calls the method createGModel() again.

And this last step, is the part which I can observer, but I do not understand why this is happen? Why is the server calling createGModel() each time the model is updated. In my case traversing through the BPMN model is a complex part, as I need to go through a complex dom tree of objects. And in case of an update it should not be necessary to re-create the GModel, as the GModel is now already the source of truth. (I assume that I will be able to update my BPMN source model during the update-opertation)

To avoid this repeating calls of createGModel() I introduced now a new boolean in my ModelState object named 'initalized'. I use this flag to only create the GModel once the first time the bpmn file was loaded.

public class BPMNGModelFactory implements GModelFactory {
    @Inject
    protected BPMNGModelState modelState;
    @Override
    public void createGModel() {
        if (!modelState.isInitalized()) {

            // Creating new GModel from BPMN metha model...
            .....
           modelState.updateRoot(newGModel);
           modelState.getRoot().setRevision(-1);
        }
    }
}

So, why is the GModelFactory called in case of an update?
I know I'm too impatient again now, but maybe someone can briefly explain the point of updating the GModel to me.

[planger]

Hi @rsoika,
in the default case, we recommend to apply edit operations directly on the source model (not the graphical model) and regenerate the GModel after the change (see also the doc):

1. The client sends an operation request to the server
2. The server invokes the registered edit operation handler, which modifies the underlying source model directly
3. The server applies the graphical model factory to the modified source model to create a new graphical model
4. The server sends the created graphical model to the client
5. The client re-renders the diagram according to the new version of the graphical model

The reasons for that are:

• Implementing a bi-directional synchronization between a GModel and a source model is often hard, error-prone and hard to test
• Directly modifying the source model from an operation is usually much easier
• The function to produce a GModel out of a source model is already available for the initialization, so better just reuse that uni-directional transformation after each update rather than coming up with a complex synchronization
• The transformation from the source model to the gmodel may be complex in some scenarios, but usually is pretty fast or can be made fast with proper caching (therefore we often introduce custom model indices by subclassing GModelIndex and extending it with additional maps, eg the EMFNotationDiagramModule introduces an extended index), so there is usually very little performance overhead for recreating the GModel on each change.

Of course, adopters may choose to do it differently, by overwriting the GModelState as you did. But in general I would recommend to stick to the pattern above ("projectional editing pattern").

I hope this helps!

[rsoika]

Hm, I am still not convinced. You wrote

Directly modifying the source model from an operation is usually much easier

I totally agree with that. If I receive a Update Operation from the client with, let's say a repositioning of one Element in the diagram, I can update my Source Model accordingly - in BPMN I have to update the bpmndi:BPMNPlane section. This is fine for me. But in the same way I can decide that the GModel is (and it should be) still valid - because it should already contain the new Position of the GElement (and I can observe this behaviour when I skip the re-generation).

So my question still is: If I update my source model in the OperationHandler, why should I discard the existing GModel and compute a complete new one - which as a result should be equal to the old one provided by the client.

The transformation from the source model to the gmodel may be complex in some scenarios, but usually is pretty fast or can be made

Yes recomputing can be fast. But in our Case a BPMN model can consist of hundreds of nodes. It seems to me as a waste of energy ;-)

Can you provide me an example how the edit operation handler will look like.

[planger]

I see, so I think you are suggesting, you will modify the source model directly anyway, but depending on the actual operation either

1. keep the GModel and modify it additionally directly instead of regenerating it from scratch (to save resources, for simple operations like move)
2. or regenerate it from scratch for more complex operations.

Is that right?

By default the framework does option 2.
So I would suggest that for operations that apply option 1 for saving resources, the respective operation handler implementations explicitly declare that they already take care of updating the GModel, e.g., by setting a flag in the GModelState gModelUpdated to true, and then if this flag is set, the regeneration of the GModel is skipped in your GModelFactory and the flag is reset to false after skipping it once.

We could consider also adjusting the API to accommodate that:

• Add an interface that operation handlers could implement to declare that they already updated the GModel
• In the operation action handler in #executeHandler(), pass on a flag whether the operation handler already updated the GModel when invoking modelSubmissionHandler.submitModel(reason, gModelUpdateToDate = true)
• In the ModelSubmissionHandler don't invoke modelFactory.createGModel() if that flag is set.

What do you think? I'd also like to get @tortmayr's opinion on whether we should consider extending the API like above to accommodate that.

However, whether or not this is part of the official API, it shouldn't keep you from following the suggestion(s) above.

[rsoika]

Yes, exactly this what I mean. And yes, the current API did not block me to implement exactly this behaviour in my own custom implementation.
So this is more a conceptual question which I come across reading the documentation and observing the behaviour of the current implementation.
I think an operation handler should be able to control if the modelFactory.createGModel() should be called or not - as you mentioned in your draft

[rsoika]

@planger I want to come back to the idea to provide a new flag gModelUpdated. There may be a misunderstanding and maybe the change is unnecessary. Let me recap the way I see the live cycle:

• The client want to open a model (e.g. the BPMN file)
• On the server loadSourceModel is called to load the BPMN file or its meta model into the GModelState
• The server calls GFactory.createModel - to generate a GModel from my meta model

Now the client can start modelling based on the GModel. Moving elements creating new ones, deleting elements, change routing.

On the server side I am not interested in these changes until the client finish the modelling phase. Finishing the modelling phase means the client save the model

At that point the sever calls saveSourceModel(). Now I can synchronize all the changes from the final GModel back into my source model (BPMN file).

So I still wonder why I should be interested in a ModelOperation event during modelling? It may be interesting for some scenarios. But in my BPMN modelling scenario I can't see why I need to hold my BPMN file in sync during modelling. It seems to me to be totally fine to sync the source model in the final saveSourceModel() call.

I still can't see the advantage of the Operation Handler in my scenario. Unfortunately, I still don't know exactly what the Operation Handler looks like. But I suspect it would be necessary to implement all possible kinds - like CreateNodeOperation, CreateEdgeOperation, LayoutOperation....

I'm lazy and just think about saving this work.

[planger]

Of course, you can implement your editor this way with GLSP, if that's what you need. However there are very good reasons, why in the usual case, you would want to let the server apply the operations (to the source model) and then regenerate the graphical model after each change in the editor:

1. The whole bi-directional synchronization topic mentioned above (re-use the model factory and avoid implementation of a bi-directional synchronization function)
2. The client shouldn't need to know how and what needs to be changed in the graphical model (let alone the source model) if an operation is applied. Sure there are easy ones, like move or change label, but for most of the operations pretty deep knowledge about the modeling language is required, e.g. creating an association in a UML class diagram (not just adding an edge, but adding an association, two or more association ends, owned by source and target element), editing a property in a UML class diagram by changing the property label e.g. from -foo:Bar [1] to +foobar:Foo [*] (may rename the property, change its type, cardinality, etc.). The server knows the source model and how to modify it on operations AND how to generate a new version of the grapical model out of it. So usually the server applies the change to the source model and regenerates the graphical model. The client just needs to know how to re-render it, but doesn't need to understand anything about the source model or how it is to be modified or what the impact of a modification on the graphical model would be.
3. There may be other clients, like property views, trees, etc. which are interested in the current state in memory of the source model too. Thus the GLSP server should actually update the source model after each change to always reflect a common in-memory state of the model, so that other views can react to it.

[rsoika]

Ok, now I see. And I totally agree with you now. Also in BPMN we have these kind of dependencies in the model. I suggest that this text should become par of the Model Operations documentation.

But please can you show me a short example how to implement a OperationHandler. I still do not get it if I need to implement all kinds of OperationHandler from the package org.eclipse.glsp.server.operations or if there is a generic operation handler telling me just the list of updated elements out from the GModel?

[planger]

Right, that part is still missing, but there is really not too much to it.
You just modify your source model according to the operation. If it is an EMF source model, you typically do that by providing an EMF command, which is then executed on the command stack. Here is an example for deleting an element:
https://github.com/eclipse-glsp/glsp-examples/blob/master/project-templates/java-emf-theia/glsp-server/src/main/java/org/eclipse/glsp/example/javaemf/handler/DeleteTaskNodeHandler.java

[rsoika]

Sorry, but I still did not get it.
I implemented my own Operation Handler now the second time:

import com.google.inject.Inject;
public class BPMNDeleteNodeHandler<T extends DeleteOperation> extends AbstractOperationHandler<DeleteOperation> {
    @Inject
    protected BPMNGModelState modelState;
    @Override
    protected void executeOperation(final DeleteOperation operation) {
        System.out.println("executeOperation..........");
    }
}

In my DiagramModule I register this handler:

public class BPMNDiagramModule extends DiagramModule {
    ......
    @Override
    protected void configureOperationHandlers(final MultiBinding<OperationHandler> binding) {
        super.configureOperationHandlers(binding);
        ..............
        // GLSP Operation handlers for ModelUpdates
        binding.add(BPMNDeleteNodeHandler.class);
    }
   .....
}

But the operation Handler is never be called. Is there some additional client code necessary to trigger the operation? I can observer that my custom GFactory.createModel method is called. So it looks like something is going on in the server. But my OperationHandler is not involved :-(

[rsoika]

@planger : I give up. I think I have to wait until the documentation is published

This is how my custom CreateNodeOperationHandler now looks like. The executeOperation is called when the user creates a new element in the diagram, but it does not provide the id of the element - should not the node ID be the most important part to create the corresponding element in my source model?:

public abstract class CreateTaskHandler extends CreateBPMNNodeOperationHandler {

    @Inject
    protected BPMNGModelState modelState;

    /**
     * Default constructor
     */
    public CreateTaskHandler() {
        super(BPMNTaskType.TASK);
    }

    public CreateTaskHandler(final String elementTypeId) {
        super(elementTypeId);
    }

    protected String getElementTypeId() {
        return getHandledElementTypeIds().get(0);
    }

    @Override
    protected GNode createNode(final Optional<GPoint> point, final Map<String, String> args) {
        return builder(point, modelState).build();
    }

    protected TaskNodeBuilder builder(final Optional<GPoint> point, final GModelState modelState) {

        int nodeCounter = GModelUtil.generateId(BpmnPackage.Literals.TASK_NODE, getElementTypeId(), modelState);
        String name = getLabel() + " " + nodeCounter;

        return new TaskNodeBuilder(getElementTypeId(), name) //
                .position(point.orElse(null));
    }

    @Override
    public void executeOperation(final CreateNodeOperation operation) {
        super.executeOperation(operation);
        System.out.println("===== > executeOperation " + operation.getElementTypeId() + " kind=" + operation.getKind());
        System.out.println("===== > Modelstate we have now " + modelState.getIndex().allIds().size() + " elements");
        // How should we generate a new element in the SourceModel if we do not know the
        // ID???
    }
}

[rsoika]

Finally I come to a solution where the creation of the sourceModel entry is done in the createNode method (after the GNode was created). The executeOperation method seems not to be relevant here.

Is this way to implement a CreateOperationHandler reflecting your ideas?

public abstract class CreateTaskHandler extends CreateBPMNNodeOperationHandler {

    @Inject
    protected BPMNGModelState modelState;

    /**
     * Default constructor
     */
    public CreateTaskHandler() {
        super(BPMNTaskType.TASK);
    }

    public CreateTaskHandler(final String elementTypeId) {
        super(elementTypeId);
    }

    protected String getElementTypeId() {
        return getHandledElementTypeIds().get(0);
    }

    @Override
    protected GNode createNode(final Optional<GPoint> point, final Map<String, String> args) {
        // create the TaskNode  
        TaskNode taskNode = builder(point, modelState).build();

        // now we add this task into the source model
        BPMNProcess process = modelState.getBpmnModel().getContext();
        BPMNActivity task = process.addTask(taskNode.getId(), taskNode.getName(), taskNode.getType());
        if (point.isPresent()) {
            task.getBounds().updateBounds(point.get().getX(), point.get().getY(), 10.0, 10.0);
        }
        return taskNode;
    }

    protected TaskNodeBuilder builder(final Optional<GPoint> point, final GModelState modelState) {
        int nodeCounter = GModelUtil.generateId(BpmnPackage.Literals.TASK_NODE, getElementTypeId(), modelState);
        String name = getLabel() + " " + nodeCounter;
        return new TaskNodeBuilder(getElementTypeId(), name) //
                .position(point.orElse(null));
    }

    @Override
    public void executeOperation(final CreateNodeOperation operation) {
        super.executeOperation(operation);
        // seems to be not relevant at all...?
    }
}

But even now as I can update my sourceModel successfully, I did not find a way to react on a DeleteOperation. I still don't have the slightest idea on how to do that. I'm afraid that I'll end up having to implement dozens of classes to keep my SourceModel in sync.

[planger]

For your scenario, where you have an EMF source model, I think it'd be best to look at the new EMF base classes that GLSP provides: https://github.com/eclipse-glsp/glsp-server/tree/master/plugins/org.eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf

There is a project template that uses those EMF base classes here in the new project templates: https://github.com/eclipse-glsp/glsp-examples/tree/master/project-templates/java-emf-theia
Here is the implementation of the operation handlers in this project templates: https://github.com/eclipse-glsp/glsp-examples/tree/master/project-templates/java-emf-theia/glsp-server/src/main/java/org/eclipse/glsp/example/javaemf/handler

This project template just has one node type (task nodes), thus it has one CreateTaskNodeHandler. It creates a task node ("semantic model") and a shape for it (in the notation model).
Also, it has one DeleteTaskNodeHandler to remove elements from the EMF source model.
As can be seen in both operation handlers, they only operate on the EMF source model. The GModel is always regenerated after each change.

This is the structure that we would recommend to follow.

Of course they could be generalized or restructured to your needs (e.g. that they either delegate to commands that you write externally or that you already have from your previous implementation, etc.).

To find out why your operation handlers are not invoked, I would recommend to debug into the OperationActionHandler. This class is responsible for dispatching the incoming operation requests to the operation handlers you've registered.

[rsoika]

ok, than I will go through the EMF examples in more detail

[rsoika]

Finally I succeeded. My DeleteHandler looks now like this:

public class BPMNDeleteNodeHandler extends AbstractOperationHandler<DeleteOperation> {
    @Inject
    protected BPMNGModelState modelState;
    @Override
    protected void executeOperation(final DeleteOperation operation) {
        List<String> elementIds = operation.getElementIds();
        if (elementIds == null || elementIds.size() == 0) {
            System.out.println("Elements to delete are not specified");
        }
        for (String id : elementIds) {
            System.out.println("...delete Element " + id);
            // Update the source model
            BPMNBaseElement baseElement = modelState.getBpmnModel().getContext().findBaseElementById(id);
            if (baseElement == null) {
                System.out.println("...no BPMN elmenet with id: " + id + " found!");
            }
            ..............
        // reset model state
        modelState.reset();
    }
}

I don't understand why it took me several hours to get to this point.

I guess it was my configureOperationHandlers() method in my DiagramModule. There I bound something like a default delete Handler

public class BPMNDiagramModule extends DiagramModule {
    .....
    @Override
    protected void configureOperationHandlers(final MultiBinding<OperationHandler> binding) {
        super.configureOperationHandlers(binding);
        ......
        // PROBLEMATIC LINE!
        binding.add(DeleteOperationHandler.class);
        ........
        ........
        // GLSP Operation handlers for ModelUpdates
        binding.add(BPMNDeleteNodeHandler.class);

    }
    .............
}

I am now able to delete the elements from my source model and I follow your advice to regenerate the complete model after the delete operation.

[rsoika]

@planger I would now like to return to our original discussion.

I now implemented the CreateOperationHandler and DeleteOperationHandler as suggested (more or less?)

In my CreateOperationHandler I overwrite the method createNode. I build the GNode and after that I add a corresponding BPMN element into my Source Model. I skip the GModel generation as it is already up to date. As I am operating on the GModel directly. The method executeOperation is ignored here.

In my DeleteOperationHandler I remove only the given elementIDs from my SourceModel and reset my ModelState. The GFactoryImpl rebuilds the new Gmodel.

So this looks fine. But now I lost the undo/redo features. I guess that I have to implement more operation handlers for such kind of functionality?

Can you provide a List of Operations I have to handle? Also the Create and the Delete operation handlers in my case follow now a different concept. The first is operating directly on the GModel the later not. That feels inconsistent now.

[rsoika]

Thanks for clarifying.

The workflow example is in that sense a really misleading example, because it ONLY operates on the GModel directly,

I think this is the most important hint. I started with the minimal-example and learned by the workflow-example. But now I slowly can follow your thoughts. With the workflow example (JSONGModel) I run in the wrong direction. This explains why I am so frustrated with all this magic stuff in my creationHanlers which do not match all the things you try to explain to me.

Maybe an example working on a different source model will help also other users. Because all have the the challenge to load and save there own models. (I have seen this question in other discussions several times)

I will now start reimplementing my creation handlers and hope that I get a more homogeneous picture at the end.

(By the way: my BPMN source model is no longer based on EMF. It is a more simplified BPMN meta model now operating directly on the XML Dom tree. Converting the BPMN spec into EMF is a nightmare - you generate 500 class files just to tweak everything afterwards. )

[planger]

Ok I see...

Maybe an example working on a different source model will help also other users. Because all have the the challenge to load and save there own models. (I have seen this question in other discussions several times)

Yeah, that's exactly, why we've been working on the project templates and we'll likely remove the minimal-example in favor of them.
The workflow example is merely a development example to help us test and to show specific features of client/server, but is a bad example for model source and operation handler related questions.

[rsoika]

@planger - I am now on the (hopeful) right way implementing my new BPMNCreationNodeOperator.

In its minimal implementation it looks like this:

public class BPMNCreateTaskHandler extends CreateNodeOperationHandler {
    @Inject
    protected BPMNGModelState modelState;

    public BPMNCreateTaskHandler() {
        super(BPMNTaskType.TASK);
    }

    public BPMNCreateTaskHandler(final String elementTypeId) {
        super(elementTypeId);
    }

    @Override
    protected GNode createNode(final Optional<GPoint> point, final Map<String, String> args) {
        System.out.println("===== > createNode is ignored here!");
        // no-op, because this class returns a command directly instead of executing the
        // command directly on a model
        return null;
    }

    @Override
    public void executeOperation(final CreateNodeOperation operation) {
        System.out.println("===== > executeOperation " + operation.getElementTypeId() + " kind=" + operation.getKind());

        // We can not call super.execute because of the missing createNode impl!
        // super.executeOperation(operation);

        // add new element into source model.....
    }

    @Override
    public String getLabel() {
        return "Just a new custom Label";
    }

}

As you can see I implement the executeOperation method to extend my source model. And(!) I am overwriting the createNode() method - just like in the EMFCreateNodeOperationHandler - and I am returning null!

But when doing this, it is also not allowed to call

    @Override
    public void executeOperation(final CreateNodeOperation operation) {
        super.executeOperation(operation);
        .......
    }

because the implementation of this method in the CreateNodeOperationHandler is calling createNode() and expects a valid GNode object. This is causing the following StackTrace

18:53:57.786 [DefaultActionDispatcher 2] ERROR org.eclipse.glsp.server.internal.gmodel.commandstack.GModelCommandStack - Error while executing command
java.lang.IllegalArgumentException: The 'no null' constraint is violated
	at org.eclipse.emf.common.util.AbstractEList.validate(AbstractEList.java:99) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.emf.ecore.util.EcoreEList.validate(EcoreEList.java:63) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.emf.common.util.BasicEList.addUnique(BasicEList.java:417) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.emf.common.notify.impl.NotifyingListImpl.doAddUnique(NotifyingListImpl.java:325) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.emf.common.notify.impl.NotifyingListImpl.addUnique(NotifyingListImpl.java:282) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.emf.common.util.AbstractEList.add(AbstractEList.java:304) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]
	at org.eclipse.glsp.server.operations.gmodel.CreateNodeOperationHandler.executeOperation(CreateNodeOperationHandler.java:61) ~[open-bpmn.server-0.1.0-SNAPSHOT-glsp.jar:?]

So extending the CreateNodeOperationHandler seems to make no sense at all. But the EMFCreateNodeOperationHandler is extending the CreateNodeOperationHandler. This is surprising me a little bit. How did you avoid to run into the same IllegalArgumentException?

So for the moment I am ignoring this side effect. But maybe the CreateNodeOperationHandler should check for null of the GModelElement returned by createNode?

[planger]

I agree this is certainly confusing and I opened #648 for that.

[rsoika]

Thanks. I can tell you - with my new understanding, I am now making really progress!!

[rsoika]

@planger - One more suggestion for the CreateOperation handler.

My final implementation of the BPMNCreateTaskHandler is very compact and works perfect. But there is one issue I came across during implementation. My BPMNCreateTaskHandler is generic and able to create all 7 BPMN Sub-Task types. To archive this I have to overwrite the constructor in a ugly way:

    public BPMNCreateTaskHandler() {
        super(BPMNTaskType.TASK);
        handledElementTypeIds = Arrays
                .asList(new String[] { //
                        BPMNTaskType.TASK, //
                        BPMNTaskType.MANUAL, //
                        BPMNTaskType.SEND, //
                        BPMNTaskType.SERVICE, //
                        BPMNTaskType.SCRIPT,
                        BPMNTaskType.USER //
                        });
    }

You would normally expect that in case you extend the AbstractCreateOperationHandler you could overwrite the method

   @Override
   public List<String> getHandledElementTypeIds() { return handledElementTypeIds; }

But this is not possible, because the class itself did not use this method consequently. For example the method

public boolean handles(final Operation operation)

uses the member variable instead of the getter. Luckily the handledElementTypeIds is protected and not private.

And also (as mentioned some time before) the CreateNodeOperationHandler hides the two constructors

 public AbstractCreateOperationHandler(final String... elementTypeIds) {
      this(Lists.newArrayList(elementTypeIds));
   }

   public AbstractCreateOperationHandler(final List<String> handledElementTypeIds) {
      this.handledElementTypeIds = handledElementTypeIds;
   }

So if the constructors would be visible I could overwrite the empty constructor (needed for the binding) like this:

    /* Overwrite the default constructor to define a list of elementTypeIDs
     * this CreateNodeOperation handler should be responsible for.
     */
    public BPMNCreateTaskHandler() {
        super( Arrays
                .asList(new String[] { //
                        BPMNTaskType.TASK, //
                        BPMNTaskType.MANUAL, //
                        BPMNTaskType.SEND, //
                        BPMNTaskType.SERVICE, //
                        BPMNTaskType.SCRIPT,
                        BPMNTaskType.USER //
                        }));
    }

[planger]

@rsoika @martin-fleck-at pushed a change that should improve the handling. Can you please double-check? eclipse-glsp/glsp-server#164 (review)
Thanks!

[rsoika]

@planger as I wrote in your pull request (eclipse-glsp/glsp-server#164 (review)), I am still not totally satisfied. In the moment it is not a big thing and I simply avoid calling super.executeOperation(operation);. And so my node operation handlers work fine.

But if you take a look at the AbstractCreateEdgeOperationHandler there is the same thing. createEdge() seems to be an important method. I can not ignore this ? What is your opinion about the package org.eclipse.glsp.server.operations.gmodel? Should I refer this package i my own implementation or is it not recommended?

[planger]

Hi @rsoika,
right, use cases that don't use the GModel as their source model should NOT use any handlers from org.eclipse.glsp.server.gmodel (unless there is a really good reason or special case). Instead those use cases should directly use AbstractCreateOperationHandler etc.
Therefore in eclipse-glsp/glsp-server#165 we further clarified this by also adding GModel as a prefix.

[rsoika]

Thanks for the clarification.

[rsoika]

Hi @planger
I know I'm a bit annoying with this topic but I have one more question:
Would you agree, that in my scenario - building the gmodel from a custom source model - I should use the org.eclipse.glsp.server.gmodel.AbstractGModelCreateEdgeOperationHandler for the creation of new edges? It seems to me that this is such a really good reason to use handlers from org.eclipse.glsp.server.gmodel ?

public class BPMNCreateSequenceFlowHandler extends AbstractGModelCreateEdgeOperationHandler {

    public CreateSequenceFlowHandler() {
        super(ModelTypes.SEQUENCE_FLOW, "Sequence Flow");
    }

    @Override
    public void executeOperation(final CreateEdgeOperation operation) {        
        super.executeOperation(operation);
        // update source model
        ...........
    }

    // Needed to show the edge live 
    @Override
    protected Optional<GEdge> createEdge(final GModelElement source, final GModelElement target,
            final GModelState modelState) {
        return Optional.of(new SequenceFlowBuilder() //
                .source(source) //
                .target(target) //
                .addArgument(GArguments.edgePadding(10)) //
                .addCssClass("sequenceflow") //
                .build());
    }
}

The edge is something which need to be shown during the connecting-process on the client side before the edge becomes part of the source model.

[rsoika]

Ok, than I must review my impl once again. The problem was that the feedback edge was not shown even if the createEdgeOperation was fired. Maybe a small mistake that happened during my massive refactoring...

[rsoika]

@planger - does this mean that the server is responsible for the routing? I need a manhantan routing and I have had the impression that this is part of the sprotty client. I don't want to implement the routing by myself. The CreateEdgeOperation does not seem to provide routing points :-(

But in the AbstractGEdgeBuilder it looks like the setProperties method did some kind of this magic to get the routing out of the client by calling

edge.getRoutingPoints().addAll(routingPoints);

So using a AbstractGEdgeBuilder in the executeOperation method seems reasonable to me to generate the GEdge object. Than I would be able to get the routing points and put these into my source model (named waypoints in BPMN).

But solving this in that way seems to violate again your strict rule of NEVER touching the GModel in the executeOperation method.

Or have I overseen something and there is some kind of RoutingProcessor in the GLSP Server part?

[rsoika]

ok, I think I now understand that there is no routing processor in GLSP. In Eclipse BPMN2 we have this to optimize the routing:

I guess I have to overtake this from Eclipse BPMN2 by myself.

[planger]

The routing is influenced by two orthogonal approaches:

1. Client-side routing: The server doesn't add any routing points on the edge, but just specifies in the property routerKind of the edge the client-side router that shall be used, e.g., routerKind: 'manhattan'. The client will then apply the router that is registered for the routerKind to the edge, which may add routing points according to a certain strategy. The manhatten edge router, for instance, might be the closest to what you are looking for. Of course you can also add custom routers by registering them similarly to how the manhattan router is registered and set the respective routerKind in the edge. There is also a work in progress to integrate the libavoid routing which is a more sophisticated algorithm to avoid overlaps as much as possible. But it is not ready yet, afaik.
2. Server-side routing: The server can add routing points to the edge when creating the GEdge in the GModelFactory. This is useful if the router wants to control the routing or applies a certain routing strategy or if there are manual routing points added by the user, thus they must be persisted on save and restored when the model is loaded. This is the approach that you would want to take, if you want to take over the routing algorithm from your existing BPMN implementation.

Both of these approaches can theoretically also be combined, if needed, as long as the edge router on the client respects existing routing points and doesn't just throw them away and recreates them (not sure if the manhatten router does that, actually).

[rsoika]

Thanks for your answer. Ok, I expected an answer in this direction. As I have already tweaked the Manhaten router, maybe I can add additional routing logic too. And also the server side routing makes sense as you explained. Puh... more work than I expected.