Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enum mapping in lowercase fails by default #198

Closed
beeondev opened this issue Oct 5, 2023 · 5 comments
Closed

enum mapping in lowercase fails by default #198

beeondev opened this issue Oct 5, 2023 · 5 comments
Milestone

Comments

@beeondev
Copy link

beeondev commented Oct 5, 2023

I have a legacy API that takes an enumeration as argument, here is a yaml fragment of OpenAPI spec :

...
components 
    schemas:
       Mode:
          type: string
          enum: 
            - a
            - b

Such enumeration should be used as string in API, but is translated to Enum with processor (why not, this is not the problem) as :

@Generated(value = "openapi-processor-spring", version = "2023.4", date = "2023-10-05T14:30:44.211810700+02:00")
public enum Mode {
    A("a"),
    B("b");
...

Internally spring use StringToEnumConverterFactory to convert the string to enumeration:

Daemon Thread [parallel-3] (Suspended)	
	owns: FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber<T,R>  (id=741)	
	owns: FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber<T,R>  (id=742)	
	Enum<E>.valueOf(Class<T>, String) line: 268	
	StringToEnumConverterFactory$StringToEnum<T>.convert(String) line: 54	
	StringToEnumConverterFactory$StringToEnum<T>.convert(Object) line: 39	
	GenericConversionService$ConverterFactoryAdapter.convert(Object, TypeDescriptor, TypeDescriptor) line: 436	
	ConversionUtils.invokeConverter(GenericConverter, Object, TypeDescriptor, TypeDescriptor) line: 41	
	DefaultFormattingConversionService(GenericConversionService).convert(Object, TypeDescriptor, TypeDescriptor) line: 192	
	TypeConverterDelegate.convertIfNecessary(String, Object, Object, Class<T>, TypeDescriptor) line: 129	
	SimpleTypeConverter(TypeConverterSupport).convertIfNecessary(Object, Class<T>, TypeDescriptor) line: 73	
	SimpleTypeConverter(TypeConverterSupport).convertIfNecessary(Object, Class<T>, MethodParameter) line: 53	
	BindingContext$ExtendedWebExchangeDataBinder(DataBinder).convertIfNecessary(Object, Class<T>, MethodParameter) line: 729	
	RequestParamMethodArgumentResolver(AbstractNamedValueArgumentResolver).applyConversion(Object, AbstractNamedValueArgumentResolver$NamedValueInfo, MethodParameter, BindingContext, ServerWebExchange) line: 193

and it fail, because Enum.valueOf(Mode.class, "a") delegates to internal enumConstantDirectory which contains only uppercase values "A" and "B" !

So the only workaround is to inject custom converter, but this is not desirable.

@beeondev
Copy link
Author

beeondev commented Oct 5, 2023

say the same shortly :

 // processor-spring impl
 Mode.fromValue(source);

is not the same as

Mode.valueOf(source);

@hauner
Copy link
Member

hauner commented Oct 8, 2023

yes, this is annoying.

the question is how to improve it.

  • generate lowercase enums

    • would need some switch to be backward compatible
    • maybe configure by enum
    • will not work for enum values that contain illegal characters (not allowed in java identifier)
  • provide/generate converter

    • StringToEnumConverter implements Converter<String, MyEnum>

any other idea?

@beeondev
Copy link
Author

beeondev commented Oct 9, 2023

  • I would like specify what is annoying :
  1. the need to add specific spring behavior
  2. per enum spring behavior (converter) injection

Second point is far more annoying, because each enum must have its converter registered. Having a special interface on the enum i.e like "Supplier<String>" would allow :
- not break the backward compatibility
- reduce '2)' to '1)' by :
* allowing to identitfy that enum must converted another way.
* avoid to depends on specific openapi-processor type

  • If breaking backward compatibily is an option, then perhaphs, because initial openapi spec declare a string type, we can just keep it as a string. enum declaration in spec could be interpreted a special validator constraint with something like :
    public Mono<String> someEndpoint(@RequestBody @DictionaryValue(allowedValues = {"a", "b"}) String mode)

with :

    import javax.validation.Constraint;
    import javax.validation.Payload;
    import java.lang.annotation.*;
    
    @Target({ElementType.FIELD, ElementType.PARAMETER})
    @Retention(RetentionPolicy.RUNTIME)
    @Constraint(validatedBy = DictionaryValueValidator.class)
    @Documented
    public @interface DictionaryValue {
        String message() default "Invalid value. Please choose from the dictionary.";
        Class<?>[] groups() default {};
        Class<? extends Payload>[] payload() default {};
        String[] allowedValues() default {};
    }

and

    public class DictionaryValueValidator implements ConstraintValidator<DictionaryValue, String> {
    
        private String[] allowedValues;
    
        @Override
        public void initialize(DictionaryValue constraintAnnotation) {
            this.allowedValues = constraintAnnotation.allowedValues();
        }
    
        @Override
        public boolean isValid(String value, ConstraintValidatorContext context) {
            return value != null && Arrays.asList(allowedValues).contains(value);
        }
    }

This is only suggestions.
Best regards

@hauner
Copy link
Member

hauner commented Oct 22, 2023

I have successfully created a prototype ConverterFactory that creates enum converters for generated enums that implement Supplier<>. Not much code which is nice :-)

import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.ConverterFactory;
import org.springframework.stereotype.Component;

import java.util.EnumSet;
import java.util.function.Supplier;

public class StringToEnumConverterFactory<T extends Enum<T> & Supplier<String>> implements ConverterFactory<String, T> {

    @Override
    @SuppressWarnings({"unchecked", "rawtypes"})
    public <E extends T> Converter<String, E> getConverter(Class<E> targetType) {
        return new StringToEnumConverter(targetType);
    }

    static class StringToEnumConverter<T extends Enum<T> & Supplier<String>> implements Converter<String, T> {

        private final Class<T> enumType;

        public StringToEnumConverter(Class<T> enumType) {
            this.enumType = enumType;
        }

        public T convert(String source) {
            String sourceValue = source.trim();

            for (T e : EnumSet.allOf(enumType)) {
                if (e.get().equals(sourceValue)) {
                    return e;
                }
            }

            throw new IllegalArgumentException(
                    String.format("No enum constant of %s has the value %s", enumType.getCanonicalName(), sourceValue));
        }
    }
}

It requires a one time registration using Web[Mvc|Flux]Configurer.addFormatters().

Your String version is also something I'm considering. I'm thinking about a global setting to switch between enum and String. enum would also generate the converter factory and it can be registered (or not if it is not needed).

Thanks for your input :-)

hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Oct 31, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 5, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 5, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 11, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 11, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 11, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit that referenced this issue Nov 12, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 12, 2023
hauner added a commit to openapi-processor/openapi-processor-base that referenced this issue Nov 18, 2023
hauner added a commit that referenced this issue Nov 18, 2023
hauner added a commit that referenced this issue Nov 18, 2023
@hauner hauner added this to the 2023.6 milestone Nov 26, 2023
@hauner
Copy link
Member

hauner commented Nov 26, 2023

improved in 2023.6, added enum-type mapping option default (like before), string (just use string) and framework (enum with spring converter factory)

@hauner hauner closed this as completed Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants