Bug 544219 - Provide number aware sorting for tables and multi

references

* Implement a number aware string comparator
* Refactor the multi references table comparator to emfforms.swt.core to
make it re-usable for the multi attribute or similar renderers
* Use the number aware comparator for table column and multi reference
sorting
* Adapt test cases

Change-Id: Iae29481a32c334ba9933712c0e70c9672e8e85bd
Signed-off-by: Lucas Koehler <lkoehler@eclipsesource.com>

11 files changed, 375 insertions, 84 deletions

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 876223d9cf..a450e5c769 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
@@ -62,6 +62,7 @@ import org.eclipse.emfforms.internal.core.services.label.BundleResolver.NoBundle
 import org.eclipse.emfforms.internal.core.services.label.BundleResolverImpl;
 import org.eclipse.emfforms.spi.common.report.AbstractReport;
 import org.eclipse.emfforms.spi.common.report.ReportService;
+import org.eclipse.emfforms.spi.common.sort.NumberAwareStringComparator;
 import org.eclipse.emfforms.spi.core.services.databinding.DatabindingFailedException;
 import org.eclipse.emfforms.spi.core.services.databinding.EMFFormsDatabinding;
 import org.eclipse.emfforms.spi.core.services.label.EMFFormsLabelProvider;
@@ -72,6 +73,7 @@ import org.eclipse.emfforms.spi.swt.core.SWTDataElementIdHelper;
 import org.eclipse.emfforms.spi.swt.core.layout.GridDescriptionFactory;
 import org.eclipse.emfforms.spi.swt.core.layout.SWTGridCell;
 import org.eclipse.emfforms.spi.swt.core.layout.SWTGridDescription;
+import org.eclipse.emfforms.spi.swt.core.ui.ObjectViewerComparator;
 import org.eclipse.jface.databinding.swt.WidgetProperties;
 import org.eclipse.jface.databinding.viewers.ObservableListContentProvider;
 import org.eclipse.jface.layout.GridDataFactory;
@@ -89,8 +91,6 @@ import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.jface.viewers.TableViewer;
 import org.eclipse.jface.viewers.TableViewerColumn;
 import org.eclipse.jface.viewers.TableViewerEditor;
-import org.eclipse.jface.viewers.Viewer;
-import org.eclipse.jface.viewers.ViewerComparator;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.SelectionAdapter;
@@ -842,7 +842,7 @@ public class MultiReferenceSWTRenderer extends AbstractControlSWTRenderer<VContr
 			| ColumnViewerEditor.KEYBOARD_ACTIVATION);
 		ColumnViewerToolTipSupport.enableFor(tableViewer);
 
