Skip to content

Commit

Permalink
story(issue-293): rest unable to register endpoint with wildcard segm…
Browse files Browse the repository at this point in the history 
…ent (#294)

* refactor(issue-293): only register endpoints in Run to avoid panic

* test(issue-293): bug replicated with test case

* fix(issue-293): remove ... wildcard before registering open api operation

* chore(docs): updated coverage badge.

---------

Co-authored-by: GitHub Action <[email protected]>
  • Loading branch information
@Zaba505 @actions-user
Zaba505 and actions-user authored Sep 29, 2024
1 parent 265fc29 commit 53d7646
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 83 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[![Mentioned in Awesome Go](https://awesome.re/mentioned-badge.svg)](https://github.com/avelino/awesome-go)
[![Go Reference](https://pkg.go.dev/badge/github.com/z5labs/bedrock.svg)](https://pkg.go.dev/github.com/z5labs/bedrock)
[![Go Report Card](https://goreportcard.com/badge/github.com/z5labs/bedrock)](https://goreportcard.com/report/github.com/z5labs/bedrock)
![Coverage](https://img.shields.io/badge/Coverage-97.2%25-brightgreen)
![Coverage](https://img.shields.io/badge/Coverage-97.0%25-brightgreen)
[![build](https://github.com/z5labs/bedrock/actions/workflows/build.yaml/badge.svg)](https://github.com/z5labs/bedrock/actions/workflows/build.yaml)

**bedrock provides a minimal, modular and composable foundation for
Expand Down
120 changes: 82 additions & 38 deletions rest/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ func Listener(ls net.Listener) Option {
// meant for serving the OpenAPI schema.
func OpenApiEndpoint(method, pattern string, f func(*openapi3.Spec) http.Handler) Option {
return func(a *App) {
a.mux.Handle(fmt.Sprintf("%s %s", method, pattern), f(a.spec))
a.openApiEndpoint = func(mux *http.ServeMux) {
mux.Handle(fmt.Sprintf("%s %s", method, pattern), f(a.spec))
}
}
}

Expand Down Expand Up @@ -83,48 +85,32 @@ type Operation interface {
OpenApi() openapi3.Operation
}

type endpoint struct {
method string
pattern string
op Operation
}

// Endpoint registers the [Operation] with both
// the App wide OpenAPI spec and the App wide HTTP server.
//
// "/" is always treated as "/{$}" because it would otherwise
// match too broadly and cause conflicts with other paths.
func Endpoint(method, pattern string, op Operation) Option {
return func(app *App) {
// Per the net/http.ServeMux docs, https://pkg.go.dev/net/http#ServeMux:
//
// The special wildcard {$} matches only the end of the URL.
// For example, the pattern "/{$}" matches only the path "/",
// whereas the pattern "/" matches every path.
//
// This means that when registering the pattern with the OpenAPI spec
// the {$} needs to be stripped because OpenAPI will believe it's
// an actual path parameter.
trimmedPattern := strings.TrimSuffix(pattern, "{$}")
err := app.spec.AddOperation(method, trimmedPattern, op.OpenApi())
if err != nil {
panic(err)
}

// enforce strict matching for top-level path
// otherwise "/" would match too broadly and http.ServeMux
// will panic when other paths are registered e.g. /openapi.json
if pattern == "/" {
pattern = "/{$}"
}
app.pathMethods[pattern] = append(app.pathMethods[pattern], method)

app.mux.Handle(
fmt.Sprintf("%s %s", method, pattern),
otelhttp.WithRouteTag(trimmedPattern, op),
)
app.endpoints = append(app.endpoints, endpoint{
method: method,
pattern: pattern,
op: op,
})
}
}

// NotFoundHandler will register the given [http.Handler] to handle
// any HTTP requests that do not match any other method-pattern combinations.
func NotFoundHandler(h http.Handler) Option {
return func(app *App) {
app.mux.Handle("/{path...}", h)
app.notFoundHandler = h
}
}

Expand Down Expand Up @@ -161,8 +147,12 @@ func Version(s string) Option {
type App struct {
ls net.Listener

spec *openapi3.Spec
mux *http.ServeMux
spec *openapi3.Spec
endpoints []endpoint

openApiEndpoint func(*http.ServeMux)

notFoundHandler http.Handler

pathMethods map[string][]string
methodNotAllowedHandler http.Handler
Expand All @@ -176,9 +166,9 @@ func NewApp(opts ...Option) *App {
spec: &openapi3.Spec{
Openapi: "3.0.3",
},
mux: http.NewServeMux(),
pathMethods: make(map[string][]string),
listen: net.Listen,
pathMethods: make(map[string][]string),
listen: net.Listen,
openApiEndpoint: func(sm *http.ServeMux) {},
}
for _, opt := range opts {
opt(app)
Expand All @@ -193,11 +183,23 @@ func (app *App) Run(ctx context.Context) error {
return err
}

app.registerMethodNotAllowedHandler()
mux := http.NewServeMux()
app.openApiEndpoint(mux)

err = app.registerEndpoints(mux)
if err != nil {
return err
}

if app.notFoundHandler != nil {
mux.Handle("/{path...}", app.notFoundHandler)
}

app.registerMethodNotAllowedHandler(mux)

httpServer := &http.Server{
Handler: otelhttp.NewHandler(
app.mux,
mux,
"server",
otelhttp.WithMessageEvents(otelhttp.ReadEvents, otelhttp.WriteEvents),
),
Expand Down Expand Up @@ -226,7 +228,49 @@ func (app *App) listener() (net.Listener, error) {
return app.listen("tcp", ":80")
}

func (app *App) registerMethodNotAllowedHandler() {
func (app *App) registerEndpoints(mux *http.ServeMux) error {
for _, e := range app.endpoints {
// Per the net/http.ServeMux docs, https://pkg.go.dev/net/http#ServeMux:
//
// The special wildcard {$} matches only the end of the URL.
// For example, the pattern "/{$}" matches only the path "/",
// whereas the pattern "/" matches every path.
//
// This means that when registering the pattern with the OpenAPI spec
// the {$} needs to be stripped because OpenAPI will believe it's
// an actual path parameter.
trimmedPattern := strings.TrimSuffix(e.pattern, "{$}")

// Per the net/http.ServeMux docs, https://pkg.go.dev/net/http#ServeMux:
//
// A path can include wildcard segments of the form {NAME} or {NAME...}.
//
// The '...' wildcard has no equivalent in OpenAPI so we must remove it
// before registering the OpenAPI operation with the spec.
trimmedPattern = strings.ReplaceAll(trimmedPattern, "...", "")

err := app.spec.AddOperation(e.method, trimmedPattern, e.op.OpenApi())
if err != nil {
return err
}

// enforce strict matching for top-level path
// otherwise "/" would match too broadly and http.ServeMux
// will panic when other paths are registered e.g. /openapi.json
if e.pattern == "/" {
e.pattern = "/{$}"
}
app.pathMethods[e.pattern] = append(app.pathMethods[e.pattern], e.method)

mux.Handle(
fmt.Sprintf("%s %s", e.method, e.pattern),
otelhttp.WithRouteTag(trimmedPattern, e.op),
)
}
return nil
}

func (app *App) registerMethodNotAllowedHandler(mux *http.ServeMux) {
if app.methodNotAllowedHandler == nil {
return
}
Expand All @@ -246,7 +290,7 @@ func (app *App) registerMethodNotAllowedHandler() {
for path, methods := range app.pathMethods {
unsupportedMethods := diffSets(supportedMethods, methods)
for _, method := range unsupportedMethods {
app.mux.Handle(fmt.Sprintf("%s %s", method, path), app.methodNotAllowedHandler)
mux.Handle(fmt.Sprintf("%s %s", method, path), app.methodNotAllowedHandler)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions rest/rest_example_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"net/http"
"strings"

"github.com/z5labs/bedrock/rest/endpoint"
re "github.com/z5labs/bedrock/rest/endpoint"
"golang.org/x/sync/errgroup"
)

Expand Down Expand Up @@ -62,7 +62,7 @@ func Example() {
Endpoint(
http.MethodPost,
"/",
endpoint.NewOperation(
re.NewOperation(
echoService{},
),
),
Expand Down
Loading

0 comments on commit 53d7646

Please sign in to comment.