-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Request Editors are broken #298
Description
There seems to be an bug introduced with the recent PR #228, resulting in request editors to not work as advertised.
Here's a snippet of code to illustrate the issue: using the example petstore-expanded.yaml OpenAPI spec provided in this project, I've generated a Go file of the client bindings (oapi-codegen -generate types,client -package main -o petstore.gen.go petstore-expanded.yaml), then I added some print statements inside the generated Client.FindPets() method as well as in Client.applyEditors():
// petstore.gen.go
func (c *Client) FindPets(ctx context.Context, params *FindPetsParams, reqEditors ...RequestEditorFn) (*http.Response, error) {
req, err := NewFindPetsRequest(c.Server, params)
if err != nil {
return nil, err
}
fmt.Printf("FindPets before applyEditors: req.Host=%s\n", req.Host)
if err := c.applyEditors(ctx, req, reqEditors); err != nil {
return nil, err
}
fmt.Printf("FindPets after applyEditors: req.Host=%s\n", req.Host)
return c.Client.Do(req)
}
// ...
func (c *Client) applyEditors(ctx context.Context, req *http.Request, additionalEditors []RequestEditorFn) error {
req = req.WithContext(ctx)
for _, r := range c.RequestEditors {
if err := r(ctx, req); err != nil {
return err
}
}
for _, r := range additionalEditors {
if err := r(ctx, req); err != nil {
return err
}
}
fmt.Printf("applyEditors: req.Host=%s\n", req.Host)
return nil
}Then, I wrote a little snippet that calls the generated code, and provides a RequestEditorFn that rewrites the request Host header:
// main.go
package main
import (
"context"
"net/http"
)
func editRequest(ctx context.Context, req *http.Request) error {
req.Host = "www.another-petstore.net"
return nil
}
func main() {
ctx := context.Background()
client, err := NewClient("https://www.petstore.net/")
if err != nil {
panic(err)
}
client.FindPets(ctx, &FindPetsParams{}, editRequest)
}When running the code, we can clearly see that the modifications performed by the reqEditors are only local to the Client.applyEditors() method, and not to the HTTP request actually sent by the Client:
$ go run .
FindPets before applyEditors: req.Host=www.petstore.net
applyEditors: req.Host=www.another-petstore.net
FindPets after applyEditors: req.Host=www.petstore.net
This looks to be caused by the use of req.WithContext(ctx) in the applyEditors() method, which cause the modifications performed by the request editor functions to be only local to the method – where in the previous versions, the req variable would still be accessible to the c.Client.Do() method: https://github.com/deepmap/oapi-codegen/pull/228/files#diff-c8306209ecceed0e799909503c21e674164c27a31308bf57f75c68cd159cc33eR104-R112
P.S.: It would have been nice to include the new reqEditors parameter to the *WithResponse methods as well.