[427869] Remove DeleteDiagramElementsTriggerOperation mechanism

DeleteDiagramElementsTriggerOperation was used to remove any
DSemanticDecorator which target semantic elements were deleted by the
external action. We do this in a rather inefficient way:
* it uses a ChangeListener which installs a ContentAdapter on the model;
* the ChangeListener keeps in memory all added/removed/modified
elements;
* after the task's execution we use APIs like
TaskHelper.getDElementToClearFromSemanticElements() which iterate on all
the content of the representation models;
* if a tool executes N different Java actions, we do all of this N
times...

In addition, what this task does is inconsistent with what happens for
other tasks (that use pure ModelOperations):
* it does not take into account the "Automatic Refresh" setting (which
is bad);
* it removes representations whose target element has been removed
(which is good, and should probably be generalized).

The IExternalJavaAction2 interface has also been removed: it was
introduced only to allow for implementers to explicitly declare when
their actions do not delete any element so that we can avoid the costly
DeleteDiagramElementsTriggerOperation in these cases.

The DeleteDRepresentationElementsTask has been modified to delete at
least its representation element if the corresponding semantic element
is
null or does not have a container: now that
DeleteDiagramElementsTriggerOperation has been removed and because
SiriusCommand.getDeletedObject() returns an empty list for
ExternalJavaActionTask, this has been done to avoid the red cross
decorator after a delete from model in manual refresh (if the delete
tool use an external java action to do the delete). This a temporary
behavior to minimize the changes for the more simple cases.

Bug: 427869
Change-Id: I32da343309e798cb5363157de4161fce5c20b616
Signed-off-by: Maxime PORHEL <maxime.porhel@obeo.fr>

6 files changed, 27 insertions, 222 deletions

diff --git a/plugins/org.eclipse.sirius.doc/doc/Release Notes.html b/plugins/org.eclipse.sirius.doc/doc/Release Notes.html
index 9bcf648aa4..a65f221352 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release Notes.html
+++ b/plugins/org.eclipse.sirius.doc/doc/Release Notes.html
@@ -74,6 +74,9 @@
 				<code>org.eclipse.sirius.ecore.extender.business.api.permission.exception.LockedInstanceException</code> now can take several EObjects in constructor and a method 
 				<code>LockedInstanceException.getLockedElements()</code> has been added to get all EObjects put in constructor.
 			</li>
+			<li>The 
+				<code>org.eclipse.sirius.tools.api.ui.IExternalAction2</code> interface has been removed. It was created to disable the change listener when the action indicates it will not delete elements.
+			</li>
 		</ul>
 		<p>
 			<code>org.eclipse.sirius.tools.api.command.InvalidPermissionCommand</code> now can take several EObjects in constructor to throw a 
@@ -1704,4 +1707,4 @@ void removeSelectedView(DView view);
 			</li>
 		</ul>
 	</body>
-</html>
\ No newline at end of file
+</html>
diff --git a/plugins/org.eclipse.sirius.doc/doc/Release Notes.textile b/plugins/org.eclipse.sirius.doc/doc/Release Notes.textile
index 4b38e84954..774dcce020 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release Notes.textile
+++ b/plugins/org.eclipse.sirius.doc/doc/Release Notes.textile
@@ -24,6 +24,7 @@ h4. Changes in @org.eclipse.sirius@
 * The @org.eclipse.sirius.business.api.helper.task.AbstractDeleteDRepresentationElementTask@ class has been renamed into @DeleteEObjectTask@.
 * @org.eclipse.sirius.ecore.extender.business.api.permission.exception.LockedInstanceException@ now can take several EObjects in constructor and a method @LockedInstanceException.getLockedElements()@ has been added to get all EObjects put in constructor.
 @org.eclipse.sirius.tools.api.command.InvalidPermissionCommand@ now can take several EObjects in constructor to throw a @LockedInstanceException@ with several EObjects.
