autorest icon indicating copy to clipboard operation
autorest copied to clipboard

M4 generates multiple responses with the same status code

Open pakrym opened this issue 3 years ago • 4 comments

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 avatar Oct 21 '20 15:10 pakrym

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

pakrym avatar Oct 21 '20 15:10 pakrym

@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

  • 200 -> string -> text/plain
  • 400 -> CloudError -> application/json
  • 401 -> string -> text/plain

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.

timotheeguerin avatar Oct 21 '21 21:10 timotheeguerin

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

AlexanderSher avatar Oct 25 '21 20:10 AlexanderSher

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.

AlexanderSher avatar Oct 26 '21 00:10 AlexanderSher