[rust-server] features broken from swagger-codegen

[bjgill]

Description

In #290, we got rust-server mostly working in openapi-generator. However, there are still a few missing features. See the commented out sections of https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/2_0/rust-server/petstore-with-fake-endpoints-models-for-testing.yaml, but in summary:

• Responses with schema: {type: <something>} and >0 headers (got a fix - [rust-server] Handle headers correctly #549)
• Enum strings (and arrays of strings) in form data params (we never supported this; see String array in form data has isString set #545 for the fix for the compilation error)
• Byte strings in form data parameters (we never supported this properly; see [rust-server] how to cope with loss of file type? #538)
• Binary string properties (we never supported this properly; see [rust-server] how to cope with loss of file type? #538)

I suspect the former might be related to the terrible hack I did in postProcessModels, whilst the latter 3 seem likely to be related to processParam.

EDIT: I have no plans to work on this for the next fortnight, so contributions gratefully welcomed.

[bjgill]

Right. I'm planning to pick this up this week.

[bjgill]

My plan is to add notes to this issue as I go. I'm likely to be working on this over a few days, and don't want to forget things.

The schema problem is interesting. From my testing, it appears that the problem is any base schema in a response with at least one header defined. both the body and first header get the type of the first header. The type is correct in the generated openapi.yaml, so maybe this is a problem with the template? It looks as if we're doing this correctly in client/mod.rs, so presumably I can copy the logic from there.

Hmm. It looks as if the problem is that inside {{#headers}}{{#-first}}, {{{dataType}}} refers to the type of the first header, rather than the type of the body as a whole. This is the only time we need both the body type and the fact that there are some headers at the same time.

EDIT: repro template:

swagger: '2.0'
info:
  description: "This spec is mainly for testing Petstore server and contains fake endpoints, models. Please do not use this for any other purpose. Special characters: \" \\"
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
host: petstore.swagger.io:80
basePath: /v2
tags:
  - name: pet
    description: Everything about your Pets
  - name: store
    description: Access to Petstore orders
  - name: user
    description: Operations about user
schemes:
  - http
paths:
  /user/login:
    get:
      tags:
        - user
      summary: Logs user into the system
      operationId: loginUser
      produces:
        - application/json
      responses:
        '200':
          description: successful operation
          schema:
            type: number
          headers:
            X-Rate-Limit:
              type: integer
              format: int32
              description: calls per hour allowed by the user
            # X-Expires-After:
            #   type: string
            #   format: date-time
            #   description: date in UTC when token expires
        '201':
          description: wtf
          schema:
            type: string
        '400':
          description: Invalid username/password supplied

EDIT2: I've gone back in time to before #290, and saw this error. This suggests to me that this is caused by the move from swagger/v2 to openapi/v3 (rather than anything I did in #290)

[bjgill]

On the byte form params - I don't think this ever worked.

What happens is that if hasFile is set, we generate entry.parse::<{{{dataType}}}>(), but "the trait std::str::FromStr is not implemented for swagger::ByteArray". In the old sample, hasFile was not set, so we fell through to the other implementation, which was just to set it to the example value.

So:

• if the API hasFile, the server code won't compile if the API has binary/byte format parameters (but should work otherwise).
• if the API doesn't hasFile, form parameters just don't work server-side (we throw away the request value, using the example value instead).

In this issue, I'm just trying to match the behaviour of the old swagger-codegen rather than fixing any pre-existing bugs. Hence, I plan to check whether the change to hasFile being set is deliberate - if so, I'll raise a follow-on issue to fix the bug; if not, I'll ensure that hasFile gets put back to its previous value.

EDIT: OK - I think what's happened is that the old mechanism for file parsing has been fundamentally broken by the switch to OpenAPI v3:

In contrast with the 2.0 specification, file input/output content in OpenAPI [v3] is described with the same semantics as any other schema type.

What this means is that the old file type no longer exists, and gets mapped to the string type with format binary. I suspect the workaround may be to add a vendor extension for back-compatibility...

[wing328]

Hmm. It looks as if the problem is that inside {{#headers}}{{#-first}}, {{{dataType}}} refers to the type of the first header, rather than the type of the body as a whole. This is the only time we need both the body type and the fact that there are some headers at the same time.

I compared Swagger Codegen and OpenAPI Generator and both show the following mustache tags:

      "responses" : [ {
        "headers" : [ {
          "baseName" : "X-Rate-Limit",
          "getter" : "getXRateLimit",
          "setter" : "setXRateLimit",
          "description" : "calls per hour allowed by the user",
          "dataType" : "Integer",
          "datatypeWithEnum" : "Integer",
          "dataFormat" : "int32",
          "name" : "x_rate_limit",
          "defaultValueWithParam" : " = data.X-Rate-Limit;",
          "baseType" : "Integer",
          "unescapedDescription" : "calls per hour allowed by the user",
          "jsonSchema" : "{\n  \"type\" : \"integer\",\n  \"format\" : \"int32\"\n}",
          "exclusiveMinimum" : false,
          "exclusiveMaximum" : false,
          "hasMore" : false,
          "required" : false,

refers to the type of the first header, rather than the type of the body as a whole

Do mean you're looking for the data type of the response body?

Can you point me to the exact lines in the mustache template having the issue and ideally what you expect?

[wing328]

Speaking of hasFile, just want to share that there's no file type in OAS3 so isFile and isBinary are now identical. I don't recall doing anyting in particular for hasFile flag so we may need to revise and test it to ensure its value is correct.

[bjgill]

Thanks for taking a look.

On the first point - see https://github.com/bjgill/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/rust-server/lib.mustache#L45 for the template. Then, SuccessfulOperation { body: String, x_rate_limit: i32, ... (https://github.com/swagger-api/swagger-codegen/blob/master/samples/server/petstore/rust-server/src/lib.rs#L246) is the old (correct) output under swagger-codegen, and SuccessfulOperation { body: i32, x_rate_limit: i32, ... is the (wrong) output under openapi-generator

On the second - I think we realised this at the same time. In swagger-codegen's rust-server, binary, byte, and file resulted in different types. This means that we need to decide on two types to carry forwards (or use a workaround to preserve the third). This seems like a fairly substantial change that I should probably not undertake unilaterally, so I'll raise a new issue and not fix it here/now.

[bjgill]

The fourth point (binary properties is just another re-occurence of the problems caused by the changes to the various binary/byte/file types. Specifically, the binary format has not got isFile set, where previously it did not.

In any case, #538 will fix this as well.

[bjgill]

Looking at the second, I wonder is this is another weird mustache-related bug.

Repro template:

swagger: '2.0'
info:
  description: "This spec is mainly for testing Petstore server and contains fake endpoints, models. Please do not use this for any other purpose. Special characters: \" \\"
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
schemes:
  - http
paths:
  /pet:
    get:
      tags:
        - fake
      description: To test enum parameters
      consumes:
        - "application/x-www-form-urlencoded"
      parameters:
        - name: enum_form_string_array
          type: array
          items:
            type: string
            default: '$'
            enum:
              - '>'
              - '$'
          in: formData
          description: Form parameter enum test (string array)
        - name: enum_form_string
          type: string
          default: '-efg'
          enum:
            - _abc
            - '-efg'
            - (xyz)
          in: formData
          description: Form parameter enum test (string)
      responses:
        '400':
          description: Invalid request

Interestingly, if I comment out either of enum_form_string_array or enum_form_string, I get code that compiles. However, with both, we get an error where something is thinking that param_enum_form_string should have the same type as enum_form_string_array

EDIT: I've not confirmed, but I suspect that param_enum_form_string_array mistakenly has isString set.

EDIT2: the compilation error is a red herring. I've confirmed that param_enum_form_string_array mistakenly has isString set.

EDIT3: I've raised #545 to cover the problem with isString.
However, this has never worked server-side. And client-side, I'm not sure this produces the right things (except by accident sometimes). As such, I don't think it needs to get fixed as part of this issue.

[bjgill]

@wing328 - I'm suspicious that something has changed in the mustache template logic.

I'll use the following spec to demonstrate:

swagger: '2.0'
info:
  description: "This spec is mainly for testing Petstore server and contains fake endpoints, models. Please do not use this for any other purpose. Special characters: \" \\"
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
host: petstore.swagger.io:80
basePath: /v2
tags:
  - name: pet
    description: Everything about your Pets
  - name: store
    description: Access to Petstore orders
  - name: user
    description: Operations about user
schemes:
  - http
paths:
  /user/login:
    get:
      tags:
        - user
      summary: Logs user into the system
      operationId: loginUser
      produces:
        - application/json
      responses:
        '200':
          description: successful operation
          schema:
            type: number
          headers:
            X-Rate-Limit:
              type: integer
              format: int32
              description: calls per hour allowed by the user
            # X-Expires-After:
            #   type: string
            #   format: date-time
            #   description: date in UTC when token expires
        '201':
          description: wtf
          schema:
            type: string
          headers:
            X-Rate-Limit:
              type: integer
              format: int32
              description: calls per hour allowed by the user
        '400':
          description: Invalid username/password supplied
          headers:
            X-Rate-Limit:
              type: integer
              format: int32
              description: calls per hour allowed by the user

Swagger-codegen

{{#apiInfo}}{{#apis}}{{#operations}}{{#operation}}
{{^isResponseFile}}
#[derive(Debug, PartialEq)]
{{/isResponseFile}}
pub enum {{{operationId}}}Response {
{{#responses}}
{{#message}}    /// {{{message}}}{{/message}}
    {{#vendorExtensions}}{{{x-responseId}}}{{/vendorExtensions}} {{#dataType}}{{^headers}}( {{{dataType}}} ) {{/headers}}{{#headers}}{{#-first}}{ body: {{{dataType}}}{{/-first}}{{/headers}}{{/dataType}}{{#headers}}{{#-first}}{{^dataType}} { {{/dataType}}{{#dataType}}, {{/dataType}}{{/-first}}{{^-first}}, {{/-first}}{{{name}}}: {{{datatype}}}{{#-last}} } {{/-last}}{{/headers}},
{{/responses}}
}{{#isResponseFile}}

yields

#[derive(Debug, PartialEq)]
pub enum LoginUserResponse {
    /// successful operation
    SuccessfulOperation { body: f64, x_rate_limit: i32 } ,
    /// wtf
    Wtf { body: String, x_rate_limit: i32 } ,
    /// Invalid username/password supplied
    InvalidUsername  { x_rate_limit: i32 } ,
}

Openapi-generator

{{#apiInfo}}{{#apis}}{{#operations}}{{#operation}}
{{^isResponseFile}}
#[derive(Debug, PartialEq)]
{{/isResponseFile}}
pub enum {{operationId}}Response {
{{#responses}}
{{#message}}    /// {{{message}}}{{/message}}
    {{#vendorExtensions}}{{{x-responseId}}}{{/vendorExtensions}} {{#dataType}}{{^headers}}( {{{dataType}}} ) {{/headers}}{{#headers}}{{#-first}}{ body: {{{dataType}}}{{/-first}}{{/headers}}{{/dataType}}{{#headers}}{{#-first}}{{^dataType}} { {{/dataType}}{{#dataType}}, {{/dataType}}{{/-first}}{{^-first}}, {{/-first}}{{{name}}}: {{{datatype}}}{{#-last}} } {{/-last}}{{/headers}},
{{/responses}}
}
{{/operation}}{{/operations}}{{/apis}}{{/apiInfo}}

yields

#[derive(Debug, PartialEq)]
pub enum LoginUserResponse {
    /// successful operation
    SuccessfulOperation { body: i32, x_rate_limit: i32 } ,
    /// wtf
    Wtf { body: i32, x_rate_limit: i32 } ,
    /// Invalid username/password supplied
    InvalidUsername , x_rate_limit: i32 } ,
}

Hypothesis

From the output from openapi-generator, it looks as if the difference is to do with what happens inside {{#headers}}{{#-first}}. In swagger-codegen, {{{dataType}}} inside those tags refers to the type of the body. Whereas in openapi-generator, {{{dataType}}} inside those tags refers to the type of the first header.

How to fix

The ideal fix would be to restore the previous (swagger-codegen) behaviour. Failing that, we could probably work-around by some mixture of template changes and Rust macros.

[bjgill]

On my previous comment. I've now read through some of the mustache spec. It looks like the new behaviour is probably correct. I think I may have found a way to fix by using hasHeaders rather than headers.

[bjgill]

OK, with #547 merged, we are now feature back-compatible.