Affects: 5.2.8.RELEASE

I have a controller like this

@RestController
@RequestMapping("/dict")
public class EnumTestController {

    @GetMapping("/convert")
    public EnabledEnum convert(@RequestParam EnabledEnum enabledEnum) {
        return enabledEnum;
    }

}

and EnabledEnum is

public enum EnabledEnum {
    DISABLE, ENABLE;
}

I use http://localhost:8080/dict/convert?enabledEnum=0 to request, but it throws exception with the stack trace:

Caused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [@org.springframework.web.bind.annotation.RequestParam org.gywebframework.web.sample.constant.EnabledEnum] for value '0'; nested exception is java.lang.IllegalArgumentException: No enum constant org.gywebframework.web.sample.constant.EnabledEnum.0
    at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:47) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:191) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:129) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:73) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:53) ~[spring-beans-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.validation.DataBinder.convertIfNecessary(DataBinder.java:693) ~[spring-context-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.web.method.annotation.AbstractNamedValueMethodArgumentResolver.resolveArgument(AbstractNamedValueMethodArgumentResolver.java:125) ~[spring-web-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    ... 92 common frames omitted
Caused by: java.lang.IllegalArgumentException: No enum constant org.gywebframework.web.sample.constant.EnabledEnum.0
    at java.base/java.lang.Enum.valueOf(Enum.java:240) ~[na:na]
    at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:52) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:38) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.core.convert.support.GenericConversionService$ConverterFactoryAdapter.convert(GenericConversionService.java:436) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41) ~[spring-core-5.2.8.RELEASE.jar:5.2.8.RELEASE]
    ... 98 common frames omitted

org.springframework.core.convert.support.GenericConversionService#convert(Object, TypeDescriptor, TypeDescriptor) is using org.springframework.core.convert.support.StringToEnumConverterFactory, not org.springframework.core.convert.support.IntegerToEnumConverterFactory.

Comment From: sbrannen

That's by design.

IntegerToEnumConverterFactory converts from an Integer to an Enum; whereas, StringToEnumConverterFactory converts from a String to an Enum.

Since the input from a request parameter is a String, StringToEnumConverterFactory gets correctly applied.

IntegerToEnumConverterFactory is intended for other conversion scenarios (i.e., not those from web request parameters).

It would be possible for StringToEnumConverterFactory to delegate to IntegerToEnumConverterFactory as a fallback mechanism in case the invocation of java.lang.Enum.valueOf(Class<T>, String) fails.

@jhoeller and @rstoyanchev, what are your thoughts on the matter?

Comment From: quaff

That's by design.

IntegerToEnumConverterFactory converts from an Integer to an Enum; whereas, StringToEnumConverterFactory converts from a String to an Enum.

Since the input from a request parameter is a String, StringToEnumConverterFactory gets correctly applied.

IntegerToEnumConverterFactory is intended for other conversion scenarios (i.e., not those from web request parameters).

It would be possible for StringToEnumConverterFactory to delegate to IntegerToEnumConverterFactory as a fallback mechanism in case the invocation of java.lang.Enum.valueOf(Class<T>, String) fails.

@jhoeller and @rstoyanchev, what are your thoughts on the matter?

It's safe to delegate to IntegerToEnumConverterFactory if input is numeric string since it's not a valid enum name.

Comment From: gcdd1993

That's by design.

IntegerToEnumConverterFactory converts from an Integer to an Enum; whereas, StringToEnumConverterFactory converts from a String to an Enum.

Since the input from a request parameter is a String, StringToEnumConverterFactory gets correctly applied.

IntegerToEnumConverterFactory is intended for other conversion scenarios (i.e., not those from web request parameters).

It would be possible for StringToEnumConverterFactory to delegate to IntegerToEnumConverterFactory as a fallback mechanism in case the invocation of java.lang.Enum.valueOf(Class<T>, String) fails.

@jhoeller and @rstoyanchev, what are your thoughts on the matter?

I know, but it seems that every parameter is a string at the beginning, how can I use IntegerToEnumConverterFactory?

Comment From: rstoyanchev

It seems okay to me to do that. If the input can be parsed to an Integer and the target is an enum, it's fairly constrained.

Comment From: rstoyanchev

On second though, it seems a bit fragile to rely on order of declaration and I'm not sure we should support it. @gcdd1993 if you want to use something shorter, why not choose enum names that are shorter like on and off? Probably more clear for users too.

Comment From: quaff

On second though, it seems a bit fragile to rely on order of declaration and I'm not sure we should support it. @gcdd1993 if you want to use something shorter, why not choose enum names that are shorter like on and off? Probably more clear for users too.

It's more common to use ordinal instead of name when using JPA, name is not effective for database, and we need to amend data if we want to rename it. It's similar here, If we published permanent link or API, we need redirection for renaming.

Comment From: gcdd1993

@rstoyanchev I'm not sure where IntegerToEnumConverterFactory will be used, but it exists. Wouldn't it be better if IntegerToEnumConverterFactory could be treated as a fallback mechanism for StringToEnumConverterFactory?

Comment From: bclozel

It's more common to use ordinal instead of name when using JPA, name is not effective for database, and we need to amend data if we want to rename it.

I would argue that using JPA as an example here makes a case against this enhancement. Changing the order in the enum (so not even renaming) breaks things. I think JPA converter annotations are really recommended these days. Also some databases support enums natively now.

While from a pure implementation perspective this fallback makes sense, I don't think it's wise to enable this case in Web APIs.

Comment From: rstoyanchev

I'm not sure where IntegerToEnumConverterFactory will be used, but it exists.

It looks like it was introduced in #18611 although the description doesn't provide more insight. On some level I would say, why not. It could potentially be useful depending on the case.

That said I agree with Brian, I'm not sure we should enable it by default for web input. You can however easily change that in the Type Converters callback of the MVC config.

Comment From: snicoll

Alright closing given the above. I think that's what Rossen meant to do anyway.