diff options
author | Christian W. Damus | 2014-06-12 20:00:23 +0000 |
---|---|---|
committer | Christian W. Damus | 2014-07-21 15:04:58 +0000 |
commit | b254781fb01527bdab7a2bd6357825166f7e6b26 (patch) | |
tree | c412cbf81add7bb6676f986b49fc0d7732cde5dc /plugins/uml | |
parent | 029e4b677bf9ad7a919565c55d1c4ed3128e5cb4 (diff) | |
download | org.eclipse.papyrus-b254781fb01527bdab7a2bd6357825166f7e6b26.tar.gz org.eclipse.papyrus-b254781fb01527bdab7a2bd6357825166f7e6b26.tar.xz org.eclipse.papyrus-b254781fb01527bdab7a2bd6357825166f7e6b26.zip |
431618: [Model Validation View] Problems on deleted elements become zombies
https://bugs.eclipse.org/bugs/show_bug.cgi?id=431618
Add a destroy-elements advice that deletes markers attached to the object being deleted and recreates them on undo. Each marker provider implementation provides an undoable marker-deletion command optimized for its storage domain.
Work around bugs in the edit infrastructure exposed by the new marker deletion advice:
- ChangeDescriptions do not record sufficient information to correctly redo deletion of elements from subset lists that have changeable supersets (not derived unions)
- visual element types specializing the GMF null element type discard the core editing command
Change-Id: Icca5e0332320f7adabd781c71cf81fd06a3bcc34
Diffstat (limited to 'plugins/uml')
3 files changed, 199 insertions, 4 deletions
diff --git a/plugins/uml/org.eclipse.papyrus.uml.service.types/plugin.xml b/plugins/uml/org.eclipse.papyrus.uml.service.types/plugin.xml index 2ffb172c335..470d733c637 100644 --- a/plugins/uml/org.eclipse.papyrus.uml.service.types/plugin.xml +++ b/plugins/uml/org.eclipse.papyrus.uml.service.types/plugin.xml @@ -1981,6 +1981,12 @@ <extension point="org.eclipse.gmf.runtime.emf.type.core.elementTypes"> <metamodel nsURI="http://www.eclipse.org/uml2/5.0.0/UML"> + + <!-- Helper advice for Activity --> + <adviceBinding id="org.eclipse.papyrus.uml.advice.ContainmentSubsetRemoval" + class="org.eclipse.papyrus.uml.service.types.helper.advice.ContainmentSubsetRemovalAdvice" + inheritance="all" typeId="org.eclipse.papyrus.uml.Element"> + </adviceBinding> <!-- Helper advice for Classifier --> <adviceBinding id="org.eclipse.papyrus.uml.advice.Classifier" @@ -2388,6 +2394,8 @@ <elementType ref="org.eclipse.papyrus.uml.Association" /> <elementType ref="org.eclipse.papyrus.uml.Trace" /> <elementType ref="org.eclipse.papyrus.uml.Refine" /> + + <advice ref="org.eclipse.papyrus.uml.advice.ContainmentSubsetRemoval" /> <advice ref="org.eclipse.papyrus.uml.advice.Classifier" /> <advice ref="org.eclipse.papyrus.uml.advice.Collaboration" /> <advice ref="org.eclipse.papyrus.uml.advice.CollaborationUse" /> diff --git a/plugins/uml/org.eclipse.papyrus.uml.service.types/src/org/eclipse/papyrus/uml/service/types/helper/advice/ContainmentSubsetRemovalAdvice.java b/plugins/uml/org.eclipse.papyrus.uml.service.types/src/org/eclipse/papyrus/uml/service/types/helper/advice/ContainmentSubsetRemovalAdvice.java new file mode 100644 index 00000000000..871314baba8 --- /dev/null +++ b/plugins/uml/org.eclipse.papyrus.uml.service.types/src/org/eclipse/papyrus/uml/service/types/helper/advice/ContainmentSubsetRemovalAdvice.java @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2014 CEA and others. + * + * 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: + * Christian W. Damus (CEA) - Initial API and implementation + * + */ +package org.eclipse.papyrus.uml.service.types.helper.advice; + +import java.util.Collection; + +import org.eclipse.core.commands.ExecutionException; +import org.eclipse.core.runtime.IAdaptable; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.ecore.EReference; +import org.eclipse.gmf.runtime.common.core.command.AbstractCommand; +import org.eclipse.gmf.runtime.common.core.command.CommandResult; +import org.eclipse.gmf.runtime.common.core.command.ICommand; +import org.eclipse.gmf.runtime.emf.type.core.edithelper.AbstractEditHelperAdvice; +import org.eclipse.gmf.runtime.emf.type.core.requests.DestroyElementRequest; +import org.eclipse.papyrus.uml.tools.model.UmlUtils; + +/** + * An advisor of element destruction that works around a problem in the redo of removal of elements from + * containment references that are subsets. The change-description that is applied by the nesting + * transactional command doesn't account for the subset-superset relationship, so we inject a command + * that makes sure to clean up supersets on redo (undo is not a problem because the addition of an + * element to the subset always internally updates the supersets). + */ +public class ContainmentSubsetRemovalAdvice extends AbstractEditHelperAdvice { + + // This parameter is set by generated semantic edit policies when delegating to "visual element types" to + // decorate the semantic element command with advice targeting the visual element type, which is a hinted + // element type specializing the semantic element type to suggest the way in which it should be presented + // in the diagram. Because our advice is already included in the semantic command, it isn't needed in + // this additional decoration and, therefore, it won't cause problems when the visual element type is + // a specialization (dubiously) of the null element type + // (see https://www.eclipse.org/forums/index.php/t/781825/ for discussion of that problem) + private static final String EDIT_POLICY_COMMAND = "edit policy command"; //$NON-NLS-1$ + + public ContainmentSubsetRemovalAdvice() { + super(); + } + + @Override + protected ICommand getAfterDestroyElementCommand(DestroyElementRequest request) { + ICommand result = super.getAfterDestroyElementCommand(request); + + if(request.getParameter(EDIT_POLICY_COMMAND) != null) { + return result; + } + + final EObject destructee = request.getElementToDestroy(); + + EReference containment = destructee.eContainmentFeature(); + final Collection<EReference> supersets = !UmlUtils.isSubset(containment) ? null : UmlUtils.getAllChangeableSupersets(containment); + + if(supersets != null) { + // Add a command that ensures removal from the supersets on redo, if necessary + final EObject container = destructee.eContainer(); + + ICommand advice = new AbstractCommand("Ensure supersets") { + + @Override + protected CommandResult doRedoWithResult(IProgressMonitor progressMonitor, IAdaptable info) throws ExecutionException { + // Ensure sanity of the supersets, which redoing the recorded ChangeDescription does not + for(EReference superset : supersets) { + remove(container, superset, destructee); + } + + return CommandResult.newOKCommandResult(); + } + + private void remove(EObject owner, EReference reference, EObject object) { + if(reference.isMany()) { + ((Collection<?>)owner.eGet(reference)).remove(object); + } else if(owner.eGet(reference) == object) { + owner.eUnset(reference); + } + } + + @Override + protected CommandResult doExecuteWithResult(IProgressMonitor progressMonitor, IAdaptable info) throws ExecutionException { + // Initial execution is never a problem + return CommandResult.newOKCommandResult(); + } + + @Override + protected CommandResult doUndoWithResult(IProgressMonitor progressMonitor, IAdaptable info) throws ExecutionException { + // Undoing the deletion is never a problem + return CommandResult.newOKCommandResult(); + } + }; + + result = (result == null) ? advice : result.compose(advice); + } + + return result; + } +} diff --git a/plugins/uml/tools/org.eclipse.papyrus.uml.tools/src/org/eclipse/papyrus/uml/tools/model/UmlUtils.java b/plugins/uml/tools/org.eclipse.papyrus.uml.tools/src/org/eclipse/papyrus/uml/tools/model/UmlUtils.java index d0a6cffb8d1..3235ce814aa 100644 --- a/plugins/uml/tools/org.eclipse.papyrus.uml.tools/src/org/eclipse/papyrus/uml/tools/model/UmlUtils.java +++ b/plugins/uml/tools/org.eclipse.papyrus.uml.tools/src/org/eclipse/papyrus/uml/tools/model/UmlUtils.java @@ -1,8 +1,27 @@ -/**
- *
- */
+/*****************************************************************************
+ * Copyright (c) 2011, 2014 LIFL, CEA, and others.
+ *
+ * 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:
+ * LIFL - Initial API and implementation
+ * Christian W. Damus (CEA) - bug 431618
+ *
+ *****************************************************************************/
package org.eclipse.papyrus.uml.tools.model;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+import org.eclipse.emf.ecore.EAnnotation;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EReference;
import org.eclipse.emf.ecore.resource.Resource;
import org.eclipse.papyrus.infra.core.resource.ModelSet;
import org.eclipse.papyrus.infra.core.resource.ModelUtils;
@@ -19,7 +38,8 @@ import org.eclipse.papyrus.uml.tools.Activator; *
*/
public class UmlUtils {
-
+ private static final String ANNOTATION_SUBSETS = "subsets"; //$NON-NLS-1$
+
/**
* Gets the UmlModel for the currently selected editor. <br>
* Warning: This method is designed to be call from ui.handlers. It is not
@@ -126,4 +146,65 @@ public class UmlUtils { return null;
}
+ public static Collection<EReference> getAllChangeableSupersets(EReference subset) {
+ Collection<EReference> result = null;
+
+ // null has no supersets
+ EAnnotation supersets = (subset == null) ? null : subset.getEAnnotation(ANNOTATION_SUBSETS);
+ if(supersets != null) {
+ result = collectChangeableSupersets(supersets.getReferences(), new HashSet<EReference>());
+ }
+
+ return (result == null) ? Collections.<EReference>emptyList() : result;
+ }
+
+ private static Collection<EReference> collectChangeableSupersets(Collection<EObject> supersets, Set<EReference> result) {
+ for(EObject next : supersets) {
+ if(next instanceof EReference) {
+ EReference superset = (EReference)next;
+ if(superset.isChangeable() && result.add(superset)) {
+ EAnnotation recursive = (superset == null) ? null : superset.getEAnnotation(ANNOTATION_SUBSETS);
+ if(recursive != null) {
+ collectChangeableSupersets(recursive.getReferences(), result);
+ }
+ }
+ }
+ }
+
+ return result;
+ }
+
+ public static boolean isSubset(EReference subset) {
+ boolean result = false;
+
+ // null is not a subset of anything
+ EAnnotation supersets = (subset == null) ? null : subset.getEAnnotation(ANNOTATION_SUBSETS);
+ if(supersets != null) {
+ result = !supersets.getReferences().isEmpty();
+ }
+
+ return result;
+ }
+
+ public static boolean isSubsetOf(EReference subset, EReference superset) {
+ boolean result = false;
+
+ // null is not a subset of anything
+ EAnnotation supersets = (subset == null) ? null : subset.getEAnnotation(ANNOTATION_SUBSETS);
+ if(supersets != null) {
+ result = supersets.getReferences().contains(superset);
+ if(!result) {
+ // Look for transitive subset, which is at least plausible
+ // considering that we do have some superset
+ for(Iterator<EObject> iter = supersets.getReferences().iterator(); !result && iter.hasNext();) {
+ EObject next = iter.next();
+ if(next instanceof EReference) {
+ result = isSubsetOf((EReference)next, superset);
+ }
+ }
+ }
+ }
+
+ return result;
+ }
}
|