+* The @org.eclipse.sirius.tools.api.ui.IExternalAction2@ interface has been removed. It was created to disable the change listener when the action indicates it will not delete elements.
 
 h4. Changes in @org.eclipse.sirius.common@
 
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/DeleteDRepresentationElementsTask.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/DeleteDRepresentationElementsTask.java
index c3e7910a88..408f0bf4ee 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/DeleteDRepresentationElementsTask.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/DeleteDRepresentationElementsTask.java
@@ -93,6 +93,14 @@ public class DeleteDRepresentationElementsTask extends AbstractCommandTask {
         }
 
         final Set<DSemanticDecorator> vpElements = taskHelper.getDElementToClearFromSemanticElements(root, completeCollection(this.cmd.getDeletedObjects()));
+
+        // In case of ExternalJavaActionTask, the command is not aware of its
+        // deleted elements, but we could at least check if the target of the
+        // current representation element has been deleted.
+        if (repElt.getTarget() == null || repElt.getTarget().eContainer() == null) {
+            vpElements.add(repElt);
+        }
+
         for (DSemanticDecorator semDec : vpElements) {
             tasks.add(new DeleteEObjectTask(semDec, modelAccessor));
         }
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/ModelOperationToTask.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/ModelOperationToTask.java
index 186627304f..f61a9d24b2 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/ModelOperationToTask.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/ModelOperationToTask.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2010 THALES GLOBAL SERVICES.
+ * Copyright (c) 2007, 2014 THALES GLOBAL SERVICES.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -51,9 +51,9 @@ import com.google.common.collect.Collections2;
 
 /**
  * Transform operation object to task.
- *
+ * 
  * @author jdupont
- *
+ * 
  */
 public class ModelOperationToTask implements Function<ModelOperation, ICommandTask> {
 
@@ -80,10 +80,9 @@ public class ModelOperationToTask implements Function<ModelOperation, ICommandTa
      */
     private final CommandContext context;
 
-
     /**
      * Transform model operations to tasks instances.
-     *
+     * 
      * @param extPackage
      *            access to semantic model.
      * @param uiCallback
@@ -103,7 +102,7 @@ public class ModelOperationToTask implements Function<ModelOperation, ICommandTa
 
     /**
      * Create a new task.
-     *
+     * 
      * @param op
      *            the operation
      * @return the created task
@@ -134,9 +133,9 @@ public class ModelOperationToTask implements Function<ModelOperation, ICommandTa
         } else if (op instanceof DeleteView) {
             task = new RemoveElementTask(extPackage, context, (DeleteView) op, interpreter);
         } else if (op instanceof ExternalJavaAction) {
-            task = new ExternalJavaActionTask(context, extPackage, (ExternalJavaAction) op, session, uiCallback);
+            task = new ExternalJavaActionTask(context, extPackage, (ExternalJavaAction) op, interpreter, uiCallback);
         } else if (op instanceof ExternalJavaActionCall) {
-            task = new ExternalJavaActionTask(context, extPackage, ((ExternalJavaActionCall) op).getAction(), session, uiCallback);
+            task = new ExternalJavaActionTask(context, extPackage, ((ExternalJavaActionCall) op).getAction(), interpreter, uiCallback);
         } else if (op instanceof Switch) {
             task = new SwitchTask(context, extPackage, (Switch) op, session, uiCallback);
         }
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/operations/ExternalJavaActionTask.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/operations/ExternalJavaActionTask.java
index 14519ffe89..f40af9cf42 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/operations/ExternalJavaActionTask.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/helper/task/operations/ExternalJavaActionTask.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2010 THALES GLOBAL SERVICES.
+ * Copyright (c) 2008, 2014 THALES GLOBAL SERVICES.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -13,44 +13,22 @@ package org.eclipse.sirius.business.internal.helper.task.operations;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.eclipse.emf.ecore.EObject;
-import org.eclipse.emf.ecore.resource.Resource;
-import org.eclipse.sirius.business.api.dialect.DialectManager;
-import org.eclipse.sirius.business.api.helper.task.DeleteEObjectTask;
-import org.eclipse.sirius.business.api.helper.task.ICommandTask;
-import org.eclipse.sirius.business.api.helper.task.TaskExecutor;
 import org.eclipse.sirius.business.api.helper.task.TaskHelper;
 import org.eclipse.sirius.business.api.logger.RuntimeLoggerInterpreter;
 import org.eclipse.sirius.business.api.logger.RuntimeLoggerManager;
-import org.eclipse.sirius.business.api.query.ResourceQuery;
-import org.eclipse.sirius.business.api.session.Session;
-import org.eclipse.sirius.business.internal.helper.task.DeleteDRepresentationTask;
+import org.eclipse.sirius.common.tools.api.interpreter.IInterpreter;
 import org.eclipse.sirius.ecore.extender.business.api.accessor.ModelAccessor;
 import org.eclipse.sirius.tools.api.command.CommandContext;
-import org.eclipse.sirius.tools.api.command.listener.ChangeListenerFactory;
-import org.eclipse.sirius.tools.api.command.listener.IChangeListener;
-import org.eclipse.sirius.tools.api.command.listener.TriggerOperation;
 import org.eclipse.sirius.tools.api.command.ui.UICallBack;
 import org.eclipse.sirius.tools.api.ui.ExternalJavaActionProvider;
 import org.eclipse.sirius.tools.api.ui.IExternalJavaAction;
-import org.eclipse.sirius.tools.api.ui.IExternalJavaAction2;
-import org.eclipse.sirius.viewpoint.DRepresentation;
-import org.eclipse.sirius.viewpoint.DRepresentationElement;
-import org.eclipse.sirius.viewpoint.DSemanticDecorator;
-import org.eclipse.sirius.viewpoint.SiriusPlugin;
 import org.eclipse.sirius.viewpoint.description.tool.ExternalJavaAction;
 import org.eclipse.sirius.viewpoint.description.tool.ExternalJavaActionParameter;
 import org.eclipse.sirius.viewpoint.description.tool.ToolPackage;
 
-import com.google.common.collect.Iterators;
-import com.google.common.collect.Sets;
-
 /**
  * A task which call an external Java Action.
  * 
@@ -68,8 +46,6 @@ public class ExternalJavaActionTask extends AbstractOperationTask {
 
     private final TaskHelper taskHelper;
 
-    private final Session session;
-
     /**
      * Default constructor.
      * 
@@ -79,16 +55,15 @@ public class ExternalJavaActionTask extends AbstractOperationTask {
      *            the extended package
      * @param op
      *            the operation
-     * @param session
-     *            the {@link Session} to be used to this task
+     * @param interpreter
+     *            the {@link IInterpreter} to be used to this task
      * @param uiCallback
      *            the {@link UICallBack}
      */
-    public ExternalJavaActionTask(final CommandContext context, final ModelAccessor extPackage, final ExternalJavaAction op, final Session session, final UICallBack uiCallback) {
-        super(context, extPackage, session.getInterpreter());
+    public ExternalJavaActionTask(final CommandContext context, final ModelAccessor extPackage, final ExternalJavaAction op, final IInterpreter interpreter, final UICallBack uiCallback) {
+        super(context, extPackage, interpreter);
         externalJavaAction = op;
         this.accessor = extPackage;
-        this.session = session;
         if (context.getCurrentTarget() != null) {
             selections.add(context.getCurrentTarget());
         }
@@ -113,46 +88,8 @@ public class ExternalJavaActionTask extends AbstractOperationTask {
                 final Object value = safeInterpreter.evaluate(context.getCurrentTarget(), parameter, ToolPackage.eINSTANCE.getExternalJavaActionParameter_Value());
                 parameters.put(parameter.getName(), value);
             }
-
-            IChangeListener listener = null;
-            try {
-                if (!(javaAction instanceof IExternalJavaAction2) || ((IExternalJavaAction2) javaAction).mayDeleteElements()) {
-                    listener = ChangeListenerFactory.INSTANCE.getNewChangeListener();
-                    startListenSemanticChange(listener);
-                }
-                javaAction.execute(selections, parameters);
-            } finally {
-                if (listener != null) {
-                    stopListenSemanticChange(listener);
-                }
-            }
-        }
-    }
-
-    private void startListenSemanticChange(IChangeListener listener) {
-
-        listener.setTriggerOperation(new DeleteDiagramElementsTriggerOperation());
-        // Add this listener on each resources that is not VSM or aird files (to
-        // improve perf).
-        for (Resource resource : session.getTransactionalEditingDomain().getResourceSet().getResources()) {
-            ResourceQuery resourceQuery = new ResourceQuery(resource);
-            if (!resourceQuery.isRepresentationsResource() && !resourceQuery.isModelerResource()) {
-                resource.eAdapters().add(listener);
-            }
+            javaAction.execute(selections, parameters);
         }
-        listener.activate();
-    }
-
-    private void stopListenSemanticChange(IChangeListener listener) {
-        listener.launchTriggerOperation();
-        // Remove this listener on each resources that is not VSM or aird files.
-        for (Resource resource : session.getTransactionalEditingDomain().getResourceSet().getResources()) {
-            ResourceQuery resourceQuery = new ResourceQuery(resource);
-            if (!resourceQuery.isRepresentationsResource() && !resourceQuery.isModelerResource()) {
-                resource.eAdapters().remove(listener);
-            }
-        }
-        listener.deactivate();
     }
 
     @Override
@@ -166,102 +103,4 @@ public class ExternalJavaActionTask extends AbstractOperationTask {
 
     }
 
-    /**
-     * This is run at the end of the execution of an {@link IExternalJavaAction}
-     * unless it was an {@link IExternalJavaAction2} which explicitly declared
-     * it never deletes model elements. It is used to remove diagram elements
-     * and diagrams themselves which target semantic elements that were deleted
-     * by the external action.
-     */
-    private class DeleteDiagramElementsTriggerOperation implements TriggerOperation {
-
-        @Override
-        public void run(Collection<Object> createdElements, Collection<Object> modifiedElements, Collection<Object> deletedElements) {
-            if (!deletedElements.isEmpty()) {
-                Set<EObject> allDeletedElements = elementsAndChildren(deletedElements);
-                final Set<DSemanticDecorator> decoratorsToDelete = getDecoratorsInRepresentationToDelete(allDeletedElements);
-                decoratorsToDelete.addAll(getRepresentationsToDelete(allDeletedElements));
-                /*
-                 * if the external java action do a real delete, then
-                 * representation elements will lost their reference to target
-                 * semantic element they decorate
-                 */
-                decoratorsToDelete.addAll(getOrphanedDecorators());
-                getCommand(decoratorsToDelete);
-            }
-        }
-
-        private Set<DSemanticDecorator> getDecoratorsInRepresentationToDelete(Set<EObject> deletedElements) {
-            return taskHelper.getDElementToClearFromSemanticElements(context.getRepresentation(), deletedElements);
-        }
-
-        private Set<DSemanticDecorator> getRepresentationsToDelete(Set<EObject> deletedElements) {
-            final Set<DSemanticDecorator> representationsToDelete = Sets.newHashSet();
-
-            if (context.getRepresentation() instanceof DSemanticDecorator) {
-                for (final DRepresentation representation : DialectManager.INSTANCE.getAllRepresentations(session)) {
-                    if (representation instanceof DSemanticDecorator && deletedElements.contains(((DSemanticDecorator) representation).getTarget())) {
-                        representationsToDelete.add((DSemanticDecorator) representation);
-                    }
-                }
-            }
-            return representationsToDelete;
-        }
-
-        private Set<DSemanticDecorator> getOrphanedDecorators() {
-            Set<DSemanticDecorator> orhanedElements = Sets.newHashSet();
-            final Iterator<EObject> it = context.getRepresentation().eAllContents();
-            while (it.hasNext()) {
-                final EObject eObj = it.next();
-                if (isOrphaned(eObj)) {
-                    orhanedElements.add((DSemanticDecorator) eObj);
-                }
-            }
-            if (context.getRepresentation() instanceof DSemanticDecorator) {
-                for (final DRepresentation representation : DialectManager.INSTANCE.getAllRepresentations(session)) {
-                    if (isOrphaned(representation)) {
-                        orhanedElements.add((DSemanticDecorator) representation);
-                    }
-                }
-            }
-            return orhanedElements;
-        }
-
-        private boolean isOrphaned(EObject eObject) {
-            /*
-             * DFeatureColumn should not be a DSemanticDecorator, until the
-             * change filter instance of that class
-             */
-            return eObject instanceof DSemanticDecorator && ((DSemanticDecorator) eObject).getTarget() == null && !"DFeatureColumn".equals(eObject.eClass().getName());
-        }
-
-        private void getCommand(final Collection<DSemanticDecorator> dElements) {
-            List<ICommandTask> tasks = new ArrayList<ICommandTask>();
-            for (final DSemanticDecorator dElement : dElements) {
-                if (dElement instanceof DRepresentationElement) {
-                    ModelAccessor modelAccessor = SiriusPlugin.getDefault().getModelAccessorRegistry().getModelAccessor(dElement);
-                    DeleteEObjectTask task = new DeleteEObjectTask(dElement, modelAccessor);
-                    tasks.add(task);
-                } else if (dElement instanceof DRepresentation) {
-                    DeleteDRepresentationTask task = new DeleteDRepresentationTask((DRepresentation) dElement);
-                    task.setDeleteIncomingReferences(true);
-                    tasks.add(task);
-                }
-            }
-            TaskExecutor.execute(tasks);
-        }
-
-        private Set<EObject> elementsAndChildren(final Collection<Object> semantics) {
-            final Set<EObject> elementsAndChildren = new LinkedHashSet<EObject>();
-            for (final Object object : semantics) {
-                if (object instanceof EObject) {
-                    final EObject eObject = (EObject) object;
-                    elementsAndChildren.add(eObject);
-                    Iterators.addAll(elementsAndChildren, eObject.eAllContents());
-                }
-            }
-            return elementsAndChildren;
-        }
-    }
-
 }
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/tools/api/ui/IExternalJavaAction2.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/tools/api/ui/IExternalJavaAction2.java
deleted file mode 100644
index 5b4687295c..0000000000
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/tools/api/ui/IExternalJavaAction2.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2012 THALES GLOBAL SERVICES.
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- *    Obeo - initial API and implementation
- *******************************************************************************/
-package org.eclipse.sirius.tools.api.ui;
-
-/**
- * This class allows the specifier to warn Sirius about the kind of
- * modification will be done during the execution of this
- * {@link IExternalJavaAction2}. As it is executed externally of Sirius,
- * {@link ChangeListener} are used to keep Sirius up-to-date. Depending on
- * the result of the provided method, the specifier will be able to warn
- * Sirius the kind of modification will be done on semantic models.
- * Therefore, Sirius will be able to decide if a {@link ChangeListener} will
- * be needed, which will have a positive impact on performances.<br/>
- * <br/>
- * 
- * Note : <a href=
- * "http://wiki.eclipse.org/Evolving_Java-based_APIs_3#.222.22_Convention"
- * >Naming Convention</a>
- * 
- * @author <a href="mailto:steve.monnier@obeo.fr">Steve Monnier</a>
- */
-public interface IExternalJavaAction2 extends IExternalJavaAction {
-
-    /**
-     * This method determine if the execution of this
-     * {@link IExternalJavaAction2} will delete elements. This information will
-     * be used to decide if a {@link ChangeListener} will be needed for element
-     * deletion. Returning false will increase performances, but the execution
-     * of the external java action MUST NOT delete any elements to avoid further
-     * issues.
-     * 
-     * @return if the execution of this {@link IExternalJavaAction2} will delete
-     *         any elements.
-     */
-    boolean mayDeleteElements();
-
-}