changeset 5740:d892fc853b2a

RT-34327: Comparator in TreeTableView has wrong generic type Reviewed-by: dgrieve
author jgiles
date Tue, 19 Nov 2013 07:59:52 +1300
parents 881baee7d434
children 2650ad7e53bf
files modules/controls/src/main/java/javafx/scene/control/TreeTableView.java modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java
diffstat 2 files changed, 61 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Mon Nov 18 13:26:53 2013 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Tue Nov 19 07:59:52 2013 +1300
@@ -1237,19 +1237,19 @@
      * order list contains the columns that have been added to it either programmatically
      * or via a user clicking on the headers themselves.
      */
-    private ReadOnlyObjectWrapper<Comparator<S>> comparator;
-    private void setComparator(Comparator<S> value) {
+    private ReadOnlyObjectWrapper<Comparator<TreeItem<S>>> comparator;
+    private void setComparator(Comparator<TreeItem<S>> value) {
         comparatorPropertyImpl().set(value);
     }
-    public final Comparator<S> getComparator() {
+    public final Comparator<TreeItem<S>> getComparator() {
         return comparator == null ? null : comparator.get();
     }
-    public final ReadOnlyObjectProperty<Comparator<S>> comparatorProperty() {
+    public final ReadOnlyObjectProperty<Comparator<TreeItem<S>>> comparatorProperty() {
         return comparatorPropertyImpl().getReadOnlyProperty();
     }
-    private ReadOnlyObjectWrapper<Comparator<S>> comparatorPropertyImpl() {
+    private ReadOnlyObjectWrapper<Comparator<TreeItem<S>>> comparatorPropertyImpl() {
         if (comparator == null) {
-            comparator = new ReadOnlyObjectWrapper<Comparator<S>>(this, "comparator");
+            comparator = new ReadOnlyObjectWrapper<>(this, "comparator");
         }
         return comparator;
     }
@@ -1596,11 +1596,11 @@
         final ObservableList<TreeTableColumn<S,?>> sortOrder = getSortOrder();
         
         // update the Comparator property
-        final Comparator<S> oldComparator = getComparator();
+        final Comparator<TreeItem<S>> oldComparator = getComparator();
         if (sortOrder.isEmpty()) {
             setComparator(null);
         } else {
-            Comparator<S> newComparator = new TableColumnComparatorBase.TreeTableColumnComparator(sortOrder);
+            Comparator<TreeItem<S>> newComparator = new TableColumnComparatorBase.TreeTableColumnComparator(sortOrder);
             setComparator(newComparator);
         }
         
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Mon Nov 18 13:26:53 2013 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Tue Nov 19 07:59:52 2013 +1300
@@ -28,12 +28,7 @@
 import static com.sun.javafx.scene.control.infrastructure.ControlTestUtils.assertStyleClassContains;
 import static javafx.scene.control.TreeTableColumn.SortType.ASCENDING;
 import static javafx.scene.control.TreeTableColumn.SortType.DESCENDING;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 import java.util.Comparator;
 import java.util.List;
@@ -2657,4 +2652,56 @@
         treeTableView.edit(0, col);
         assertEquals(1, rt_29849_start_count);
     }
+
+    @Test public void test_rt_34327() {
+        // by default the comparator is null.
+        // NOTE: this method (prior to the fix as part of RT-34327) would have
+        // returned Comparator<String>, but after the fix it correctly returns
+        // a Comparator<TreeItem<String>>
+        Comparator nonGenericComparator = treeTableView.getComparator();
+        Comparator<TreeItem<String>> genericComparator = treeTableView.getComparator();
+        assertNull(nonGenericComparator);
+        assertNull(genericComparator);
+
+        // add in a column and some data
+        TreeTableColumn<String, String> col = new TreeTableColumn<>("column");
+        col.setEditable(true);
+        col.setCellValueFactory(new Callback<TreeTableColumn.CellDataFeatures<String, String>, ObservableValue<String>>() {
+            @Override public ObservableValue<String> call(TreeTableColumn.CellDataFeatures<String, String> param) {
+                return new ReadOnlyObjectWrapper<>(param.getValue().getValue());
+            }
+        });
+        treeTableView.getColumns().add(col);
+
+        installChildren();
+
+        // sort by that column
+        treeTableView.getSortOrder().add(col);
+
+        // get the new comparator, which should no longer be null
+        nonGenericComparator = treeTableView.getComparator();
+        genericComparator = treeTableView.getComparator();
+        assertNotNull(nonGenericComparator);
+        assertNotNull(genericComparator);
+
+        // now, as noted above, previously we would use the Comparator to compare
+        // two String instances, which would fail at runtime as the Comparator
+        // was actually expecting to compare two TreeItem<String>, but the API
+        // was failing us.
+        try {
+            nonGenericComparator.compare("abc", "def");
+            fail("This should not work!");
+        } catch (ClassCastException e) {
+            // if we get the exception, we're happy
+        }
+
+        try {
+            Object string1 = "abc";
+            Object string2 = "def";
+            genericComparator.compare((TreeItem<String>)string1, (TreeItem<String>)string2);
+            fail("This should not work!");
+        } catch (ClassCastException e) {
+            // if we get the exception, we're happy
+        }
+    }
 }