Fix item caption generators in RadioButton/CheckBoxGroup

This patch corrects the behavior of setting caption generators
that may return null.

Additionally renames RadioButtonGroup's ItemIconProviders to
generators, unifying it with other select components.

Change-Id: I5e8491834dccce1c37ab9c2002acc475e8945f4b

8 files changed, 124 insertions, 80 deletions

diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java
index 3d2cb151b8..4094ee383c 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java
@@ -229,8 +229,13 @@ public abstract class AbstractMultiSelect<T>
     private class MultiSelectDataGenerator implements DataGenerator<T> {
         @Override
         public void generateData(T data, JsonObject jsonObject) {
-            jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE,
-                    getItemCaptionGenerator().apply(data));
+            String caption = getItemCaptionGenerator().apply(data);
+            if (caption != null) {
+                jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE,
+                        caption);
+            } else {
+                jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, "");
+            }
             Resource icon = getItemIconGenerator().apply(data);
             if (icon != null) {
                 String iconUrl = ResourceReference
@@ -381,8 +386,8 @@ public abstract class AbstractMultiSelect<T>
 
     /**
      * Sets the item enabled predicate for this multiselect. The predicate is
-     * applied to each item to determine whether the item should be enabled
-     * ({@code true}) or disabled ({@code false}). Disabled items are displayed
+     * applied to each item to determine whether the item should be enabled (
+     * {@code true}) or disabled ({@code false}). Disabled items are displayed
      * as grayed out and the user cannot select them. The default predicate
      * always returns {@code true} (all the items are enabled).
      * <p>
diff --git a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java
index f22e4a9535..2e7ff8a706 100644
--- a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java
+++ b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java
@@ -16,6 +16,10 @@
 
 package com.vaadin.ui;
 
+import java.util.Collection;
+import java.util.Objects;
+import java.util.function.Predicate;
+
 import com.vaadin.data.Listing;
 import com.vaadin.server.Resource;
 import com.vaadin.server.ResourceReference;
@@ -23,16 +27,12 @@ import com.vaadin.server.data.DataGenerator;
 import com.vaadin.server.data.DataSource;
 import com.vaadin.shared.ui.ListingJsonConstants;
 import com.vaadin.shared.ui.optiongroup.RadioButtonGroupState;
-import elemental.json.JsonObject;
 
-import java.util.Collection;
-import java.util.Objects;
-import java.util.function.Function;
-import java.util.function.Predicate;
+import elemental.json.JsonObject;
 
 /**
- * A group of RadioButtons. Individual radiobuttons are made from items supplied by
- * a {@link DataSource}. RadioButtons may have captions and icons.
+ * A group of RadioButtons. Individual radiobuttons are made from items supplied
+ * by a {@link DataSource}. RadioButtons may have captions and icons.
  *
  * @param <T>
  *            item type
@@ -41,9 +41,9 @@ import java.util.function.Predicate;
  */
 public class RadioButtonGroup<T> extends AbstractSingleSelect<T> {
 
-    private Function<T, Resource> itemIconProvider = item -> null;
+    private IconGenerator<T> itemIconGenerator = item -> null;
 
-    private Function<T, String> itemCaptionProvider = String::valueOf;
+    private ItemCaptionGenerator<T> itemCaptionGenerator = String::valueOf;
 
     private Predicate<T> itemEnabledProvider = item -> true;
 
@@ -98,9 +98,14 @@ public class RadioButtonGroup<T> extends AbstractSingleSelect<T> {
         addDataGenerator(new DataGenerator<T>() {
             @Override
             public void generateData(T data, JsonObject jsonObject) {
-                jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE,
-                        itemCaptionProvider.apply(data));
-                Resource icon = itemIconProvider.apply(data);
+                String caption = getItemCaptionGenerator().apply(data);
+                if (caption != null) {
+                    jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE,
+                            caption);
+                } else {
+                    jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, "");
+                }
+                Resource icon = getItemIconGenerator().apply(data);
                 if (icon != null) {
                     String iconUrl = ResourceReference
                             .create(icon, RadioButtonGroup.this, null).getURL();
@@ -161,52 +166,58 @@ public class RadioButtonGroup<T> extends AbstractSingleSelect<T> {
     }
 
     /**
-     * Returns the item icons provider.
+     * Returns the item icon generator.
      *
-     * @return the icons provider for items
+     * @return the currently set icon generator
      * @see #setItemIconProvider
+     * @see IconGenerator
      */
-    public Function<T, Resource> getItemIconProvider() {
-        return itemIconProvider;
+    public IconGenerator<T> getItemIconGenerator() {
+        return itemIconGenerator;
     }
 
     /**
-     * Sets the item icon provider for this radiobutton group. The icon provider is
-     * queried for each item to optionally display an icon next to the item
-     * caption. If the provider returns null for an item, no icon is displayed.
-     * The default provider always returns null (no icons).
+     * Sets the item icon generator for this radiobutton group. The icon
+     * generator is queried for each item to optionally display an icon next to
+     * the item caption. If the provider returns null for an item, no icon is
+     * displayed. The default provider always returns null (no icons).
      *
-     * @param itemIconProvider
-     *            icons provider, not null
+     * @param itemIconGenerator
+     *            the icon generator, not null
+     * @see IconGenerator
      */
-    public void setItemIconProvider(Function<T, Resource> itemIconProvider) {
-        Objects.requireNonNull(itemIconProvider);
-        this.itemIconProvider = itemIconProvider;
+    public void setItemIconGenerator(IconGenerator<T> itemIconGenerator) {
+        Objects.requireNonNull(itemIconGenerator,
+                "Item icon generator cannot be null.");
+        this.itemIconGenerator = itemIconGenerator;
     }
 
     /**
-     * Returns the item caption provider.
+     * Returns the currently set item caption generator.
      *
-     * @return the captions provider
-     * @see #setItemCaptionProvider
+     * @return the currently set caption generator
+     * @see #setItemCaptionGenerator
+     * @see ItemCaptionGenerator
      */
-    public Function<T, String> getItemCaptionProvider() {
-        return itemCaptionProvider;
+    public ItemCaptionGenerator<T> getItemCaptionGenerator() {
+        return itemCaptionGenerator;
     }
 
     /**
-     * Sets the item caption provider for this radiobutton group. The caption
-     * provider is queried for each item to optionally display an item textual
-     * representation. The default provider returns
+     * Sets the item caption generator for this radiobutton group. The caption
+     * generator is queried for each item to optionally display an item textual
+     * representation. The default generator returns
      * {@code String.valueOf(item)}.
      *
-     * @param itemCaptionProvider
-     *            the item caption provider, not null
+     * @param itemCaptionGenerator
+     *            the item caption generator, not null
+     * @see ItemCaptionGenerator
      */
-    public void setItemCaptionProvider(
-            Function<T, String> itemCaptionProvider) {
-        Objects.requireNonNull(itemCaptionProvider);
-        this.itemCaptionProvider = itemCaptionProvider;
+    public void setItemCaptionGenerator(
+            ItemCaptionGenerator<T> itemCaptionGenerator) {
+        Objects.requireNonNull(itemCaptionGenerator,
+                "Item caption generator cannot be null.");
+        this.itemCaptionGenerator = itemCaptionGenerator;
     }
 
     /**
@@ -220,8 +231,8 @@ public class RadioButtonGroup<T> extends AbstractSingleSelect<T> {
     }
 
     /**
-     * Sets the item enabled predicate for this radiobutton group. The predicate is
-     * applied to each item to determine whether the item should be enabled
+     * Sets the item enabled predicate for this radiobutton group. The predicate
+     * is applied to each item to determine whether the item should be enabled
      * (true) or disabled (false). Disabled items are displayed as grayed out
      * and the user cannot select them. The default predicate always returns
      * true (all the items are enabled).
diff --git a/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java b/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java
index 2ec2404515..2fa64ee0ec 100644
--- a/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java
+++ b/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java
@@ -40,7 +40,7 @@ public class RadioButtonGroupBoVTest
     public void createOptionGroup() {
         RadioButtonGroup<Status> s = new RadioButtonGroup<>();
         s.setItems(EnumSet.allOf(Status.class));
-        s.setItemCaptionProvider(Status::getCaption);
+        s.setItemCaptionGenerator(Status::getCaption);
     }
 
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java
index fadd94a4be..71fd0fd37d 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java
@@ -1,11 +1,13 @@
 package com.vaadin.tests.components.abstractlisting;
 
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import com.vaadin.shared.data.selection.SelectionModel.Multi;
 import com.vaadin.ui.AbstractMultiSelect;
+import com.vaadin.ui.ItemCaptionGenerator;
 
 public abstract class AbstractMultiSelectTestUI<MULTISELECT extends AbstractMultiSelect<Object>>
         extends AbstractListingTestUI<MULTISELECT> {
@@ -21,19 +23,18 @@ public abstract class AbstractMultiSelectTestUI<MULTISELECT extends AbstractMult
     }
 
     protected void createItemCaptionGeneratorMenu() {
-        createBooleanAction("Use Item Caption Generator", "Item Generator",
-                false, this::useItemCaptionProvider);
-    }
+        LinkedHashMap<String, ItemCaptionGenerator<Object>> options = new LinkedHashMap<>();
+        options.put("Null Caption Generator", item -> null);
+        options.put("Default Caption Generator", item -> item.toString());
+        options.put("Custom Caption Generator",
+                item -> item.toString() + " Caption");
 
-    private void useItemCaptionProvider(MULTISELECT select, boolean activate,
-            Object data) {
-        if (activate) {
-            select.setItemCaptionGenerator(
-                    item -> item.toString() + " Caption");
-        } else {
-            select.setItemCaptionGenerator(item -> item.toString());
-        }
-        select.getDataSource().refreshAll();
+        createSelectAction("Item Caption Generator", "Item Generator", options,
+                "None", (abstractMultiSelect, captionGenerator, data) -> {
+                    abstractMultiSelect
+                            .setItemCaptionGenerator(captionGenerator);
+                    abstractMultiSelect.getDataSource().refreshAll();
+                }, true);
     }
 
     protected void createSelectionMenu() {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
index 19d9ffd3f3..041c578f1b 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
@@ -15,10 +15,12 @@
  */
 package com.vaadin.tests.components.radiobutton;
 
+import java.util.LinkedHashMap;
 import java.util.stream.IntStream;
 
 import com.vaadin.shared.data.selection.SelectionModel;
 import com.vaadin.tests.components.abstractlisting.AbstractListingTestUI;
+import com.vaadin.ui.ItemCaptionGenerator;
 import com.vaadin.ui.RadioButtonGroup;
 
 /**
@@ -42,7 +44,7 @@ public class RadioButtonGroupTestUI
         super.createActions();
         createListenerMenu();
         createSelectionMenu();
-        createItemProviderMenu();
+        createItemGeneratorMenu();
     }
 
     protected void createSelectionMenu() {
@@ -59,19 +61,18 @@ public class RadioButtonGroupTestUI
                         selectionCategory, toggleSelection, item));
     }
 
-    private void createItemProviderMenu() {
-        createBooleanAction("Use Item Caption Provider", "Item Provider", false,
-                this::useItemCaptionProvider);
-    }
+    private void createItemGeneratorMenu() {
+        LinkedHashMap<String, ItemCaptionGenerator<Object>> options = new LinkedHashMap<>();
+        options.put("Null Caption Generator", item -> null);
+        options.put("Default Caption Generator", item -> item.toString());
+        options.put("Custom Caption Generator",
+                item -> item.toString() + " Caption");
 
-    private void useItemCaptionProvider(RadioButtonGroup<Object> group,
-            boolean activate, Object data) {
-        if (activate) {
-            group.setItemCaptionProvider(item -> item.toString() + " Caption");
-        } else {
-            group.setItemCaptionProvider(item -> item.toString());
-        }
-        group.getDataSource().refreshAll();
+        createSelectAction("Item Caption Generator", "Item Generator", options,
+                "None", (radioButtonGroup, captionGenerator, data) -> {
+                    radioButtonGroup.setItemCaptionGenerator(captionGenerator);
+                    radioButtonGroup.getDataSource().refreshAll();
+                }, true);
     }
 
     private void toggleSelection(String item) {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
index 4bb1defbf6..b4e462a983 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
@@ -123,12 +123,21 @@ public class CheckBoxGroupTest extends MultiBrowserTest {
 
     @Test
     public void itemCaptionGenerator() {
-        selectMenuPath("Component", "Item Generator",
-                "Use Item Caption Generator");
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Custom Caption Generator");
         assertItems(20, " Caption");
     }
 
     @Test
+    public void nullItemCaptionGenerator() {
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Null Caption Generator");
+        for (String text : getSelect().getOptions()) {
+            Assert.assertEquals("", text);
+        }
+    }
+
+    @Test
     public void itemIconGenerator() {
         selectMenuPath("Component", "Item Generator",
                 "Use Item Icon Generator");
diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java
index c909333290..42d912edd6 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java
@@ -112,13 +112,22 @@ public class RadioButtonGroupTest extends MultiBrowserTest {
     }
 
     @Test
-    public void itemCaptionProvider() {
-        selectMenuPath("Component", "Item Provider",
-                "Use Item Caption Provider");
+    public void itemCaptionGenerator() {
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Custom Caption Generator");
         assertItems(20, " Caption");
     }
 
     @Test
+    public void nullItemCaptionGenerator() {
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Null Caption Generator");
+        for (String text : getSelect().getOptions()) {
+            Assert.assertEquals("", text);
+        }
+    }
+
+    @Test
     public void selectProgramatically() {
         selectMenuPath("Component", "Listeners", "Selection listener");
 
@@ -162,5 +171,4 @@ public class RadioButtonGroupTest extends MultiBrowserTest {
         }
         assertEquals("Number of items", count, i);
     }
-
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java b/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java
index e5f44a405b..ad2752112f 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java
@@ -167,13 +167,22 @@ public class TwinColSelectTest extends MultiBrowserTest {
     }
 
     @Test
-    public void itemCaptionProvider() {
-        selectMenuPath("Component", "Item Generator",
-                "Use Item Caption Generator");
+    public void itemCaptionGenerator() {
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Custom Caption Generator");
         assertItems(20, " Caption");
     }
 
     @Test
+    public void nullItemCaptionGenerator() {
+        selectMenuPath("Component", "Item Generator", "Item Caption Generator",
+                "Null Caption Generator");
+        for (String text : getTwinColSelect().getOptions()) {
+            Assert.assertEquals("", text);
+        }
+    }
+
+    @Test
     public void selectProgramatically() {
         selectMenuPath("Component", "Listeners", "Selection listener");