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

simplify AppModule #6231

Merged
merged 25 commits into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5efee7d
add test of alternative impl
jgimeno May 15, 2020
1319b03
simplify query route too
jgimeno May 15, 2020
c0c1d35
change name querier
jgimeno May 15, 2020
206e08e
simplify register routes
jgimeno May 15, 2020
784b28a
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno May 18, 2020
a3fcbe4
revert change
jgimeno May 18, 2020
90650f8
add route
jgimeno May 18, 2020
87f40ce
add router
jgimeno May 18, 2020
97f3b9c
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno May 27, 2020
8e4b0cb
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno Jun 9, 2020
1114876
first step refactor
jgimeno Jun 9, 2020
726b8cb
refactor
jgimeno Jun 9, 2020
0d7875a
update documentation
jgimeno Jun 9, 2020
8e191e4
update
jgimeno Jun 9, 2020
7411a3c
add format
jgimeno Jun 9, 2020
085d4f5
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno Jun 9, 2020
acbe301
simplify appmodule
jgimeno Jun 10, 2020
01ac041
update changelog
jgimeno Jun 10, 2020
ede7c3c
Merge branch 'jonathan/module-avoid-route-name' of github.com:cosmos/…
jgimeno Jun 10, 2020
6a04b9c
rename vars
jgimeno Jun 10, 2020
9bac536
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno Jun 10, 2020
3da1f32
Merge branch 'master' into jonathan/module-avoid-route-name
jgimeno Jun 10, 2020
cee833d
remove interface
jgimeno Jun 10, 2020
d0d3f76
Merge branch 'master' into jonathan/module-avoid-route-name
fedekunze Jun 10, 2020
a16b885
Update CHANGELOG.md
fedekunze Jun 10, 2020
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ older clients.

### API Breaking Changes

* [\#6231](https://github.com/cosmos/cosmos-sdk/pull/6231) Simplify `AppModule` interface, `Route` and `NewHandler` methods become only `Route`
and returns a new `Route` type.
* [\#6212](https://github.com/cosmos/cosmos-sdk/pull/6212) Remove `Get*` prefixes from key construction functions
* [\#6079](https://github.com/cosmos/cosmos-sdk/pull/6079) Remove `UpgradeOldPrivValFile` (deprecated in Tendermint Core v0.28).
* (modules) [\#5664](https://github.com/cosmos/cosmos-sdk/pull/5664) Remove amino `Codec` from simulation `StoreDecoder`, which now returns a function closure in order to unmarshal the key-value pairs.
Expand Down
44 changes: 28 additions & 16 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,9 @@ func TestCheckTx(t *testing.T) {
anteOpt := func(bapp *BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, counterKey)) }
routerOpt := func(bapp *BaseApp) {
// TODO: can remove this once CheckTx doesnt process msgs.
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
bapp.Router().AddRoute(sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
return &sdk.Result{}, nil
})
}))
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -831,7 +831,8 @@ func TestDeliverTx(t *testing.T) {
// test increments in the handler
deliverKey := []byte("deliver-key")
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
r := sdk.NewRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -884,8 +885,10 @@ func TestMultiMsgDeliverTx(t *testing.T) {
deliverKey := []byte("deliver-key")
deliverKey2 := []byte("deliver-key2")
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
bapp.Router().AddRoute(routeMsgCounter2, handlerMsgCounter(t, capKey1, deliverKey2))
r1 := sdk.NewRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
r2 := sdk.NewRoute(routeMsgCounter2, handlerMsgCounter(t, capKey1, deliverKey2))
bapp.Router().AddRoute(r1)
bapp.Router().AddRoute(r2)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -960,10 +963,11 @@ func TestSimulateTx(t *testing.T) {
}

routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
ctx.GasMeter().ConsumeGas(gasConsumed, "test")
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1024,9 +1028,10 @@ func TestRunInvalidTransaction(t *testing.T) {
})
}
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1149,11 +1154,12 @@ func TestTxGasLimits(t *testing.T) {
}

routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
count := msg.(msgCounter).Counter
ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler")
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1232,11 +1238,12 @@ func TestMaxBlockGasLimits(t *testing.T) {
}

routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
count := msg.(msgCounter).Counter
ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler")
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1317,9 +1324,10 @@ func TestCustomRunTxPanicHandler(t *testing.T) {
})
}
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1356,7 +1364,8 @@ func TestBaseAppAnteHandler(t *testing.T) {

deliverKey := []byte("deliver-key")
routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
r := sdk.NewRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
bapp.Router().AddRoute(r)
}

cdc := codec.New()
Expand Down Expand Up @@ -1451,11 +1460,12 @@ func TestGasConsumptionBadTx(t *testing.T) {
}

routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
count := msg.(msgCounter).Counter
ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler")
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

cdc := codec.New()
Expand Down Expand Up @@ -1504,11 +1514,12 @@ func TestQuery(t *testing.T) {
}

routerOpt := func(bapp *BaseApp) {
bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
store := ctx.KVStore(capKey1)
store.Set(key, value)
return &sdk.Result{}, nil
})
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down Expand Up @@ -1639,8 +1650,8 @@ type testCustomRouter struct {
routes sync.Map
}

func (rtr *testCustomRouter) AddRoute(path string, h sdk.Handler) sdk.Router {
rtr.routes.Store(path, h)
func (rtr *testCustomRouter) AddRoute(route *sdk.Route) sdk.Router {
rtr.routes.Store(route.Path(), route.Handler())
return rtr
}

Expand All @@ -1662,7 +1673,8 @@ func TestWithRouter(t *testing.T) {
deliverKey := []byte("deliver-key")
routerOpt := func(bapp *BaseApp) {
bapp.SetRouter(&testCustomRouter{routes: sync.Map{}})
bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
r := sdk.NewRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey))
bapp.Router().AddRoute(r)
}

