diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java index 5086322b6e2..57152a0dd81 100755 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java @@ -446,6 +446,7 @@ public static class Column extends AbstractColumn> { }; private SerializableComparator comparator; + private SerializableComparator reversedComparator; private Registration columnDataGeneratorRegistration; private Registration editorDataGeneratorRegistration; @@ -735,10 +736,28 @@ public Element getElement() { public Column setComparator(Comparator comparator) { Objects.requireNonNull(comparator, "Comparator must not be null"); setSortable(true); - this.comparator = comparator::compare; + + internalSetComparator(comparator); return this; } + void internalSetComparator(Comparator comparator) { + // Store the main comparator + if (comparator instanceof SerializableComparator) { + this.comparator = (SerializableComparator) comparator; + } else { + this.comparator = comparator::compare; + } + + // Compute and store the reversed comparator + Comparator reversed = comparator.reversed(); + if (reversed instanceof SerializableComparator) { + this.reversedComparator = (SerializableComparator) reversed; + } else { + this.reversedComparator = reversed::compare; + } + } + /** * Sets a comparator to use with in-memory sorting with this column * based on the return type of the given {@link ValueProvider}.Sorting @@ -782,7 +801,8 @@ public SerializableComparator getComparator( "No comparator defined for sorted column."); setSortable(true); boolean reverse = sortDirection != SortDirection.ASCENDING; - return reverse ? comparator.reversed()::compare : comparator; + + return reverse ? reversedComparator : comparator; } /** @@ -1864,7 +1884,7 @@ protected > C addColumn( item -> formatValueToSendToTheClient( applyValueProvider(valueProvider, item))), columnFactory); - ((Column) column).comparator = ((a, b) -> compareMaybeComparables( + column.internalSetComparator((a, b) -> compareMaybeComparables( applyValueProvider(valueProvider, a), applyValueProvider(valueProvider, b))); return column; diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridColumnTest.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridColumnTest.java index cf937971ac8..32657f8df97 100755 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridColumnTest.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridColumnTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertNotNull; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; import java.util.function.BiFunction; @@ -330,6 +331,140 @@ public void setColumnRowHeader_updatedPropertyValue() { Assert.assertTrue(rowHeaderColumn.isRowHeader()); } + @Test + public void testCustomReversedComparatorIsRespected() { + Grid grid = new Grid<>(); + Column column = grid.addColumn(ValueProvider.identity()); + + // Custom comparator that keeps favorites (even numbers) at the top + // regardless of sort direction + Comparator customComparator = new Comparator() { + private boolean isFavorite(Integer value) { + return value != null && value % 2 == 0; + } + + @Override + public int compare(Integer o1, Integer o2) { + // Favorites first, then natural order + return Comparator.comparing(this::isFavorite).reversed() + .thenComparing(Comparator.naturalOrder()) + .compare(o1, o2); + } + + @Override + public Comparator reversed() { + // Custom reversed: favorites still first, but reversed within + // groups + return (o1, o2) -> { + boolean fav1 = isFavorite(o1); + boolean fav2 = isFavorite(o2); + + if (fav1 != fav2) { + // Favorites always come first + return fav1 ? -1 : 1; + } + // Within the same group, reverse the natural order + return Comparator. reverseOrder().compare(o1, o2); + }; + } + }; + + column.setComparator(customComparator); + + // Get comparators for both directions + SerializableComparator ascComparator = column + .getComparator(SortDirection.ASCENDING); + SerializableComparator descComparator = column + .getComparator(SortDirection.DESCENDING); + + // Test ascending sort + // Favorites (even) should come first: 2, 4, then non-favorites: 1, 3 + Assert.assertTrue("Even 2 should come before odd 1 in ascending", + ascComparator.compare(2, 1) < 0); + Assert.assertTrue("Even 4 should come before odd 3 in ascending", + ascComparator.compare(4, 3) < 0); + Assert.assertTrue( + "Within favorites, 2 should come before 4 in ascending", + ascComparator.compare(2, 4) < 0); + Assert.assertTrue( + "Within non-favorites, 1 should come before 3 in ascending", + ascComparator.compare(1, 3) < 0); + + // Test descending sort with custom reversed implementation + // Favorites (even) should still come first: 4, 2, then non-favorites: + // 3, 1 + Assert.assertTrue( + "Even 2 should come before odd 1 in descending (custom reversed)", + descComparator.compare(2, 1) < 0); + Assert.assertTrue( + "Even 4 should come before odd 3 in descending (custom reversed)", + descComparator.compare(4, 3) < 0); + Assert.assertTrue( + "Within favorites, 4 should come before 2 in descending", + descComparator.compare(4, 2) < 0); + Assert.assertTrue( + "Within non-favorites, 3 should come before 1 in descending", + descComparator.compare(3, 1) < 0); + } + + @Test + public void testSerializableComparatorPreservesOriginal() { + Grid grid = new Grid<>(); + Column column = grid.addColumn(ValueProvider.identity()); + + // Create a serializable comparator with custom reversed + SerializableComparator serializableComparator = new SerializableComparator() { + @Override + public int compare(Integer o1, Integer o2) { + // Even numbers first, then natural order + boolean even1 = o1 != null && o1 % 2 == 0; + boolean even2 = o2 != null && o2 % 2 == 0; + if (even1 != even2) { + return even1 ? -1 : 1; + } + return Integer.compare(o1, o2); + } + + @Override + public Comparator reversed() { + // Custom reversed: evens still first, but reversed within + // groups + return new SerializableComparator() { + @Override + public int compare(Integer o1, Integer o2) { + boolean even1 = o1 != null && o1 % 2 == 0; + boolean even2 = o2 != null && o2 % 2 == 0; + if (even1 != even2) { + return even1 ? -1 : 1; + } + // Within groups, reverse order + return Integer.compare(o2, o1); + } + }; + } + }; + + column.setComparator(serializableComparator); + + // Get comparators for both directions + SerializableComparator ascComparator = column + .getComparator(SortDirection.ASCENDING); + SerializableComparator descComparator = column + .getComparator(SortDirection.DESCENDING); + + // Test that ascending uses original comparator + Assert.assertTrue("2 (even) should come before 1 (odd)", + ascComparator.compare(2, 1) < 0); + Assert.assertTrue("2 should come before 4 within evens", + ascComparator.compare(2, 4) < 0); + + // Test that descending uses custom reversed + Assert.assertTrue("2 (even) should still come before 1 (odd) in desc", + descComparator.compare(2, 1) < 0); + Assert.assertTrue("4 should come before 2 within evens in desc", + descComparator.compare(4, 2) < 0); + } + private void assertEqualColumnClasses(Class columnClass, Class compareTo) { assertNotNull(columnClass); Assert.assertEquals(compareTo, columnClass);