diff --git a/group.go b/group.go index 28ce0dd9..749a5caa 100644 --- a/group.go +++ b/group.go @@ -23,10 +23,12 @@ func (g *Group) Use(middleware ...MiddlewareFunc) { if len(g.middleware) == 0 { return } - // Allow all requests to reach the group as they might get dropped if router - // doesn't find a match, making none of the group middleware process. - g.Any("", NotFoundHandler) - g.Any("/*", NotFoundHandler) + // group level middlewares are different from Echo `Pre` and `Use` middlewares (those are global). Group level middlewares + // are only executed if they are added to the Router with route. + // So we register catch all route (404 is a safe way to emulate route match) for this group and now during routing the + // Router would find route to match our request path and therefore guarantee the middleware(s) will get executed. + g.RouteNotFound("", NotFoundHandler) + g.RouteNotFound("/*", NotFoundHandler) } // CONNECT implements `Echo#CONNECT()` for sub-routes within the Group. diff --git a/group_test.go b/group_test.go index 01c304d0..d22f564b 100644 --- a/group_test.go +++ b/group_test.go @@ -184,3 +184,73 @@ func TestGroup_RouteNotFound(t *testing.T) { }) } } + +func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) { + var testCases = []struct { + name string + givenCustom404 bool + whenURL string + expectBody interface{} + expectCode int + }{ + { + name: "ok, custom 404 handler is called with middleware", + givenCustom404: true, + whenURL: "/group/test3", + expectBody: "GET /group/*", + expectCode: http.StatusNotFound, + }, + { + name: "ok, default group 404 handler is called with middleware", + givenCustom404: false, + whenURL: "/group/test3", + expectBody: "{\"message\":\"Not Found\"}\n", + expectCode: http.StatusNotFound, + }, + { + name: "ok, (no slash) default group 404 handler is called with middleware", + givenCustom404: false, + whenURL: "/group", + expectBody: "{\"message\":\"Not Found\"}\n", + expectCode: http.StatusNotFound, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + okHandler := func(c Context) error { + return c.String(http.StatusOK, c.Request().Method+" "+c.Path()) + } + notFoundHandler := func(c Context) error { + return c.String(http.StatusNotFound, c.Request().Method+" "+c.Path()) + } + + e := New() + e.GET("/test1", okHandler) + e.RouteNotFound("/*", notFoundHandler) + + g := e.Group("/group") + g.GET("/test1", okHandler) + + middlewareCalled := false + g.Use(func(next HandlerFunc) HandlerFunc { + return func(c Context) error { + middlewareCalled = true + return next(c) + } + }) + if tc.givenCustom404 { + g.RouteNotFound("/*", notFoundHandler) + } + + req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil) + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + + assert.True(t, middlewareCalled) + assert.Equal(t, tc.expectCode, rec.Code) + assert.Equal(t, tc.expectBody, rec.Body.String()) + }) + } +}