Bug 559267 - ConditionalDeleteService assumes containment references
- add a canRemove(...) API to the ConditionalDeleteService
- use it instead of canDelete(...) where appropriate
Change-Id: I488060ca9f4ce2217722292fc8d16600c136e96b
Signed-off-by: Christian W. Damus <give.a.damus@gmail.com>
diff --git a/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/ConditionalDeleteService.java b/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/ConditionalDeleteService.java
index b65f506..a74c0d3 100644
--- a/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/ConditionalDeleteService.java
+++ b/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/ConditionalDeleteService.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2019 Christian W. Damus and others.
+ * Copyright (c) 2019-2020 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 2.0
@@ -51,14 +51,49 @@
boolean canDelete(Iterable<?> objects);
/**
+ * Query whether an {@code object} in some {@code reference} feature of an {@code owner} of that
+ * feature can be removed from the {@code reference} feature. In the case of containment references,
+ * this amounts to {@linkplain #canDelete(Object) deletion} of the {@code object}.
+ *
+ * @param owner the owner of a {@code reference} to some {@code object}
+ * @param reference a reference feature of the {@code owner}
+ * @param object an object referenced by the {@code owner} from which it is to be removed
+ *
+ * @return whether the {@code object} may be removed
+ *
+ * @since 1.24
+ */
+ default boolean canRemove(Object owner, Object reference, Object object) {
+ return canRemove(owner, reference, Collections.singleton(object));
+ }
+
+ /**
+ * Query whether <em>all</em> of the given {@code objects} in some {@code reference} feature of an {@code owner} of
+ * that
+ * feature can be removed from the {@code reference} feature. In the case of containment references,
+ * this amounts to {@linkplain #canDelete(Iterable) deletion} of the {@code object}.
+ *
+ * @param owner the owner of a {@code reference} to some {@code object}
+ * @param reference a reference feature of the {@code owner}
+ * @param objects a group of objects referenced by the {@code owner} from which they are to be removed
+ *
+ * @return whether the {@code objects} may be removed
+ *
+ * @since 1.24
+ */
+ default boolean canRemove(Object owner, Object reference, Iterable<?> objects) {
+ return canDelete(objects);
+ }
+
+ /**
* Obtain a conditional delete service for a given {@code context}, if necessary
* {@linkplain #adapt(DeleteService) adapting} its service implementation.
*
* @param context a form context
* @return the delete service, or an adapter if the actual delete service implementation does not provide
- * the {@link ConditionalDeleteService} protocol or if there isn't a delete service at all (in which case nothing can
- * be deleted)
- *
+ * the {@link ConditionalDeleteService} protocol or if there isn't a delete service at all (in which case
+ * nothing can be deleted)
+ *
* @see #adapt(DeleteService)
*/
static ConditionalDeleteService getDeleteService(EMFFormsViewContext context) {
@@ -67,8 +102,8 @@
/**
* Obtain a view of a given {@code service} as conditional delete service, if necessary adapting a service
- * implementation that does not provide the {@link ConditionalDeleteService} protocol with a default deletion condition.
- * In that case, an object is assumed to be deletable if it is not {@code null} and it is an
+ * implementation that does not provide the {@link ConditionalDeleteService} protocol with a default deletion
+ * condition. In that case, an object is assumed to be deletable if it is not {@code null} and it is an
* {@link EObject} that has some container from which it can be removed (root elements not logically
* being deletable).
*
diff --git a/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/EMFDeleteServiceImpl.java b/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/EMFDeleteServiceImpl.java
index dad5963..0d067ff 100644
--- a/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/EMFDeleteServiceImpl.java
+++ b/bundles/org.eclipse.emf.ecp.edit/src/org/eclipse/emf/ecp/edit/spi/EMFDeleteServiceImpl.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011-2019 EclipseSource Muenchen GmbH and others.
+ * Copyright (c) 2011-2020 EclipseSource Muenchen GmbH and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,7 +10,7 @@
*
* Contributors:
* Johannes Faltermeier - initial API and implementation
- * Christian W. Damus - bug 552385
+ * Christian W. Damus - bugs 552385, 559267
******************************************************************************/
package org.eclipse.emf.ecp.edit.spi;
@@ -21,10 +21,12 @@
import org.eclipse.emf.common.command.Command;
import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EReference;
import org.eclipse.emf.ecore.util.EcoreUtil;
import org.eclipse.emf.ecp.view.spi.context.ViewModelContext;
import org.eclipse.emf.edit.command.ChangeCommand;
import org.eclipse.emf.edit.command.DeleteCommand;
+import org.eclipse.emf.edit.command.RemoveCommand;
import org.eclipse.emf.edit.domain.AdapterFactoryEditingDomain;
import org.eclipse.emf.edit.domain.EditingDomain;
@@ -123,13 +125,7 @@
boolean result;
if (editingDomain != null) {
- Collection<?> collection;
- if (objects instanceof Collection<?>) {
- collection = (Collection<?>) objects;
- } else {
- collection = StreamSupport.stream(objects.spliterator(), false).collect(Collectors.toList());
- }
-
+ final Collection<?> collection = asCollection(objects);
final Command deleteCommand = DeleteCommand.create(editingDomain, collection);
result = deleteCommand.canExecute();
} else {
@@ -148,4 +144,41 @@
return result;
}
+ private static <T> Collection<T> asCollection(Iterable<T> iterable) {
+ Collection<T> result;
+
+ if (iterable instanceof Collection<?>) {
+ result = (Collection<T>) iterable;
+ } else {
+ result = StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ }
+
+ return result;
+ }
+
+ @Override
+ public boolean canRemove(Object owner, Object feature, Iterable<?> objects) {
+ if (!(feature instanceof EReference)) {
+ return canDelete(objects);
+ }
+
+ final EReference reference = (EReference) feature;
+ if (reference.isContainment()) {
+ return canDelete(objects);
+ }
+
+ boolean result;
+
+ if (editingDomain != null) {
+ final Collection<?> collection = asCollection(objects);
+ final Command removeCommand = RemoveCommand.create(editingDomain, owner, reference, collection);
+ result = removeCommand.canExecute();
+ } else {
+ // No commands? Then there's nothing to say 'no'
+ result = true;
+ }
+
+ return result;
+ }
+
}
diff --git a/bundles/org.eclipse.emf.ecp.view.control.multireference/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceSWTRenderer.java b/bundles/org.eclipse.emf.ecp.view.control.multireference/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceSWTRenderer.java
index 7b3830a..da538b4 100644
--- a/bundles/org.eclipse.emf.ecp.view.control.multireference/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceSWTRenderer.java
+++ b/bundles/org.eclipse.emf.ecp.view.control.multireference/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceSWTRenderer.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011-2019 EclipseSource Muenchen GmbH and others.
+ * Copyright (c) 2011-2020 EclipseSource Muenchen GmbH and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -12,7 +12,7 @@
* Eugen Neufeld - initial API and implementation
* Lucas Koehler - use data binding services
* Martin Fleck - bug 487101
- * Christian W. Damus - bugs 527736, 548592, 552385
+ * Christian W. Damus - bugs 527736, 548592, 552385, 559267
******************************************************************************/
package org.eclipse.emf.ecp.view.internal.control.multireference;
@@ -140,6 +140,11 @@
private Optional<EObject> cachedContainer;
/**
+ * The structural feature presented in this multi reference control.
+ */
+ private Optional<EStructuralFeature> cachedFeature;
+
+ /**
* A user-presentable display name for the reference, used in tool-tips.
*/
private String referenceDisplayName;
@@ -781,8 +786,10 @@
private void enableDeleteButton(boolean baseEnable, int listSize, int selectionIndex) {
if (btnDelete != null && showDeleteButton()) {
btnDelete.setEnabled(baseEnable && listSize > 0 && selectionIndex != -1
+ && getContainer().isPresent() && cachedFeature.isPresent()
&& ConditionalDeleteService.getDeleteService(getViewModelContext())
- .canDelete(tableViewer.getStructuredSelection().toList()));
+ .canRemove(getContainer().get(), cachedFeature.get(),
+ tableViewer.getStructuredSelection().toList()));
}
}
@@ -822,6 +829,7 @@
final Composite buttonComposite = new Composite(parent, SWT.NONE);
final EStructuralFeature structuralFeature = getEStructuralFeature();
+ cachedFeature = Optional.ofNullable(structuralFeature);
int nrButtons = 0;
diff --git a/bundles/org.eclipse.emf.ecp.view.table.ui.swt/src/org/eclipse/emf/ecp/view/spi/table/swt/TableControlSWTRenderer.java b/bundles/org.eclipse.emf.ecp.view.table.ui.swt/src/org/eclipse/emf/ecp/view/spi/table/swt/TableControlSWTRenderer.java
index 6e92318..8faba07 100644
--- a/bundles/org.eclipse.emf.ecp.view.table.ui.swt/src/org/eclipse/emf/ecp/view/spi/table/swt/TableControlSWTRenderer.java
+++ b/bundles/org.eclipse.emf.ecp.view.table.ui.swt/src/org/eclipse/emf/ecp/view/spi/table/swt/TableControlSWTRenderer.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011-2019 EclipseSource Muenchen GmbH and others.
+ * Copyright (c) 2011-2020 EclipseSource Muenchen GmbH and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -11,7 +11,7 @@
* Contributors:
* Eugen Neufeld - initial API and implementation
* Johannes Faltermeier - refactorings
- * Christian W. Damus - bugs 544116, 544537, 545686, 530314, 547271, 547787, 548592, 552385
+ * Christian W. Damus - bugs 544116, 544537, 545686, 530314, 547271, 547787, 548592, 552385, 559267
******************************************************************************/
package org.eclipse.emf.ecp.view.spi.table.swt;
@@ -638,8 +638,10 @@
@Override
public boolean canExecute() {
+ final Setting setting = getActionContext().getSetting();
return super.canExecute() && ConditionalDeleteService.getDeleteService(getViewModelContext())
- .canDelete(getActionContext().getViewer().getStructuredSelection().toList());
+ .canRemove(setting.getEObject(), setting.getEStructuralFeature(),
+ getActionContext().getViewer().getStructuredSelection().toList());
}
};
diff --git a/tests/org.eclipse.emf.ecp.edit.swt.test/src/org/eclipse/emf/ecp/edit/internal/swt/util/EMFDeleteSerivceImpl_PTest.java b/tests/org.eclipse.emf.ecp.edit.swt.test/src/org/eclipse/emf/ecp/edit/internal/swt/util/EMFDeleteSerivceImpl_PTest.java
index 0344274..b0c3c97 100644
--- a/tests/org.eclipse.emf.ecp.edit.swt.test/src/org/eclipse/emf/ecp/edit/internal/swt/util/EMFDeleteSerivceImpl_PTest.java
+++ b/tests/org.eclipse.emf.ecp.edit.swt.test/src/org/eclipse/emf/ecp/edit/internal/swt/util/EMFDeleteSerivceImpl_PTest.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011-2019 EclipseSource Muenchen GmbH and others.
+ * Copyright (c) 2011-2020 EclipseSource Muenchen GmbH and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,7 +10,7 @@
*
* Contributors:
* jfaltermeier - initial API and implementation
- * Christian W. Damus - bug 552385
+ * Christian W. Damus - bugs 552385, 559267
******************************************************************************/
package org.eclipse.emf.ecp.edit.internal.swt.util;
@@ -25,6 +25,7 @@
import java.util.Arrays;
import java.util.Collection;
+import java.util.HashMap;
import org.eclipse.emf.common.command.BasicCommandStack;
import org.eclipse.emf.common.command.Command;
@@ -46,6 +47,7 @@
import org.eclipse.emf.edit.domain.AdapterFactoryEditingDomain;
import org.eclipse.emf.edit.provider.ComposedAdapterFactory;
import org.eclipse.emf.emfstore.bowling.BowlingFactory;
+import org.eclipse.emf.emfstore.bowling.BowlingPackage;
import org.eclipse.emf.emfstore.bowling.Game;
import org.eclipse.emf.emfstore.bowling.League;
import org.eclipse.emf.emfstore.bowling.Player;
@@ -202,4 +204,58 @@
assertThat("can delete a root object", deleteService.canDelete(league), is(false));
}
+ @Test
+ public void canRemoveElement_containment() {
+ assertThat("can delete returns false",
+ deleteService.canRemove(league, BowlingPackage.Literals.LEAGUE__PLAYERS, player1), is(true));
+
+ denyDeletion = player1;
+ assertThat("can delete returns true",
+ deleteService.canRemove(league, BowlingPackage.Literals.LEAGUE__PLAYERS, player1), is(false));
+ }
+
+ @Test
+ public void canRemoveElement_crossReferenced() {
+ assertThat("cannot remove a referenced object",
+ deleteService.canRemove(tournament, BowlingPackage.Literals.TOURNAMENT__PLAYERS, player1), is(true));
+ }
+
+ @Test
+ public void canRemoveElement_notReferenced() {
+ tournament.getPlayers().remove(player3);
+
+ assertThat("cannot remove a referenced object",
+ deleteService.canRemove(tournament, BowlingPackage.Literals.TOURNAMENT__PLAYERS, player3), is(false));
+ }
+
+ @Test
+ public void canRemoveElement_crossReferenced_readOnly() {
+ final Resource readOnly = resource.getResourceSet().createResource(URI.createURI("READ_ONLY"));
+ final League others = BowlingFactory.eINSTANCE.createLeague();
+ others.setName("The Others");
+ others.getPlayers().add(player3);
+ domain.setResourceToReadOnlyMap(new HashMap<>());
+ domain.getResourceToReadOnlyMap().put(readOnly, true);
+
+ assertThat("cannot remove a referenced object",
+ deleteService.canRemove(tournament, BowlingPackage.Literals.TOURNAMENT__PLAYERS, player3), is(true));
+ }
+
+ @Test
+ public void testRemove_noEditingDomain() {
+ // Disconnect everything from the editing domain
+ resource.getContents().clear();
+ resource.getResourceSet().eAdapters().clear();
+ resource.getResourceSet().getResources().clear();
+ resource.unload();
+ resource.eAdapters().clear();
+
+ // Re-initialize to forget the editing domain
+ deleteService.dispose();
+ deleteService.instantiate(context);
+
+ assertThat("cannot remove a referenced object",
+ deleteService.canRemove(tournament, BowlingPackage.Literals.TOURNAMENT__PLAYERS, player1), is(true));
+ }
+
}
