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

feat(client): overwrite client context instead of setting new one #20356

Merged
merged 3 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
* (genutil) [#19971](https://github.com/cosmos/cosmos-sdk/pull/19971) Allow manually setting the consensus key type in genesis
* (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.
* (client) [#20356](https://github.com/cosmos/cosmos-sdk/pull/20356) Overwrite client context instead of setting new one

### Improvements

Expand Down
13 changes: 8 additions & 5 deletions client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,17 @@ func GetClientContextFromCmd(cmd *cobra.Command) Context {
// SetCmdClientContext sets a command's Context value to the provided argument.
// If the context has not been set, set the given context as the default.
func SetCmdClientContext(cmd *cobra.Command, clientCtx Context) error {
var cmdCtx context.Context

if cmd.Context() == nil {
cmdCtx := cmd.Context()
if cmdCtx == nil {
cmdCtx = context.Background()
}

v := cmd.Context().Value(ClientContextKey)
if clientCtxPtr, ok := v.(*Context); ok {
Copy link
Member

@julienrbrt julienrbrt May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good compromise with what we had in v0.47 (https://github.com/cosmos/cosmos-sdk/blob/release/v0.47.x/client/cmd.go#L350-L360) vs what we have now in v0.50 (https://github.com/cosmos/cosmos-sdk/blob/v0.50.6/client/cmd.go#L361-L371).
Adding the backport label.

*clientCtxPtr = clientCtx
} else {
cmdCtx = cmd.Context()
cmd.SetContext(context.WithValue(cmdCtx, ClientContextKey, &clientCtx))
}

cmd.SetContext(context.WithValue(cmdCtx, ClientContextKey, &clientCtx))
return nil
}
26 changes: 23 additions & 3 deletions client/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,21 @@ func TestSetCmdClientContextHandler(t *testing.T) {
name string
expectedContext client.Context
args []string
ctx context.Context
}{
{
"no flags set",
initClientCtx,
[]string{},
context.WithValue(context.Background(), client.ClientContextKey, &client.Context{}),
},
{
"flags set",
initClientCtx.WithChainID("new-chain-id"),
[]string{
fmt.Sprintf("--%s=new-chain-id", flags.FlagChainID),
},
context.WithValue(context.Background(), client.ClientContextKey, &client.Context{}),
},
{
"flags set with space",
Expand All @@ -99,20 +102,37 @@ func TestSetCmdClientContextHandler(t *testing.T) {
fmt.Sprintf("--%s", flags.FlagHome),
"/tmp/dir",
},
context.Background(),
},
{
"no context provided",
initClientCtx.WithHomeDir("/tmp/noctx"),
[]string{
fmt.Sprintf("--%s", flags.FlagHome),
"/tmp/noctx",
},
nil,
},
{
"with invalid client value in the context",
initClientCtx.WithHomeDir("/tmp/invalid"),
[]string{
fmt.Sprintf("--%s", flags.FlagHome),
"/tmp/invalid",
},
context.WithValue(context.Background(), client.ClientContextKey, "invalid"),
},
}

for _, tc := range testCases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
ctx := context.WithValue(context.Background(), client.ClientContextKey, &client.Context{})

cmd := newCmd()
_ = testutil.ApplyMockIODiscardOutErr(cmd)
cmd.SetArgs(tc.args)

require.NoError(t, cmd.ExecuteContext(ctx))
require.NoError(t, cmd.ExecuteContext(tc.ctx))

clientCtx := client.GetClientContextFromCmd(cmd)
require.Equal(t, tc.expectedContext, clientCtx)
Expand Down
Loading