From 1a76e021f3e427f45f0184c26af88bb299310ac7 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:23:18 -0500 Subject: [PATCH 01/14] [ffapi] Support for Text and Binary Streams Signed-off-by: hfuss --- pkg/ffapi/apiserver.go | 24 ++++++++++++++++++------ pkg/ffapi/handler.go | 9 +++++++-- pkg/ffapi/openapi3.go | 16 +++++++++++++++- pkg/ffapi/routes.go | 13 ++++++++++--- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/pkg/ffapi/apiserver.go b/pkg/ffapi/apiserver.go index 2d43f02..17a61c9 100644 --- a/pkg/ffapi/apiserver.go +++ b/pkg/ffapi/apiserver.go @@ -86,6 +86,7 @@ type APIServerOptions[T any] struct { type APIServerRouteExt[T any] struct { JSONHandler func(*APIRequest, T) (output interface{}, err error) UploadHandler func(*APIRequest, T) (output interface{}, err error) + StreamHandler func(*APIRequest, T) (output interface{}, err error) } // NewAPIServer makes a new server, with the specified configuration, and @@ -201,13 +202,24 @@ func (as *apiServer[T]) routeHandler(hf *HandlerFactory, route *Route) http.Hand // We extend the base ffapi functionality, with standardized DB filter support for all core resources. // We also pass the Orchestrator context through ext := route.Extensions.(*APIServerRouteExt[T]) - route.JSONHandler = func(r *APIRequest) (output interface{}, err error) { - er, err := as.EnrichRequest(r) - if err != nil { - return nil, err + if route.OutputType == "stream" && ext.StreamHandler != nil { + route.StreamHandler = func(r *APIRequest) (output interface{}, err error) { + er, err := as.EnrichRequest(r) + if err != nil { + return nil, err + } + return ext.StreamHandler(r, er) + } + } else { + route.JSONHandler = func(r *APIRequest) (output interface{}, err error) { + er, err := as.EnrichRequest(r) + if err != nil { + return nil, err + } + return ext.JSONHandler(r, er) } - return ext.JSONHandler(r, er) } + return hf.RouteHandler(route) } @@ -247,7 +259,7 @@ func (as *apiServer[T]) createMuxRouter(ctx context.Context) *mux.Router { return ce.UploadHandler(r, er) } } - if ce.JSONHandler != nil || ce.UploadHandler != nil { + if ce.JSONHandler != nil || ce.UploadHandler != nil || ce.StreamHandler != nil { r.HandleFunc(fmt.Sprintf("/api/v1/%s", route.Path), as.routeHandler(hf, route)). Methods(route.Method) } diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index 4a7229b..cf12125 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -219,6 +219,8 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { r.FP = multipart.formParams r.Part = multipart.part output, err = route.FormUploadHandler(r) + } else if route.StreamHandler != nil { + output, err = route.StreamHandler(r) } else { output, err = route.JSONHandler(r) } @@ -234,13 +236,13 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } } if err == nil { - status, err = hs.handleOutput(req.Context(), res, status, output) + status, err = hs.handleOutput(req.Context(), res, status, output, route) } return status, err }) } -func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}) (int, error) { +func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}, route *Route) (int, error) { vOutput := reflect.ValueOf(output) outputKind := vOutput.Kind() isPointer := outputKind == reflect.Ptr @@ -260,6 +262,9 @@ func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWri case reader != nil: defer reader.Close() res.Header().Add("Content-Type", "application/octet-stream") + if route.StreamOutputContentType != "" { + res.Header().Set("Content-Type", route.StreamOutputContentType) + } res.WriteHeader(status) _, marshalErr = io.Copy(res, reader) default: diff --git a/pkg/ffapi/openapi3.go b/pkg/ffapi/openapi3.go index c67868b..5acd0a7 100644 --- a/pkg/ffapi/openapi3.go +++ b/pkg/ffapi/openapi3.go @@ -277,9 +277,23 @@ func CheckObjectDocumented(example interface{}) { } func (sg *SwaggerGen) addOutput(ctx context.Context, doc *openapi3.T, route *Route, op *openapi3.Operation) { + s := i18n.Expand(ctx, i18n.APISuccessResponse) + if route.OutputType == "stream" { + contentType := "application/octet-stream" + if route.StreamOutputContentType != "" { + contentType = route.StreamOutputContentType + } + op.Responses.Set("200", &openapi3.ResponseRef{ + Value: &openapi3.Response{ + Description: &s, + Content: openapi3.Content{ + contentType: &openapi3.MediaType{}, + }, + }, + }) + } var schemaRef *openapi3.SchemaRef var err error - s := i18n.Expand(ctx, i18n.APISuccessResponse) schemaCustomizer := func(name string, t reflect.Type, tag reflect.StructTag, schema *openapi3.Schema) error { sg.addCustomType(t, schema) return sg.ffOutputTagHandler(ctx, route, name, tag, schema) diff --git a/pkg/ffapi/routes.go b/pkg/ffapi/routes.go index 98b9203..3beb0bb 100644 --- a/pkg/ffapi/routes.go +++ b/pkg/ffapi/routes.go @@ -61,12 +61,19 @@ type Route struct { JSONOutputSchema func(ctx context.Context, schemaGen SchemaGenerator) (*openapi3.SchemaRef, error) // JSONOutputValue is a function that returns a pointer to a structure to take JSON output JSONOutputValue func() interface{} - // JSONOutputCodes is the success response code + // JSONOutputCodes is the success response codes that could be returned by the API. Error codes are explicitly not supported by the framework since they could be subject to change by the errors thrown or how errors are handled. JSONOutputCodes []int - // JSONHandler is a function for handling JSON content type input. Input/Ouptut objects are returned by JSONInputValue/JSONOutputValue funcs + // JSONHandler is a function for handling JSON content type input. Input/Output objects are returned by JSONInputValue/JSONOutputValue funcs JSONHandler func(r *APIRequest) (output interface{}, err error) // FormUploadHandler takes a single file upload, and returns a JSON object - FormUploadHandler func(r *APIRequest) (output interface{}, err error) + FormUploadHandler func(r *APIRequest) (output interface{}, err error) + StreamOutputContentType string + // StreamHandler allows for custom request handling and non-JSON responses + StreamHandler func(r *APIRequest) (output interface{}, err error) + + // json or stream + OutputType string + // Deprecated whether this route is deprecated Deprecated bool // Tag a category identifier for this route in the generated OpenAPI spec From a7aceb2b42da76b1895c0e1ed2ee7fbf146cfc64 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:26:08 -0500 Subject: [PATCH 02/14] io.ReadCloser to prescribe return type Signed-off-by: hfuss --- pkg/ffapi/apiserver.go | 5 +++-- pkg/ffapi/routes.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/ffapi/apiserver.go b/pkg/ffapi/apiserver.go index 17a61c9..acbbd3c 100644 --- a/pkg/ffapi/apiserver.go +++ b/pkg/ffapi/apiserver.go @@ -19,6 +19,7 @@ package ffapi import ( "context" "fmt" + "io" "net" "net/http" "time" @@ -86,7 +87,7 @@ type APIServerOptions[T any] struct { type APIServerRouteExt[T any] struct { JSONHandler func(*APIRequest, T) (output interface{}, err error) UploadHandler func(*APIRequest, T) (output interface{}, err error) - StreamHandler func(*APIRequest, T) (output interface{}, err error) + StreamHandler func(*APIRequest, T) (output io.ReadCloser, err error) } // NewAPIServer makes a new server, with the specified configuration, and @@ -203,7 +204,7 @@ func (as *apiServer[T]) routeHandler(hf *HandlerFactory, route *Route) http.Hand // We also pass the Orchestrator context through ext := route.Extensions.(*APIServerRouteExt[T]) if route.OutputType == "stream" && ext.StreamHandler != nil { - route.StreamHandler = func(r *APIRequest) (output interface{}, err error) { + route.StreamHandler = func(r *APIRequest) (output io.ReadCloser, err error) { er, err := as.EnrichRequest(r) if err != nil { return nil, err diff --git a/pkg/ffapi/routes.go b/pkg/ffapi/routes.go index 3beb0bb..cb72e51 100644 --- a/pkg/ffapi/routes.go +++ b/pkg/ffapi/routes.go @@ -18,6 +18,7 @@ package ffapi import ( "context" + "io" "github.com/getkin/kin-openapi/openapi3" "github.com/hyperledger/firefly-common/pkg/config" @@ -69,7 +70,7 @@ type Route struct { FormUploadHandler func(r *APIRequest) (output interface{}, err error) StreamOutputContentType string // StreamHandler allows for custom request handling and non-JSON responses - StreamHandler func(r *APIRequest) (output interface{}, err error) + StreamHandler func(r *APIRequest) (output io.ReadCloser, err error) // json or stream OutputType string From 6cd38168866d59c444dcfeee9dab9966d1dbd27a Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:37:10 -0500 Subject: [PATCH 03/14] comments and enum Signed-off-by: hfuss --- pkg/ffapi/apiserver.go | 2 +- pkg/ffapi/openapi3.go | 2 +- pkg/ffapi/routes.go | 18 ++++++++++++------ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/ffapi/apiserver.go b/pkg/ffapi/apiserver.go index acbbd3c..cd3bf34 100644 --- a/pkg/ffapi/apiserver.go +++ b/pkg/ffapi/apiserver.go @@ -203,7 +203,7 @@ func (as *apiServer[T]) routeHandler(hf *HandlerFactory, route *Route) http.Hand // We extend the base ffapi functionality, with standardized DB filter support for all core resources. // We also pass the Orchestrator context through ext := route.Extensions.(*APIServerRouteExt[T]) - if route.OutputType == "stream" && ext.StreamHandler != nil { + if route.OutputType == RouteOutputTypeStream && ext.StreamHandler != nil { route.StreamHandler = func(r *APIRequest) (output io.ReadCloser, err error) { er, err := as.EnrichRequest(r) if err != nil { diff --git a/pkg/ffapi/openapi3.go b/pkg/ffapi/openapi3.go index 5acd0a7..b737d89 100644 --- a/pkg/ffapi/openapi3.go +++ b/pkg/ffapi/openapi3.go @@ -278,7 +278,7 @@ func CheckObjectDocumented(example interface{}) { func (sg *SwaggerGen) addOutput(ctx context.Context, doc *openapi3.T, route *Route, op *openapi3.Operation) { s := i18n.Expand(ctx, i18n.APISuccessResponse) - if route.OutputType == "stream" { + if route.OutputType == RouteOutputTypeStream { contentType := "application/octet-stream" if route.StreamOutputContentType != "" { contentType = route.StreamOutputContentType diff --git a/pkg/ffapi/routes.go b/pkg/ffapi/routes.go index cb72e51..2b6d98b 100644 --- a/pkg/ffapi/routes.go +++ b/pkg/ffapi/routes.go @@ -67,14 +67,13 @@ type Route struct { // JSONHandler is a function for handling JSON content type input. Input/Output objects are returned by JSONInputValue/JSONOutputValue funcs JSONHandler func(r *APIRequest) (output interface{}, err error) // FormUploadHandler takes a single file upload, and returns a JSON object - FormUploadHandler func(r *APIRequest) (output interface{}, err error) + FormUploadHandler func(r *APIRequest) (output interface{}, err error) + // StreamOutputContentType allows for overriding the default binary (application/octet-stream) with a custom MIME type StreamOutputContentType string - // StreamHandler allows for custom request handling and non-JSON responses + // StreamHandler allows for custom request handling with explicit stream (io.ReadCloser) responses StreamHandler func(r *APIRequest) (output io.ReadCloser, err error) - - // json or stream - OutputType string - + // OutputType for OpenAPI generation, either 'json' or 'stream'. Defaults to 'json' if none is provided + OutputType RouteOutputType // Deprecated whether this route is deprecated Deprecated bool // Tag a category identifier for this route in the generated OpenAPI spec @@ -83,6 +82,13 @@ type Route struct { Extensions interface{} } +type RouteOutputType string + +const ( + RouteOutputTypeJSON RouteOutputType = "json" + RouteOutputTypeStream RouteOutputType = "stream" +) + // PathParam is a description of a path parameter type PathParam struct { // Name is the name of the parameter, from the Gorilla path mux From 5e2ff09d3a29b7c8ba9d3c9112ad5556748261dc Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:38:06 -0500 Subject: [PATCH 04/14] headers Signed-off-by: hfuss --- pkg/ffapi/apiserver.go | 2 +- pkg/ffapi/handler.go | 2 +- pkg/ffapi/routes.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/ffapi/apiserver.go b/pkg/ffapi/apiserver.go index cd3bf34..f4c5d89 100644 --- a/pkg/ffapi/apiserver.go +++ b/pkg/ffapi/apiserver.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2024 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index cf12125..0bb3d64 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2024 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // diff --git a/pkg/ffapi/routes.go b/pkg/ffapi/routes.go index 2b6d98b..a74aa72 100644 --- a/pkg/ffapi/routes.go +++ b/pkg/ffapi/routes.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2024 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // From 729ca040770abdd93f466c220f5049d156cc3703 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:41:56 -0500 Subject: [PATCH 05/14] lint Signed-off-by: hfuss --- pkg/ffapi/handler.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index 0bb3d64..7d6de17 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -215,13 +215,14 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { if len(route.JSONOutputCodes) > 0 { r.SuccessStatus = route.JSONOutputCodes[0] } - if multipart != nil { + switch { + case multipart != nil: r.FP = multipart.formParams r.Part = multipart.part output, err = route.FormUploadHandler(r) - } else if route.StreamHandler != nil { + case route.OutputType == RouteOutputTypeStream && route.StreamHandler != nil: output, err = route.StreamHandler(r) - } else { + default: output, err = route.JSONHandler(r) } status = r.SuccessStatus // Can be updated by the route From e26c9d200c5eed3f7352da0357a49df38134e2a9 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:43:45 -0500 Subject: [PATCH 06/14] avoiding passing route Signed-off-by: hfuss --- pkg/ffapi/handler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index 7d6de17..a7860b0 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -237,13 +237,13 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } } if err == nil { - status, err = hs.handleOutput(req.Context(), res, status, output, route) + status, err = hs.handleOutput(req.Context(), res, status, output, route.StreamOutputContentType) } return status, err }) } -func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}, route *Route) (int, error) { +func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}, streamContentType string) (int, error) { vOutput := reflect.ValueOf(output) outputKind := vOutput.Kind() isPointer := outputKind == reflect.Ptr @@ -263,8 +263,8 @@ func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWri case reader != nil: defer reader.Close() res.Header().Add("Content-Type", "application/octet-stream") - if route.StreamOutputContentType != "" { - res.Header().Set("Content-Type", route.StreamOutputContentType) + if streamContentType != "" { + res.Header().Set("Content-Type", streamContentType) } res.WriteHeader(status) _, marshalErr = io.Copy(res, reader) From 2d5deff21fbd6fc17e21673c642e14d86b75a885 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:49:36 -0500 Subject: [PATCH 07/14] struct to avoid awkward optional string param for streams Signed-off-by: hfuss --- pkg/ffapi/handler.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index a7860b0..009ba7a 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -189,8 +189,8 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } } - var status = 400 // if fail parsing input - var output interface{} + status := 400 // if fail parsing input + output := &handlerOutput{} if err == nil { queryParams, pathParams, queryArrayParams = hs.getParams(req, route) } @@ -219,11 +219,12 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { case multipart != nil: r.FP = multipart.formParams r.Part = multipart.part - output, err = route.FormUploadHandler(r) + output.out, err = route.FormUploadHandler(r) case route.OutputType == RouteOutputTypeStream && route.StreamHandler != nil: - output, err = route.StreamHandler(r) + output.out, err = route.StreamHandler(r) + output.contentType = route.StreamOutputContentType default: - output, err = route.JSONHandler(r) + output.out, err = route.JSONHandler(r) } status = r.SuccessStatus // Can be updated by the route } @@ -237,14 +238,20 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } } if err == nil { - status, err = hs.handleOutput(req.Context(), res, status, output, route.StreamOutputContentType) + status, err = hs.handleOutput(req.Context(), res, status, output) } return status, err }) } -func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}, streamContentType string) (int, error) { - vOutput := reflect.ValueOf(output) +// a wrapper around a dynamic output, allowing for customizing returned content types +type handlerOutput struct { + out interface{} + contentType string +} + +func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output *handlerOutput) (int, error) { + vOutput := reflect.ValueOf(output.out) outputKind := vOutput.Kind() isPointer := outputKind == reflect.Ptr invalid := outputKind == reflect.Invalid @@ -263,8 +270,8 @@ func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWri case reader != nil: defer reader.Close() res.Header().Add("Content-Type", "application/octet-stream") - if streamContentType != "" { - res.Header().Set("Content-Type", streamContentType) + if output.contentType != "" { + res.Header().Set("Content-Type", output.contentType) } res.WriteHeader(status) _, marshalErr = io.Copy(res, reader) From 1edbce4c298f53368b37d79bd4b0adccf9bb7adc Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 08:52:55 -0500 Subject: [PATCH 08/14] fix bugs with handlerOutput Signed-off-by: hfuss --- pkg/ffapi/handler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index 009ba7a..5e41917 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -190,7 +190,7 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } status := 400 // if fail parsing input - output := &handlerOutput{} + output := handlerOutput{} if err == nil { queryParams, pathParams, queryArrayParams = hs.getParams(req, route) } @@ -250,12 +250,12 @@ type handlerOutput struct { contentType string } -func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output *handlerOutput) (int, error) { +func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output handlerOutput) (int, error) { vOutput := reflect.ValueOf(output.out) outputKind := vOutput.Kind() isPointer := outputKind == reflect.Ptr invalid := outputKind == reflect.Invalid - isNil := output == nil || invalid || (isPointer && vOutput.IsNil()) + isNil := output.out == nil || invalid || (isPointer && vOutput.IsNil()) var reader io.ReadCloser var marshalErr error if !isNil && vOutput.CanInterface() { @@ -278,7 +278,7 @@ func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWri default: res.Header().Add("Content-Type", "application/json") res.WriteHeader(status) - marshalErr = json.NewEncoder(res).Encode(output) + marshalErr = json.NewEncoder(res).Encode(output.out) } if marshalErr != nil { err := i18n.WrapError(ctx, marshalErr, i18n.MsgResponseMarshalError) From e3f3b81273d43a7a0612321a370318daa91c7596 Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 15:12:06 -0500 Subject: [PATCH 09/14] simplifying openapi generation options for streams and other edge cases; allow handlers to manually specify content-type for non-JSON i.e. streams Signed-off-by: hfuss --- pkg/ffapi/handler.go | 46 +++++++++++++++++++------------------------ pkg/ffapi/openapi3.go | 19 ++++-------------- pkg/ffapi/routes.go | 13 ++---------- 3 files changed, 26 insertions(+), 52 deletions(-) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index 5e41917..b75fae3 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -190,7 +190,7 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { } status := 400 // if fail parsing input - output := handlerOutput{} + var output interface{} if err == nil { queryParams, pathParams, queryArrayParams = hs.getParams(req, route) } @@ -202,15 +202,17 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { if err == nil { r := &APIRequest{ - Req: req, - PP: pathParams, - QP: queryParams, - QAP: queryArrayParams, - Filter: filter, - Input: jsonInput, - SuccessStatus: http.StatusOK, + Req: req, + PP: pathParams, + QP: queryParams, + QAP: queryArrayParams, + Filter: filter, + Input: jsonInput, + SuccessStatus: http.StatusOK, + AlwaysPaginate: hs.AlwaysPaginate, + + // res.Header() returns a map which is a ref type so handler header edits are persisted ResponseHeaders: res.Header(), - AlwaysPaginate: hs.AlwaysPaginate, } if len(route.JSONOutputCodes) > 0 { r.SuccessStatus = route.JSONOutputCodes[0] @@ -219,12 +221,11 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { case multipart != nil: r.FP = multipart.formParams r.Part = multipart.part - output.out, err = route.FormUploadHandler(r) + output, err = route.FormUploadHandler(r) case route.OutputType == RouteOutputTypeStream && route.StreamHandler != nil: - output.out, err = route.StreamHandler(r) - output.contentType = route.StreamOutputContentType + output, err = route.StreamHandler(r) default: - output.out, err = route.JSONHandler(r) + output, err = route.JSONHandler(r) } status = r.SuccessStatus // Can be updated by the route } @@ -244,18 +245,12 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { }) } -// a wrapper around a dynamic output, allowing for customizing returned content types -type handlerOutput struct { - out interface{} - contentType string -} - -func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output handlerOutput) (int, error) { - vOutput := reflect.ValueOf(output.out) +func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWriter, status int, output interface{}) (int, error) { + vOutput := reflect.ValueOf(output) outputKind := vOutput.Kind() isPointer := outputKind == reflect.Ptr invalid := outputKind == reflect.Invalid - isNil := output.out == nil || invalid || (isPointer && vOutput.IsNil()) + isNil := output == nil || invalid || (isPointer && vOutput.IsNil()) var reader io.ReadCloser var marshalErr error if !isNil && vOutput.CanInterface() { @@ -269,16 +264,15 @@ func (hs *HandlerFactory) handleOutput(ctx context.Context, res http.ResponseWri res.WriteHeader(204) case reader != nil: defer reader.Close() - res.Header().Add("Content-Type", "application/octet-stream") - if output.contentType != "" { - res.Header().Set("Content-Type", output.contentType) + if res.Header().Get("Content-Type") == "" { + res.Header().Add("Content-Type", "application/octet-stream") } res.WriteHeader(status) _, marshalErr = io.Copy(res, reader) default: res.Header().Add("Content-Type", "application/json") res.WriteHeader(status) - marshalErr = json.NewEncoder(res).Encode(output.out) + marshalErr = json.NewEncoder(res).Encode(output) } if marshalErr != nil { err := i18n.WrapError(ctx, marshalErr, i18n.MsgResponseMarshalError) diff --git a/pkg/ffapi/openapi3.go b/pkg/ffapi/openapi3.go index b737d89..35ba549 100644 --- a/pkg/ffapi/openapi3.go +++ b/pkg/ffapi/openapi3.go @@ -277,23 +277,9 @@ func CheckObjectDocumented(example interface{}) { } func (sg *SwaggerGen) addOutput(ctx context.Context, doc *openapi3.T, route *Route, op *openapi3.Operation) { - s := i18n.Expand(ctx, i18n.APISuccessResponse) - if route.OutputType == RouteOutputTypeStream { - contentType := "application/octet-stream" - if route.StreamOutputContentType != "" { - contentType = route.StreamOutputContentType - } - op.Responses.Set("200", &openapi3.ResponseRef{ - Value: &openapi3.Response{ - Description: &s, - Content: openapi3.Content{ - contentType: &openapi3.MediaType{}, - }, - }, - }) - } var schemaRef *openapi3.SchemaRef var err error + s := i18n.Expand(ctx, i18n.APISuccessResponse) schemaCustomizer := func(name string, t reflect.Type, tag reflect.StructTag, schema *openapi3.Schema) error { sg.addCustomType(t, schema) return sg.ffOutputTagHandler(ctx, route, name, tag, schema) @@ -327,6 +313,9 @@ func (sg *SwaggerGen) addOutput(ctx context.Context, doc *openapi3.T, route *Rou }, }) } + for code, res := range route.CustomResponseRefs { + op.Responses.Set(code, res) + } } func (sg *SwaggerGen) AddParam(ctx context.Context, op *openapi3.Operation, in, name, def, example string, description i18n.MessageKey, deprecated bool, msgArgs ...interface{}) { diff --git a/pkg/ffapi/routes.go b/pkg/ffapi/routes.go index a74aa72..12578ef 100644 --- a/pkg/ffapi/routes.go +++ b/pkg/ffapi/routes.go @@ -68,12 +68,10 @@ type Route struct { JSONHandler func(r *APIRequest) (output interface{}, err error) // FormUploadHandler takes a single file upload, and returns a JSON object FormUploadHandler func(r *APIRequest) (output interface{}, err error) - // StreamOutputContentType allows for overriding the default binary (application/octet-stream) with a custom MIME type - StreamOutputContentType string // StreamHandler allows for custom request handling with explicit stream (io.ReadCloser) responses StreamHandler func(r *APIRequest) (output io.ReadCloser, err error) - // OutputType for OpenAPI generation, either 'json' or 'stream'. Defaults to 'json' if none is provided - OutputType RouteOutputType + // CustomResponseRefs allows for specifying custom responses for a route + CustomResponseRefs map[string]*openapi3.ResponseRef // Deprecated whether this route is deprecated Deprecated bool // Tag a category identifier for this route in the generated OpenAPI spec @@ -82,13 +80,6 @@ type Route struct { Extensions interface{} } -type RouteOutputType string - -const ( - RouteOutputTypeJSON RouteOutputType = "json" - RouteOutputTypeStream RouteOutputType = "stream" -) - // PathParam is a description of a path parameter type PathParam struct { // Name is the name of the parameter, from the Gorilla path mux From 1c74e1a5899fd5758dd0456510014cc08384abbf Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 15:14:50 -0500 Subject: [PATCH 10/14] Fixes Signed-off-by: hfuss --- pkg/ffapi/apiserver.go | 5 +++-- pkg/ffapi/handler.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/ffapi/apiserver.go b/pkg/ffapi/apiserver.go index f4c5d89..d1d639a 100644 --- a/pkg/ffapi/apiserver.go +++ b/pkg/ffapi/apiserver.go @@ -203,7 +203,8 @@ func (as *apiServer[T]) routeHandler(hf *HandlerFactory, route *Route) http.Hand // We extend the base ffapi functionality, with standardized DB filter support for all core resources. // We also pass the Orchestrator context through ext := route.Extensions.(*APIServerRouteExt[T]) - if route.OutputType == RouteOutputTypeStream && ext.StreamHandler != nil { + switch { + case ext.StreamHandler != nil: route.StreamHandler = func(r *APIRequest) (output io.ReadCloser, err error) { er, err := as.EnrichRequest(r) if err != nil { @@ -211,7 +212,7 @@ func (as *apiServer[T]) routeHandler(hf *HandlerFactory, route *Route) http.Hand } return ext.StreamHandler(r, er) } - } else { + case ext.JSONHandler != nil: route.JSONHandler = func(r *APIRequest) (output interface{}, err error) { er, err := as.EnrichRequest(r) if err != nil { diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index b75fae3..40e5487 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -222,7 +222,7 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc { r.FP = multipart.formParams r.Part = multipart.part output, err = route.FormUploadHandler(r) - case route.OutputType == RouteOutputTypeStream && route.StreamHandler != nil: + case route.StreamHandler != nil: output, err = route.StreamHandler(r) default: output, err = route.JSONHandler(r) From 0868da912227bfc5482163a48422729a66efe78d Mon Sep 17 00:00:00 2001 From: hfuss Date: Tue, 13 Feb 2024 15:19:27 -0500 Subject: [PATCH 11/14] set description if none provided Signed-off-by: hfuss --- pkg/ffapi/openapi3.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/ffapi/openapi3.go b/pkg/ffapi/openapi3.go index 35ba549..2bcee7f 100644 --- a/pkg/ffapi/openapi3.go +++ b/pkg/ffapi/openapi3.go @@ -314,6 +314,9 @@ func (sg *SwaggerGen) addOutput(ctx context.Context, doc *openapi3.T, route *Rou }) } for code, res := range route.CustomResponseRefs { + if res.Value != nil && res.Value.Description == nil { + res.Value.Description = &s + } op.Responses.Set(code, res) } } From 5582e08202b34195cc0965d62c81a133c8804c32 Mon Sep 17 00:00:00 2001 From: hfuss Date: Mon, 26 Feb 2024 14:05:36 -0500 Subject: [PATCH 12/14] handler tests for text and binary streams Signed-off-by: hfuss --- pkg/ffapi/handler_test.go | 68 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/pkg/ffapi/handler_test.go b/pkg/ffapi/handler_test.go index 3f8375b..f196a87 100644 --- a/pkg/ffapi/handler_test.go +++ b/pkg/ffapi/handler_test.go @@ -21,6 +21,8 @@ import ( "context" "encoding/json" "fmt" + "github.com/getkin/kin-openapi/openapi3" + "github.com/stretchr/testify/require" "io" "mime/multipart" "net/http" @@ -156,6 +158,72 @@ func TestJSONHTTPNilResponseNon204(t *testing.T) { assert.Regexp(t, "FF00164", resJSON["error"]) } +func TestStreamHttpResponsePlainText200(t *testing.T) { + text := ` +some stream +of +text +!!! +` + s, _, done := newTestServer(t, []*Route{{ + Name: "testRoute", + Path: "/test", + Method: "GET", + CustomResponseRefs: map[string]*openapi3.ResponseRef{ + "200": { + Value: &openapi3.Response{ + Content: openapi3.Content{ + "text/plain": &openapi3.MediaType{}, + }, + }, + }, + }, + StreamHandler: func(r *APIRequest) (output io.ReadCloser, err error) { + r.ResponseHeaders.Add("Content-Type", "text/plain") + return io.NopCloser(strings.NewReader(text)), nil + }, + }}, "", nil) + defer done() + + res, err := http.Get(fmt.Sprintf("http://%s/test", s.Addr())) + require.NoError(t, err) + assert.Equal(t, 200, res.StatusCode) + assert.Equal(t, "text/plain", res.Header.Get("Content-Type")) + b, err := io.ReadAll(res.Body) + require.NoError(t, err) + assert.Equal(t, text, string(b)) +} + +func TestStreamHttpResponseBinary200(t *testing.T) { + randomBytes := []byte{3, 255, 192, 201, 33, 50} + s, _, done := newTestServer(t, []*Route{{ + Name: "testRoute", + Path: "/test", + Method: "GET", + CustomResponseRefs: map[string]*openapi3.ResponseRef{ + "200": { + Value: &openapi3.Response{ + Content: openapi3.Content{ + "application/octet-stream": &openapi3.MediaType{}, + }, + }, + }, + }, + StreamHandler: func(r *APIRequest) (output io.ReadCloser, err error) { + return io.NopCloser(bytes.NewReader(randomBytes)), nil + }, + }}, "", nil) + defer done() + + res, err := http.Get(fmt.Sprintf("http://%s/test", s.Addr())) + require.NoError(t, err) + assert.Equal(t, 200, res.StatusCode) + assert.Equal(t, "application/octet-stream", res.Header.Get("Content-Type")) + b, err := io.ReadAll(res.Body) + require.NoError(t, err) + assert.Equal(t, randomBytes, b) +} + func TestJSONHTTPDefault500Error(t *testing.T) { s, _, done := newTestServer(t, []*Route{{ Name: "testRoute", From a707576a8aa23d7ca11c77711181b1e88d051a73 Mon Sep 17 00:00:00 2001 From: hfuss Date: Mon, 26 Feb 2024 14:13:51 -0500 Subject: [PATCH 13/14] openapi ut Signed-off-by: hfuss --- pkg/ffapi/openapi3_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/pkg/ffapi/openapi3_test.go b/pkg/ffapi/openapi3_test.go index ee1eb98..f44911a 100644 --- a/pkg/ffapi/openapi3_test.go +++ b/pkg/ffapi/openapi3_test.go @@ -19,6 +19,7 @@ package ffapi import ( "context" "fmt" + "github.com/stretchr/testify/require" "net/http" "testing" @@ -298,6 +299,36 @@ func TestFFExcludeTag(t *testing.T) { assert.Regexp(t, "no schema", err) } +func TestCustomResponseRefs(t *testing.T) { + routes := []*Route{ + { + Name: "CustomResponseRefTest", + Path: "/test", + Method: http.MethodGet, + CustomResponseRefs: map[string]*openapi3.ResponseRef{ + "200": { + Value: &openapi3.Response{ + Content: openapi3.Content{ + "text/plain": &openapi3.MediaType{}, + }, + }, + }, + }, + }, + } + swagger := NewSwaggerGen(&SwaggerGenOptions{ + Title: "UnitTest", + Version: "1.0", + BaseURL: "http://localhost:12345/api/v1", + }).Generate(context.Background(), routes) + assert.Nil(t, swagger.Paths.Find("/test").Get.RequestBody) + require.NotEmpty(t, swagger.Paths.Find("/test").Get.Responses) + require.NotNil(t, swagger.Paths.Find("/test").Get.Responses.Value("200")) + require.NotNil(t, swagger.Paths.Find("/test").Get.Responses.Value("200").Value) + assert.NotNil(t, swagger.Paths.Find("/test").Get.Responses.Value("200").Value.Content.Get("text/plain")) + assert.Nil(t, swagger.Paths.Find("/test").Get.Responses.Value("201")) +} + func TestPanicOnMissingDescription(t *testing.T) { routes := []*Route{ { From 26af1df869ed8d177785df96b1315667d7b16563 Mon Sep 17 00:00:00 2001 From: hfuss Date: Mon, 26 Feb 2024 14:27:47 -0500 Subject: [PATCH 14/14] apiserver test Signed-off-by: hfuss --- pkg/ffapi/apiserver_test.go | 51 ++++++++++++++++++++++++++++++++++++- pkg/ffapi/handler_test.go | 2 +- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/pkg/ffapi/apiserver_test.go b/pkg/ffapi/apiserver_test.go index 8abad5d..f91b5bd 100644 --- a/pkg/ffapi/apiserver_test.go +++ b/pkg/ffapi/apiserver_test.go @@ -19,6 +19,7 @@ package ffapi import ( "context" "fmt" + "github.com/getkin/kin-openapi/openapi3" "io" "net/http" "strings" @@ -38,6 +39,7 @@ type utManager struct { mockEnrichErr error calledJSONHandler string calledUploadHandler string + calledStreamHandler string } type sampleInput struct { @@ -80,6 +82,35 @@ var utAPIRoute1 = &Route{ }, } +var utAPIRoute2 = &Route{ + Name: "utAPIRoute2", + Path: "ut/utresource/{resourceid}/getit", + Method: http.MethodGet, + Description: "random GET stream route for testing", + PathParams: []*PathParam{ + {Name: "resourceid", Description: "My resource"}, + }, + FormParams: nil, + JSONInputValue: nil, + JSONOutputValue: nil, + JSONOutputCodes: nil, + CustomResponseRefs: map[string]*openapi3.ResponseRef{ + "200": { + Value: &openapi3.Response{ + Content: openapi3.Content{ + "application/octet-stream": {}, + }, + }, + }, + }, + Extensions: &APIServerRouteExt[*utManager]{ + StreamHandler: func(r *APIRequest, um *utManager) (output io.ReadCloser, err error) { + um.calledStreamHandler = r.PP["resourceid"] + return io.NopCloser(strings.NewReader("a stream!")), nil + }, + }, +} + func initUTConfig() (config.Section, config.Section, config.Section) { config.RootConfigReset() apiConfig := config.RootSection("ut.api") @@ -97,7 +128,7 @@ func newTestAPIServer(t *testing.T, start bool) (*utManager, *apiServer[*utManag um := &utManager{t: t} as := NewAPIServer(ctx, APIServerOptions[*utManager]{ MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"), - Routes: []*Route{utAPIRoute1}, + Routes: []*Route{utAPIRoute1, utAPIRoute2}, EnrichRequest: func(r *APIRequest) (*utManager, error) { // This could be some dynamic object based on extra processing in the request, // but the most common case is you just have a "manager" that you inject into each @@ -125,6 +156,24 @@ func newTestAPIServer(t *testing.T, start bool) (*utManager, *apiServer[*utManag } } +func TestAPIServerInvokeAPIRouteStream(t *testing.T) { + um, as, done := newTestAPIServer(t, true) + defer done() + + <-as.Started() + + var o sampleOutput + res, err := resty.New().R(). + SetBody(nil). + SetResult(&o). + Get(fmt.Sprintf("%s/api/v1/ut/utresource/id12345/getit", as.APIPublicURL())) + assert.NoError(t, err) + assert.Equal(t, 200, res.StatusCode()) + assert.Equal(t, "application/octet-stream", res.Header().Get("Content-Type")) + assert.Equal(t, "id12345", um.calledStreamHandler) + assert.Equal(t, "a stream!", string(res.Body())) +} + func TestAPIServerInvokeAPIRouteJSON(t *testing.T) { um, as, done := newTestAPIServer(t, true) defer done() diff --git a/pkg/ffapi/handler_test.go b/pkg/ffapi/handler_test.go index f196a87..b8ffd21 100644 --- a/pkg/ffapi/handler_test.go +++ b/pkg/ffapi/handler_test.go @@ -173,7 +173,7 @@ text "200": { Value: &openapi3.Response{ Content: openapi3.Content{ - "text/plain": &openapi3.MediaType{}, + "text/plain": {}, }, }, },