Skip to content

Commit

Permalink
Restore ability to modify selection in GroupRequest within DeleteAction
Browse files Browse the repository at this point in the history
The list returned by getSelectedEditParts() should be a view of the
internal selection, similar to getSelectedObjects(). Meaning callers are
able to modify by the selection by calling set(int,Object).

In order to reduce the chance of an undesirable ClassCastException, we
only check whether the first element in the list is an EditPart,
assuming that all remaining elements are as well. I.e. we don't expect a
"mixed" selection of different types. This is similar to what has
already been done in e.g. the DeleteAction or the MatchSizeAction.

This is a follow-up to 78c3fed.

Resolves #616
  • Loading branch information
ptziegler authored and azoitl committed Nov 17, 2024
1 parent 20aa0b4 commit b75478c
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,27 @@

package org.eclipse.gef.test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import java.util.List;

import org.eclipse.jface.viewers.IStructuredSelection;
import org.eclipse.jface.viewers.StructuredSelection;
import org.eclipse.ui.IWorkbenchPart;
import org.eclipse.ui.PlatformUI;

import org.eclipse.draw2d.Figure;
import org.eclipse.draw2d.IFigure;

import org.eclipse.gef.EditPart;
import org.eclipse.gef.GraphicalViewer;
import org.eclipse.gef.Request;
import org.eclipse.gef.commands.Command;
import org.eclipse.gef.editparts.AbstractGraphicalEditPart;
import org.eclipse.gef.requests.GroupRequest;
import org.eclipse.gef.ui.actions.DeleteAction;
import org.eclipse.gef.ui.parts.GraphicalViewerImpl;

import org.junit.Before;
Expand Down Expand Up @@ -54,4 +67,42 @@ public void testSetNullSelection() {
assertThrows(NullPointerException.class, () -> viewer.setSelection(selection));
assertTrue(viewer.getSelectedEditParts().isEmpty());
}

@Test
public void testDeleteSelection() {
IStructuredSelection selection = new StructuredSelection(new DummyEditPart());
viewer.setSelection(selection);

DeleteAction deleteAction = new DeleteAction((IWorkbenchPart) null);
deleteAction.setSelectionProvider(viewer);
deleteAction.update();
deleteAction.run();
}

private static class DummyEditPart extends AbstractGraphicalEditPart {
@Override
protected IFigure createFigure() {
return new Figure();
}

@Override
protected void createEditPolicies() {
// nothing to do
}

@Override
public Command getCommand(Request request) {
assertEquals(request.getType(), REQ_DELETE);
if (REQ_DELETE.equals(request.getType())) {
GroupRequest groupRequest = (GroupRequest) request;
DummyEditPart newEditPart = new DummyEditPart();

@SuppressWarnings("unchecked")
List<EditPart> editParts = (List<EditPart>) groupRequest.getEditParts();
editParts.set(0, newEditPart);
assertEquals(groupRequest.getEditParts(), List.of(newEditPart));
}
return super.getCommand(request);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protected boolean calculateEnabled() {
*/
protected Command createPasteCommand() {
Command result = null;
List<Object> selection = getSelectedObjects();
List<?> selection = getSelectedObjects();
if (selection != null && selection.size() == 1 && selection.get(0) instanceof GraphicalEditPart gep) {
Object template = getClipboardContents();
if (template != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,41 @@ public void dispose() {
}

/**
* Returns a <code>List</code> containing the currently selected objects.
* Returns a {@code List<EditPart} containing the currently selected objects.
* The {@code set} method is supported but the {@code add}, {@code addAll},
* {@code remove} {@code removeAll} methods are not. Changes to this list modify
* the internal {@link #selection}.
*
* @return A List containing the currently selected objects.
*/
protected List<Object> getSelectedObjects() {
protected List<?> getSelectedObjects() {
if (!(getSelection() instanceof IStructuredSelection)) {
return Collections.emptyList();
}
return ((IStructuredSelection) getSelection()).toList();
}

/**
* Returns a <code>List<EditPart></code> containing the currently selected
* EditParts.
* Returns a {@code List<EditPart} containing the currently selected EditParts.
* If elements not of type EditPart are selected, an empty list is returned. The
* {@code set} method is supported but the {@code add}, {@code addAll},
* {@code remove} {@code removeAll} methods are not. Changes to this list modify
* the internal {@link #selection}.<br>
* <em>Note:</em> This method only checks whether the first selected element is
* of type {@link EditPart} and then performs a lazy cast from {@code List<?>}
* to {@code List<EditPart>}. It should therefore only be called when a single
* type of objects are selected.
*
* @return A List containing the currently selected EditParts.
* @since 3.20
*/
@SuppressWarnings("unchecked") // We don't expect a "mixed" selection
protected final List<EditPart> getSelectedEditParts() {
return getSelectedObjects().stream().filter(EditPart.class::isInstance).map(EditPart.class::cast).toList();
List<?> selectedObjects = getSelectedObjects();
if (selectedObjects.isEmpty() || !(selectedObjects.get(0) instanceof EditPart)) {
return Collections.emptyList();
}
return (List<EditPart>) selectedObjects;
}

/**
Expand Down

0 comments on commit b75478c

Please sign in to comment.