Use converter for Select/Tree/ComboBox when using item id for caption

(#11433)

Change-Id: I10c47986b98e132e874b2882fbb2323409d67a25

5 files changed, 201 insertions, 15 deletions

diff --git a/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java b/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java
index 29bf8fc400..e91dd2a303 100644
--- a/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java
+++ b/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java
@@ -37,6 +37,28 @@ public class StringToEnumConverter implements Converter<String, Enum> {
         if (value == null || value.trim().equals("")) {
             return null;
         }
+
+        return stringToEnum(value, targetType, locale);
+    }
+
+    /**
+     * Converts the given string to the given enum type using the given locale
+     * <p>
+     * Compatible with {@link #enumToString(Enum, Locale)}
+     * 
+     * @param value
+     *            The string value to convert
+     * @param enumType
+     *            The type of enum to create
+     * @param locale
+     *            The locale to use for conversion. If null, the JVM default
+     *            locale will be used
+     * @return The enum which matches the given string
+     * @throws ConversionException
+     *             if the conversion fails
+     */
+    public static <T extends Enum<T>> T stringToEnum(String value,
+            Class<T> enumType, Locale locale) throws ConversionException {
         if (locale == null) {
             locale = Locale.getDefault();
         }
@@ -45,13 +67,13 @@ public class StringToEnumConverter implements Converter<String, Enum> {
         // Foo bar -> FOO_BAR
         String result = value.replace(" ", "_").toUpperCase(locale);
         try {
-            return Enum.valueOf(targetType, result);
+            return Enum.valueOf(enumType, result);
         } catch (Exception ee) {
             // There was no match. Try to compare the available values to see if
             // the constant is using something else than all upper case
             try {
-                EnumSet<?> set = EnumSet.allOf(targetType);
-                for (Enum e : set) {
+                EnumSet<T> set = EnumSet.allOf(enumType);
+                for (T e : set) {
                     if (e.name().toUpperCase(locale).equals(result)) {
                         return e;
                     }
@@ -65,13 +87,21 @@ public class StringToEnumConverter implements Converter<String, Enum> {
         }
     }
 
-    @Override
-    public String convertToPresentation(Enum value,
-            Class<? extends String> targetType, Locale locale)
-            throws ConversionException {
-        if (value == null) {
-            return null;
-        }
+    /**
+     * Converts the given enum to a human readable string using the given locale
+     * <p>
+     * Compatible with {@link #stringToEnum(String, Class, Locale)}
+     * 
+     * @param value
+     *            The enum value to convert
+     * @param locale
+     *            The locale to use for conversion. If null, the JVM default
+     *            locale will be used
+     * @return A human readable string based on the enum
+     * @throws ConversionException
+     *             if the conversion fails
+     */
+    public static String enumToString(Enum<?> value, Locale locale) {
         if (locale == null) {
             locale = Locale.getDefault();
         }
@@ -82,11 +112,21 @@ public class StringToEnumConverter implements Converter<String, Enum> {
         // _FOO -> _foo
         String result = enumString.substring(0, 1).toUpperCase(locale);
         result += enumString.substring(1).toLowerCase(locale).replace('_', ' ');
-
         return result;
     }
 
     @Override
+    public String convertToPresentation(Enum value,
+            Class<? extends String> targetType, Locale locale)
+            throws ConversionException {
+        if (value == null) {
+            return null;
+        }
+
+        return enumToString(value, locale);
+    }
+
+    @Override
     public Class<Enum> getModelType() {
         return Enum.class;
     }
diff --git a/server/src/com/vaadin/ui/AbstractSelect.java b/server/src/com/vaadin/ui/AbstractSelect.java
index 70f08c95d8..1a3eeb88a3 100644
--- a/server/src/com/vaadin/ui/AbstractSelect.java
+++ b/server/src/com/vaadin/ui/AbstractSelect.java
@@ -33,6 +33,8 @@ import com.vaadin.data.Container;
 import com.vaadin.data.Item;
 import com.vaadin.data.Property;
 import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.data.util.converter.Converter;
+import com.vaadin.data.util.converter.ConverterUtil;
 import com.vaadin.event.DataBoundTransferable;
 import com.vaadin.event.Transferable;
 import com.vaadin.event.dd.DragAndDropEvent;
@@ -1181,7 +1183,7 @@ public abstract class AbstractSelect extends AbstractField<Object> implements
         switch (getItemCaptionMode()) {
 
         case ID:
-            caption = itemId.toString();
+            caption = idToCaption(itemId);
             break;
 
         case INDEX:
@@ -1207,7 +1209,7 @@ public abstract class AbstractSelect extends AbstractField<Object> implements
         case EXPLICIT_DEFAULTS_ID:
             caption = itemCaptions.get(itemId);
             if (caption == null) {
-                caption = itemId.toString();
+                caption = idToCaption(itemId);
             }
             break;
 
@@ -1227,6 +1229,17 @@ public abstract class AbstractSelect extends AbstractField<Object> implements
         return caption != null ? caption : "";
     }
 
+    private String idToCaption(Object itemId) {
+        try {
+            Converter<String, Object> c = (Converter<String, Object>) ConverterUtil
+                    .getConverter(String.class, itemId.getClass(), getSession());
+            return ConverterUtil.convertFromModel(itemId, String.class, c,
+                    getLocale());
+        } catch (Exception e) {
+            return itemId.toString();
+        }
+    }
+
     /**
      * Sets tqhe icon for an item.
      * 
diff --git a/uitest/src/com/vaadin/tests/components/notification/NotificationsWaiAriaTest.java b/uitest/src/com/vaadin/tests/components/notification/NotificationsWaiAriaTest.java
index 6b517e9887..252efe2824 100644
--- a/uitest/src/com/vaadin/tests/components/notification/NotificationsWaiAriaTest.java
+++ b/uitest/src/com/vaadin/tests/components/notification/NotificationsWaiAriaTest.java
@@ -19,6 +19,7 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.openqa.selenium.WebElement;
 
+import com.vaadin.data.util.converter.StringToEnumConverter;
 import com.vaadin.shared.ui.ui.NotificationRole;
 import com.vaadin.testbench.By;
 import com.vaadin.testbench.elements.ButtonElement;
@@ -57,7 +58,8 @@ public class NotificationsWaiAriaTest extends MultiBrowserTest {
         postfix.clear();
         postfix.sendKeys("- press ESC to close");
 
-        type.selectByText(NotificationRole.ALERT.toString());
+        type.selectByText(StringToEnumConverter.enumToString(
+                NotificationRole.ALERT, null));
 
         show.click();
         waitForElementPresent(By.className("v-Notification"));
@@ -83,7 +85,8 @@ public class NotificationsWaiAriaTest extends MultiBrowserTest {
         } catch (Exception e) {
         }
 
-        type.selectByText("STATUS");
+        type.selectByText(StringToEnumConverter.enumToString(
+                NotificationRole.STATUS, null));
 
         show.click();
         waitForElementPresent(By.className("v-Notification"));
diff --git a/uitest/src/com/vaadin/tests/components/select/EnumSelect.java b/uitest/src/com/vaadin/tests/components/select/EnumSelect.java
new file mode 100644
index 0000000000..5976952f8c
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/select/EnumSelect.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2000-2014 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.tests.components.select;
+
+import java.util.Locale;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.combobox.FilteringMode;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.NativeSelect;
+import com.vaadin.ui.Tree;
+
+public class EnumSelect extends AbstractTestUIWithLog {
+
+    public enum Constant {
+        SOME_VALUE, SOME_OTHER_VALUE, FOO, BAR;
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        setLocale(new Locale("fi", "FI"));
+        ComboBox cb = new ComboBox();
+        cb.setFilteringMode(FilteringMode.CONTAINS);
+        for (Constant c : Constant.values()) {
+            cb.addItem(c);
+        }
+        addComponent(cb);
+
+        NativeSelect ns = new NativeSelect();
+        for (Constant c : Constant.values()) {
+            ns.addItem(c);
+        }
+        addComponent(ns);
+
+        Tree t = new Tree();
+        t.addItem(Constant.SOME_OTHER_VALUE);
+        t.addItem(2500.12);
+        t.setParent(2500.12, Constant.SOME_OTHER_VALUE);
+
+        addComponent(t);
+
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Test formatting captions with enum converters in selection components";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11433;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/select/EnumSelectTest.java b/uitest/src/com/vaadin/tests/components/select/EnumSelectTest.java
new file mode 100644
index 0000000000..c0429baa31
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/select/EnumSelectTest.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2000-2014 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.tests.components.select;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elements.NativeSelectElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class EnumSelectTest extends SingleBrowserTest {
+
+    @Test
+    public void enumInNativeSelect() {
+        openTestURL();
+        NativeSelectElement ns = $(NativeSelectElement.class).first();
+        List<TestBenchElement> options = ns.getOptions();
+        Assert.assertEquals("Some value", options.get(1).getText());
+        Assert.assertEquals("Some other value", options.get(2).getText());
+    }
+
+    @Test
+    public void enumInComboBox() {
+        openTestURL();
+        ComboBoxElement cb = $(ComboBoxElement.class).first();
+        cb.openPopup();
+        List<String> options = cb.getPopupSuggestions();
+        Assert.assertEquals("Some value", options.get(1));
+        Assert.assertEquals("Some other value", options.get(2));
+    }
+
+    @Test
+    public void enumInComboBoxFiltering() {
+        openTestURL();
+        ComboBoxElement cb = $(ComboBoxElement.class).first();
+        cb.findElement(By.vaadin("#textbox")).sendKeys(" other ");
+        List<String> options = cb.getPopupSuggestions();
+        Assert.assertEquals("Only one item should match filter", 1,
+                options.size());
+        Assert.assertEquals("Invalid option matched filter",
+                "Some other value", options.get(0));
+    }
+}