diff options
3 files changed, 101 insertions, 25 deletions
diff --git a/WebContent/release-notes.html b/WebContent/release-notes.html index 463f366069..c95c75e248 100644 --- a/WebContent/release-notes.html +++ b/WebContent/release-notes.html @@ -115,6 +115,8 @@ on the reasoning behind the change.</li> <li>Grid SelectionModels are now Extensions. This update removes all selection related variables and API from GridConnector, GridState, GridServerRpc and GridClientRpc</li> + <li>StringToEnumConverter now explicitly supports Enum types with custom toString() implementations. + This may affect applications that relied on the undefined behavior in previous versions.</li> </ul> <h3 id="knownissues">Known Issues and Limitations</h3> <ul> diff --git a/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java b/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java index 96fdc7baa8..7b2f43f972 100644 --- a/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java +++ b/server/src/com/vaadin/data/util/converter/StringToEnumConverter.java @@ -22,8 +22,15 @@ import java.util.Locale; * A converter that converts from {@link String} to an {@link Enum} and back. * <p> * Designed to provide nice human readable strings for {@link Enum} classes - * where the constants are named SOME_UPPERCASE_WORDS. Will not necessarily work - * correctly for other cases. + * conforming to one of these patterns: + * <ul> + * <li>The constants are named SOME_UPPERCASE_WORDS and there's no toString + * implementation.</li> + * <li>toString() always returns the same human readable string that is not the + * same as its name() value. Each constant in the enum type returns a distinct + * toString() value.</li> + * </ul> + * Will not necessarily work correctly for other cases. * </p> * * @author Vaadin Ltd @@ -63,26 +70,35 @@ public class StringToEnumConverter implements Converter<String, Enum> { locale = Locale.getDefault(); } - // Foo -> FOO - // Foo bar -> FOO_BAR - String result = value.replace(" ", "_").toUpperCase(locale); - try { - 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<T> set = EnumSet.allOf(enumType); - for (T e : set) { - if (e.name().toUpperCase(locale).equals(result)) { - return e; - } + if (!enumType.isEnum()) { + throw new ConversionException(enumType.getName() + + " is not an enum type"); + } + + // First test for the human-readable value since that's the more likely + // input + String upperCaseValue = value.toUpperCase(locale); + T match = null; + for (T e : EnumSet.allOf(enumType)) { + String upperCase = enumToString(e, locale).toUpperCase(locale); + if (upperCase.equals(upperCaseValue)) { + if (match != null) { + throw new ConversionException("Both " + match.name() + + " and " + e.name() + + " are matching the input string " + value); } - } catch (Exception e) { + match = e; } + } - // Fallback did not work either, re-throw original exception so - // user knows what went wrong + if (match != null) { + return match; + } + + // Then fall back to using a strict match based on name() + try { + return Enum.valueOf(enumType, upperCaseValue); + } catch (Exception ee) { throw new ConversionException(ee); } } @@ -107,12 +123,17 @@ public class StringToEnumConverter implements Converter<String, Enum> { } String enumString = value.toString(); - // FOO -> Foo - // FOO_BAR -> Foo bar - // _FOO -> _foo - String result = enumString.substring(0, 1).toUpperCase(locale); - result += enumString.substring(1).toLowerCase(locale).replace('_', ' '); - return result; + if (enumString.equals(value.name())) { + // FOO -> Foo + // FOO_BAR -> Foo bar + // _FOO -> _foo + String result = enumString.substring(0, 1).toUpperCase(locale); + result += enumString.substring(1).toLowerCase(locale) + .replace('_', ' '); + return result; + } else { + return enumString; + } } @Override diff --git a/server/tests/src/com/vaadin/tests/data/converter/StringToEnumConverterTest.java b/server/tests/src/com/vaadin/tests/data/converter/StringToEnumConverterTest.java index 59c1ed133a..6660ecc9d5 100644 --- a/server/tests/src/com/vaadin/tests/data/converter/StringToEnumConverterTest.java +++ b/server/tests/src/com/vaadin/tests/data/converter/StringToEnumConverterTest.java @@ -14,10 +14,36 @@ public class StringToEnumConverterTest { VALUE1, SOME_VALUE, FOO_BAR_BAZ, Bar, nonStandardCase, _HUGH; } + public static enum EnumWithCustomToString { + ONE, TWO, THREE; + + @Override + public String toString() { + return "case " + (ordinal() + 1); + } + } + + public static enum EnumWithAmbigousToString { + FOO, FOOBAR, FOO_BAR; + + @Override + public String toString() { + return name().replaceAll("_", ""); + } + } + StringToEnumConverter converter = new StringToEnumConverter(); Converter<Enum, String> reverseConverter = new ReverseConverter<Enum, String>( converter); + private String convertToString(Enum value) { + return converter.convertToPresentation(value, String.class, null); + } + + public Enum convertToEnum(String string, Class<? extends Enum> type) { + return converter.convertToModel(string, type, null); + } + @Test public void testEmptyStringConversion() { Assert.assertEquals(null, @@ -79,4 +105,31 @@ public class StringToEnumConverterTest { } + @Test + public void preserveFormattingWithCustomToString() { + for (EnumWithCustomToString e : EnumWithCustomToString.values()) { + Assert.assertEquals(e.toString(), convertToString(e)); + } + } + + @Test + public void findEnumWithCustomToString() { + for (EnumWithCustomToString e : EnumWithCustomToString.values()) { + Assert.assertSame(e, + convertToEnum(e.toString(), EnumWithCustomToString.class)); + Assert.assertSame(e, + convertToEnum(e.name(), EnumWithCustomToString.class)); + } + } + + @Test + public void unambigousValueInEnumWithAmbigous_succeed() { + Assert.assertSame(EnumWithAmbigousToString.FOO, + convertToEnum("foo", EnumWithAmbigousToString.class)); + } + + @Test(expected = ConversionException.class) + public void ambigousValue_throws() { + convertToEnum("foobar", EnumWithAmbigousToString.class); + } } |