-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
docs: demonstrate how to define hooks using depinject #21892
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,64 @@ sidebar_position: 1 | |
# `x/counter` | ||
|
||
Counter is a module used for testing purposes within the Cosmos SDK. | ||
|
||
## Hooks | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, but I meant creating a page at build-modules/hooks on the website instead. No one will ever find this otherwise :D Basically, if you could move that section under docs/build-modules/define-hooks that would be great. We can use code snippet, as |
||
|
||
The module supports hook injection using `depinject`. Follow these steps to implement hooks: | ||
|
||
1. Define the hook interface and a wrapper implementing `depinject.OnePerModuleType`: | ||
```go | ||
type CounterHooks interface { | ||
AfterIncreaseCount(ctx context.Context, newCount int64) error | ||
} | ||
|
||
type CounterHooksWrapper struct{ CounterHooks } | ||
|
||
// IsOnePerModuleType implements the depinject.OnePerModuleType interface. | ||
func (CounterHooksWrapper) IsOnePerModuleType() {} | ||
|
||
``` | ||
2. Add a `CounterHooks` field to the keeper: | ||
```go | ||
type Keeper struct{ | ||
//... | ||
hooks CounterHooks | ||
} | ||
|
||
func (k *Keeper) SetHooks(hooks ...CounterHooks){ | ||
k.hooks = hooks | ||
} | ||
``` | ||
3. Create a `depinject` invoker function | ||
```go | ||
func InvokeSetHooks(keeper *keeper.Keeper, counterHooks map[string]types.CounterHooksWrapper) error { | ||
//... | ||
keeper.SetHooks(multiHooks) | ||
return nil | ||
} | ||
|
||
``` | ||
Comment on lines
+36
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve the depinject invoker function example The Consider providing a more complete example of the func InvokeSetHooks(keeper *keeper.Keeper, counterHooks map[string]types.CounterHooksWrapper) error {
for _, hook := range counterHooks {
keeper.SetHooks(hook)
}
return nil
} Also, improve the formatting:
3. Create a `depinject` invoker function:
```go
func InvokeSetHooks(keeper *keeper.Keeper, counterHooks map[string]types.CounterHooksWrapper) error {
for _, hook := range counterHooks {
keeper.SetHooks(hook)
}
return nil
}
|
||
4. Inject the hooks during app initialization: | ||
```go | ||
appConfig = appconfig.Compose(&appv1alpha1.Config{ | ||
Modules: []*appv1alpha1.ModuleConfig{ | ||
// .... | ||
{ | ||
Name: types.ModuleName, | ||
Config: appconfig.WrapAny(&types.Module{}), | ||
}, | ||
} | ||
}) | ||
appConfig = depinject.Configs( | ||
AppConfig(), | ||
runtime.DefaultServiceBindings(), | ||
depinject.Supply( | ||
logger, | ||
viper, | ||
map[string]types.CounterHooksWrapper{ | ||
"counter": types.CounterHooksWrapper{&types.Hooks{}}, | ||
}, | ||
)) | ||
|
||
``` | ||
By following these steps, you can successfully integrate hooks into the `x/counter` module using `depinject`. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,10 @@ | ||||||||||||||||||||||
package counter | ||||||||||||||||||||||
|
||||||||||||||||||||||
import ( | ||||||||||||||||||||||
"fmt" | ||||||||||||||||||||||
"maps" | ||||||||||||||||||||||
"slices" | ||||||||||||||||||||||
|
||||||||||||||||||||||
"cosmossdk.io/core/appmodule" | ||||||||||||||||||||||
"cosmossdk.io/depinject" | ||||||||||||||||||||||
"cosmossdk.io/depinject/appconfig" | ||||||||||||||||||||||
|
@@ -17,6 +21,7 @@ func (am AppModule) IsOnePerModuleType() {} | |||||||||||||||||||||
func init() { | ||||||||||||||||||||||
appconfig.RegisterModule( | ||||||||||||||||||||||
&types.Module{}, | ||||||||||||||||||||||
appconfig.Invoke(InvokeSetHooks), | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider reordering To ensure that all dependencies are available when invoked, it's customary to call Apply this diff to reorder the calls: appconfig.RegisterModule(
&types.Module{},
+ appconfig.Provide(ProvideModule),
appconfig.Invoke(InvokeSetHooks),
- appconfig.Provide(ProvideModule),
) Committable suggestion
Suggested change
|
||||||||||||||||||||||
appconfig.Provide(ProvideModule), | ||||||||||||||||||||||
) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
@@ -31,7 +36,7 @@ type ModuleInputs struct { | |||||||||||||||||||||
type ModuleOutputs struct { | ||||||||||||||||||||||
depinject.Out | ||||||||||||||||||||||
|
||||||||||||||||||||||
Keeper keeper.Keeper | ||||||||||||||||||||||
Keeper *keeper.Keeper | ||||||||||||||||||||||
Module appmodule.AppModule | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -44,3 +49,24 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { | |||||||||||||||||||||
Module: m, | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
func InvokeSetHooks(keeper *keeper.Keeper, counterHooks map[string]types.CounterHooksWrapper) error { | ||||||||||||||||||||||
if keeper == nil || counterHooks == nil { | ||||||||||||||||||||||
return nil | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+54
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return an error when dependencies are missing Returning Apply this diff to return an error when dependencies are missing: func InvokeSetHooks(keeper *keeper.Keeper, counterHooks map[string]types.CounterHooksWrapper) error {
- if keeper == nil || counterHooks == nil {
- return nil
- }
+ if keeper == nil {
+ return fmt.Errorf("keeper is nil")
+ }
+ if counterHooks == nil {
+ return fmt.Errorf("counterHooks is nil")
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
// Default ordering is lexical by module name. | ||||||||||||||||||||||
// Explicit ordering can be added to the module config if required. | ||||||||||||||||||||||
modNames := slices.Sorted(maps.Keys(counterHooks)) | ||||||||||||||||||||||
var multiHooks types.MultiCounterHooks | ||||||||||||||||||||||
for _, modName := range modNames { | ||||||||||||||||||||||
hook, ok := counterHooks[modName] | ||||||||||||||||||||||
if !ok { | ||||||||||||||||||||||
return fmt.Errorf("can't find hooks for module %s", modName) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
multiHooks = append(multiHooks, hook) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
keeper.SetHooks(multiHooks) | ||||||||||||||||||||||
return nil | ||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package keeper | ||
|
||
import ( | ||
"context" | ||
) | ||
|
||
type Hooks struct { | ||
AfterCounterIncreased bool | ||
} | ||
|
||
func (h *Hooks) AfterIncreaseCount(ctx context.Context, n int64) error { | ||
h.AfterCounterIncreased = true | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,11 +21,13 @@ type Keeper struct { | |
appmodule.Environment | ||
|
||
CountStore collections.Item[int64] | ||
|
||
hooks types.CounterHooks | ||
Comment on lines
+24
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ensure Consistent Use of Pointer Receivers for The Suggested changes: Update the method receivers to use pointers: -var _ types.QueryServer = Keeper{}
+var _ types.QueryServer = &Keeper{}
-func (k Keeper) GetCount(ctx context.Context, _ *types.QueryGetCountRequest) (*types.QueryGetCountResponse, error) {
+func (k *Keeper) GetCount(ctx context.Context, _ *types.QueryGetCountRequest) (*types.QueryGetCountResponse, error) {
-var _ types.MsgServer = Keeper{}
+var _ types.MsgServer = &Keeper{}
-func (k Keeper) IncreaseCount(ctx context.Context, msg *types.MsgIncreaseCounter) (*types.MsgIncreaseCountResponse, error) {
+func (k *Keeper) IncreaseCount(ctx context.Context, msg *types.MsgIncreaseCounter) (*types.MsgIncreaseCountResponse, error) {
|
||
} | ||
|
||
func NewKeeper(env appmodule.Environment) Keeper { | ||
func NewKeeper(env appmodule.Environment) *Keeper { | ||
sb := collections.NewSchemaBuilder(env.KVStoreService) | ||
return Keeper{ | ||
return &Keeper{ | ||
Environment: env, | ||
CountStore: collections.NewItem(sb, collections.NewPrefix(0), "count", collections.Int64Value), | ||
} | ||
|
@@ -67,6 +69,10 @@ func (k Keeper) IncreaseCount(ctx context.Context, msg *types.MsgIncreaseCounter | |
return nil, err | ||
} | ||
|
||
if err := k.Hooks().AfterIncreaseCount(ctx, num+msg.Count); err != nil { | ||
return nil, err | ||
} | ||
|
||
if err := k.EventService.EventManager(ctx).EmitKV( | ||
"increase_counter", | ||
event.NewAttribute("signer", msg.Signer), | ||
|
@@ -78,3 +84,23 @@ func (k Keeper) IncreaseCount(ctx context.Context, msg *types.MsgIncreaseCounter | |
NewCount: num + msg.Count, | ||
}, nil | ||
} | ||
|
||
// Hooks gets the hooks for counter Keeper | ||
func (k *Keeper) Hooks() types.CounterHooks { | ||
if k.hooks == nil { | ||
// return a no-op implementation if no hooks are set | ||
return types.MultiCounterHooks{} | ||
} | ||
|
||
return k.hooks | ||
} | ||
|
||
// SetHooks sets the hooks for counter | ||
func (k *Keeper) SetHooks(gh types.CounterHooks) *Keeper { | ||
if k.hooks != nil { | ||
panic("cannot set governance hooks twice") | ||
} | ||
Comment on lines
+100
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid Using Using Suggested change: func (k *Keeper) SetHooks(gh types.CounterHooks) *Keeper {
if k.hooks != nil {
- panic("cannot set governance hooks twice")
+ return errors.New("cannot set hooks twice")
}
k.hooks = gh
- return k
+ return nil
}
|
||
|
||
k.hooks = gh | ||
return k | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package types | ||
|
||
import "context" | ||
|
||
type CounterHooks interface { | ||
AfterIncreaseCount(ctx context.Context, newCount int64) error | ||
} | ||
|
||
type CounterHooksWrapper struct{ CounterHooks } | ||
|
||
// IsOnePerModuleType implements the depinject.OnePerModuleType interface. | ||
func (CounterHooksWrapper) IsOnePerModuleType() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package types | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
) | ||
|
||
var _ CounterHooks = MultiCounterHooks{} | ||
|
||
// MultiCounterHooks is a slice of hooks to be called in sequence. | ||
type MultiCounterHooks []CounterHooks | ||
|
||
// NewMultiCounterHooks returns a MultiCounterHooks from a list of CounterHooks | ||
func NewMultiCounterHooks(hooks ...CounterHooks) MultiCounterHooks { | ||
return hooks | ||
} | ||
|
||
// AfterIncreaseCount calls AfterIncreaseCount on all hooks and collects the errors if any. | ||
func (ch MultiCounterHooks) AfterIncreaseCount(ctx context.Context, newCount int64) error { | ||
var errs error | ||
for i := range ch { | ||
errs = errors.Join(errs, ch[i].AfterIncreaseCount(ctx, newCount)) | ||
} | ||
|
||
return errs | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be deleted