poem-web / poem

A full-featured and easy-to-use web framework with the Rust programming language.
Apache License 2.0
3.46k stars 283 forks source link

Poem/Openapi : Strategy for generating generic structure names #774

Open rxdiscovery opened 5 months ago

rxdiscovery commented 5 months ago

Hello, Poem #[aoi] , must allow you to choose the strategy for generating generic structure names,

Response\ for example, should have the symbol name Reponse_T or ResponseForT or ResponseOfT and Response\<[T]> => ResponseOfListT for example.

when I want to generate code with the swagger spec using openapi generator, I get this error because the generic names contain special characters :

Exception in thread "main" org.openapitools.codegen.SpecValidationException: There were issues with the specification. The option can be disabled via validateSpec (Maven/Gradle) or --skip-validate-spec (CLI).
 | Error count: 4, Warning count: 0
Errors: 
        -components.schemas.Schema name Response<string> doesn't adhere to regular expression ^[a-zA-Z0-9\.\-_]+$
        -attribute info.summary is unexpected
        -components.schemas.Schema name Response<Auth> doesn't adhere to regular expression ^[a-zA-Z0-9\.\-_]+$
        -components.schemas.Schema name Response<[City]> doesn't adhere to regular expression ^[a-zA-Z0-9\.\-_]+$

Response and Response<[City]> will generate the same structure : ResponseCity , ResponseCity, the generated code has no way of differentiating between them.

but if we rename the [City] to ListCity, for example, the result will be : ResponseCity and ResponseListCity

to simplify, here is the part concerned :

impl<T: Type> Type for Vec<T> {
    const IS_REQUIRED: bool = true;

    type RawValueType = Self;

    type RawElementValueType = T::RawValueType;

    fn name() -> Cow<'static, str> {
        format!("[{}]", T::name()).into()  //<<------------------------------------- !!!
    }

    fn schema_ref() -> MetaSchemaRef {
        MetaSchemaRef::Inline(Box::new(MetaSchema {
            items: Some(Box::new(T::schema_ref())),
            ..MetaSchema::new("array")
        }))
    }

transform [T] into ListOfT , because [ and ] characters are not allowed

    fn name() -> Cow<'static, str> {
        format!("[{}]", T::name()).into()  //<<------------------------------------- !!!
    }

to

    fn name() -> Cow<'static, str> {
        format!("ListOf{}", T::name()).into()  //<<------------------------------------- !!!
    }

Thanks in advance :1st_place_medal:

sunli829 commented 5 months ago

Currently it is not possible to customize the OpenAPI name corresponding to a generic type. If you need a legal name, you can only avoid using generics.

rxdiscovery commented 5 months ago

I have a proposition, why not take the template ( [{}] ) directly from a global variable, which can be modified?

  fn name() -> Cow<'static, str> {
        format!("[{}]", T::name()).into() 
    }

to

  fn name() -> Cow<'static, str> {
        format!(get_vec_gen_template(), T::name()).into() 
    }
amtelekom commented 4 months ago

318 and #670 are about similar problems.

https://github.com/poem-web/poem/pull/671 implemented a solution for the generics problem, but sadly got no feedback so far.

The current output is non-compliant with the OpenAPI spec, even if SwaggerUI & Co don't complain:

All the fixed fields declared above are objects that MUST use keys that match the regular expression: ^[a-zA-Z0-9.-_]+$

(https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#components-object)

The problem I see with both your suggestion (Prefix with ListOf instead of wrapping in brackets) as well as the one in #671 (replace special characters with underscore) is that it might collide with non-generics that use the same name - but IMHO both solutions would be preferable to just not be able to use Generics at all without generating invalid OpenAPI specs.

@sunli829 would you be open to a PR that removes the disallowed characters, and if so, do you have a preference for the format (i.e., replace punctuation with underscores, Prefix with a String like "Of", ...)?

rxdiscovery commented 4 months ago

Hello,

@amtelekom if we make the template dynamic so that each developer can customize it only for their needs

I think @sunli829 is so busy lately, it would be nice if he assigned a moderator to help him.