From e0e16f62f938f2e72c6e63a5f856c8a8535c293f Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 29 Oct 2020 23:01:09 +0100 Subject: [PATCH] Remove duplicated Module.Route calls (#7716) * Remove duplicated Module.Route calls * make a proper test for registering empty route Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- types/module/module.go | 8 ++++---- types/module/module_test.go | 18 +++++++----------- types/router.go | 4 ++-- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index 6b678745e2..c58555a980 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -277,11 +277,11 @@ func (m *Manager) RegisterInvariants(ir sdk.InvariantRegistry) { // RegisterRoutes registers all module routes and module querier routes func (m *Manager) RegisterRoutes(router sdk.Router, queryRouter sdk.QueryRouter, legacyQuerierCdc *codec.LegacyAmino) { for _, module := range m.Modules { - if !module.Route().Empty() { - router.AddRoute(module.Route()) + if r := module.Route(); !r.Empty() { + router.AddRoute(r) } - if module.QuerierRoute() != "" { - queryRouter.AddRoute(module.QuerierRoute(), module.LegacyQuerierHandler(legacyQuerierCdc)) + if r := module.QuerierRoute(); r != "" { + queryRouter.AddRoute(r, module.LegacyQuerierHandler(legacyQuerierCdc)) } } } diff --git a/types/module/module_test.go b/types/module/module_test.go index cc331cf58f..630c576192 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -147,19 +147,15 @@ func TestManager_RegisterRoutes(t *testing.T) { require.Equal(t, 2, len(mm.Modules)) router := mocks.NewMockRouter(mockCtrl) - handler1, handler2 := sdk.Handler(func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { - return nil, nil - }), sdk.Handler(func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { - return nil, nil - }) - route1 := sdk.NewRoute("route1", handler1) - route2 := sdk.NewRoute("route2", handler2) - mockAppModule1.EXPECT().Route().Times(2).Return(route1) - mockAppModule2.EXPECT().Route().Times(2).Return(route2) - router.EXPECT().AddRoute(gomock.Any()).Times(2) // Use of Any due to limitations to compare Functions as the sdk.Handler + noopHandler := sdk.Handler(func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { return nil, nil }) + route1 := sdk.NewRoute("route1", noopHandler) + route2 := sdk.NewRoute("", noopHandler) + mockAppModule1.EXPECT().Route().Times(1).Return(route1) + mockAppModule2.EXPECT().Route().Times(1).Return(route2) + router.EXPECT().AddRoute(gomock.Any()).Times(1) // Use of Any due to limitations to compare Functions as the sdk.Handler queryRouter := mocks.NewMockQueryRouter(mockCtrl) - mockAppModule1.EXPECT().QuerierRoute().Times(2).Return("querierRoute1") + mockAppModule1.EXPECT().QuerierRoute().Times(1).Return("querierRoute1") mockAppModule2.EXPECT().QuerierRoute().Times(1).Return("") handler3 := sdk.Querier(nil) amino := codec.NewLegacyAmino() diff --git a/types/router.go b/types/router.go index 27c2127d5f..9dab11f606 100644 --- a/types/router.go +++ b/types/router.go @@ -40,7 +40,7 @@ type Route struct { // NewRoute returns an instance of Route. func NewRoute(p string, h Handler) Route { - return Route{path: p, handler: h} + return Route{path: strings.TrimSpace(p), handler: h} } // Path returns the path the route has assigned. @@ -55,7 +55,7 @@ func (r Route) Handler() Handler { // Empty returns true only if both handler and path are not empty. func (r Route) Empty() bool { - return r.handler == nil || r.path == strings.TrimSpace("") + return r.handler == nil || r.path == "" } // QueryRouter provides queryables for each query path.