-		final ECPTableViewerComparator comparator = new ECPTableViewerComparator();
+		final ObjectViewerComparator comparator = new ObjectViewerComparator(this::compare);
 		final boolean isMoveDisabled = !showMoveUpButton() && !showMoveDownButton();
 		if (isMoveDisabled) {
 			tableViewer.setComparator(comparator);
@@ -903,7 +903,7 @@ public class MultiReferenceSWTRenderer extends AbstractControlSWTRenderer<VContr
 	}
 
 	private SelectionAdapter getSelectionAdapter(final TableViewer tableViewer,
-		final ECPTableViewerComparator comparator, final TableColumn column) {
+		final ObjectViewerComparator comparator, final TableColumn column) {
 		final SelectionAdapter selectionAdapter = new SelectionAdapter() {
 			@Override
 			public void widgetSelected(SelectionEvent e) {
@@ -1043,47 +1043,6 @@ public class MultiReferenceSWTRenderer extends AbstractControlSWTRenderer<VContr
 		}
 	}
 
-	/**
-	 * The {@link ViewerComparator} for the multi reference renderer's table which allows 3 states for sort order:
-	 * none, up and down. This comparator does not use column specific values but only the objects' labels because the
-	 * multi reference renderer always only has one column that shows the label.
-	 *
-	 * @author Eugen Neufeld
-	 *
-	 */
-	private class ECPTableViewerComparator extends ViewerComparator {
-		private static final int NONE = 0;
-		private int direction = NONE;
-
-		ECPTableViewerComparator() {
-			direction = NONE;
-		}
-
-		/** Toggles through the sorting directions: NONE -&gt; UP -&gt; DOWN -&gt; NONE. */
-		public void toggleDirection() {
-			direction = (direction + 1) % 3;
-		}
-
-		public int getDirection() {
-			switch (direction) {
-			case 0:
-				return SWT.NONE;
-			case 1:
-				return SWT.UP;
-			case 2:
-				return SWT.DOWN;
-			default:
-				return SWT.NONE;
-			}
-
-		}
-
-		@Override
-		public int compare(Viewer viewer, Object e1, Object e2) {
-			return MultiReferenceSWTRenderer.this.compare(direction, e1, e2);
-		}
-	}
-
 	@Override
 	protected void rootDomainModelChanged() throws DatabindingFailedException {
 		// TODO rebinding of text and tooltip needed? If yes, complete!
@@ -1155,11 +1114,15 @@ public class MultiReferenceSWTRenderer extends AbstractControlSWTRenderer<VContr
 		final String label1 = labelProvider.getText(object1);
 		final String label2 = labelProvider.getText(object2);
 		if (label1 == null) {
-			rc = 1;
+			if (label2 == null) {
+				rc = 0;
+			} else {
+				rc = 1;
+			}
 		} else if (label2 == null) {
 			rc = -1;
 		} else {
-			rc = label1.toString().compareTo(label2.toString());
+			rc = NumberAwareStringComparator.getInstance().compare(label1.toString(), label2.toString());
 		}
 		// If descending order, flip the direction
 		if (direction == 2) {
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 103dfb1d95..19fef30ae4 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
@@ -113,6 +113,7 @@ import org.eclipse.emf.edit.ui.dnd.ViewerDragAdapter;
 import org.eclipse.emfforms.common.Optional;
 import org.eclipse.emfforms.spi.common.report.AbstractReport;
 import org.eclipse.emfforms.spi.common.report.ReportService;
+import org.eclipse.emfforms.spi.common.sort.NumberAwareStringComparator;
 import org.eclipse.emfforms.spi.core.services.databinding.DatabindingFailedException;
 import org.eclipse.emfforms.spi.core.services.databinding.DatabindingFailedReport;
 import org.eclipse.emfforms.spi.core.services.databinding.EMFFormsDatabinding;
@@ -1656,14 +1657,19 @@ public class TableControlSWTRenderer extends AbstractControlSWTRenderer<VTableCo
 		}
 
 		if (leftValue == null) {
-			rc = 1;
+			if (rightValue == null) {
+				rc = 0;
+			} else {
+				rc = 1;
+			}
 		} else if (rightValue == null) {
 			rc = -1;
 		} else {
-			if (leftValue instanceof Comparable && leftValue.getClass().isInstance(rightValue)) {
+			if (!(leftValue instanceof String) && leftValue instanceof Comparable
+				&& leftValue.getClass().isInstance(rightValue)) {
 				rc = Comparable.class.cast(leftValue).compareTo(rightValue);
 			} else {
-				rc = leftValue.toString().compareTo(rightValue.toString());
+				rc = NumberAwareStringComparator.getInstance().compare(leftValue.toString(), rightValue.toString());
 			}
 		}
 		// If descending order, flip the direction
diff --git a/bundles/org.eclipse.emfforms.common/META-INF/MANIFEST.MF b/bundles/org.eclipse.emfforms.common/META-INF/MANIFEST.MF
index f55bb98604..067eeff5c1 100644
--- a/bundles/org.eclipse.emfforms.common/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.emfforms.common/META-INF/MANIFEST.MF
@@ -11,6 +11,7 @@ Export-Package: org.eclipse.emfforms.common;version="1.20.0",
  org.eclipse.emfforms.spi.common.converter;version="1.20.0",
  org.eclipse.emfforms.spi.common.locale;version="1.20.0",
  org.eclipse.emfforms.spi.common.report;version="1.20.0",
+ org.eclipse.emfforms.spi.common.sort;version="1.20.0",
  org.eclipse.emfforms.spi.common.validation;version="1.20.0"
 Require-Bundle: org.eclipse.emf.ecore;bundle-version="[2.7.0,3.0.0)"
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
diff --git a/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/common/TriFunction.java b/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/common/TriFunction.java
new file mode 100644
index 0000000000..23e8b228b9
--- /dev/null
+++ b/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/common/TriFunction.java
@@ -0,0 +1,48 @@
+/*******************************************************************************
+ * Copyright (c) 2011-2019 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 v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Lucas Koehler - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.emfforms.common;
+
+import java.util.function.Function;
+
+/**
+ * Represents a function that accepts three arguments and produces a result.
+ * This is the three-arity specialization of {@link Function}.
+ *
+ * <p>
+ * This is a functional interface whose functional method is {@link #apply(Object, Object, Object)}.
+ *
+ * <p>
+ * This interface was introduced because Java 8 only provides {@link java.util.function.Function Function} and
+ * {@link java.util.function.BiFunction BiFunction}.
+ *
+ * @param <R> the type of the result of the function
+ * @param <T> the type of the first argument to the function
+ * @param <U> the type of the second argument to the function
+ * @param <V> the type of the third argument to the function
+ *
+ * @author Lucas Koehler
+ * @since 1.20
+ *
+ */
+@FunctionalInterface
+public interface TriFunction<R, T, U, V> {
+
+	/**
+	 * Applies this function to the given arguments.
+	 *
+	 * @param t the first function argument
+	 * @param u the second function argument
+	 * @param v the third function argument
+	 * @return the function result
+	 */
+	R apply(T t, U u, V v);
+}
diff --git a/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/spi/common/sort/NumberAwareStringComparator.java b/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/spi/common/sort/NumberAwareStringComparator.java
new file mode 100644
index 0000000000..fb8c10763e
--- /dev/null
+++ b/bundles/org.eclipse.emfforms.common/src/org/eclipse/emfforms/spi/common/sort/NumberAwareStringComparator.java
@@ -0,0 +1,86 @@
+/*******************************************************************************
+ * Copyright (c) 2011-2019 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 v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Lucas Koehler - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.emfforms.spi.common.sort;
+
+import java.math.BigInteger;
+import java.util.Comparator;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * A comparator for strings that compares numbers which are part of compared string as numbers and not as strings.
+ * This allows to sort strings that are a mixture of numbers and text (e.g. house numbers) in an intuitive fashion.
+ * For instance, plain string sorting sorts 200A greater than 1000A. This comparator sorts 1000A greater than 200A.
+ *
+ * @author Lucas Koehler
+ * @since 1.20
+ *
+ */
+public final class NumberAwareStringComparator implements Comparator<String> {
+
+	// First group matches zero or more non-digits. Second group matches zero or more digits
+	private static final Pattern PATTERN = Pattern.compile("(\\D*)(\\d*)"); //$NON-NLS-1$
+	private static NumberAwareStringComparator instance;
+
+	/**
+	 * @return the static {@link NumberAwareStringComparator} instance.
+	 */
+	public static NumberAwareStringComparator getInstance() {
+		if (instance == null) {
+			instance = new NumberAwareStringComparator();
+		}
+		return instance;
+	}
+
+	private NumberAwareStringComparator() {
+		// Static instance should be used.
+	}
+
+	@Override
+	public int compare(String o1, String o2) {
+		final Matcher matcher1 = PATTERN.matcher(o1);
+		final Matcher matcher2 = PATTERN.matcher(o2);
+
+		// For our pattern Matcher::find only returns false if the end of the string was reached.
+		while (matcher1.find() && matcher2.find()) {
+			// group(1) gets the results matched by \\D* (non-digits)
+			final int wordCompare = matcher1.group(1).compareTo(matcher2.group(1));
+			if (wordCompare != 0) {
+				return wordCompare;
+			}
+
+			// group(2) gets the results matched by \\d* (digits)
+			final String numberString1 = matcher1.group(2);
+			final String numberString2 = matcher2.group(2);
+
+			if (numberString1.isEmpty()) {
+				// Empty string is smaller than any other string
+				return numberString2.isEmpty() ? 0 : -1;
+			} else if (numberString2.isEmpty()) {
+				return 1;
+			}
+
+			final BigInteger number1 = new BigInteger(numberString1);
+			final BigInteger number2 = new BigInteger(numberString2);
+			final int numberCompare = number1.compareTo(number2);
+			if (numberCompare != 0) {
+				return numberCompare;
+			}
+		}
+
+		if (matcher1.hitEnd() && matcher2.hitEnd()) {
+			return 0;
+		}
+		return matcher1.hitEnd() ? -1 : 1;
+	}
+
+}
diff --git a/bundles/org.eclipse.emfforms.swt.core/META-INF/MANIFEST.MF b/bundles/org.eclipse.emfforms.swt.core/META-INF/MANIFEST.MF
index 3353bdf8d5..63ab198b97 100644
--- a/bundles/org.eclipse.emfforms.swt.core/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.emfforms.swt.core/META-INF/MANIFEST.MF
@@ -8,6 +8,7 @@ Export-Package: org.eclipse.emfforms.internal.swt.core;version="1.20.0";x-intern
  org.eclipse.emfforms.spi.swt.core;version="1.20.0",
  org.eclipse.emfforms.spi.swt.core.data;version="1.20.0",
  org.eclipse.emfforms.spi.swt.core.layout;version="1.20.0",
+ org.eclipse.emfforms.spi.swt.core.ui;version="1.20.0",
  org.eclipse.emfforms.spi.swt.core.util;version="1.20.0",
  org.eclipse.emfforms.swt.core;version="1.20.0"
 Require-Bundle: org.eclipse.emf.ecp.view.model;bundle-version="[1.20.0,1.21.0)",
@@ -22,6 +23,7 @@ Bundle-ActivationPolicy: lazy
 Import-Package: org.eclipse.emf.ecp.view.model.common;version="[1.20.0,1.21.0)",
  org.eclipse.emfforms.spi.common.report;version="[1.20.0,1.21.0)",
  org.eclipse.jface.layout;version="0.0.0",
+ org.eclipse.jface.viewers;version="0.0.0",
  org.eclipse.swt;version="0.0.0",
  org.eclipse.swt.custom;version="0.0.0",
  org.eclipse.swt.events;version="0.0.0",
diff --git a/bundles/org.eclipse.emfforms.swt.core/src/org/eclipse/emfforms/spi/swt/core/ui/ObjectViewerComparator.java b/bundles/org.eclipse.emfforms.swt.core/src/org/eclipse/emfforms/spi/swt/core/ui/ObjectViewerComparator.java
new file mode 100644
index 0000000000..26667cfbe5
--- /dev/null
+++ b/bundles/org.eclipse.emfforms.swt.core/src/org/eclipse/emfforms/spi/swt/core/ui/ObjectViewerComparator.java
@@ -0,0 +1,78 @@
+/*******************************************************************************
+ * Copyright (c) 2011-2019 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 v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Lucas Koehler - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.emfforms.spi.swt.core.ui;
+
+import org.eclipse.emfforms.common.TriFunction;
+import org.eclipse.jface.viewers.Viewer;
+import org.eclipse.jface.viewers.ViewerComparator;
+import org.eclipse.swt.SWT;
+
+/**
+ * The {@link ObjectViewerComparator} allows to rotate between three sorting states:
+ * <ol>
+ * <li>no sorting
+ * <li>ascending
+ * <li>descending
+ * </ol>
+ * To sort the objects, the comparator applies the configured sorting function to the given objects.
+ *
+ * @author Lucas Koehler
+ * @since 1.20
+ *
+ */
+public class ObjectViewerComparator extends ViewerComparator {
+
+	private static final int NONE = 0;
+	private int direction = NONE;
+	private final TriFunction<Integer, Integer, Object, Object> compareFunction;
+
+	/**
+	 * Creates a new instance.
+	 *
+	 * @param compareFunction The function used to compare objects of the viewer. This tri-function accepts the sorting
+	 *            direction as its first argument and the objects to compare as the following arguments. The sorting
+	 *            directions are: 0 = none, 1 = ascending, 2 = descending
+	 */
+	public ObjectViewerComparator(TriFunction<Integer, Integer, Object, Object> compareFunction) {
+		this.compareFunction = compareFunction;
+		direction = NONE;
+	}
+
+	/** Toggles through the sorting directions: NONE -&gt; UP (ascending) -&gt; DOWN (descending) -&gt; NONE. */
+	public void toggleDirection() {
+		direction = (direction + 1) % 3;
+	}
+
+	/**
+	 * Get the current sorting direction as an SWT constant.
+	 *
+	 * @return SWT.NONE, SWT.UP (ascending), or SWT.DOWN (descending)
+	 */
+	public int getDirection() {
+		switch (direction) {
+		case 0:
+			return SWT.NONE;
+		case 1:
+			return SWT.UP;
+		case 2:
+			return SWT.DOWN;
+		default:
+			return SWT.NONE;
+		}
+
+	}
+
+	@Override
+	public int compare(Viewer viewer, Object e1, Object e2) {
+		return compareFunction.apply(direction, e1, e2);
+	}
+}
diff --git a/tests/org.eclipse.emf.ecp.view.control.multireference.tests/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceRenderer_PTest.java b/tests/org.eclipse.emf.ecp.view.control.multireference.tests/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceRenderer_PTest.java
index 41638a8078..e13179a7df 100644
--- a/tests/org.eclipse.emf.ecp.view.control.multireference.tests/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceRenderer_PTest.java
+++ b/tests/org.eclipse.emf.ecp.view.control.multireference.tests/src/org/eclipse/emf/ecp/view/internal/control/multireference/MultiReferenceRenderer_PTest.java
@@ -19,6 +19,7 @@ import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
@@ -89,6 +90,7 @@ import org.eclipse.emfforms.spi.localization.EMFFormsLocalizationService;
 import org.eclipse.emfforms.spi.swt.core.SWTDataElementIdHelper;
 import org.eclipse.emfforms.spi.swt.core.layout.SWTGridCell;
 import org.eclipse.emfforms.spi.swt.core.layout.SWTGridDescription;
+import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
@@ -96,6 +98,7 @@ import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.Table;
 import org.eclipse.swt.widgets.TableColumn;
+import org.eclipse.swt.widgets.TableItem;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -595,10 +598,14 @@ public class MultiReferenceRenderer_PTest {
 	}
 
 	protected Table createLeaguePlayersTable() {
+		final League league = BowlingFactory.eINSTANCE.createLeague();
+		return createLeaguePlayersTable(league);
+	}
+
+	protected Table createLeaguePlayersTable(final League league) {
 		final EditingDomain domain = new AdapterFactoryEditingDomain(
 			new ComposedAdapterFactory(ComposedAdapterFactory.Descriptor.Registry.INSTANCE), new BasicCommandStack());
 		final Resource bowling = domain.getResourceSet().createResource(URI.createURI("foo.ecore")); //$NON-NLS-1$
-		final League league = BowlingFactory.eINSTANCE.createLeague();
 		bowling.getContents().add(league);
 
 		final IObservableList<?> observableList = EMFEditProperties
@@ -855,38 +862,62 @@ public class MultiReferenceRenderer_PTest {
 		}
 	}
 
-	public void compare() {
-		// final MultiReferenceSWTRenderer multi = new MultiReferenceSWTRenderer(mock(VControl.class),
-		// mock(ViewModelContext.class), mock(ReportService.class), mock(EMFFormsDatabinding.class),
-		// mock(EMFFormsLabelProvider.class), mock(VTViewTemplateProvider.class), mock(ImageRegistryService.class));
+	@Test
+	public void compare()
+		throws NoRendererFoundException, NoPropertyDescriptorFoundExeption, DatabindingFailedException {
+		showMoveButtons = false;
+
+		// Label: Player a2
+		final Player p1 = BowlingFactory.eINSTANCE.createPlayer();
+		p1.setName("a2"); //$NON-NLS-1$
 
-		// Label: Player A
-		final Player pA = BowlingFactory.eINSTANCE.createPlayer();
-		pA.setName("A"); //$NON-NLS-1$
+		// Label: Player a10
+		final Player p2 = BowlingFactory.eINSTANCE.createPlayer();
+		p2.setName("a10a"); //$NON-NLS-1$
 
-		// Label: Player C
-		final Player pC = BowlingFactory.eINSTANCE.createPlayer();
-		pA.setName("C"); //$NON-NLS-1$
+		// Label: Player a10a
+		final Player p3 = BowlingFactory.eINSTANCE.createPlayer();
+		p3.setName("a10"); //$NON-NLS-1$
+
+		final League league = BowlingFactory.eINSTANCE.createLeague();
+		league.getPlayers().add(p1);
+		league.getPlayers().add(p2);
+		league.getPlayers().add(p3);
 
-		// Label: Player B
-		final Player pB = BowlingFactory.eINSTANCE.createPlayer();
-		pA.setName("B"); //$NON-NLS-1$
+		final Table playersTable = createLeaguePlayersTable(league);
 
-		assertEquals(0, renderer.compare(0, pA, pB));
-		assertEquals(0, renderer.compare(0, pA, pC));
-		assertEquals(0, renderer.compare(0, pB, pC));
+		// Initially, items should be sorted by insertion order
+		assertEquals(SWT.NONE, playersTable.getSortDirection());
+		assertItemOrder(playersTable, p1, p2, p3);
 
-		// direction UP
-		assertEquals(0, renderer.compare(1, pA, pA));
-		assertEquals(-1, renderer.compare(1, pA, pB));
-		assertEquals(-1, renderer.compare(1, pA, pC));
-		assertEquals(-1, renderer.compare(1, pB, pC));
+		SWTTestUtil.selectWidget(playersTable.getColumn(0));
+		SWTTestUtil.waitForUIThread();
 
-		// direction DOWN
-		assertEquals(0, renderer.compare(2, pA, pA));
-		assertEquals(1, renderer.compare(2, pA, pB));
-		assertEquals(1, renderer.compare(2, pA, pC));
-		assertEquals(1, renderer.compare(2, pB, pC));
+		// ascending
+		assertEquals(SWT.UP, playersTable.getSortDirection());
+		assertItemOrder(playersTable, p1, p3, p2);
+
+		SWTTestUtil.selectWidget(playersTable.getColumn(0));
+		SWTTestUtil.waitForUIThread();
+
+		// descending
+		assertEquals(SWT.DOWN, playersTable.getSortDirection());
+		assertItemOrder(playersTable, p2, p3, p1);
+
+		SWTTestUtil.selectWidget(playersTable.getColumn(0));
+		SWTTestUtil.waitForUIThread();
+
+		// insertion order again
+		assertEquals(SWT.NONE, playersTable.getSortDirection());
+		assertItemOrder(playersTable, p1, p2, p3);
+	}
+
+	private static void assertItemOrder(Table table, Object... objects) {
+		assertEquals(objects.length, table.getItemCount());
+		final TableItem[] items = table.getItems();
+		for (int i = 0; i < items.length; i++) {
+			assertSame(objects[i], items[i].getData());
+		}
 	}
 
 	/**
diff --git a/tests/org.eclipse.emf.ecp.view.table.ui.swt.test/src/org/eclipse/emf/ecp/view/spi/table/swt/SWTTable_PTest.java b/tests/org.eclipse.emf.ecp.view.table.ui.swt.test/src/org/eclipse/emf/ecp/view/spi/table/swt/SWTTable_PTest.java
index b089939031..51d6e56ce9 100644
--- a/tests/org.eclipse.emf.ecp.view.table.ui.swt.test/src/org/eclipse/emf/ecp/view/spi/table/swt/SWTTable_PTest.java
+++ b/tests/org.eclipse.emf.ecp.view.table.ui.swt.test/src/org/eclipse/emf/ecp/view/spi/table/swt/SWTTable_PTest.java
@@ -483,9 +483,9 @@ public class SWTTable_PTest {
 		EMFFormsNoRendererException {
 		// domain
 		((EClass) domainElement).getEStructuralFeatures().clear();
-		final EAttribute attribute1 = createEAttribute("a", EcorePackage.Literals.ESTRING, 0, 2);
-		final EAttribute attribute2 = createEAttribute("b", EcorePackage.Literals.ESTRING, 0, 11);
-		final EAttribute attribute3 = createEAttribute("c", EcorePackage.Literals.ESTRING, 0, 1);
+		final EAttribute attribute1 = createEAttribute("a2", EcorePackage.Literals.ESTRING, 0, 2);
+		final EAttribute attribute2 = createEAttribute("a10", EcorePackage.Literals.ESTRING, 0, 11);
+		final EAttribute attribute3 = createEAttribute("a10b", EcorePackage.Literals.ESTRING, 0, 1);
 		((EClass) domainElement).getEStructuralFeatures().add(attribute1);
 		((EClass) domainElement).getEStructuralFeatures().add(attribute2);
 		((EClass) domainElement).getEStructuralFeatures().add(attribute3);
diff --git a/tests/org.eclipse.emfforms.common.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.emfforms.common.tests/META-INF/MANIFEST.MF
index 0cea2b7bcb..61bd2950d2 100644
--- a/tests/org.eclipse.emfforms.common.tests/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.emfforms.common.tests/META-INF/MANIFEST.MF
@@ -5,13 +5,15 @@ Bundle-SymbolicName: org.eclipse.emfforms.common.tests;singleton:=true
 Bundle-Version: 1.20.0.qualifier
 Bundle-Vendor: Eclipse Modeling Project
 Fragment-Host: org.eclipse.emfforms.common;bundle-version="[1.20.0,1.21.0)"
-Export-Package: org.eclipse.emfforms.common.tests;version="1.20.0";x-internal:=true,
- org.eclipse.emfforms.spi.common.locale;version="1.20.0"
-Require-Bundle: org.junit;bundle-version="[4.11.0,5.0.0)",
+Export-Package: org.eclipse.emfforms.common.sort;version="1.20.0";x-internal:=true,
+ org.eclipse.emfforms.common.tests;version="1.20.0";x-internal:=true,
+ org.eclipse.emfforms.spi.common.locale;version="1.20.0";x-internal:=true
+Require-Bundle: org.hamcrest.library;bundle-version="[1.3.0,2.0.0)",
  org.mockito.mockito-core-hamcrest-modified;bundle-version="[1.9.5,2.0.0)",
  org.eclipse.emf.ecore.xmi;bundle-version="[2.13.0,3.0.0)",
  org.eclipse.emfforms.editor;bundle-version="[1.20.0,1.21.0)",
  org.eclipse.emf.edit;bundle-version="[2.14.0,3.0.0)",
- org.eclipse.emf.ecp.view.model.common;bundle-version="[1.20.0,1.21.0)"
+ org.eclipse.emf.ecp.view.model.common;bundle-version="[1.20.0,1.21.0)",
+ org.junit;bundle-version="[4.12.0,5.0.0)"
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
 Automatic-Module-Name: org.eclipse.emfforms.common.tests
diff --git a/tests/org.eclipse.emfforms.common.tests/src/org/eclipse/emfforms/common/sort/NumberAwareStringComparator_PTest.java b/tests/org.eclipse.emfforms.common.tests/src/org/eclipse/emfforms/common/sort/NumberAwareStringComparator_PTest.java
new file mode 100644
index 0000000000..a3193550aa
--- /dev/null
+++ b/tests/org.eclipse.emfforms.common.tests/src/org/eclipse/emfforms/common/sort/NumberAwareStringComparator_PTest.java
@@ -0,0 +1,74 @@
+/*******************************************************************************
+ * Copyright (c) 2011-2019 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 v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Lucas Koehler - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.emfforms.common.sort;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+
+import org.eclipse.emfforms.spi.common.sort.NumberAwareStringComparator;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link NumberAwareStringComparator}. Run as plugin test because otherwise the hamcrest matchers do not
+ * work due to a security exception.
+ *
+ * @author Lucas Koehler
+ *
+ */
+public class NumberAwareStringComparator_PTest {
+
+	private static NumberAwareStringComparator comparator;
+
+	@BeforeClass
+	public static void setUp() {
+		comparator = NumberAwareStringComparator.getInstance();
+	}
+
+	@Test
+	public void houseNumbers() {
+		// With normal string compare a letter is bigger than a digit => 100A would be bigger than 1000A
+		// With number aware compare 100A should be smaller
+		assertThat(comparator.compare("100A", "1000A"), lessThanOrEqualTo(-1)); //$NON-NLS-1$//$NON-NLS-2$
+
+		assertThat(comparator.compare("2A", "10A"), lessThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("2B", "10A"), lessThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("2B", "2A"), greaterThanOrEqualTo(1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("25C", "25C"), equalTo(0)); //$NON-NLS-1$ //$NON-NLS-2$
+	}
+
+	@Test
+	public void mixedSameSegments() {
+		assertThat(comparator.compare("n1a1", "n1aa"), lessThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1aa", "n1a1"), greaterThanOrEqualTo(1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1aa", "n1aa"), equalTo(0)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1a1", "n1a1"), equalTo(0)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1a1", "n1a"), greaterThanOrEqualTo(1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1a1", "n1aa1"), lessThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1b1", "n1a2"), greaterThanOrEqualTo(1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1a1", "n1a1a"), lessThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+
+		assertThat(comparator.compare("n1a2", "n1a1a"), greaterThanOrEqualTo(-1)); //$NON-NLS-1$ //$NON-NLS-2$
+	}
+}