2

I have implemented a GEF editor for a graph-like EMF model, with a remove command for a certain type of node in the graph. I think I've done all the necessary steps in order to make this set up work (vainolo's blog has been a great help).

However, when I'm deleting a model element, the view doesn't get refreshed, i.e., the figure for the model element isn't removed from the editor view, and I have no idea why. I'd be extremely grateful if somebody could have a look at my sources and point me to any problems (and possibly solutions :)). Many thanks in advance!

Below are what I think are the important classes for this issue. Please do let me know should I add further code/edit the code, etc. (I've left out code that I thought doesn't help, e.g., getters and setters, class variables). Thanks!

DiagramEditPart

public class DiagramEditPart extends AbstractGraphicalEditPart {

    public DiagramEditPart(Diagram model) {
        this.setModel(model);
        adapter = new DiagramAdapter();
    }

    @Override protected IFigure createFigure() {
        Figure figure = new FreeformLayer();
        return figure;
      }

      @Override protected void createEditPolicies() {
        installEditPolicy(EditPolicy.LAYOUT_ROLE, new DiagramXYLayoutPolicy());
      }

      @Override protected List<EObject> getModelChildren() {
          List<EObject> allModelObjects = new ArrayList<EObject>();
          if (((Diagram) getModel()).getMyNodes() != null)
          allModelObjects.addAll(((Diagram) getModel()).getMyNodes());
          return allModelObjects;
      }

      @Override public void activate() {
          if(!isActive()) {
              ((Diagram) getModel()).eAdapters().add(adapter);
          }
          super.activate();
      }


      @Override public void deactivate() {
          if(isActive()) {
              ((Diagram) getModel()).eAdapters().remove(adapter);
          }
          super.deactivate();
      }

    public class DiagramAdapter implements Adapter {

          @Override public void notifyChanged(Notification notification) {
              switch (notification.getEventType()) {
            case Notification.REMOVE: refreshChildren();
                break;
            default:
                break;
            }
          }

          @Override public Notifier getTarget() {
              return (Diagram) getModel();
          }

          @Override public void setTarget(Notifier newTarget) {
              // Do nothing.
          }

          @Override public boolean isAdapterForType(Object type) {
              return type.equals(Diagram.class);
          } 

      }

}

MyNodeEditPart

public class MyNodeEditPart extends AbstractGraphicalEditPart {

    public MyNodeEditPart(MyNode model) {
         this.setModel(model);
         adapter = new MyNodeAdapter();
    }

    @Override protected IFigure createFigure() {
        return new MyNodeFigure();
    }

    @Override protected void createEditPolicies() {
        installEditPolicy(EditPolicy.COMPONENT_ROLE, new MyNodeComponentEditPolicy());
    }

    @Override protected void refreshVisuals() {
        MyNodeFigure figure = (MyNodeFigure) getFigure();
        DiagramEditPart parent = (DiagramEditPart) getParent();
        Dimension labelSize = figure.getLabel().getPreferredSize();
        Rectangle layout = new Rectangle((getParent().getChildren().indexOf(this) * 50), 
                (getParent().getChildren().indexOf(this) * 50), (labelSize.width + 20), 
                (labelSize.height + 20));
        parent.setLayoutConstraint(this, figure, layout);
    }

    public List<Edge> getModelSourceConnections() {
        if ((MyNode) getModel() != null && ((MyNode) getModel()).getDiagram() != null) {
            ArrayList<Edge> sourceConnections = new ArrayList<Edge>();
            for (Edge edge : ((MyNode) getModel()).getDiagram().getOutEdges(((MyNode) getModel()).getId())) {
                sourceConnections.add(edge);
            }
            return sourceConnections;
        }
        return null;
    }

    // + the same method for targetconnections

    @Override public void activate() {
        if (!isActive()) {
            ((MyNode) getModel()).eAdapters().add(adapter);
        }
        super.activate();
    }

    @Override public void deactivate() {
        if (isActive()) {
            ((MyNode) getModel()).eAdapters().remove(adapter);
        }
        super.deactivate();
    }

    public class MyNodeAdapter implements Adapter {

        @Override
        public void notifyChanged(Notification notification) {
            refreshVisuals();
        }

        @Override
        public Notifier getTarget() {
            return (MyNode) getModel();
        }

        @Override
        public void setTarget(Notifier newTarget) {
            // Do nothing
        }

