Skip to content

Commit

Permalink
Register separate route for each method.
Browse files Browse the repository at this point in the history
  • Loading branch information
soundvibe committed Nov 12, 2020
1 parent 9b9c6da commit 9af0b72
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 86 deletions.
94 changes: 33 additions & 61 deletions src/query/api/v1/httpd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import (
"fmt"
"net/http"
_ "net/http/pprof" // needed for pprof handler registration
"path"
"sort"
"strings"
"time"

"go.uber.org/zap"
Expand Down Expand Up @@ -324,22 +321,24 @@ func (h *Handler) RegisterRoutes() error {

// Register custom endpoints.
for _, custom := range h.customHandlers {
routeName := routeName(custom.Route(), custom.Methods())
route := h.router.Get(routeName)
var prevHandler http.Handler
if route != nil {
prevHandler = route.GetHandler()
}
customHandler, err := custom.Handler(nativeSourceOpts, prevHandler)
if err != nil {
return fmt.Errorf("failed to register custom handler with path %s: %w",
routeName, err)
}

if route == nil {
wrappedRouteFn(custom.Route(), customHandler, custom.Methods()...)
} else {
route.Handler(wrapped(customHandler))
for _, method := range custom.Methods() {
routeName := routeName(custom.Route(), method)
route := h.router.Get(routeName)
var prevHandler http.Handler
if route != nil {
prevHandler = route.GetHandler()
}
customHandler, err := custom.Handler(nativeSourceOpts, prevHandler)
if err != nil {
return fmt.Errorf("failed to register custom handler with path %s: %w",
routeName, err)
}

if route == nil {
wrappedRouteFn(custom.Route(), customHandler, method)
} else {
route.Handler(wrapped(customHandler))
}
}
}

Expand All @@ -356,53 +355,26 @@ func (h *Handler) addRouteHandlerFn(
handlerFn http.HandlerFunc,
methods ...string,
) {
routeName := routeName(path, methods)
if previousRoute := router.Get(routeName); previousRoute != nil {
h.logger.Warn("route already exists",
zap.String("routeName", routeName))
return
}

route := router.
HandleFunc(path, handlerFn).
Name(routeName)

if len(methods) < 1 {
h.logger.Warn("adding route without methods",
zap.String("routeName", routeName))
return
}

route.Methods(methods...)
}
for _, method := range methods {
routeName := routeName(path, method)
if previousRoute := router.Get(routeName); previousRoute != nil {
h.logger.Warn("route already exists",
zap.String("routeName", routeName))
continue
}

func routeName(p string, methods []string) string {
if len(methods) > 1 {
distinctMethods := distinct(methods)
return path.Join(p, strings.Join(distinctMethods, ":"))
router.
HandleFunc(path, handlerFn).
Name(routeName).
Methods(method)
}

return path.Join(p, strings.Join(methods, ":"))
}

func distinct(arr []string) []string {
if len(arr) <= 1 {
return arr
func routeName(p string, method string) string {
if method == "" {
return p
}

sort.Strings(arr)

var prev string
distinct := arr[:0]
for i, entry := range arr {
if i == 0 || prev != entry {
distinct = append(distinct, entry)
}

prev = entry
}

return distinct
return p + " " + method
}

func (h *Handler) placementOpts() (placement.HandlerOptions, error) {
Expand Down
30 changes: 5 additions & 25 deletions src/query/api/v1/httpd/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,42 +448,22 @@ func TestCustomRoutes(t *testing.T) {
require.Equal(t, res.Code, http.StatusOK)
}

func TestDistinct(t *testing.T) {
assert.Equal(
t,
[]string{"a"},
distinct([]string{"a"}),
)

assert.Equal(
t,
[]string{"a", "b", "c"},
distinct([]string{"c", "a", "b", "c"}),
)

assert.Equal(
t,
[]string{"a", "b", "c"},
distinct([]string{"c", "b", "a"}),
)
}

func TestRouteName(t *testing.T) {
assert.Equal(
t,
"/api/v1/test/GET:POST",
routeName("/api/v1/test", []string{"GET", "POST"}),
"/api/v1/test GET",
routeName("/api/v1/test", "GET"),
)

assert.Equal(
t,
"/api/v1/test",
routeName("/api/v1/test", []string{}),
routeName("/api/v1/test", ""),
)

assert.Equal(
t,
"/api/v1/test/GET:HEAD:POST",
routeName("/api/v1/test", []string{"POST", "GET", "GET", "HEAD"}),
"/api/v1/test POST",
routeName("/api/v1/test", "POST"),
)
}

0 comments on commit 9af0b72

Please sign in to comment.