Azure / autorest

OpenAPI (f.k.a Swagger) Specification code generator. Supports C#, PowerShell, Go, Java, Node.js, TypeScript, Python
MIT License
4.61k stars 737 forks source link

M4 generates multiple responses with the same status code #3632

Open pakrym opened 4 years ago

pakrym commented 4 years ago

M4 generates multiple responses with the same status code

For https://github.com/Azure/azure-rest-api-specs/blob/d9cf7c7ed3d674ebd482836e82b274014245ae67/specification/attestation/data-plane/readme.md

Operation PrepareToSet

responses:
          - !<!SchemaResponse> 
            schema: *ref_13
            language: !<!Languages> 
              default:
                name: ''
                description: ''
            protocol: !<!Protocols> 
              http: !<!HttpResponse> 
                knownMediaType: text
                mediaTypes:
                  - text/plain
                statusCodes:
                  - '200'
          - !<!SchemaResponse> 
            schema: *ref_13
            language: !<!Languages> 
              default:
                name: ''
                description: ''
            protocol: !<!Protocols> 
              http: !<!HttpResponse> 
                knownMediaType: json
                mediaTypes:
                  - application/json
                statusCodes:
                  - '200'
          - !<!SchemaResponse> 
            schema: *ref_14
            language: !<!Languages> 
              default:
                name: ''
                description: ''
            protocol: !<!Protocols> 
              http: !<!HttpResponse> 
                knownMediaType: json
                mediaTypes:
                  - application/json
                statusCodes:
                  - '400'
          - !<!SchemaResponse> 
            schema: *ref_13
            language: !<!Languages> 
              default:
                name: ''
                description: ''
            protocol: !<!Protocols> 
              http: !<!HttpResponse> 
                knownMediaType: text
                mediaTypes:
                  - text/plain
                statusCodes:
                  - '401'
          - !<!SchemaResponse> 
            schema: *ref_13
            language: !<!Languages> 
              default:
                name: ''
                description: ''
            protocol: !<!Protocols> 
              http: !<!HttpResponse> 
                knownMediaType: json
                mediaTypes:
                  - application/json
                statusCodes:
                  - '401'

cc @daviwil

pakrym commented 4 years ago

Seems to be related to the recent text/plain change.

timotheeguerin commented 3 years ago

@allenjzhang @AlexanderSher Problem happening here: Spec where the issue is

Swagger 2.0 only lets you define the produce content type at the operation level(or global) but not per status code. In this case depending on the status code it returns a different type and can guess here its also returning the different content type

Now for this scenario we could see multiple solutions:

  1. Try to be smart and guess which content-type is the correct one depending on the schema. This will probably backfire and result in unexpected behaviors.
  2. Error out/ignore: Can't support that if defined this way. Pick one at m4 level and remove the duplicates and log a warning
  3. The generator support both content type for the same exit code and depending on the Content-Type header parse it differently.

Option 2 would basically move/tweak the current behavior from generator to m4

Problem is this would still not cover the scenario where an api could return different content type for the exact same status code. (e.g. application/json or application/xml) probably dependent on what accept code was sent.

Option 3 solve this but would require the generator to always first group the responses by statusCode. The code model isn't really designed for this to work well. We could change that but that would be a breaking change for all the generators.

AlexanderSher commented 2 years ago

Another one: https://github.com/Azure/autorest.csharp/issues/1605

AlexanderSher commented 2 years ago

I don't know if having two different content types under the same status code is a valid case. If it is, then we have to change it on the level of generator plugin. If it isn't, then probably we should throw an error.