[356449] Don't add null data to selection, deprecate redundant method

fsteeg · Sep 2, 2011 · 0e530e4 · 0e530e4
1 parent 9655eeb
commit 0e530e4
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 10 deletions.
diff --git a/org.eclipse.zest.core/src/org/eclipse/zest/core/widgets/GraphConnection.java b/org.eclipse.zest.core/src/org/eclipse/zest/core/widgets/GraphConnection.java
@@ -193,6 +193,7 @@ public Connection getConnectionFigure() {
 	 * Gets the external connection object.
 	 * 
 	 * @return Object
+	 * @deprecated Use {@link #getData()} instead
 	 */
 	public Object getExternalConnection() {
 		return this.getData();

diff --git a/org.eclipse.zest.jface/src/org/eclipse/zest/core/viewers/AbstractStructuredGraphViewer.java b/org.eclipse.zest.jface/src/org/eclipse/zest/core/viewers/AbstractStructuredGraphViewer.java
@@ -412,16 +412,9 @@ protected List getSelectionFromWidget() {
 		List internalSelection = getWidgetSelection();
 		LinkedList externalSelection = new LinkedList();
 		for (Iterator i = internalSelection.iterator(); i.hasNext();) {
-			// @tag zest.todo : should there be a method on IGraphItem to get
-			// the external data?
-			GraphItem item = (GraphItem) i.next();
-			if (item instanceof GraphNode) {
-				externalSelection.add(((GraphNode) item).getData());
-			} else if (item instanceof GraphConnection) {
-				externalSelection.add(((GraphConnection) item)
-						.getExternalConnection());
-			} else if (item instanceof Widget) {
-				externalSelection.add(((Widget) item).getData());
+			Object data = ((GraphItem) i.next()).getData();
+			if (data != null) {
+				externalSelection.add(data);
 			}
 		}
 		return externalSelection;

diff --git a/org.eclipse.zest.tests/src/org/eclipse/zest/tests/GraphViewerTests.java b/org.eclipse.zest.tests/src/org/eclipse/zest/tests/GraphViewerTests.java
@@ -12,6 +12,7 @@
 import junit.framework.TestCase;
 
 import org.eclipse.jface.util.DelegatingDragAdapter;
+import org.eclipse.jface.viewers.StructuredSelection;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.dnd.DND;
 import org.eclipse.swt.dnd.DropTarget;
@@ -21,6 +22,7 @@
 import org.eclipse.zest.core.viewers.GraphViewer;
 import org.eclipse.zest.core.widgets.Graph;
 import org.eclipse.zest.core.widgets.GraphConnection;
+import org.eclipse.zest.core.widgets.GraphItem;
 import org.eclipse.zest.core.widgets.GraphNode;
 
 /**
@@ -90,4 +92,23 @@ public void testFindGraphItem() {
 				viewer.findGraphItem(new Integer(5)));
 	}
 
+	/**
+	 * Assert that no invalid selections with null data are produced by the
+	 * viewer (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=356449)
+	 */
+	public void testValidSelection() {
+		Graph graph = new Graph(shell, SWT.NONE);
+		GraphNode n1 = new GraphNode(graph, SWT.NONE);
+		GraphNode n2 = new GraphNode(graph, SWT.NONE);
+		GraphConnection c = new GraphConnection(graph, SWT.NONE, n1, n2);
+		graph.setSelection(new GraphItem[] { n1, n2, c });
+		GraphViewer viewer = new GraphViewer(graph);
+		assertEquals("No null data should be in the selection", 0,
+				((StructuredSelection) viewer.getSelection()).size());
+		n1.setData("1");
+		n2.setData("2");
+		assertEquals("Other data should be in the selection", 2,
+				((StructuredSelection) viewer.getSelection()).size());
+	}
+
 }