Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

story(issue-293): rest unable to register endpoint with wildcard segment #294

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
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
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
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