From 613f36ff23fe06c676f9a71e92c04ccabcd19c63 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Olli=20Tiet=C3=A4v=C3=A4inen?= Date: Fri, 28 Dec 2018 09:25:11 +0200 Subject: [PATCH] Grid column npe behavior (#11390) * add possibility to configure nested null behavior to Grid.Column * added addColumn method that allows configuring the nested null handling behavior of the column * added uitest for nested null value handling, implements #11137 --- server/src/main/java/com/vaadin/ui/Grid.java | 145 +++++++++++++++++- .../GridNullSafeNestedPropertyColumn.java | 65 ++++++++ .../GridNullSafeNestedPropertyColumnTest.java | 46 ++++++ 3 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumn.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumnTest.java diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 7c3467eaa3..81f46f8d59 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -845,6 +845,23 @@ public class Grid extends AbstractListing implements HasComponents, */ public static class Column extends AbstractExtension { + /** + * behavior when parsing nested properties which may contain + * null values in the property chain + */ + public enum NestedNullBehavior { + /** + * throw a NullPointerException if there is a nested + * null value + */ + THROW, + /** + * silently ignore any exceptions caused by nested null + * values + */ + ALLOW_NULLS + } + private final ValueProvider valueProvider; private ValueProvider presentationProvider; @@ -856,6 +873,7 @@ public class Grid extends AbstractListing implements HasComponents, return Stream.of(new QuerySortOrder(id, direction)); }; + private NestedNullBehavior nestedNullBehavior = NestedNullBehavior.THROW; private boolean sortable = true; private SerializableComparator comparator; private StyleGenerator styleGenerator = item -> null; @@ -990,6 +1008,38 @@ public class Grid extends AbstractListing implements HasComponents, } } + /** + * Constructs a new Column configuration with given renderer and value + * provider. + *

+ * For a more complete explanation on presentation provider, see + * {@link #setRenderer(ValueProvider, Renderer)}. + * + * @param valueProvider + * the function to get values from items, not + * null + * @param presentationProvider + * the function to get presentations from the value of this + * column, not null. For more details, see + * {@link #setRenderer(ValueProvider, Renderer)} + * @param nestedNullBehavior + * behavior on encountering nested null values + * when reading the value from the bean + * @param renderer + * the presentation renderer, not null + * @param

+ * the presentation type + * + * @since + */ + protected

Column(ValueProvider valueProvider, + ValueProvider presentationProvider, + Renderer renderer, + NestedNullBehavior nestedNullBehavior) { + this(valueProvider, presentationProvider, renderer); + this.nestedNullBehavior = nestedNullBehavior; + } + private static int compareMaybeComparables(Object a, Object b) { if (hasCommonComparableBaseType(a, b)) { return compareComparables(a, b); @@ -1053,8 +1103,16 @@ public class Grid extends AbstractListing implements HasComponents, @SuppressWarnings("unchecked") private

JsonValue generateRendererValue(T item, ValueProvider presentationProvider, Connector renderer) { - P presentationValue = presentationProvider - .apply(valueProvider.apply(item)); + V value; + try { + value = valueProvider.apply(item); + } catch (NullPointerException npe) { + value = null; + if (NestedNullBehavior.THROW == nestedNullBehavior) { + throw npe; + } + } + P presentationValue = presentationProvider.apply(value); // Make Grid track components. if (renderer instanceof ComponentRenderer @@ -2722,6 +2780,59 @@ public class Grid extends AbstractListing implements HasComponents, return column; } + /** + * Adds a new column with the given property name and renderer. The property + * name will be used as the {@link Column#getId() column id} and the + * {@link Column#getCaption() column caption} will be set based on the + * property definition. + *

+ * This method can only be used for a Grid created using + * {@link Grid#Grid(Class)} or {@link #withPropertySet(PropertySet)}. + *

+ * You can add columns for nested properties with dot notation, eg. + * "property.nestedProperty" + * + * @param propertyName + * the property name of the new column, not null + * @param renderer + * the renderer to use, not null + * @param nestedNullBehavior + * the behavior when + * @return the newly added column, not null + */ + public Column addColumn(String propertyName, + AbstractRenderer renderer, + Column.NestedNullBehavior nestedNullBehavior) { + Objects.requireNonNull(propertyName, "Property name cannot be null"); + Objects.requireNonNull(renderer, "Renderer cannot be null"); + + if (getColumn(propertyName) != null) { + throw new IllegalStateException( + "There is already a column for " + propertyName); + } + + PropertyDefinition definition = propertySet + .getProperty(propertyName) + .orElseThrow(() -> new IllegalArgumentException( + "Could not resolve property name " + propertyName + + " from " + propertySet)); + + if (!renderer.getPresentationType() + .isAssignableFrom(definition.getType())) { + throw new IllegalArgumentException( + renderer + " cannot be used with a property of type " + + definition.getType().getName()); + } + @SuppressWarnings({ "unchecked", "rawtypes" }) + Column column = createColumn(definition.getGetter(), + ValueProvider.identity(), (AbstractRenderer) renderer, + nestedNullBehavior); + String generatedIdentifier = getGeneratedIdentifier(); + addColumn(generatedIdentifier, column); + column.setId(definition.getName()).setCaption(definition.getCaption()); + return column; + } + /** * 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 @@ -2824,7 +2935,7 @@ public class Grid extends AbstractListing implements HasComponents, /** * Adds a column that shows components. *

- * This is a shorthand for {@link #addColum()} with a + * This is a shorthand for {@link #addColumn()} with a * {@link ComponentRenderer}. * * @param componentProvider @@ -2865,6 +2976,34 @@ public class Grid extends AbstractListing implements HasComponents, return new Column<>(valueProvider, presentationProvider, renderer); } + /** + * Creates a column instance from a value provider, presentation provider + * and a renderer. + * + * @param valueProvider + * the value provider + * @param presentationProvider + * the presentation provider + * @param renderer + * the renderer + * @param nestedNullBehavior + * the behavior when facing nested null values + * @return a new column instance + * @param + * the column value type + * @param

+ * the column presentation type + * + * @since + */ + private Column createColumn(ValueProvider valueProvider, + ValueProvider presentationProvider, + AbstractRenderer renderer, + Column.NestedNullBehavior nestedNullBehavior) { + return new Column<>(valueProvider, presentationProvider, renderer, + nestedNullBehavior); + } + private void addColumn(String identifier, Column column) { if (getColumns().contains(column)) { return; diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumn.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumn.java new file mode 100644 index 0000000000..888296d5f8 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumn.java @@ -0,0 +1,65 @@ +package com.vaadin.tests.components.grid; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.data.provider.ListDataProvider; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.tests.data.bean.Address; +import com.vaadin.tests.data.bean.Country; +import com.vaadin.tests.data.bean.Person; +import com.vaadin.tests.data.bean.Sex; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; +import com.vaadin.ui.TextField; +import com.vaadin.ui.renderers.TextRenderer; + +import java.util.ArrayList; +import java.util.List; + +@Widgetset("com.vaadin.DefaultWidgetSet") +public class GridNullSafeNestedPropertyColumn extends AbstractTestUI { + + private List personList = new ArrayList<>(); + private ListDataProvider listDataProvider; + private Grid.Column nullSafeColumn = null; + private Grid.Column regularColumn = null; + + @Override + protected void setup(VaadinRequest request) { + Grid grid = new Grid<>(Person.class); + grid.setSizeFull(); + + personList.add(new Person("person", "with", "an address", 0, + Sex.UNKNOWN, new Address("street", 0, "", Country.FINLAND))); + listDataProvider = new ListDataProvider<>(personList); + grid.setDataProvider(listDataProvider); + + Button addPersonButton = new Button("add person with a null address", + event -> { + Address address = null; + Person person = new Person("person", "without", "address", + 42, Sex.UNKNOWN, address); + personList.add(person); + listDataProvider.refreshAll(); + }); + addPersonButton.setId("add"); + + Button addSafeColumnButton = new Button( + "add 'address.streetAddress' as a null-safe column", event -> { + nullSafeColumn = grid.addColumn("address.streetAddress", + new TextRenderer(), + Grid.Column.NestedNullBehavior.ALLOW_NULLS); + }); + addSafeColumnButton.setId("safe"); + + Button addUnsafeColumnButton = new Button( + "add 'address.streetAddress' column without nested null safety", + event -> { + regularColumn = grid.addColumn("address.streetAddress"); + }); + addUnsafeColumnButton.setId("unsafe"); + + addComponents(grid, addPersonButton, addSafeColumnButton, + addUnsafeColumnButton); + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumnTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumnTest.java new file mode 100644 index 0000000000..2ac67b7fc2 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridNullSafeNestedPropertyColumnTest.java @@ -0,0 +1,46 @@ +package com.vaadin.tests.components.grid; + +import com.vaadin.testbench.By; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.parallel.TestCategory; +import com.vaadin.tests.tb3.MultiBrowserTest; +import org.junit.Test; +import org.openqa.selenium.WebElement; + +import java.util.List; + +import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertFalse; + +/** + * Tests that using a nested property name with a null bean child property won't + * cause an exception. + */ +@TestCategory("grid") +public class GridNullSafeNestedPropertyColumnTest extends MultiBrowserTest { + + @Test + public void testNullNestedPropertyInSafeGridColumn() { + openTestURL(); + waitForElementPresent(org.openqa.selenium.By.className("v-grid")); + $(ButtonElement.class).id("safe").click(); + $(ButtonElement.class).id("add").click(); + List errorIndicator = findElements( + By.className("v-errorindicator")); + assertTrue(errorIndicator.isEmpty()); + } + + @Test + public void testNullNestedPropertyInUnsafeGridColumn() { + openTestURL(); + waitForElementPresent(org.openqa.selenium.By.className("v-grid")); + $(ButtonElement.class).id("unsafe").click(); + $(ButtonElement.class).id("add").click(); + List errorIndicator = findElements( + By.className("v-errorindicator")); + assertFalse( + "There should be an error indicator when adding nested null values to Grid", + errorIndicator.isEmpty()); + } + +} -- 2.39.5