From a256a7c444c31f08cbab140c15e244cee3f467f1 Mon Sep 17 00:00:00 2001 From: Ewen Quimerc'h <46993939+EwenQuim@users.noreply.github.com> Date: Sat, 14 Dec 2024 16:46:58 +0100 Subject: [PATCH] Description option : override + default description contains the list of middlewares (#275) * Refactored the Description/AddDescription to a clearer system * New more explicit OverrideDescription option * Tests for the description option with middlewares * Make tag order deterministic * Update option.go Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com> * Makes overrideDescription private in BaseRoute * Moved the operation ID generation to a method on the BaseRoute struct. * Generate description is in OpenAPI registration, not in mux registration --------- Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com> --- examples/full-app-gourmet/views/views.go | 4 +- examples/petstore/controllers/middlewares.go | 11 ++++ examples/petstore/controllers/pets.go | 9 ++- .../lib/testdata/doc/openapi.golden.json | 28 ++++----- mux.go | 63 ++++++++++++------- mux_test.go | 6 +- openapi.go | 21 ++++++- openapi_operations_test.go | 2 +- option.go | 16 ++++- option/option.go | 7 +++ option_test.go | 53 ++++++++++++++-- 11 files changed, 167 insertions(+), 53 deletions(-) create mode 100644 examples/petstore/controllers/middlewares.go diff --git a/examples/full-app-gourmet/views/views.go b/examples/full-app-gourmet/views/views.go index d583f907..50a3e793 100644 --- a/examples/full-app-gourmet/views/views.go +++ b/examples/full-app-gourmet/views/views.go @@ -67,6 +67,8 @@ func (rs Resource) Routes(s *fuego.Server) { fuego.Get(adminRoutes, "/ingredients/create", rs.adminIngredientCreationPage) fuego.All(adminRoutes, "/ingredients/{id}", rs.adminOneIngredient) - fuego.Post(adminRoutes, "/ingredients/new", rs.adminCreateIngredient) + fuego.Post(adminRoutes, "/ingredients/new", rs.adminCreateIngredient, + option.Description("Create a new ingredient"), + ) fuego.Get(adminRoutes, "/users", rs.adminRecipes) } diff --git a/examples/petstore/controllers/middlewares.go b/examples/petstore/controllers/middlewares.go new file mode 100644 index 00000000..99deb7be --- /dev/null +++ b/examples/petstore/controllers/middlewares.go @@ -0,0 +1,11 @@ +package controller + +import "net/http" + +func dummyMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // do something before + next.ServeHTTP(w, r) + // do something after + }) +} diff --git a/examples/petstore/controllers/pets.go b/examples/petstore/controllers/pets.go index 72a2d0d2..734a8031 100644 --- a/examples/petstore/controllers/pets.go +++ b/examples/petstore/controllers/pets.go @@ -47,13 +47,18 @@ func (rs PetsResources) Routes(s *fuego.Server) { option.Description("Get all pets"), ) - fuego.Get(petsGroup, "/by-age", rs.getAllPetsByAge, option.Description("Returns an array of pets grouped by age")) + fuego.Get(petsGroup, "/by-age", rs.getAllPetsByAge, + option.Description("Returns an array of pets grouped by age"), + option.Middleware(dummyMiddleware), + ) fuego.Post(petsGroup, "/", rs.postPets, option.DefaultStatusCode(201), option.AddResponse(409, "Conflict: Pet with the same name already exists", fuego.Response{Type: PetsError{}}), ) fuego.Get(petsGroup, "/{id}", rs.getPets, + option.OverrideDescription("Replace description with this sentence."), + option.OperationID("getPet"), option.Path("id", "Pet ID", param.Example("example", "123")), ) fuego.Get(petsGroup, "/by-name/{name...}", rs.getPetByName) @@ -77,14 +82,12 @@ func (rs PetsResources) Routes(s *fuego.Server) { if err := json.NewEncoder(w).Encode(pets); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) } - }, option.AddResponse(http.StatusOK, "all the pets", fuego.Response{ Type: []models.Pets{}, ContentTypes: []string{"application/json"}, }, )) - } func (rs PetsResources) getAllPets(c fuego.ContextNoBody) ([]models.Pets, error) { diff --git a/examples/petstore/lib/testdata/doc/openapi.golden.json b/examples/petstore/lib/testdata/doc/openapi.golden.json index beb1b056..167c6f2e 100644 --- a/examples/petstore/lib/testdata/doc/openapi.golden.json +++ b/examples/petstore/lib/testdata/doc/openapi.golden.json @@ -152,7 +152,7 @@ "paths": { "/pets/": { "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.filterPets`\n\n---\n\nFilter pets", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.filterPets`\n\n---\n\nFilter pets", "operationId": "GET_/pets/", "parameters": [ { @@ -302,7 +302,7 @@ ] }, "post": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.postPets`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.postPets`\n\n---\n\n", "operationId": "POST_/pets/", "parameters": [ { @@ -395,7 +395,7 @@ }, "/pets/all": { "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getAllPets`\n\n---\n\nGet all pets", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getAllPets`\n\n---\n\nGet all pets", "operationId": "GET_/pets/all", "parameters": [ { @@ -525,7 +525,7 @@ }, "/pets/by-age": { "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getAllPetsByAge`\n\n---\n\nReturns an array of pets grouped by age", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getAllPetsByAge`\n\n#### Middlewares:\n\n- `github.com/go-fuego/fuego/examples/petstore/controllers.dummyMiddleware`\n\n---\n\nReturns an array of pets grouped by age", "operationId": "GET_/pets/by-age", "parameters": [ { @@ -604,7 +604,7 @@ }, "/pets/by-name/{name...}": { "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getPetByName`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getPetByName`\n\n---\n\n", "operationId": "GET_/pets/by-name/:name...", "parameters": [ { @@ -680,7 +680,7 @@ }, "/pets/std/all": { "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.Routes.func1`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.Routes.func1`\n\n---\n\n", "operationId": "GET_/pets/std/all", "parameters": [ { @@ -739,7 +739,7 @@ }, "/pets/{id}": { "delete": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.deletePets`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.deletePets`\n\n---\n\n", "operationId": "DELETE_/pets/:id", "parameters": [ { @@ -812,8 +812,8 @@ ] }, "get": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.getPets`\n\n---\n\n", - "operationId": "GET_/pets/:id", + "description": "Replace description with this sentence.", + "operationId": "getPet", "parameters": [ { "in": "header", @@ -891,7 +891,7 @@ ] }, "put": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.putPets`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.putPets`\n\n---\n\n", "operationId": "PUT_/pets/:id", "parameters": [ { @@ -977,7 +977,7 @@ }, "/pets/{id}/json": { "put": { - "description": "controller: `github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.putPets`\n\n---\n\n", + "description": "#### Controller: \n\n`github.com/go-fuego/fuego/examples/petstore/controllers.PetsResources.putPets`\n\n---\n\n", "operationId": "PUT_/pets/:id/json", "parameters": [ { @@ -1070,13 +1070,13 @@ ], "tags": [ { - "name": "pets" + "name": "my-tag" }, { - "name": "std" + "name": "pets" }, { - "name": "my-tag" + "name": "std" } ] } \ No newline at end of file diff --git a/mux.go b/mux.go index 2cbc51b7..7b348708 100644 --- a/mux.go +++ b/mux.go @@ -60,6 +60,19 @@ type BaseRoute struct { Hidden bool // If true, the route will not be documented in the OpenAPI spec DefaultStatusCode int // Default status code for the response OpenAPI *OpenAPI // Ref to the whole OpenAPI spec + + overrideDescription bool // Override the default description +} + +func (r *BaseRoute) GenerateDefaultDescription() { + if r.overrideDescription { + return + } + r.Operation.Description = DefaultDescription(r.FullName, r.Middlewares) + r.Operation.Description +} + +func (r *BaseRoute) GenerateDefaultOperationID() { + r.Operation.OperationID = r.Method + "_" + strings.ReplaceAll(strings.ReplaceAll(r.Path, "{", ":"), "}", "") } // Capture all methods (GET, POST, PUT, PATCH, DELETE) and register a controller. @@ -93,41 +106,26 @@ func Register[T, B any](s *Server, route Route[T, B], controller http.Handler, o o(&route.BaseRoute) } route.Handler = controller + route.Path = s.basePath + route.Path - fullPath := s.basePath + route.Path + fullPath := route.Path if route.Method != "" { fullPath = route.Method + " " + fullPath } slog.Debug("registering controller " + fullPath) - allMiddlewares := append(s.middlewares, route.Middlewares...) - s.Mux.Handle(fullPath, withMiddlewares(route.Handler, allMiddlewares...)) + route.Middlewares = append(s.middlewares, route.Middlewares...) + s.Mux.Handle(fullPath, withMiddlewares(route.Handler, route.Middlewares...)) if s.DisableOpenapi || route.Hidden || route.Method == "" { return &route } - route.Path = s.basePath + route.Path - err := route.RegisterOpenAPIOperation(s.OpenAPI) if err != nil { slog.Warn("error documenting openapi operation", "error", err) } - if route.FullName == "" { - route.FullName = route.Path - } - - if route.Operation.Summary == "" { - route.Operation.Summary = route.NameFromNamespace(camelToHuman) - } - - route.Operation.Description = "controller: `" + route.FullName + "`\n\n---\n\n" + route.Operation.Description - - if route.Operation.OperationID == "" { - route.Operation.OperationID = route.Method + "_" + strings.ReplaceAll(strings.ReplaceAll(route.Path, "{", ":"), "}", "") - } - return &route } @@ -180,7 +178,7 @@ func registerFuegoController[T, B any, Contexted ctx[B]](s *Server, method, path Path: path, Params: make(map[string]OpenAPIParam), FullName: FuncName(controller), - Operation: openapi3.NewOperation(), + Operation: &openapi3.Operation{}, OpenAPI: s.OpenAPI, } @@ -199,8 +197,10 @@ func registerStdController(s *Server, method, path string, controller func(http. route := BaseRoute{ Method: method, Path: path, + Params: make(map[string]OpenAPIParam), FullName: FuncName(controller), - Operation: openapi3.NewOperation(), + Operation: &openapi3.Operation{}, + Handler: http.HandlerFunc(controller), OpenAPI: s.OpenAPI, } @@ -253,3 +253,24 @@ func camelToHuman(s string) string { } return result.String() } + +// DefaultDescription returns a default .md description for a controller +func DefaultDescription[T any](handler string, middlewares []T) string { + description := "#### Controller: \n\n`" + + handler + "`" + + if len(middlewares) > 0 { + description += "\n\n#### Middlewares:\n" + + for i, fn := range middlewares { + description += "\n- `" + FuncName(fn) + "`" + + if i == 4 { + description += "\n- more middleware..." + break + } + } + } + + return description + "\n\n---\n\n" +} diff --git a/mux_test.go b/mux_test.go index 0d73a66e..cbd719f3 100644 --- a/mux_test.go +++ b/mux_test.go @@ -402,21 +402,21 @@ func TestRegister(t *testing.T) { Operation: &openapi3.Operation{ Tags: []string{"my-tag"}, Summary: "my-summary", - Description: "my-description", + Description: "my-description\n", OperationID: "my-operation-id", }, }, }, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), OptionOperationID("new-operation-id"), OptionSummary("new-summary"), - OptionDescription("new-description"), + OptionOverrideDescription("new-description"), OptionTags("new-tag"), ) require.NotNil(t, route) require.Equal(t, []string{"my-tag", "new-tag"}, route.Operation.Tags) require.Equal(t, "new-summary", route.Operation.Summary) - require.Equal(t, "controller: `/test`\n\n---\n\nnew-description", route.Operation.Description) + require.Equal(t, "new-description", route.Operation.Description) require.Equal(t, "new-operation-id", route.Operation.OperationID) }) } diff --git a/openapi.go b/openapi.go index 83cbd280..7611efa5 100644 --- a/openapi.go +++ b/openapi.go @@ -89,6 +89,11 @@ func declareAllTagsFromOperations(s *Server) { } } } + + // Make sure tags are sorted + slices.SortFunc(s.OpenAPI.Description().Tags, func(a, b *openapi3.Tag) int { + return strings.Compare(a.Name, b.Name) + }) } // OutputOpenAPISpec takes the OpenAPI spec and outputs it to a JSON file and/or serves it on a URL. @@ -209,6 +214,20 @@ func RegisterOpenAPIOperation[T, B any](openapi *OpenAPI, route Route[T, B]) (*o route.Operation = openapi3.NewOperation() } + if route.FullName == "" { + route.FullName = route.Path + } + + route.GenerateDefaultDescription() + + if route.Operation.Summary == "" { + route.Operation.Summary = route.NameFromNamespace(camelToHuman) + } + + if route.Operation.OperationID == "" { + route.GenerateDefaultOperationID() + } + // Request Body if route.Operation.RequestBody == nil { bodyTag := SchemaTagFromType(openapi, *new(B)) @@ -263,7 +282,7 @@ func RegisterOpenAPIOperation[T, B any](openapi *OpenAPI, route Route[T, B]) (*o for _, params := range route.Operation.Parameters { if params.Value.In == "path" { if !strings.Contains(route.Path, "{"+params.Value.Name) { - return nil, fmt.Errorf("path parameter '%s' is not declared in the path", params.Value.Name) + panic(fmt.Errorf("path parameter '%s' is not declared in the path", params.Value.Name)) } } } diff --git a/openapi_operations_test.go b/openapi_operations_test.go index c03e129f..ec88d614 100644 --- a/openapi_operations_test.go +++ b/openapi_operations_test.go @@ -16,7 +16,7 @@ func TestTags(t *testing.T) { ) require.Equal(t, []string{"my-tag"}, route.Operation.Tags) - require.Equal(t, "controller: `github.com/go-fuego/fuego.testController`\n\n---\n\nmy description", route.Operation.Description) + require.Equal(t, "#### Controller: \n\n`github.com/go-fuego/fuego.testController`\n\n---\n\nmy description", route.Operation.Description) require.Equal(t, "my summary", route.Operation.Summary) require.Equal(t, true, route.Operation.Deprecated) } diff --git a/option.go b/option.go index 28e51258..5695968e 100644 --- a/option.go +++ b/option.go @@ -258,10 +258,10 @@ func OptionSummary(summary string) func(*BaseRoute) { } } -// Description sets the description to the route. +// Description adds a description to the route. // By default, the description is set by Fuego with some info, // like the controller function name and the package name. -// If you want to add a description, please use [AddDescription] instead. +// If you want to override Fuego's description, please use [OptionOverrideDescription] instead. func OptionDescription(description string) func(*BaseRoute) { return func(r *BaseRoute) { r.Operation.Description = description @@ -273,7 +273,17 @@ func OptionDescription(description string) func(*BaseRoute) { // like the controller function name and the package name. func OptionAddDescription(description string) func(*BaseRoute) { return func(r *BaseRoute) { - r.Operation.Description += "\n\n" + description + r.Operation.Description += description + } +} + +// OptionOverrideDescription overrides the default description set by Fuego. +// By default, the description is set by Fuego with some info, +// like the controller function name and the package name. +func OptionOverrideDescription(description string) func(*BaseRoute) { + return func(r *BaseRoute) { + r.overrideDescription = true + r.Operation.Description = description } } diff --git a/option/option.go b/option/option.go index c3ade403..3dd1db8e 100644 --- a/option/option.go +++ b/option/option.go @@ -105,8 +105,15 @@ var Description = fuego.OptionDescription // AddDescription adds a description to the route. // By default, the description is set by Fuego with some info, // like the controller function name and the package name. +// +// Deprecated: Use [Description] instead. var AddDescription = fuego.OptionAddDescription +// OverrideDescription overrides the default description set by Fuego. +// By default, the description is set by Fuego with some info, +// like the controller function name and the package name. +var OverrideDescription = fuego.OptionOverrideDescription + // Security configures security requirements to the route. // // Single Scheme (AND Logic): diff --git a/option_test.go b/option_test.go index 382becde..fecd57df 100644 --- a/option_test.go +++ b/option_test.go @@ -11,6 +11,7 @@ import ( "github.com/thejerf/slogassert" "github.com/go-fuego/fuego" + "github.com/go-fuego/fuego/option" "github.com/go-fuego/fuego/param" ) @@ -239,7 +240,7 @@ func TestOpenAPI(t *testing.T) { ) require.Equal(t, "test summary", route.Operation.Summary) - require.Equal(t, "controller: `github.com/go-fuego/fuego_test.helloWorld`\n\n---\n\ntest description", route.Operation.Description) + require.Equal(t, "#### Controller: \n\n`github.com/go-fuego/fuego_test.helloWorld`\n\n---\n\ntest description", route.Operation.Description) require.Equal(t, []string{"first-tag", "second-tag"}, route.Operation.Tags) require.True(t, route.Operation.Deprecated) }) @@ -778,16 +779,56 @@ func TestSecurity(t *testing.T) { }) } -func TestOptionAddDescription(t *testing.T) { - t.Run("Declare a description for the route with multiple descriptions", func(t *testing.T) { +func TestOptionDescription(t *testing.T) { + t.Run("Declare a description for the route", func(t *testing.T) { s := fuego.NewServer() route := fuego.Get(s, "/test", helloWorld, - fuego.OptionDescription("test description"), - fuego.OptionAddDescription("another description"), + option.Description("test description"), + ) + + require.Equal(t, "#### Controller: \n\n`github.com/go-fuego/fuego_test.helloWorld`\n\n---\n\ntest description", route.Operation.Description) + }) + + t.Run("Override Fuego's description for the route", func(t *testing.T) { + s := fuego.NewServer() + + route := fuego.Get(s, "/test", helloWorld, + option.OverrideDescription("another description"), + ) + + require.Equal(t, "another description", route.Operation.Description) + }) + + t.Run("Add description to the route, route middleware is included", func(t *testing.T) { + s := fuego.NewServer() + + route := fuego.Get(s, "/test", helloWorld, + option.Middleware(dummyMiddleware), + option.Description("another description"), + ) + + require.Equal(t, "#### Controller: \n\n`github.com/go-fuego/fuego_test.helloWorld`\n\n#### Middlewares:\n\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n\n---\n\nanother description", route.Operation.Description) + }) + + t.Run("Add description to the route, route middleware is included", func(t *testing.T) { + s := fuego.NewServer() + + fuego.Use(s, dummyMiddleware) + + group := fuego.Group(s, "/group", option.Middleware(dummyMiddleware)) + + fuego.Use(group, dummyMiddleware) + + route := fuego.Get(s, "/test", helloWorld, + option.Middleware(dummyMiddleware), + option.Description("another description"), + option.Middleware(dummyMiddleware), // After the description + option.Middleware(dummyMiddleware), // 6th middleware + option.Middleware(dummyMiddleware), // 7th middleware, should not be included ) - require.Equal(t, "controller: `github.com/go-fuego/fuego_test.helloWorld`\n\n---\n\ntest description\n\nanother description", route.Operation.Description) + require.Equal(t, "#### Controller: \n\n`github.com/go-fuego/fuego_test.helloWorld`\n\n#### Middlewares:\n\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n- `github.com/go-fuego/fuego_test.dummyMiddleware`\n- more middleware...\n\n---\n\nanother description", route.Operation.Description) }) }