Azure / autorest.go

Extension for AutoRest (https://github.com/Azure/autorest) that generates Go code
MIT License
70 stars 43 forks source link

Allow specification of dynamic query keyvalue pairs #1383

Open jim-minter opened 5 months ago

jim-minter commented 5 months ago

My use case is that I want to generate a client that can add arbitrary key-value pairs to the query-string of a request. I think it is possible to define this in OpenAPI 3 (not 2), but I don't think that autorest.go supports it yet.

For example, I can specify the following:

{
  "openapi": "3.0.0",
   ...
  "paths": {
    "/api/invoke": {
      "post": {
        "parameters": [
          {
            "name": "query",
            "in": "query",
            "schema": {
              "type": "array",
              "items": {
                "type": "string"
              }
            },
            "explode": true,
            "style": "form"
          }
        ]
      }
    }
  }
}

And that will generate a Go client that contains this snippet:

       reqQP := req.Raw().URL.Query()
       if options != nil && options.Query != nil {
               for _, qv := range options.Query {
                       reqQP.Add("query", qv)
               }
       }
       req.Raw().URL.RawQuery = reqQP.Encode()

This is really close, but I end up with a dynamic querystring ?query=a&query=b, and I want to express a dynamic querystring ?foo=a&bar=b, etc.

What I want to be able to do is instead of specifying an array of type string, specify an object with additionalProperties string.

Reading https://swagger.io/docs/specification/serialization/, I think this is a legitimate thing to be able to express in OpenAPI 3.

However when I do that, I get the error Error: unexpected type map[string]*string for QueryCollectionParameter Query.

Would it be possible to make this work? Thanks!

jhendrixMSFT commented 5 months ago

So generate something like this?

func (client *AliasClient) dynamicQueryStringCreateRequest(ctx context.Context, queries map[string]string, _ *DynamicQueryStringOptions) (*policy.Request, error) {
    urlPath := "/dynamicquerystring"
    req, err := runtime.NewRequest(ctx, http.MethodPost, runtime.JoinPaths(host, urlPath))
    if err != nil {
        return nil, err
    }
    reqQP := req.Raw().URL.Query()
    for qp, qv := range queries {
        reqQP.Add(qp, qv)
    }
    req.Raw().URL.RawQuery = reqQP.Encode()
    return req, nil
}

Although, I wonder if we should use url.Values instead of a map[string]string. It's not quite the same as what's being asked for in the OpenAPI but it would provide the most flexibility.

jim-minter commented 5 months ago

Looks good. I think I'd be inclined to implement as documented by openapi (i.e using type: object, and not supporting multiple query items with the same key) rather than a special case that might be unique to autorest.go?

jhendrixMSFT commented 5 months ago

Yeah I agree.