Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian W. Damus2016-09-22 11:52:07 -0400
committerGerrit Code Review @ Eclipse.org2016-10-07 04:34:49 -0400
commitc6273756089c46770af1f38e8e0c543d7c4a27c7 (patch)
tree009e6f7c851342dfbd3044c265a688049f1ed3ba /plugins
parent721ef7d021121732a8ec45f5df6daab3ee4abce7 (diff)
downloadorg.eclipse.papyrus-c6273756089c46770af1f38e8e0c543d7c4a27c7.tar.gz
org.eclipse.papyrus-c6273756089c46770af1f38e8e0c543d7c4a27c7.tar.xz
org.eclipse.papyrus-c6273756089c46770af1f38e8e0c543d7c4a27c7.zip
Bug 501946: Undo creation of capsule with structure diagram open causes exception to be thrown
https://bugs.eclipse.org/bugs/show_bug.cgi?id=501946 Ensure that, when a notation view is removed from the notation resource, then if any editor pages open on it are not closed deliberately, they are closed implicitly. Also do not attempt to refresh a diagram edit part that has lots its semantic element, on the assumption that it is being deleted. In order to reuse the asynchronous executor for an editing domain, factor it out of the DiagramHelper into a more suitable common API. (cherry-picked from streams/2.0-maintenance) Change-Id: Ic0b59ff9b3475f8ab4e7d852b0719c9ba479758b
Diffstat (limited to 'plugins')
-rw-r--r--plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF6
-rw-r--r--plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/plugin.xml4
-rw-r--r--plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/editpart/PapyrusDiagramEditPart.java30
-rw-r--r--plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/helper/DiagramHelper.java50
-rw-r--r--plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/internal/common/model/NotationPageSnippet.java127
-rw-r--r--plugins/infra/ui/org.eclipse.papyrus.infra.ui/src/org/eclipse/papyrus/infra/ui/util/TransactionUIHelper.java61
6 files changed, 216 insertions, 62 deletions
diff --git a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF
index 84840f206fb..3d174ab26b1 100644
--- a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF
+++ b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/META-INF/MANIFEST.MF
@@ -44,7 +44,8 @@ Export-Package: org.eclipse.papyrus.infra.gmfdiag.common,
org.eclipse.papyrus.infra.gmfdiag.common.undocontext,
org.eclipse.papyrus.infra.gmfdiag.common.updater,
org.eclipse.papyrus.infra.gmfdiag.common.utils,
- org.eclipse.papyrus.infra.gmfdiag.internal.common.expressions;x-internal:=true
+ org.eclipse.papyrus.infra.gmfdiag.internal.common.expressions;x-internal:=true,
+ org.eclipse.papyrus.infra.gmfdiag.internal.common.model;x-internal:=true
Require-Bundle: org.eclipse.emf.ecore.edit;bundle-version="[2.9.0,3.0.0)",
org.eclipse.gmf.runtime.diagram.ui.resources.editor;bundle-version="[1.7.0,2.0.0)";visibility:=reexport,
org.eclipse.papyrus.infra.emf.appearance;bundle-version="[1.2.0,2.0.0)",
@@ -65,7 +66,8 @@ Require-Bundle: org.eclipse.emf.ecore.edit;bundle-version="[2.9.0,3.0.0)",
org.eclipse.papyrus.infra.types.core;bundle-version="[3.0.0,4.0.0)";visibility:=reexport,
org.eclipse.papyrus.infra.sync;bundle-version="[1.2.0,2.0.0)";visibility:=reexport,
org.eclipse.papyrus.infra.services.edit.ui;bundle-version="[3.0.0,4.0.0)";visibility:=reexport,
- org.eclipse.papyrus.infra.emf.gmf
+ org.eclipse.papyrus.infra.emf.gmf,
+ org.eclipse.papyrus.infra.ui;bundle-version="[2.0.0,3.0.0)"
Bundle-Vendor: %providerName
Bundle-ActivationPolicy: lazy
Bundle-ClassPath: .
diff --git a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/plugin.xml b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/plugin.xml
index f9ef42b7cb7..e0381ecdba3 100644
--- a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/plugin.xml
+++ b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/plugin.xml
@@ -84,6 +84,10 @@
classname="org.eclipse.papyrus.infra.core.resource.AdjunctResourceModelSnippet"
description="Snippet for notation resource of referenced model resources">
</modelSnippet>
+ <modelSnippet
+ classname="org.eclipse.papyrus.infra.gmfdiag.internal.common.model.NotationPageSnippet"
+ description="Snippet that cleans up editor pages for deleted notation views">
+ </modelSnippet>
</model>
</extension>
diff --git a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/editpart/PapyrusDiagramEditPart.java b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/editpart/PapyrusDiagramEditPart.java
index 8484ba3bbfc..9810d54c940 100644
--- a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/editpart/PapyrusDiagramEditPart.java
+++ b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/editpart/PapyrusDiagramEditPart.java
@@ -1,5 +1,5 @@
/*****************************************************************************
- * Copyright (c) 2012, 2015 CEA LIST, Christian W. Damus, and others.
+ * Copyright (c) 2012, 2016 CEA LIST, Christian W. Damus, and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -10,8 +10,7 @@
* Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation
* Christian W. Damus (CEA) - support pluggable edit-part conflict detection (CDO)
* C´┐Żline Janssens (ALL4TEC) - Override getDragTracker with the PapyrusRubberbandDragTracker
- * Christian W. Damus - bug 451230
- * Christian W. Damus - bug 461629
+ * Christian W. Damus - bugs 451230, 461629, 501946
* Mickael ADAM (ALL4TEC) mickael.adam@all4tec.net - add refresh of SVGPostProcessor - Bug 467569
*
*****************************************************************************/
@@ -78,24 +77,21 @@ public class PapyrusDiagramEditPart extends DiagramEditPart {
}
}
- /**
- * Refresh.
- *
- * @see org.eclipse.gmf.runtime.diagram.ui.editparts.GraphicalEditPart#refresh()
- */
@Override
public void refresh() {
- if (SVGPostProcessor.instance instanceof IRefreshHandlerPart) {
- IEditorPart activeEditor = null;
- try {
- IMultiDiagramEditor multiDiagramEditor = ServiceUtilsForEditPart.getInstance().getService(IMultiDiagramEditor.class, this);
- activeEditor = multiDiagramEditor.getActiveEditor();
- } catch (ServiceException e) {
- Activator.log.error(e);
+ if (hasNotationView() && (getNotationView().eResource() != null)) {
+ if (SVGPostProcessor.instance instanceof IRefreshHandlerPart) {
+ IEditorPart activeEditor = null;
+ try {
+ IMultiDiagramEditor multiDiagramEditor = ServiceUtilsForEditPart.getInstance().getService(IMultiDiagramEditor.class, this);
+ activeEditor = multiDiagramEditor.getActiveEditor();
+ } catch (ServiceException e) {
+ Activator.log.error(e);
+ }
+ ((IRefreshHandlerPart) SVGPostProcessor.instance).refresh(activeEditor);
}
- ((IRefreshHandlerPart) SVGPostProcessor.instance).refresh(activeEditor);
+ super.refresh();
}
- super.refresh();
}
/**
diff --git a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/helper/DiagramHelper.java b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/helper/DiagramHelper.java
index 9f7a6a6b54b..cab10bc5fff 100644
--- a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/helper/DiagramHelper.java
+++ b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/common/helper/DiagramHelper.java
@@ -8,7 +8,7 @@
*
* Contributors:
* Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation
- * Christian W. Damus - bugs 433206, 461629, 466997, 478556, 485220
+ * Christian W. Damus - bugs 433206, 461629, 466997, 478556, 485220, 501946
*
*****************************************************************************/
package org.eclipse.papyrus.infra.gmfdiag.common.helper;
@@ -30,21 +30,16 @@ import org.eclipse.gmf.runtime.diagram.ui.parts.DiagramEditor;
import org.eclipse.papyrus.infra.core.sasheditor.editor.ISashWindowsContainer;
import org.eclipse.papyrus.infra.core.services.ServiceException;
import org.eclipse.papyrus.infra.core.services.ServicesRegistry;
-import org.eclipse.papyrus.infra.core.utils.IExecutorPolicy;
import org.eclipse.papyrus.infra.core.utils.ServiceUtils;
-import org.eclipse.papyrus.infra.core.utils.TransactionHelper;
import org.eclipse.papyrus.infra.gmfdiag.common.Activator;
import org.eclipse.papyrus.infra.gmfdiag.common.editpolicies.IPapyrusCanonicalEditPolicy;
import org.eclipse.papyrus.infra.gmfdiag.common.utils.DiagramEditPartsUtil;
import org.eclipse.papyrus.infra.ui.editor.IMultiDiagramEditor;
import org.eclipse.papyrus.infra.ui.util.EditorUtils;
-import org.eclipse.papyrus.infra.ui.util.UIUtil;
+import org.eclipse.papyrus.infra.ui.util.TransactionUIHelper;
import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.IEditorPart;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.MapMaker;
import com.google.common.util.concurrent.ListenableFuture;
@@ -56,33 +51,6 @@ public class DiagramHelper {
private static final ConcurrentMap<DiagramEditPart, Boolean> pendingDiagramRefresh = new MapMaker().concurrencyLevel(4).weakKeys().makeMap();
- private static final Executor uiExecutor = UIUtil.createUIExecutor(Display.getDefault());
-
- // Don't need weak values because the executor doesn't retain a reference to the domain
- private static final LoadingCache<TransactionalEditingDomain, Executor> domainExecutors = CacheBuilder.newBuilder().weakKeys().build(new CacheLoader<TransactionalEditingDomain, Executor>() {
- @Override
- public Executor load(TransactionalEditingDomain domain) {
- // Diagram refreshes must happen on the UI thread, so we must exclude the transaction
- // executor, itself, in the case that the transaction is not running on the UI thread
- IExecutorPolicy policy = new IExecutorPolicy() {
- @Override
- public Ranking rank(Runnable task, Executor executor) {
- if (executor == uiExecutor) {
- // Always OK to fall back to the UI-thread executor
- return Ranking.ACCEPTABLE;
- } else {
- // The case of the transaction executor
- return (Display.getCurrent() == null) ? Ranking.DEPRECATED : Ranking.PREFERRED;
- }
- }
- };
-
- // Edit-parts will be asked to refresh, and they would do this in read-only transaction, which subsequently
- // requires canonical edit policies invoked recursively to run unprotected transactions, breaking undo/redo
- return TransactionHelper.createTransactionExecutor(domain, uiExecutor, policy, TransactionHelper.mergeReadOnlyOption(true));
- }
- });
-
public static void refresh(EditPart editPart, boolean recursive) {
editPart.refresh();
@@ -198,8 +166,9 @@ public class DiagramHelper {
}
private static Executor getExecutor(DiagramEditPart diagram) {
- TransactionalEditingDomain domain = diagram.getEditingDomain();
- return (domain != null) ? domainExecutors.getUnchecked(domain) : uiExecutor;
+ TransactionalEditingDomain domain = (diagram == null) ? null : diagram.getEditingDomain();
+ // There is a default executor for the null domain
+ return TransactionUIHelper.getExecutor(domain);
}
/**
@@ -307,11 +276,7 @@ public class DiagramHelper {
*/
public static void asyncExec(EditPart context, Runnable task) {
DiagramEditPart diagram = DiagramEditPartsUtil.getDiagramEditPart(context);
- if (diagram != null) {
- asyncExec(diagram, task);
- } else {
- uiExecutor.execute(task);
- }
+ getExecutor(diagram).execute(task);
}
/**
@@ -344,7 +309,8 @@ public class DiagramHelper {
result = submit(diagram, task);
} else {
ListenableFutureTask<V> runnable = ListenableFutureTask.create(task);
- uiExecutor.execute(runnable);
+ // Yes, 'diagram' is null and that is okay
+ getExecutor(diagram).execute(runnable);
result = runnable;
}
diff --git a/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/internal/common/model/NotationPageSnippet.java b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/internal/common/model/NotationPageSnippet.java
new file mode 100644
index 00000000000..b43beac7523
--- /dev/null
+++ b/plugins/infra/gmfdiag/org.eclipse.papyrus.infra.gmfdiag.common/src/org/eclipse/papyrus/infra/gmfdiag/internal/common/model/NotationPageSnippet.java
@@ -0,0 +1,127 @@
+/*****************************************************************************
+ * Copyright (c) 2016 Christian W. Damus 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 - Initial API and implementation
+ *
+ *****************************************************************************/
+
+package org.eclipse.papyrus.infra.gmfdiag.internal.common.model;
+
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.transaction.ResourceSetListener;
+import org.eclipse.emf.transaction.TransactionalEditingDomain;
+import org.eclipse.papyrus.infra.core.resource.EMFLogicalModel;
+import org.eclipse.papyrus.infra.core.resource.IModel;
+import org.eclipse.papyrus.infra.core.resource.IModelSnippet;
+import org.eclipse.papyrus.infra.core.resource.ModelSet;
+import org.eclipse.papyrus.infra.core.resource.ResourceAdapter;
+import org.eclipse.papyrus.infra.core.sashwindows.di.service.IPageManager;
+import org.eclipse.papyrus.infra.core.services.ServiceException;
+import org.eclipse.papyrus.infra.emf.gmf.util.GMFUnsafe;
+import org.eclipse.papyrus.infra.emf.utils.ServiceUtilsForResourceSet;
+import org.eclipse.papyrus.infra.gmfdiag.common.Activator;
+import org.eclipse.papyrus.infra.ui.util.TransactionUIHelper;
+
+/**
+ * A snippet on the notation {@link IModel} that ensures
+ * closure of editor pages when notation views (diagrams, tables)
+ * are removed by sneaky means (such as undo).
+ */
+public class NotationPageSnippet implements IModelSnippet {
+
+ private EMFLogicalModel model;
+ private TransactionalEditingDomain domain;
+ private ResourceSetListener listener;
+ private IPageManager pageManager;
+
+ /**
+ * Initializes me.
+ */
+ public NotationPageSnippet() {
+ super();
+ }
+
+ /**
+ * If the {@link startingModel} has a resource, attach
+ * a listener to the editing domain that cleans up editor pages
+ * for notations that are deleted.
+ */
+ @Override
+ public void start(IModel startingModel) {
+ if (startingModel instanceof EMFLogicalModel) {
+ model = (EMFLogicalModel) startingModel;
+ Resource resource = model.getResource();
+ if (resource != null) {
+ // Of course it's a model-set. I am a model snippet
+ ModelSet modelSet = (ModelSet) resource.getResourceSet();
+ domain = modelSet.getTransactionalEditingDomain();
+ if (domain != null) {
+ listener = createNotationDeletionListener();
+ domain.addResourceSetListener(listener);
+ }
+ }
+ }
+ }
+
+ @Override
+ public void dispose(IModel stoppingModel) {
+ if (stoppingModel == model) {
+ try {
+ if ((domain != null) && (listener != null)) {
+ domain.removeResourceSetListener(listener);
+ }
+ } finally {
+ pageManager = null;
+ listener = null;
+ domain = null;
+ model = null;
+ }
+ }
+ }
+
+ private ResourceSetListener createNotationDeletionListener() {
+ return new ResourceAdapter.Transactional() {
+ @Override
+ protected void handleRootRemoved(Resource resource, EObject root) {
+ if (model.getResources().contains(resource)) {
+ if (pageManager == null) {
+ try {
+ pageManager = ServiceUtilsForResourceSet.getInstance().getIPageManager(domain.getResourceSet());
+ } catch (ServiceException e) {
+ // No page manager? Then we have nothing to do
+ domain.removeResourceSetListener(this);
+ return;
+ }
+ }
+
+ TransactionUIHelper.getExecutor(domain).execute(() -> {
+ if (pageManager.isOpen(root)) {
+ try {
+ // Need an unsafe write in this context but that
+ // is okay because we never record edits in the
+ // sash model for undo/redo anyways
+ GMFUnsafe.write(domain, () -> {
+ // Because the diagram/table/whatever is deleted
+ // from the resource, any and all pages that were
+ // showing it must be closed. Usually a notation
+ // is only opened in at most one page in the editor,
+ // but the API allows for more than one
+ pageManager.closeAllOpenedPages(root);
+ });
+ } catch (Exception e) {
+ Activator.log.error(e);
+ }
+ }
+ });
+ }
+ }
+ };
+ }
+}
diff --git a/plugins/infra/ui/org.eclipse.papyrus.infra.ui/src/org/eclipse/papyrus/infra/ui/util/TransactionUIHelper.java b/plugins/infra/ui/org.eclipse.papyrus.infra.ui/src/org/eclipse/papyrus/infra/ui/util/TransactionUIHelper.java
index 6ee4ed2b14c..92c977dce87 100644
--- a/plugins/infra/ui/org.eclipse.papyrus.infra.ui/src/org/eclipse/papyrus/infra/ui/util/TransactionUIHelper.java
+++ b/plugins/infra/ui/org.eclipse.papyrus.infra.ui/src/org/eclipse/papyrus/infra/ui/util/TransactionUIHelper.java
@@ -8,12 +8,13 @@
*
* Contributors:
* Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation
- * Christian W. Damus - bugs 465416, 498140
+ * Christian W. Damus - bugs 465416, 498140, 501946
*
*****************************************************************************/
package org.eclipse.papyrus.infra.ui.util;
import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.Executor;
import java.util.function.Consumer;
import org.eclipse.core.runtime.IProgressMonitor;
@@ -21,6 +22,14 @@ import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.emf.common.util.WrappedException;
import org.eclipse.emf.transaction.TransactionalEditingDomain;
import org.eclipse.jface.operation.IRunnableWithProgress;
+import org.eclipse.papyrus.infra.core.utils.IExecutorPolicy;
+import org.eclipse.papyrus.infra.core.utils.TransactionHelper;
+import org.eclipse.papyrus.infra.tools.util.CoreExecutors;
+import org.eclipse.swt.widgets.Display;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
/**
@@ -30,6 +39,56 @@ import org.eclipse.jface.operation.IRunnableWithProgress;
*/
public class TransactionUIHelper {
+ private static final Executor uiExecutor = CoreExecutors.getUIExecutorService();
+
+ // Don't need weak values because the executor doesn't retain a reference to the domain
+ private static final LoadingCache<TransactionalEditingDomain, Executor> domainExecutors = CacheBuilder.newBuilder().weakKeys().build(new CacheLoader<TransactionalEditingDomain, Executor>() {
+ @Override
+ public Executor load(TransactionalEditingDomain domain) {
+ // Diagram refreshes must happen on the UI thread, so we must exclude the transaction
+ // executor, itself, in the case that the transaction is not running on the UI thread
+ IExecutorPolicy policy = new IExecutorPolicy() {
+ @Override
+ public Ranking rank(Runnable task, Executor executor) {
+ if (executor == uiExecutor) {
+ // Always OK to fall back to the UI-thread executor
+ return Ranking.ACCEPTABLE;
+ } else {
+ // The case of the transaction executor
+ return (Display.getCurrent() == null) ? Ranking.DEPRECATED : Ranking.PREFERRED;
+ }
+ }
+ };
+
+ // Edit-parts will be asked to refresh, and they would do this in read-only transaction, which subsequently
+ // requires canonical edit policies invoked recursively to run unprotected transactions, breaking undo/redo
+ return TransactionHelper.createTransactionExecutor(domain, uiExecutor, policy, TransactionHelper.mergeReadOnlyOption(true));
+ }
+ });
+
+ /**
+ * Obtains an executor for asynchronous execution of UI updates (such as
+ * refreshes) in the context of a transactional editing domain. This executor
+ * is optimized to minimally postpone any task: it is run at commit of the
+ * current transaction, if the UI thread currently has a transaction active,
+ * otherwise as usually as an {@linkplain Display#asyncExec(Runnable) async runnable}
+ * on the UI thread.
+ *
+ * @param domain
+ * an editing domain (may be {@code null})
+ * @return an asynchronous executor for the {@code domain}. If the
+ * {@code domain} is {@code null}, a UI-thread executor is returned.
+ * Exactly one executor exists per {@code domain} (including the
+ * {@code null} domain)
+ *
+ * @since 2.0
+ */
+ public static Executor getExecutor(TransactionalEditingDomain domain) {
+ return (domain != null)
+ ? domainExecutors.getUnchecked(domain)
+ : uiExecutor;
+ }
+
/**
* <p>
* Create a privileged runnable with progress, which is like a regular {@linkplain TransactionalEditingDomain#createPrivilegedRunnable(Runnable)

Back to the top