app := setupBaseApp(t, anteOpt, routerOpt)
Expand Down
10 changes: 5 additions & 5 deletions baseapp/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ func NewRouter() *Router {

// AddRoute adds a route path to the router with a given handler. The route must
// be alphanumeric.
func (rtr *Router) AddRoute(path string, h sdk.Handler) sdk.Router {
if !sdk.IsAlphaNumeric(path) {
func (rtr *Router) AddRoute(route *sdk.Route) sdk.Router {
if !sdk.IsAlphaNumeric(route.Path()) {
panic("route expressions can only contain alphanumeric characters")
}
if rtr.routes[path] != nil {
panic(fmt.Sprintf("route %s has already been initialized", path))
if rtr.routes[route.Path()] != nil {
panic(fmt.Sprintf("route %s has already been initialized", route.Path()))
}

rtr.routes[path] = h
rtr.routes[route.Path()] = route.Handler()
return rtr
}

Expand Down
6 changes: 3 additions & 3 deletions baseapp/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ func TestRouter(t *testing.T) {

// require panic on invalid route
require.Panics(t, func() {
rtr.AddRoute("*", testHandler)
rtr.AddRoute(sdk.NewRoute("*", testHandler))
})

rtr.AddRoute("testRoute", testHandler)
rtr.AddRoute(sdk.NewRoute("testRoute", testHandler))
h := rtr.Route(sdk.Context{}, "testRoute")
require.NotNil(t, h)

// require panic on duplicate route
require.Panics(t, func() {
rtr.AddRoute("testRoute", testHandler)
rtr.AddRoute(sdk.NewRoute("testRoute", testHandler))
})
}
4 changes: 2 additions & 2 deletions docs/building-modules/handler.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ Let us break it down:

Module `handler`s are typically implemented in a `./handler.go` file inside the module's folder. The
[module manager](./module-manager.md) is used to add the module's `handler`s to the
[application's `router`](../core/baseapp.md#message-routing) via the `NewHandler()` method. Typically,
the manager's `NewHandler()` method simply calls a `NewHandler()` method defined in `handler.go`,
[application's `router`](../core/baseapp.md#message-routing) via the `Route()` method. Typically,
the manager's `Route()` method simply constructs a Route that calls a `NewHandler()` method defined in `handler.go`,
which looks like the following:

```go
Expand Down
5 changes: 2 additions & 3 deletions docs/building-modules/module-manager.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ The `AppModule` interface defines the inter-dependent methods modules need to im
Let us go through the methods of `AppModule`:

- `RegisterInvariants(sdk.InvariantRegistry)`: Registers the [`invariants`](./invariants.md) of the module. If the invariants deviates from its predicted value, the [`InvariantRegistry`](./invariants.md#registry) triggers appropriate logic (most often the chain will be halted).
- `Route()`: Returns the name of the module's route, for [`message`s](./messages-and-queries.md#messages) to be routed to the module by [`baseapp`](../core/baseapp.md#message-routing).
- `NewHandler()`: Returns a [`handler`](./handler.md) given the `Type()` of the `message`, in order to process the `message`.
- `Route()`: Returns the route for [`message`s](./messages-and-queries.md#messages) to be routed to the module by [`baseapp`](../core/baseapp.md#message-routing).
- `QuerierRoute()`: Returns the name of the module's query route, for [`queries`](./messages-and-queries.md#queries) to be routes to the module by [`baseapp`](../core/baseapp.md#query-routing).
- `NewQuerierHandler()`: Returns a [`querier`](./querier.md) given the query `path`, in order to process the `query`.
- `BeginBlock(sdk.Context, abci.RequestBeginBlock)`: This method gives module developers the option to implement logic that is automatically triggered at the beginning of each block. Implement empty if no logic needs to be triggered at the beginning of each block for this module.
Expand All @@ -78,7 +77,7 @@ Let us go through the methods of `AppModule`:

Typically, the various application module interfaces are implemented in a file called `module.go`, located in the module's folder (e.g. `./x/module/module.go`).

Almost every module need to implement the `AppModuleBasic` and `AppModule` interfaces. If the module is only used for genesis, it will implement `AppModuleGenesis` instead of `AppModule`. The concrete type that implements the interface can add parameters that are required for the implementation of the various methods of the interface. For example, the `NewHandler()` function often calls a `NewHandler(k keeper)` function defined in [`handler.go`](./handler.md) and therefore needs to pass the module's [`keeper`](./keeper.md) as parameter.
Almost every module need to implement the `AppModuleBasic` and `AppModule` interfaces. If the module is only used for genesis, it will implement `AppModuleGenesis` instead of `AppModule`. The concrete type that implements the interface can add parameters that are required for the implementation of the various methods of the interface. For example, the `Route()` function often calls a `NewHandler(k keeper)` function defined in [`handler.go`](./handler.md) and therefore needs to pass the module's [`keeper`](./keeper.md) as parameter.

```go
// example
Expand Down
4 changes: 2 additions & 2 deletions server/mock/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func NewApp(rootDir string, logger log.Logger) (abci.Application, error) {

baseApp.SetInitChainer(InitChainer(capKeyMainStore))

// Set a handler Route.
baseApp.Router().AddRoute("kvstore", KVStoreHandler(capKeyMainStore))
// Set a Route.
baseApp.Router().AddRoute(sdk.NewRoute("kvstore", KVStoreHandler(capKeyMainStore)))

// Load latest version.
if err := baseApp.LoadLatestVersion(); err != nil {
Expand Down
18 changes: 2 additions & 16 deletions tests/mocks/types_module_module.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions tests/mocks/types_router.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 4 additions & 8 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ type AppModule interface {
RegisterInvariants(sdk.InvariantRegistry)

// routes
Route() string
NewHandler() sdk.Handler
Route() *sdk.Route
// Deprecated: use RegisterQueryService
QuerierRoute() string
// Deprecated: use RegisterQueryService
Expand Down Expand Up @@ -173,10 +172,7 @@ func NewGenesisOnlyAppModule(amg AppModuleGenesis) AppModule {
func (GenesisOnlyAppModule) RegisterInvariants(_ sdk.InvariantRegistry) {}

// Route empty module message route
func (GenesisOnlyAppModule) Route() string { return "" }

// NewHandler returns an empty module handler
func (GenesisOnlyAppModule) NewHandler() sdk.Handler { return nil }
func (GenesisOnlyAppModule) Route() *sdk.Route { return nil }

// QuerierRoute returns an empty module querier route
func (GenesisOnlyAppModule) QuerierRoute() string { return "" }
Expand Down Expand Up @@ -255,8 +251,8 @@ 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) {
for _, module := range m.Modules {
if module.Route() != "" {
router.AddRoute(module.Route(), module.NewHandler())
if module.Route() != nil {
router.AddRoute(module.Route())
}
if module.QuerierRoute() != "" {
queryRouter.AddRoute(module.QuerierRoute(), module.NewQuerierHandler())
Expand Down
13 changes: 5 additions & 8 deletions types/module/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ func TestGenesisOnlyAppModule(t *testing.T) {
mockInvariantRegistry := mocks.NewMockInvariantRegistry(mockCtrl)
goam := module.NewGenesisOnlyAppModule(mockModule)

require.Empty(t, goam.Route())
require.Nil(t, goam.Route())
require.Empty(t, goam.QuerierRoute())
require.Nil(t, goam.NewHandler())
require.Nil(t, goam.NewQuerierHandler())

// no-op
Expand Down Expand Up @@ -142,12 +141,10 @@ func TestManager_RegisterRoutes(t *testing.T) {

router := mocks.NewMockRouter(mockCtrl)
handler1, handler2 := sdk.Handler(nil), sdk.Handler(nil)
mockAppModule1.EXPECT().Route().Times(2).Return("route1")
mockAppModule2.EXPECT().Route().Times(2).Return("route2")
mockAppModule1.EXPECT().NewHandler().Times(1).Return(handler1)
mockAppModule2.EXPECT().NewHandler().Times(1).Return(handler2)
router.EXPECT().AddRoute(gomock.Eq("route1"), gomock.Eq(handler1)).Times(1)
router.EXPECT().AddRoute(gomock.Eq("route2"), gomock.Eq(handler2)).Times(1)
mockAppModule1.EXPECT().Route().Times(2).Return(sdk.NewRoute("route1", handler1))
mockAppModule2.EXPECT().Route().Times(2).Return(sdk.NewRoute("route2", handler2))
router.EXPECT().AddRoute(gomock.Eq(sdk.NewRoute("route1", handler1))).Times(1)
router.EXPECT().AddRoute(gomock.Eq(sdk.NewRoute("route2", handler2))).Times(1)

queryRouter := mocks.NewMockQueryRouter(mockCtrl)
mockAppModule1.EXPECT().QuerierRoute().Times(2).Return("querierRoute1")
Expand Down
19 changes: 18 additions & 1 deletion types/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,27 @@ var (

// Router provides handlers for each transaction type.
type Router interface {
AddRoute(r string, h Handler) Router
AddRoute(r *Route) Router
Route(ctx Context, path string) Handler
}

type Route struct {
path string
handler Handler
}

func NewRoute(p string, h Handler) *Route {
return &Route{path: p, handler: h}
}

func (r Route) Path() string {
return r.path
}

func (r Route) Handler() Handler {
return r.handler
}

// QueryRouter provides queryables for each query path.
type QueryRouter interface {
AddRoute(r string, h Querier) QueryRouter
Expand Down
Loading