diff options
9 files changed, 197 insertions, 29 deletions
diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index f3dc3d9389..bb740abbcf 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -805,8 +805,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, * @param renderer * the type of value, not <code>null</code> */ - protected Column(ValueProvider<T, ? extends V> valueProvider, - Renderer<V> renderer) { + protected Column(ValueProvider<T, V> valueProvider, + Renderer<? super V> renderer) { Objects.requireNonNull(valueProvider, "Value provider can't be null"); Objects.requireNonNull(renderer, "Renderer can't be null"); @@ -824,12 +824,11 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, // removed addExtension(renderer); - Class<V> valueType = renderer.getPresentationType(); + Class<? super V> valueType = renderer.getPresentationType(); if (Comparable.class.isAssignableFrom(valueType)) { comparator = (a, b) -> compareComparables( valueProvider.apply(a), valueProvider.apply(b)); - state.sortable = true; } else if (Number.class.isAssignableFrom(valueType)) { /* * Value type will be Number whenever using NumberRenderer. @@ -839,12 +838,39 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, comparator = (a, b) -> compareNumbers( (Number) valueProvider.apply(a), (Number) valueProvider.apply(b)); - state.sortable = true; } else { - state.sortable = false; + comparator = (a, b) -> compareMaybeComparables( + valueProvider.apply(a), valueProvider.apply(b)); + } + } + + private static int compareMaybeComparables(Object a, Object b) { + if (hasCommonComparableBaseType(a, b)) { + return compareComparables(a, b); + } else { + return compareComparables(a.toString(), b.toString()); } } + private static boolean hasCommonComparableBaseType(Object a, Object b) { + if (a instanceof Comparable<?> && b instanceof Comparable<?>) { + Class<?> aClass = a.getClass(); + Class<?> bClass = b.getClass(); + + if (aClass == bClass) { + return true; + } + + Class<?> baseType = ReflectTools.findCommonBaseType(aClass, + bClass); + if (!baseType.isAssignableFrom(Comparable.class)) { + return false; + } + } + + return false; + } + @SuppressWarnings("unchecked") private static int compareComparables(Object a, Object b) { return ((Comparator) Comparator @@ -2103,17 +2129,18 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, /** * Adds a new text column to this {@link Grid} with a value provider. The * column will use a {@link TextRenderer}. The value is converted to a - * String using {@link Object#toString()}. Sorting in memory is executed by - * comparing the String values. + * String using {@link Object#toString()}. In-memory sorting will use the + * natural ordering of elements if they are mutually comparable and + * otherwise fall back to comparing the string representations of the + * values. * * @param valueProvider * the value provider * * @return the new column */ - public Column<T, String> addColumn(ValueProvider<T, ?> valueProvider) { - return addColumn(t -> String.valueOf(valueProvider.apply(t)), - new TextRenderer()); + public <V> Column<T, V> addColumn(ValueProvider<T, V> valueProvider) { + return addColumn(valueProvider, new TextRenderer()); } /** @@ -2131,9 +2158,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, * * @see AbstractRenderer */ - public <V> Column<T, V> addColumn( - ValueProvider<T, ? extends V> valueProvider, - AbstractRenderer<? super T, V> renderer) { + public <V> Column<T, V> addColumn(ValueProvider<T, V> valueProvider, + AbstractRenderer<? super T, ? super V> renderer) { String generatedIdentifier = getGeneratedIdentifier(); Column<T, V> column = new Column<>(valueProvider, renderer); addColumn(generatedIdentifier, column); @@ -3047,7 +3073,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, /** * This method is a shorthand that delegates to the currently set selection * model. - * + * * @see #getSelectionModel() * @see GridSelectionModel */ @@ -3058,7 +3084,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, /** * This method is a shorthand that delegates to the currently set selection * model. - * + * * @see #getSelectionModel() * @see GridSelectionModel */ @@ -3069,7 +3095,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, /** * This method is a shorthand that delegates to the currently set selection * model. - * + * * @see #getSelectionModel() * @see GridSelectionModel */ @@ -3080,7 +3106,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, /** * This method is a shorthand that delegates to the currently set selection * model. - * + * * @see #getSelectionModel() * @see GridSelectionModel */ diff --git a/server/src/main/java/com/vaadin/ui/renderers/TextRenderer.java b/server/src/main/java/com/vaadin/ui/renderers/TextRenderer.java index b85e3230bc..eb749d1e8f 100644 --- a/server/src/main/java/com/vaadin/ui/renderers/TextRenderer.java +++ b/server/src/main/java/com/vaadin/ui/renderers/TextRenderer.java @@ -17,29 +17,44 @@ package com.vaadin.ui.renderers; import com.vaadin.shared.ui.grid.renderers.TextRendererState; +import elemental.json.Json; +import elemental.json.JsonValue; + /** - * A renderer for presenting simple plain-text string values. + * A renderer for presenting a plain text representation of any value. + * {@link Object#toString()} is used for determining the text to show. * * @since 7.4 * @author Vaadin Ltd */ -public class TextRenderer extends AbstractRenderer<Object, String> { +public class TextRenderer extends AbstractRenderer<Object, Object> { /** - * Creates a new text renderer + * Creates a new text renderer that uses <code>""</code> to represent null + * values. */ public TextRenderer() { this(""); } /** - * Creates a new text renderer + * Creates a new text renderer with the given string to represent null + * values. * * @param nullRepresentation * the textual representation of {@code null} value */ public TextRenderer(String nullRepresentation) { - super(String.class, nullRepresentation); + super(Object.class, nullRepresentation); + } + + @Override + public JsonValue encode(Object value) { + if (value == null) { + return super.encode(null); + } else { + return Json.create(value.toString()); + } } @Override diff --git a/server/src/main/java/com/vaadin/util/ReflectTools.java b/server/src/main/java/com/vaadin/util/ReflectTools.java index bd1035dac5..beda104230 100644 --- a/server/src/main/java/com/vaadin/util/ReflectTools.java +++ b/server/src/main/java/com/vaadin/util/ReflectTools.java @@ -216,4 +216,37 @@ public class ReflectTools implements Serializable { private ReflectTools() { } + + /** + * Finds the most specific class that both provided classes extend from. + * + * @param a + * one class to get the base type for, not <code>null</code> + * @param b + * another class to get the base type for, not <code>null</code> + * @return the most specific base class, not <code>null</code> + * + * @since + */ + public static Class<?> findCommonBaseType(Class<?> a, Class<?> b) { + if (a.isInterface()) { + throw new IllegalArgumentException("a cannot be an interface"); + } + if (b.isInterface()) { + throw new IllegalArgumentException("b cannot be an interface"); + } + + if (a.isAssignableFrom(b)) { + return a; + } else if (b.isAssignableFrom(a)) { + return b; + } + + Class<?> currentClass = a; + while (!currentClass.isAssignableFrom(b)) { + currentClass = currentClass.getSuperclass(); + } + + return currentClass; + } } diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java index f216593c2f..57c4107a6b 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java @@ -6,10 +6,13 @@ import static org.junit.Assert.assertNotNull; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Random; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -25,6 +28,7 @@ import com.vaadin.data.ValueProvider; import com.vaadin.data.provider.GridSortOrder; import com.vaadin.data.provider.bov.Person; import com.vaadin.event.selection.SelectionEvent; +import com.vaadin.server.SerializableComparator; import com.vaadin.shared.data.sort.SortDirection; import com.vaadin.shared.ui.grid.HeightMode; import com.vaadin.tests.util.MockUI; @@ -41,8 +45,8 @@ public class GridTest { private Grid<String> grid; private Column<String, String> fooColumn; - private Column<String, Number> lengthColumn; - private Column<String, String> objectColumn; + private Column<String, Integer> lengthColumn; + private Column<String, Object> objectColumn; private Column<String, String> randomColumn; @Before @@ -453,6 +457,43 @@ public class GridTest { grid.setColumnOrder("randomColumnId", "length"); } + @Test + public void defaultSorting_comparableTypes() { + testValueProviderSorting(1, 2, 3); + } + + @Test + public void defaultSorting_strings() { + testValueProviderSorting("a", "b", "c"); + } + + @Test + public void defaultSorting_notComparable() { + assert !Comparable.class.isAssignableFrom(AtomicInteger.class); + + testValueProviderSorting(new AtomicInteger(10), new AtomicInteger(8), + new AtomicInteger(9)); + } + + @Test + public void defaultSorting_differentComparables() { + testValueProviderSorting(10.1, 200, 3000.1, 4000); + } + + private static void testValueProviderSorting(Object... expectedOrder) { + SerializableComparator<Object> comparator = new Grid<>() + .addColumn(ValueProvider.identity()) + .getComparator(SortDirection.ASCENDING); + + Assert.assertNotNull(comparator); + + List<Object> values = new ArrayList<>(Arrays.asList(expectedOrder)); + Collections.shuffle(values, new Random(42)); + + Assert.assertArrayEquals(expectedOrder, + values.stream().sorted(comparator).toArray()); + } + private static <T> JsonObject getRowData(Grid<T> grid, T row) { JsonObject json = Json.createObject(); if (grid.getColumns().isEmpty()) { diff --git a/server/src/test/java/com/vaadin/util/ReflectToolsTest.java b/server/src/test/java/com/vaadin/util/ReflectToolsTest.java new file mode 100644 index 0000000000..353bf0972c --- /dev/null +++ b/server/src/test/java/com/vaadin/util/ReflectToolsTest.java @@ -0,0 +1,53 @@ +/* + * Copyright 2000-2016 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.util; + +import java.io.Serializable; + +import org.junit.Assert; +import org.junit.Test; + +public class ReflectToolsTest implements Serializable { + @Test + public void findCommonBaseType_sameType() { + Assert.assertSame(Number.class, + ReflectTools.findCommonBaseType(Number.class, Number.class)); + } + + @Test + public void findCommonBaseType_aExtendsB() { + Assert.assertSame(Number.class, + ReflectTools.findCommonBaseType(Integer.class, Number.class)); + } + + @Test + public void findCommonBaseType_bExtendsA() { + Assert.assertSame(Number.class, + ReflectTools.findCommonBaseType(Number.class, Integer.class)); + } + + @Test + public void findCommonBaseType_commonBase() { + Assert.assertSame(Number.class, + ReflectTools.findCommonBaseType(Double.class, Integer.class)); + } + + @Test + public void findCommonBaseType_noCommonBase() { + Assert.assertSame(Object.class, + ReflectTools.findCommonBaseType(String.class, Number.class)); + } +} diff --git a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java index 6bf90afc03..05fbb68721 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java +++ b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java @@ -21,7 +21,7 @@ public class ColumnState extends AbstractGridExtensionState { public String caption; public String internalId; - public boolean sortable; + public boolean sortable = true; public boolean editable = false; /** The caption for the column hiding toggle. */ diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnHiding.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnHiding.java index 1a5a62810b..beea9a99c1 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnHiding.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnHiding.java @@ -18,7 +18,7 @@ public class GridColumnHiding extends AbstractReindeerTestUI { Grid<Person> grid = new Grid<>(); Column<Person, String> nameColumn = grid.addColumn(Person::getFirstName) .setHidable(true).setCaption("Name"); - Column<Person, Number> ageColumn = grid + Column<Person, Integer> ageColumn = grid .addColumn(Person::getAge, new NumberRenderer()) .setHidable(true) .setHidingToggleCaption("custom age column caption") diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnResizing.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnResizing.java index d827e7aee4..b610e01e68 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnResizing.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnResizing.java @@ -22,7 +22,7 @@ public class GridColumnResizing extends AbstractReindeerTestUI { Grid<Person> grid = new Grid<>(); Column<Person, String> nameColumn = grid.addColumn(Person::getFirstName) .setCaption("Name"); - Column<Person, Number> ageColumn = grid + Column<Person, Integer> ageColumn = grid .addColumn(Person::getAge, new NumberRenderer()) .setCaption("Age"); grid.addColumnResizeListener(event -> { diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorMultiselect.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorMultiselect.java index b216ef9fbf..aa2a05ba81 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorMultiselect.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorMultiselect.java @@ -21,7 +21,7 @@ public class GridEditorMultiselect extends AbstractTestUI { Column<Person, String> nameColumn = grid.addColumn(Person::getFirstName) .setCaption("name"); - Column<Person, Number> ageColumn = grid + Column<Person, Integer> ageColumn = grid .addColumn(Person::getAge, new NumberRenderer()) .setCaption("age"); |