Skip to content

Commit

Permalink
move locking and count decrements to deleteRoute and add test that ca…
Browse files Browse the repository at this point in the history
…lls RemoveRoute
  • Loading branch information
ckoch786 committed Dec 8, 2024
1 parent 04764d8 commit 590152e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 18 deletions.
27 changes: 14 additions & 13 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,16 +305,9 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler
}

// Duplicate Route Handling
app.mutex.Lock()
if app.routeExists(method, pathRaw) {
// TODO does RemoveRoute also need these operations?
//Decrement global handler count
atomic.AddUint32(&app.handlersCount, ^uint32(len(handlers)-1)) //nolint:gosec // Not a concern
// Decrement global route position
atomic.AddUint32(&app.routesCount, ^uint32(0))
app.deleteRoute(pathRaw, []string{method})
app.deleteRoute(pathRaw, []string{method}, len(handlers))
}
app.mutex.Unlock()

// is mounted app
isMount := group != nil && group.app != app
Expand Down Expand Up @@ -398,14 +391,22 @@ func (app *App) routeExists(method string, pathRaw string) bool {

// RemoveRoute is used to remove a route from the stack
// You should call RebuildTree after using this to ensure consistency of the tree
// TODO write tests for this that explicitally delete a route right after adding it
func (app *App) RemoveRoute(path string, methods ...string) {
func (app *App) RemoveRoute(path string, middlewareCount int, methods ...string) {
app.deleteRoute(path, methods, middlewareCount)
}

func (app *App) deleteRoute(path string, methods []string, middlewareCount int) {
app.mutex.Lock()
defer app.mutex.Unlock()
app.deleteRoute(path, methods)
}

func (app *App) deleteRoute(path string, methods []string) {
if middlewareCount == 0 {
middlewareCount++
}

//Decrement global handler count

Check failure on line 406 in router.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
atomic.AddUint32(&app.handlersCount, ^uint32(middlewareCount-1)) //nolint:gosec // Not a concern
// Decrement global route position
atomic.AddUint32(&app.routesCount, ^uint32(0))
for _, method := range methods {
// Uppercase HTTP methods
method = utils.ToUpper(method)
Expand Down
28 changes: 23 additions & 5 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,23 @@ func Test_App_Remove_Route(t *testing.T) {
t.Parallel()
app := New()

app.Get("/test", func(c Ctx) error {
return c.SendStatus(http.StatusOK)
})

app.RemoveRoute("/test", 0, http.MethodGet)
app.RebuildTree()

verifyRequest(t, app, "/test", http.StatusNotFound)
require.Equal(t, uint32(0), app.handlersCount)
require.Equal(t, uint32(0), app.routesCount)
verifyRouteHandlerCounts(t, app, 0)
}

func Test_App_Route_Registration_Prevent_Duplicate(t *testing.T) {
t.Parallel()
app := New()

registerTreeManipulationRoutes(app)
registerTreeManipulationRoutes(app)

Expand All @@ -406,10 +423,10 @@ func Test_App_Remove_Route(t *testing.T) {
require.Equal(t, uint32(2), app.handlersCount)
require.Equal(t, uint32(2), app.routesCount)

verifyRouteHandlerCounts(t, app)
verifyRouteHandlerCounts(t, app, 1)
}

func Test_App_Remove_Route_With_Middleware(t *testing.T) {
func Test_Route_Registration_Prevent_Duplicate_With_Middleware(t *testing.T) {
t.Parallel()
app := New()

Expand All @@ -436,10 +453,11 @@ func Test_App_Remove_Route_With_Middleware(t *testing.T) {
require.Equal(t, uint32(3), app.handlersCount)
require.Equal(t, uint32(2), app.routesCount)

verifyRouteHandlerCounts(t, app)
verifyRouteHandlerCounts(t, app, 1)
}

func registerTreeManipulationRoutes(app *App, middleware ...func(Ctx) error) {

Check failure on line 459 in router_test.go

View workflow job for this annotation

GitHub Actions / lint

empty-lines: extra empty line at the start of a block (revive)

Check failure on line 459 in router_test.go

View workflow job for this annotation

GitHub Actions / lint

empty-lines: extra empty line at the end of a block (revive)

Check failure on line 459 in router_test.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary leading newline (whitespace)

app.Get("/test", func(c Ctx) error {
app.Get("/dynamically-defined", func(c Ctx) error {
return c.SendStatus(http.StatusOK)
Expand All @@ -463,7 +481,7 @@ func verifyRequest(tb testing.TB, app *App, path string, expectedStatus int) *ht
}

// this is taken from listen.go's printRoutesMessage app method
func verifyRouteHandlerCounts(tb testing.TB, app *App) {
func verifyRouteHandlerCounts(tb testing.TB, app *App, expectedRoutesCount int) {
tb.Helper()

var routes []RouteMessage
Expand All @@ -485,7 +503,7 @@ func verifyRouteHandlerCounts(tb testing.TB, app *App) {
}

for _, route := range routes {
require.Equal(tb, 1, strings.Count(route.handlers, " "))
require.Equal(tb, expectedRoutesCount, strings.Count(route.handlers, " "))
}
}

Expand Down

0 comments on commit 590152e

Please sign in to comment.