        @Override
        public boolean isAdapterForType(Object type) {
            return type.equals(MyNode.class);
        }

    }

}

MyNodeComponentEditPolicy

public class MyNodeComponentEditPolicy extends ComponentEditPolicy {

    @Override
    protected Command createDeleteCommand(GroupRequest deleteRequest) {
        DeleteMyNodeCommand nodeDeleteCommand = new DeleteMyNodeCommand((MyNode) getHost().getModel());
        return nodeDeleteCommand;
    }

}

DeleteMyNodeCommand

public class DeleteMyNodeCommand extends Command {

    public DeleteMyNodeCommand(MyNode model) {
        this.node = model;
        this.graph = node.getDiagram();
    }

    @Override public void execute() {
        getMyNode().setDiagram(null);
        System.out.println("Is the model still present in the graph? " + getGraph().getMyNodes().contains(getMyNode())); 
            // Returns false, i.e., graph doesn't contain model object at this point!
    }

    @Override public void undo() {
        getMyNode().setDiagram(getGraph());
    }

}

EDIT

Re execc's comment: Yes, refreshChildren() is being called. I've tested this by overriding it and adding a simple System.err line, which is being displayed on the console on deletion of a node:

@Override
public void refreshChildren() {
    super.refreshChildren();
    System.err.println("refreshChildren() IS being called!");
}

EDIT 2

The funny (well...) thing is, when I close the editor and persist the model, then re-open the same file, the node isn't painted anymore, and is not present in the model. But what does this mean? Am I working on a stale model? Or is refreshing/getting the model children not working properly?


EDIT 3

I've just found a peculiar thing, which might explain the isues I have? In the getModelChildren() method I call allModelObjects.addAll(((Diagram) getModel()).getMyNodes());, and getMyNodes() returns an unmodifiable EList. I found out when I tried to do something along the lines of ((Diagram) getModel()).getMyNodes().remove(getMyNode()) in the delete command, and it threw an UnsupportedOperationException... Hm.


EDIT 4

Er, somebody kill me please? I've double-checked whether I'm handling the same Diagram object at all times, and while doing this I stumbled across a very embarassing thing:

The getModelChildren() method in DiagramEditPart in the last version read approx. like this:

@Override protected List<EObject> getModelChildren() {
    List<EObject> allModelObjects = new ArrayList<EObject>();
    EList<MyNode> nodes = ((Diagram) getModel()).getMyNodes();
    for (MyNode node : nodes) {
        if (node.getDiagram() != null); // ### D'Uh! ###
            allModelObjects.add(node);
    }
    return allModelObjects;
 }

I'd like to apologize for stealing everyone's time! Your suggestions were very helpful, and indeed helped my to finally track down the bug!

I've also learned a number of lessons, amongst them: Always paste the original code, over-simplifaction may cloak your bugs! And I've learned a lot about EMF, Adapter, and GEF. Still:

enter image description here

Community
  • 1
  • 1
