springdoc/springdoc-openapi

Wrong enum values in OpenAPI spec when `toString()` is overridden

ch4mpy opened this issue · 13 comments

Describe the Bug

When an enum overrides toString(), then the output of this method is used to set the acceptable values in the generated OpenAPI spec.

This goes against Java specs for enums serilization and, more importantly, against how Spring behaves by default. In both cases, if no more than a toString() method is provided what is used is still:

  • name() for serialization
  • valueof(name) for deserialization

Using toString() output could make sense when it is decorated with @JsonValue, but this is a slippery ground as:

  • there could be other annotations involved for XML & other serialization mechanisms
  • there is a difference between @RequestBody and @RequestParam deserialization in spring:
    • requests body is deserialized just like @ResponseBody is serialized: using HttpMessageConverter beans. Default implementations of which use Jackson and will use toString() for (de)serialization because of @JsonValue. So; it seems fine to use whatever is decorated with @JsonValue for @RequestBody and @ResponseBody.
    • request params are converted from a String using a Converter<String, E>. Spring Web generates default converters using enums valueOf() methods (ignoring @JsonValue). To use something else (like the output of toString()), one has to provide a custom converter. The spec generator can hardly do better than guess what such a custom converter accepts as input, and should probably keep using name() for request parameters, unless explicitly instructed to do otherwise.

I don't really care what swagger-core does. What matters to me is the generated OpenAPI spec to be aligned with inputs and outputs of my Spring REST APIs. When an enum contains just a toString() method (without @JsonValue or whatever), this is not the case.

To Reproduce

Minimal project

Consider the following Spring RestController

@RestController
public class DemoController {
    
    @GetMapping("/demo")
    public Dto getDemo(Dto dto) {
        return dto;
    }

    public static enum EnumSerializedByName {
        ON("is on"), OFF("is off");
        
        String label;
        
        EnumSerializedByName(String label) {
            this.label = label;
        }
        
        @Override
        public String toString() {
            return label;
        }
    }
    
    public static record Dto(EnumSerializedByName status) {}
}

and the following test:

@WebMvcTest(controllers = DemoController.class)
class DemoControllerTest {
    
    @Autowired
    MockMvc mockMvc;

    @Test
    void whenGetDemo_thenStatusIsSerializedByName() throws Exception {
        mockMvc.perform(get("/demo").param("status", EnumSerializedByName.ON.name())).andExpect(status().isOk()).andExpect(jsonPath("status", is(EnumSerializedByName.ON.name())));
        mockMvc.perform(get("/demo").param("status", EnumSerializedByName.ON.toString())).andExpect(status().is4xxClientError());
    }

}

We see here that toString() is ignored for (de)serialization. The serialization and deserialization are done respectively using the name() and valueOf() methods.

Expected Behavior

The schema should be generated with the values expected by the endpoint and generated by it.

At bare minimum, the default for enum values should be what spring uses by default. Here we don't even have something like an annotation to notify the spec generator that we want it to use name() for the values it put in the OpenAPI spec for an enum.

Additional context

I know about this other issue but can't comment on it as it is locked.

I also know about the workaround provided by @bnasslahsen in the same issue, but it affects all enums, even those implemented as below.

For illustration purposes, here is how an enum has to be implemented to behave as currently documented in the generated spec:

public static enum BijectiveEnumSerializedByToString {
    A("bij a"),
    B("bij b");

    String label;

    BijectiveEnumSerializedByToString(String label) {
        this.label = label;
    }

    @Override
    @JsonValue // Forces serialization using toString()
    public String toString() {
        return label;
    }

    /**
     * Inverse operation for the toString() method
     *
     * @param str the serialized value of the enum
     * @return deserialized enum value
     */
    public static BijectiveEnumSerializedByToString fromString(String str) {
        for (final var e : BijectiveEnumSerializedByToString.values()) {
            if (Objects.equals(e.toString(), str)) {
                return e;
            }
        }
        return null;
    }

    /**
     * Register a Spring converter deserialize &#64;RequestParam from String to {@link BijectiveEnumSerializedByToString}
     */
    @Component
    static class StringEnumSerializedByToStringConverter implements Converter<String, BijectiveEnumSerializedByToString> {
        @Override
        public BijectiveEnumSerializedByToString convert(String source) {
            return BijectiveEnumSerializedByToString.fromString(source);
        }
    }
}

Note:

  • @JsonValue decorating public String toString()
  • the custom converter bean for request params

Detailed Observations on the Currently Generated Spec

considering the following enums in addition to the BijectiveEnumSerializedByToString above:

public static enum EnumSerializedByName {
    A("name a"),
    B("name b");

    String label;

    EnumSerializedByName(String label) {
        this.label = label;
    }

    @Override
    public String toString() {
        return label;
    }
}
public static enum EnumSerializedByToString {
    A("str a"),
    B("str b");

    String label;

    EnumSerializedByToString(String label) {
        this.label = label;
    }

    @Override
    @JsonValue // Forces serialization using toString()
    public String toString() {
        return label;
    }
}

with Spring default:

  • HttpMessageConverter beans
  • Converter<String, EnumSerializedByName>
  • Converter<String, EnumSerializedByToString>
  • Converter<String, BijectiveEnumSerializedByToString> (this one is not generated because we registered a StringEnumSerializedByToStringConverter instance as bean)

the generated spec for a Dto containing the three enum types is:

  "components": {
    "schemas": {
      "Dto": {
        "required": ["bij", "name", "str"],
        "type": "object",
        "properties": {
          "name": { "type": "string", "enum": ["name a", "name b"] },
          "str": { "type": "string", "enum": ["str a", "str b"] },
          "bij": { "type": "string", "enum": ["bij a", "bij b"] }
        }
      }
    }
  }

this is:

  • wrong for EnumSerializedByName wich is always serialized using name() and deserialised using valueOf() (acceptable values should be ["A", "B"])
  • right for BijectiveEnumSerializedByToString because we provided quite some code for it:
    • toString() decorated with @JsonValue
    • a custom Converter<String, BijectiveEnumSerializedByToString> registered as a Spring bean
  • tricky for EnumSerializedByToString:
    • right when HttpMessageConverter beans are used (@RequestBody and @ResponseBody)
    • wrong when the default Converter<String, EnumSerializedByToString> is used (@RequestParam)