Azure / autorest.typescript

Extension for AutoRest (https://github.com/Azure/autorest) that generates TypeScript code. The transpiled javascript code is isomorphic. It can be run in browser and in node.js environment.
MIT License
177 stars 75 forks source link

missing parameter for patch operation #452

Closed tomasz-grobelny closed 5 years ago

tomasz-grobelny commented 5 years ago

I have an operation with following specification:

      "patch": {
        "tags": ["Leads"],
        "operationId": "Leads_PatchLead",
        "consumes": ["application/json", "application/json-patch+json"],
        "produces": ["application/json"],
        "parameters": [
          {
            "name": "leadId",
            "in": "path",
            "required": true,
            "type": "string"
          },
          {
            "name": "patchData",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/JsonPatchDocument[Lead]"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "schema": {
              "type": "string"
            }
          },
          "422": {
            "description": "422",
            "schema": {
              "type": "array",
              "items": {
                "$ref": "#/definitions/ValidationError"
              }
            }
          }
        }
      }

However the operation that gets produced has this signature:

    patchLead(leadId: string, options?: msRest.RequestOptionsBase): Promise<Models.LeadsPatchLeadResponse>;
    patchLead(leadId: string, callback: msRest.ServiceCallback<any>): void;
    patchLead(leadId: string, options: msRest.RequestOptionsBase, callback: msRest.ServiceCallback<any>): void;
    patchLead(leadId: string, options?: msRest.RequestOptionsBase | msRest.ServiceCallback<any>, callback?: msRest.ServiceCallback<any>): Promise<Models.LeadsPatchLeadResponse> {
    return this.client.sendOperationRequest(
      {
        leadId,
        options
      },
      patchLeadOperationSpec,
      callback) as Promise<Models.LeadsPatchLeadResponse>;
    }

Any thoughts why there is no patchData argument in the generated calls even if it is in operationSpec:

const patchLeadOperationSpec: msRest.OperationSpec = {
  httpMethod: "PATCH",
  path: "AgentAssist.Api/api/leads/{leadId}",
  urlParameters: [
    Parameters.leadId0
  ],
  requestBody: {
    parameterPath: "patchData",
    mapper: {
      ...Mappers.JsonPatchDocumentLead,
      required: true
    }
  },
  responses: {
    200: {
      bodyMapper: {
        serializedName: "parsedResponse",
        type: {
          name: "String"
        }
      }
    },
    422: {
      bodyMapper: {
        serializedName: "parsedResponse",
        type: {
          name: "Sequence",
          element: {
            type: {
              name: "Composite",
              className: "ValidationError"
            }
          }
        }
      }
    },
    default: {}
  },
  serializer
};
amarzavery commented 5 years ago
tomasz-grobelny commented 5 years ago

I will try to create minimal example and post it here.

As for the second part: the name was generated by ASP.NET WebAPI from signature that looked more or less like this:

public IActionResult PatchLead(string leadId, [FromBody]JsonPatchDocument<Lead> patchData);

Do you think it might cause the issue?

amarzavery commented 5 years ago

I hope that shouldn't be the case. However you can try to generate the client after making that change, just to be sure if that is not causing the issue.

tomasz-grobelny commented 5 years ago

Ok, here comes the full example. Download the swagger.txt file and execute this command: autorest --typescript --input-file=swagger.txt --use=@microsoft.azure/autorest.typescript@4.1.1

Open generated/src/operations/values.ts and have a look at patchLead methods - they should contain patchData parameter but they do not.

amarzavery commented 5 years ago

@tomasz-grobelny - I found out the root cause. The body parameter is referring to "#/definitions/JsonPatchDocument[Lead]". You have declared all the properties of this model as "readOnly": true. Which means that the user cannot send anything inside the request body while making the request. All the properties of the model JsonPatchDocument[Lead] will be populated by the Server in the response. Thus the generator strips that model from being a parameter to the method as it does not want the user to mislead.

"JsonPatchDocument[Lead]": {
      "type": "object",
      "properties": {
        "Operations": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/JsonPatchOperation"
          },
          "readOnly": true
        },
        "HasOperations": {
          "type": "boolean",
          "readOnly": true
        }
      }
    },

If you remove "readOnly": true from atleast 1 property of the model "JsonPatchDocument[Lead]" then you will see the parameter show up in the method definition.

Having a model with all it's properties set to readOnly: true in the request body makes no sense.

tomasz-grobelny commented 5 years ago

Thanks a lot for the analysis. It seems then that I should look into how the swagger definition is generated. I see that JsonPatch NuGet package that I am using uses JsonPatchFormatter that does custom deserializing logic which is not taken into account at all when generating swagger definition. Seems I need to write custom ISchemaFilter to fix this.

tomasz-grobelny commented 5 years ago

As a side note: shouldn't autorest at least generate a warning when this situation occurs? Because now we effectively have an unusable operation.

amarzavery commented 5 years ago

Yeah, a warning will definitely help. Will update the generator to issue a warning for such scenarios.