s.d
  • 4,017
  • 5
  • 35
  • 65
  • 1
    Does the refreshChildren() method of a DiagramEditPart ever get called? You can override it and add a simple log message to see if it is. – execc Jun 28 '12 at 09:13
  • @execc: Thanks. Yes, the method seems to be called, cf. my edit. – s.d Jun 28 '12 at 13:59
  • 1
    Did you also check if `getModelChildren` gets called also? Furthermore, in the `Command` you do `getGraph().getSNodes()` but in the `DiagramEditPart` you invoke `getMyNodes()`. What is the difference between them? – vainolo Jun 28 '12 at 14:53
  • @vainolo: Yes, `getModelChildren()` is being called. The `getMy/SNodes()` thing was a typo, sorry, I've corrected it in the question. – s.d Jun 28 '12 at 15:31
  • I think perhaps I'm working on two different graph objects. There's a bit of a mess with the `mode` class variable and calls to `setModel()`/`getModel()`. I guess I should call `this.model = model` in the constructor, and then call methods on this class variable only? E.g., `model.getMyNodes()`, etc.? Or would it be better to call `super.setModel(model)` in the constructor? Or just keep `setModel(model)`? I'm a bit confused... Thanks for all your help! – s.d Jun 28 '12 at 15:35
  • 1
    use `setModel` and `getModel`. But the code looks OK, and if `getModelChildren` is getting called then something is wrong with the model itself. Seems to me you have to debug the code to see what is happening. – vainolo Jun 28 '12 at 15:41
  • @vainolo: Thanks! In the meantime I've found out that although the node is removed from the model in `DeleteMyNodeCommand` (check via both `getModel().getMyNodes().contains(getNode())` and `getNode().getDiagram()` there), the subsequently called `getModelChildren()` (called, I guess, from `refreshChildren()`) lists the node as being a model child. I can't for the life of me figure out why... – s.d Jun 28 '12 at 15:49
  • @vainolo: Strange, once I persist the model, the node is gone. Cf. Edit 2 in the question. – s.d Jun 28 '12 at 17:08
  • Can you upload the code somewhere? – vainolo Jun 28 '12 at 20:16
  • So, If you're not getting the proper list of a model child in you getModelChildren() - comething is wrong at the level of model/commands. Make shure that tha node is gone after executing a delete command. – execc Jun 29 '12 at 07:24
  • @vainolo: Will upload the code later today. Thanks! – s.d Jun 29 '12 at 11:30
  • @execc: Thanks, I'm looking into that as well now. – s.d Jun 29 '12 at 11:31
  • @vainolo: http://pastebin.com/tpsn8UfQ - If this isn't enough context, I'll be happy to upload a zip with the complete editor project somewhere, or whichever other format/way you want. Thanks so much! – s.d Jun 29 '12 at 16:52
  • Also, why are you not using GMF? It's a perfect match for GEF project with EMF model. – execc Jun 29 '12 at 18:20
  • @execc: Haha, I've actually been waiting for that question :). I don't use for the following reasons, which I think are valid, although I don't know a lot about GMF to be honest. 1) I don't want to display the whole model in the editor, the editor's contents are set to be a graph of a document in a graph in a project. 2) I need to display model elements differently, e.g., a node can have multiple annotations, which in the model are children nodes of the node, but in the editor they need to be labels in the NodeFigure, but perhaps this is no problem in GMF? cont. ... – s.d Jun 29 '12 at 18:38
  • @execc: cont.: 3) I dont't have enough knowledge, time, and resources (in that order, haha) to learn yet another tool. Just now at least. I've only started to learn Java a good year ago, have jumped traight into RCP development (with Maven *and* Tycho, phew), and needed some basic grasp of EMF to work with the model I have (3rd party). So yea: While GMF is very probably a very good tool, I don't think it's my best option at the moment. However, please do correct me if I'm completely wrong. Which wouldn't surprise me... – s.d Jun 29 '12 at 18:41
  • You're not terribly wrong, but IMHO you should at least give it a try. It will generate all the nice things for you, such a basic figures and edit parts and commands. It will provide you an excellent runtime, which is basically a GEF, enriched with functions related to EMF models. But, yeah - it's a LOT more generated code, yet another monster-framework to leatn and there is even less documentation for GMF then for GEF. – execc Jun 29 '12 at 18:58
  • @execc: I've actually tried it once, by going through a tutorial. Something went wrong in the early stages (something to do with the .ecore file), and I left it at that. Perhaps I *should* give it another shot at some point. – s.d Jun 29 '12 at 19:16
  • @execc - I tried many times to learn GMF and it always became too complex, specially handling changes to the model. That is what I like about GEF - the model and the view are completely separated. – vainolo Jun 30 '12 at 19:39
  • @s.d I went over the code and could not find anything that popped up as an error. :-(. Good luck debugging. – vainolo Jun 30 '12 at 20:04
  • Try using EcoreUtils.delete(eObject) to delete en object from you model. – execc Jul 01 '12 at 15:21
  • @vainolo: I solved it, cf. Edit 4. OMG, WTF, sorry, and thanks! – s.d Jul 02 '12 at 09:32

1 Answers1

0

There is one semi-colon too many in line 5 of the following part of the code, namely after the if statement: if (node.getDiagram() != null);:

1   @Override protected List<EObject> getModelChildren() {
2       List<EObject> allModelObjects = new ArrayList<EObject>();
3       EList<MyNode> nodes = ((Diagram) getModel()).getMyNodes();
4       for (MyNode node : nodes) {
5           if (node.getDiagram() != null); 
6               allModelObjects.add(node);
7       }
8       return allModelObjects;
9    }
Chandrayya G K
  • 8,719
  • 5
  • 40
  • 68
s.d
  • 4,017
  • 5
  • 35
  • 65