From c775a79850f926ec36195becb5b094f8f72e48bb Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Mon, 18 Dec 2023 16:55:37 +0000 Subject: [PATCH 1/5] Remove checks for `unexpected reference depth` In my opinion, this check doesn't seem to be providing much value, and we've got a few folks requesting this be disabled. Instead of gating this behind a feature flag, we'll just remove it. Closes #1348. --- pkg/codegen/utils.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/codegen/utils.go b/pkg/codegen/utils.go index 49a2f53f2c..a282756d70 100644 --- a/pkg/codegen/utils.go +++ b/pkg/codegen/utils.go @@ -352,14 +352,6 @@ func RefPathToGoType(refPath string) (string, error) { func refPathToGoType(refPath string, local bool) (string, error) { if refPath[0] == '#' { pathParts := strings.Split(refPath, "/") - depth := len(pathParts) - if local { - if depth != 4 { - return "", fmt.Errorf("unexpected reference depth: %d for ref: %s local: %t", depth, refPath, local) - } - } else if depth != 4 && depth != 2 { - return "", fmt.Errorf("unexpected reference depth: %d for ref: %s local: %t", depth, refPath, local) - } // Schemas may have been renamed locally, so look up the actual name in // the spec. From 2327423772d7f1d41b93c978da2f5125954cc62d Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Mon, 18 Dec 2023 16:56:45 +0000 Subject: [PATCH 2/5] Add test case for reference depth checks For #1348. --- internal/test/issues/issue-1348/config.yaml | 7 + internal/test/issues/issue-1348/gen.go | 3 + .../test/issues/issue-1348/issue1348.gen.go | 523 ++++++++++++++++++ internal/test/issues/issue-1348/spec.yaml | 38 ++ 4 files changed, 571 insertions(+) create mode 100644 internal/test/issues/issue-1348/config.yaml create mode 100644 internal/test/issues/issue-1348/gen.go create mode 100644 internal/test/issues/issue-1348/issue1348.gen.go create mode 100644 internal/test/issues/issue-1348/spec.yaml diff --git a/internal/test/issues/issue-1348/config.yaml b/internal/test/issues/issue-1348/config.yaml new file mode 100644 index 0000000000..3d3f316fbf --- /dev/null +++ b/internal/test/issues/issue-1348/config.yaml @@ -0,0 +1,7 @@ +package: issue1348 +generate: + models: true + client: true + gorilla-server: true + strict-server: true +output: issue1348.gen.go diff --git a/internal/test/issues/issue-1348/gen.go b/internal/test/issues/issue-1348/gen.go new file mode 100644 index 0000000000..d582c8a13e --- /dev/null +++ b/internal/test/issues/issue-1348/gen.go @@ -0,0 +1,3 @@ +package issue1348 + +//go:generate go run github.com/deepmap/oapi-codegen/v2/cmd/oapi-codegen --config=config.yaml spec.yaml diff --git a/internal/test/issues/issue-1348/issue1348.gen.go b/internal/test/issues/issue-1348/issue1348.gen.go new file mode 100644 index 0000000000..46b92d181b --- /dev/null +++ b/internal/test/issues/issue-1348/issue1348.gen.go @@ -0,0 +1,523 @@ +// Package issue1348 provides primitives to interact with the openapi HTTP API. +// +// Code generated by github.com/deepmap/oapi-codegen/v2 version v2.0.0-00010101000000-000000000000 DO NOT EDIT. +package issue1348 + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + + "github.com/gorilla/mux" + strictnethttp "github.com/oapi-codegen/runtime/strictmiddleware/nethttp" + openapi_types "github.com/oapi-codegen/runtime/types" +) + +// CreateSample defines model for CreateSample. +type CreateSample struct { + Field *Field `json:"field,omitempty"` +} + +// Sample defines model for Sample. +type Sample struct { + Field *string `json:"field,omitempty"` + Id *openapi_types.UUID `json:"id,omitempty"` +} + +// PostSamplesJSONRequestBody defines body for PostSamples for application/json ContentType. +type PostSamplesJSONRequestBody = CreateSample + +// RequestEditorFn is the function signature for the RequestEditor callback function +type RequestEditorFn func(ctx context.Context, req *http.Request) error + +// Doer performs HTTP requests. +// +// The standard http.Client implements this interface. +type HttpRequestDoer interface { + Do(req *http.Request) (*http.Response, error) +} + +// Client which conforms to the OpenAPI3 specification for this service. +type Client struct { + // The endpoint of the server conforming to this interface, with scheme, + // https://api.deepmap.com for example. This can contain a path relative + // to the server, such as https://api.deepmap.com/dev-test, and all the + // paths in the swagger spec will be appended to the server. + Server string + + // Doer for performing requests, typically a *http.Client with any + // customized settings, such as certificate chains. + Client HttpRequestDoer + + // A list of callbacks for modifying requests which are generated before sending over + // the network. + RequestEditors []RequestEditorFn +} + +// ClientOption allows setting custom parameters during construction +type ClientOption func(*Client) error + +// Creates a new Client, with reasonable defaults +func NewClient(server string, opts ...ClientOption) (*Client, error) { + // create a client with sane default values + client := Client{ + Server: server, + } + // mutate client and add all optional params + for _, o := range opts { + if err := o(&client); err != nil { + return nil, err + } + } + // ensure the server URL always has a trailing slash + if !strings.HasSuffix(client.Server, "/") { + client.Server += "/" + } + // create httpClient, if not already present + if client.Client == nil { + client.Client = &http.Client{} + } + return &client, nil +} + +// WithHTTPClient allows overriding the default Doer, which is +// automatically created using http.Client. This is useful for tests. +func WithHTTPClient(doer HttpRequestDoer) ClientOption { + return func(c *Client) error { + c.Client = doer + return nil + } +} + +// WithRequestEditorFn allows setting up a callback function, which will be +// called right before sending the request. This can be used to mutate the request. +func WithRequestEditorFn(fn RequestEditorFn) ClientOption { + return func(c *Client) error { + c.RequestEditors = append(c.RequestEditors, fn) + return nil + } +} + +// The interface specification for the client above. +type ClientInterface interface { + // PostSamplesWithBody request with any body + PostSamplesWithBody(ctx context.Context, contentType string, body io.Reader, reqEditors ...RequestEditorFn) (*http.Response, error) + + PostSamples(ctx context.Context, body PostSamplesJSONRequestBody, reqEditors ...RequestEditorFn) (*http.Response, error) +} + +func (c *Client) PostSamplesWithBody(ctx context.Context, contentType string, body io.Reader, reqEditors ...RequestEditorFn) (*http.Response, error) { + req, err := NewPostSamplesRequestWithBody(c.Server, contentType, body) + if err != nil { + return nil, err + } + req = req.WithContext(ctx) + if err := c.applyEditors(ctx, req, reqEditors); err != nil { + return nil, err + } + return c.Client.Do(req) +} + +func (c *Client) PostSamples(ctx context.Context, body PostSamplesJSONRequestBody, reqEditors ...RequestEditorFn) (*http.Response, error) { + req, err := NewPostSamplesRequest(c.Server, body) + if err != nil { + return nil, err + } + req = req.WithContext(ctx) + if err := c.applyEditors(ctx, req, reqEditors); err != nil { + return nil, err + } + return c.Client.Do(req) +} + +// NewPostSamplesRequest calls the generic PostSamples builder with application/json body +func NewPostSamplesRequest(server string, body PostSamplesJSONRequestBody) (*http.Request, error) { + var bodyReader io.Reader + buf, err := json.Marshal(body) + if err != nil { + return nil, err + } + bodyReader = bytes.NewReader(buf) + return NewPostSamplesRequestWithBody(server, "application/json", bodyReader) +} + +// NewPostSamplesRequestWithBody generates requests for PostSamples with any type of body +func NewPostSamplesRequestWithBody(server string, contentType string, body io.Reader) (*http.Request, error) { + var err error + + serverURL, err := url.Parse(server) + if err != nil { + return nil, err + } + + operationPath := fmt.Sprintf("/samples") + if operationPath[0] == '/' { + operationPath = "." + operationPath + } + + queryURL, err := serverURL.Parse(operationPath) + if err != nil { + return nil, err + } + + req, err := http.NewRequest("POST", queryURL.String(), body) + if err != nil { + return nil, err + } + + req.Header.Add("Content-Type", contentType) + + return req, nil +} + +func (c *Client) applyEditors(ctx context.Context, req *http.Request, additionalEditors []RequestEditorFn) error { + 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 + } + } + return nil +} + +// ClientWithResponses builds on ClientInterface to offer response payloads +type ClientWithResponses struct { + ClientInterface +} + +// NewClientWithResponses creates a new ClientWithResponses, which wraps +// Client with return type handling +func NewClientWithResponses(server string, opts ...ClientOption) (*ClientWithResponses, error) { + client, err := NewClient(server, opts...) + if err != nil { + return nil, err + } + return &ClientWithResponses{client}, nil +} + +// WithBaseURL overrides the baseURL. +func WithBaseURL(baseURL string) ClientOption { + return func(c *Client) error { + newBaseURL, err := url.Parse(baseURL) + if err != nil { + return err + } + c.Server = newBaseURL.String() + return nil + } +} + +// ClientWithResponsesInterface is the interface specification for the client with responses above. +type ClientWithResponsesInterface interface { + // PostSamplesWithBodyWithResponse request with any body + PostSamplesWithBodyWithResponse(ctx context.Context, contentType string, body io.Reader, reqEditors ...RequestEditorFn) (*PostSamplesResponse, error) + + PostSamplesWithResponse(ctx context.Context, body PostSamplesJSONRequestBody, reqEditors ...RequestEditorFn) (*PostSamplesResponse, error) +} + +type PostSamplesResponse struct { + Body []byte + HTTPResponse *http.Response + JSON201 *Sample +} + +// Status returns HTTPResponse.Status +func (r PostSamplesResponse) Status() string { + if r.HTTPResponse != nil { + return r.HTTPResponse.Status + } + return http.StatusText(0) +} + +// StatusCode returns HTTPResponse.StatusCode +func (r PostSamplesResponse) StatusCode() int { + if r.HTTPResponse != nil { + return r.HTTPResponse.StatusCode + } + return 0 +} + +// PostSamplesWithBodyWithResponse request with arbitrary body returning *PostSamplesResponse +func (c *ClientWithResponses) PostSamplesWithBodyWithResponse(ctx context.Context, contentType string, body io.Reader, reqEditors ...RequestEditorFn) (*PostSamplesResponse, error) { + rsp, err := c.PostSamplesWithBody(ctx, contentType, body, reqEditors...) + if err != nil { + return nil, err + } + return ParsePostSamplesResponse(rsp) +} + +func (c *ClientWithResponses) PostSamplesWithResponse(ctx context.Context, body PostSamplesJSONRequestBody, reqEditors ...RequestEditorFn) (*PostSamplesResponse, error) { + rsp, err := c.PostSamples(ctx, body, reqEditors...) + if err != nil { + return nil, err + } + return ParsePostSamplesResponse(rsp) +} + +// ParsePostSamplesResponse parses an HTTP response from a PostSamplesWithResponse call +func ParsePostSamplesResponse(rsp *http.Response) (*PostSamplesResponse, error) { + bodyBytes, err := io.ReadAll(rsp.Body) + defer func() { _ = rsp.Body.Close() }() + if err != nil { + return nil, err + } + + response := &PostSamplesResponse{ + Body: bodyBytes, + HTTPResponse: rsp, + } + + switch { + case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 201: + var dest Sample + if err := json.Unmarshal(bodyBytes, &dest); err != nil { + return nil, err + } + response.JSON201 = &dest + + } + + return response, nil +} + +// ServerInterface represents all server handlers. +type ServerInterface interface { + + // (POST /samples) + PostSamples(w http.ResponseWriter, r *http.Request) +} + +// ServerInterfaceWrapper converts contexts to parameters. +type ServerInterfaceWrapper struct { + Handler ServerInterface + HandlerMiddlewares []MiddlewareFunc + ErrorHandlerFunc func(w http.ResponseWriter, r *http.Request, err error) +} + +type MiddlewareFunc func(http.Handler) http.Handler + +// PostSamples operation middleware +func (siw *ServerInterfaceWrapper) PostSamples(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.PostSamples(w, r) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r.WithContext(ctx)) +} + +type UnescapedCookieParamError struct { + ParamName string + Err error +} + +func (e *UnescapedCookieParamError) Error() string { + return fmt.Sprintf("error unescaping cookie parameter '%s'", e.ParamName) +} + +func (e *UnescapedCookieParamError) Unwrap() error { + return e.Err +} + +type UnmarshalingParamError struct { + ParamName string + Err error +} + +func (e *UnmarshalingParamError) Error() string { + return fmt.Sprintf("Error unmarshaling parameter %s as JSON: %s", e.ParamName, e.Err.Error()) +} + +func (e *UnmarshalingParamError) Unwrap() error { + return e.Err +} + +type RequiredParamError struct { + ParamName string +} + +func (e *RequiredParamError) Error() string { + return fmt.Sprintf("Query argument %s is required, but not found", e.ParamName) +} + +type RequiredHeaderError struct { + ParamName string + Err error +} + +func (e *RequiredHeaderError) Error() string { + return fmt.Sprintf("Header parameter %s is required, but not found", e.ParamName) +} + +func (e *RequiredHeaderError) Unwrap() error { + return e.Err +} + +type InvalidParamFormatError struct { + ParamName string + Err error +} + +func (e *InvalidParamFormatError) Error() string { + return fmt.Sprintf("Invalid format for parameter %s: %s", e.ParamName, e.Err.Error()) +} + +func (e *InvalidParamFormatError) Unwrap() error { + return e.Err +} + +type TooManyValuesForParamError struct { + ParamName string + Count int +} + +func (e *TooManyValuesForParamError) Error() string { + return fmt.Sprintf("Expected one value for %s, got %d", e.ParamName, e.Count) +} + +// Handler creates http.Handler with routing matching OpenAPI spec. +func Handler(si ServerInterface) http.Handler { + return HandlerWithOptions(si, GorillaServerOptions{}) +} + +type GorillaServerOptions struct { + BaseURL string + BaseRouter *mux.Router + Middlewares []MiddlewareFunc + ErrorHandlerFunc func(w http.ResponseWriter, r *http.Request, err error) +} + +// HandlerFromMux creates http.Handler with routing matching OpenAPI spec based on the provided mux. +func HandlerFromMux(si ServerInterface, r *mux.Router) http.Handler { + return HandlerWithOptions(si, GorillaServerOptions{ + BaseRouter: r, + }) +} + +func HandlerFromMuxWithBaseURL(si ServerInterface, r *mux.Router, baseURL string) http.Handler { + return HandlerWithOptions(si, GorillaServerOptions{ + BaseURL: baseURL, + BaseRouter: r, + }) +} + +// HandlerWithOptions creates http.Handler with additional options +func HandlerWithOptions(si ServerInterface, options GorillaServerOptions) http.Handler { + r := options.BaseRouter + + if r == nil { + r = mux.NewRouter() + } + if options.ErrorHandlerFunc == nil { + options.ErrorHandlerFunc = func(w http.ResponseWriter, r *http.Request, err error) { + http.Error(w, err.Error(), http.StatusBadRequest) + } + } + wrapper := ServerInterfaceWrapper{ + Handler: si, + HandlerMiddlewares: options.Middlewares, + ErrorHandlerFunc: options.ErrorHandlerFunc, + } + + r.HandleFunc(options.BaseURL+"/samples", wrapper.PostSamples).Methods("POST") + + return r +} + +type PostSamplesRequestObject struct { + Body *PostSamplesJSONRequestBody +} + +type PostSamplesResponseObject interface { + VisitPostSamplesResponse(w http.ResponseWriter) error +} + +type PostSamples201JSONResponse Sample + +func (response PostSamples201JSONResponse) VisitPostSamplesResponse(w http.ResponseWriter) error { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(201) + + return json.NewEncoder(w).Encode(response) +} + +// StrictServerInterface represents all server handlers. +type StrictServerInterface interface { + + // (POST /samples) + PostSamples(ctx context.Context, request PostSamplesRequestObject) (PostSamplesResponseObject, error) +} + +type StrictHandlerFunc = strictnethttp.StrictHTTPHandlerFunc +type StrictMiddlewareFunc = strictnethttp.StrictHTTPMiddlewareFunc + +type StrictHTTPServerOptions struct { + RequestErrorHandlerFunc func(w http.ResponseWriter, r *http.Request, err error) + ResponseErrorHandlerFunc func(w http.ResponseWriter, r *http.Request, err error) +} + +func NewStrictHandler(ssi StrictServerInterface, middlewares []StrictMiddlewareFunc) ServerInterface { + return &strictHandler{ssi: ssi, middlewares: middlewares, options: StrictHTTPServerOptions{ + RequestErrorHandlerFunc: func(w http.ResponseWriter, r *http.Request, err error) { + http.Error(w, err.Error(), http.StatusBadRequest) + }, + ResponseErrorHandlerFunc: func(w http.ResponseWriter, r *http.Request, err error) { + http.Error(w, err.Error(), http.StatusInternalServerError) + }, + }} +} + +func NewStrictHandlerWithOptions(ssi StrictServerInterface, middlewares []StrictMiddlewareFunc, options StrictHTTPServerOptions) ServerInterface { + return &strictHandler{ssi: ssi, middlewares: middlewares, options: options} +} + +type strictHandler struct { + ssi StrictServerInterface + middlewares []StrictMiddlewareFunc + options StrictHTTPServerOptions +} + +// PostSamples operation middleware +func (sh *strictHandler) PostSamples(w http.ResponseWriter, r *http.Request) { + var request PostSamplesRequestObject + + var body PostSamplesJSONRequestBody + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + sh.options.RequestErrorHandlerFunc(w, r, fmt.Errorf("can't decode JSON body: %w", err)) + return + } + request.Body = &body + + handler := func(ctx context.Context, w http.ResponseWriter, r *http.Request, request interface{}) (interface{}, error) { + return sh.ssi.PostSamples(ctx, request.(PostSamplesRequestObject)) + } + for _, middleware := range sh.middlewares { + handler = middleware(handler, "PostSamples") + } + + response, err := handler(r.Context(), w, r, request) + + if err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } else if validResponse, ok := response.(PostSamplesResponseObject); ok { + if err := validResponse.VisitPostSamplesResponse(w); err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } + } else if response != nil { + sh.options.ResponseErrorHandlerFunc(w, r, fmt.Errorf("unexpected response type: %T", response)) + } +} diff --git a/internal/test/issues/issue-1348/spec.yaml b/internal/test/issues/issue-1348/spec.yaml new file mode 100644 index 0000000000..801fe84de2 --- /dev/null +++ b/internal/test/issues/issue-1348/spec.yaml @@ -0,0 +1,38 @@ +openapi: 3.0.3 +info: + title: test-service + version: 1.0.0 + +paths: + /samples: + post: + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/CreateSample" + responses: + 201: + description: ".." + content: + application/json: + schema: + $ref: "#/components/schemas/Sample" + +components: + schemas: + Sample: + type: object + properties: + id: + type: string + format: uuid + field: + type: string + # more properties here + + CreateSample: + type: object + properties: + field: + $ref: "#/components/schemas/Sample/properties/field" From 936ca062fda1d45aa7fe8499515e6ab4d44f75b6 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Tue, 19 Dec 2023 14:00:23 +0000 Subject: [PATCH 3/5] rew! WIP: add a `jsonLookup` method --- pkg/codegen/utils.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pkg/codegen/utils.go b/pkg/codegen/utils.go index a282756d70..8ea733bf2a 100644 --- a/pkg/codegen/utils.go +++ b/pkg/codegen/utils.go @@ -25,6 +25,7 @@ import ( "unicode" "github.com/getkin/kin-openapi/openapi3" + "github.com/go-openapi/jsonpointer" ) var ( @@ -955,3 +956,31 @@ func isAdditionalPropertiesExplicitFalse(s *openapi3.Schema) bool { return *s.AdditionalProperties.Has == false //nolint:gosimple } + +// jsonLookup allows taking a JSON pointer and looking up the value present at the given OpenAPI specification at that path. +func jsonLookup(spec *openapi3.T, ref string) (any, error) { + parts := strings.Split(ref, "/") + + var pointed any // TODO doc + pointed = spec + + // fmt.Println(jsonpointer.GetForToken(spec, parts[1])) + for i, v := range parts { + if i == 0 { + if v != "#" { + return nil, fmt.Errorf("TODO: %v", v) + } + + continue + } + + t, _, err := jsonpointer.GetForToken(pointed, v) + if err != nil { + return nil, err // TODO + } + + pointed = t + } + + return pointed, nil +} From cb50572b3c7e78f703cc7096e92f44282d7443c1 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Tue, 19 Dec 2023 14:04:47 +0000 Subject: [PATCH 4/5] WIP-wire --- pkg/codegen/utils.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/codegen/utils.go b/pkg/codegen/utils.go index 8ea733bf2a..284caedaec 100644 --- a/pkg/codegen/utils.go +++ b/pkg/codegen/utils.go @@ -354,6 +354,20 @@ func refPathToGoType(refPath string, local bool) (string, error) { if refPath[0] == '#' { pathParts := strings.Split(refPath, "/") + t, err := jsonLookup(globalState.spec, refPath) + if err != nil { + fmt.Printf("err: %v\n", err) + } + fmt.Printf("t: %v\n", t) + fmt.Printf("t: %#v\n", t) + + schema, ok := t.(*openapi3.Schema) + if ok { + if schema.Type == "string" { + return "string", nil + } + } + // Schemas may have been renamed locally, so look up the actual name in // the spec. name, err := findSchemaNameByRefPath(refPath, globalState.spec) From 87bdd5428369cc7bc315d3295b3c390605ab75f2 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Tue, 19 Dec 2023 14:13:44 +0000 Subject: [PATCH 5/5] WIP --- pkg/codegen/utils.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/codegen/utils.go b/pkg/codegen/utils.go index 284caedaec..d8436f7d0a 100644 --- a/pkg/codegen/utils.go +++ b/pkg/codegen/utils.go @@ -358,13 +358,24 @@ func refPathToGoType(refPath string, local bool) (string, error) { if err != nil { fmt.Printf("err: %v\n", err) } - fmt.Printf("t: %v\n", t) - fmt.Printf("t: %#v\n", t) + // fmt.Printf("t: %v\n", t) + // fmt.Printf("t: %#v\n", t) schema, ok := t.(*openapi3.Schema) if ok { - if schema.Type == "string" { - return "string", nil + var s Schema + // if schema.Type == "string" { + // return "string", nil + // } + fmt.Printf("s: %#v\n", s) + fmt.Printf("oapiSchemaToGoType(schema, nil, &s): %v\n", oapiSchemaToGoType(schema, nil, &s)) + fmt.Printf("s: %v\n", s) + if schema.Type != "object" { + err = oapiSchemaToGoType(schema, nil, &s) + if err != nil { + return "", err // TODO + } + return s.GoType, nil } }