From 9df6019de6ee7999fe9864bac836deb2f36dd44a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 4 Jul 2024 10:21:49 +0200 Subject: [PATCH] fix(server/v2): remove dup commands + reduce api surface (#20810) --- server/v2/cometbft/server.go | 6 +- server/v2/commands.go | 134 ++++++++++++++----------------- server/v2/server.go | 16 +++- simapp/v2/simdv2/cmd/commands.go | 19 +++-- 4 files changed, 87 insertions(+), 88 deletions(-) diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index f8454d3b13..4088a4bd84 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -195,11 +195,13 @@ func (s *CometBFTServer[AppT, T]) CLICommands() serverv2.CLIConfig { s.ShowValidatorCmd(), s.ShowAddressCmd(), s.VersionCmd(), + cmtcmd.ResetAllCmd, + cmtcmd.ResetStateCmd, + }, + Queries: []*cobra.Command{ s.QueryBlockCmd(), s.QueryBlocksCmd(), s.QueryBlockResultsCmd(), - cmtcmd.ResetAllCmd, - cmtcmd.ResetStateCmd, }, } } diff --git a/server/v2/commands.go b/server/v2/commands.go index 9f4b3e1816..db385a832d 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -33,22 +33,76 @@ func Execute(rootCmd *cobra.Command, envPrefix, defaultHome string) error { return rootCmd.Execute() } -// Commands creates the start command of an application and gives back the CLIConfig containing all the server commands. -// This API is for advanced user only, most users should use AddCommands instead that abstract more. -func Commands[AppT AppI[T], T transaction.Tx]( +// AddCommands add the server commands to the root command +// It configure the config handling and the logger handling +func AddCommands[AppT AppI[T], T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[AppT, T], logger log.Logger, components ...ServerComponent[AppT, T], -) (CLIConfig, error) { +) error { if len(components) == 0 { - return CLIConfig{}, errors.New("no components provided") + return errors.New("no components provided") } server := NewServer(logger, components...) + originalPersistentPreRunE := rootCmd.PersistentPreRunE + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + // set the default command outputs + cmd.SetOut(cmd.OutOrStdout()) + cmd.SetErr(cmd.ErrOrStderr()) + + if err := configHandle(server, cmd); err != nil { + return err + } + + // call the original PersistentPreRun(E) if it exists + if rootCmd.PersistentPreRun != nil { + rootCmd.PersistentPreRun(cmd, args) + return nil + } + + return originalPersistentPreRunE(cmd, args) + } + + cmds := server.CLICommands() + startCmd := createStartCommand(server, newApp) + startCmd.SetContext(rootCmd.Context()) + cmds.Commands = append(cmds.Commands, startCmd) + rootCmd.AddCommand(cmds.Commands...) + + if len(cmds.Queries) > 0 { + if queryCmd := findSubCommand(rootCmd, "query"); queryCmd != nil { + queryCmd.AddCommand(cmds.Queries...) + } else { + queryCmd := topLevelCmd(rootCmd.Context(), "query", "Querying subcommands") + queryCmd.Aliases = []string{"q"} + queryCmd.AddCommand(cmds.Queries...) + rootCmd.AddCommand(queryCmd) + } + } + + if len(cmds.Txs) > 0 { + if txCmd := findSubCommand(rootCmd, "tx"); txCmd != nil { + txCmd.AddCommand(cmds.Txs...) + } else { + txCmd := topLevelCmd(rootCmd.Context(), "tx", "Transactions subcommands") + txCmd.AddCommand(cmds.Txs...) + rootCmd.AddCommand(txCmd) + } + } + + return nil +} + +// createStartCommand creates the start command for the application. +func createStartCommand[AppT AppI[T], T transaction.Tx]( + server *Server[AppT, T], + newApp AppCreator[AppT, T], +) *cobra.Command { flags := server.StartFlags() - startCmd := &cobra.Command{ + return &cobra.Command{ Use: "start", Short: "Run the application", RunE: func(cmd *cobra.Command, args []string) error { @@ -65,9 +119,7 @@ func Commands[AppT AppI[T], T transaction.Tx]( return err } - app := newApp(l, v) - - if err := server.Init(app, v, l); err != nil { + if err := server.Init(newApp(l, v), v, l); err != nil { return err } @@ -91,70 +143,6 @@ func Commands[AppT AppI[T], T transaction.Tx]( return nil }, } - startCmd.SetContext(rootCmd.Context()) - - cmds := server.CLICommands() - cmds.Commands = append(cmds.Commands, startCmd) - - return cmds, nil -} - -// AddCommands add the server commands to the root command -// It configure the config handling and the logger handling -func AddCommands[AppT AppI[T], T transaction.Tx]( - rootCmd *cobra.Command, - newApp AppCreator[AppT, T], - logger log.Logger, - components ...ServerComponent[AppT, T], -) error { - cmds, err := Commands(rootCmd, newApp, logger, components...) - if err != nil { - return err - } - - srv := NewServer(logger, components...) - originalPersistentPreRunE := rootCmd.PersistentPreRunE - rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { - // set the default command outputs - cmd.SetOut(cmd.OutOrStdout()) - cmd.SetErr(cmd.ErrOrStderr()) - - if err = configHandle(srv, cmd); err != nil { - return err - } - - if rootCmd.PersistentPreRun != nil { - rootCmd.PersistentPreRun(cmd, args) - return nil - } - - return originalPersistentPreRunE(cmd, args) - } - - rootCmd.AddCommand(cmds.Commands...) - - if len(cmds.Queries) > 0 { - if queryCmd := findSubCommand(rootCmd, "query"); queryCmd != nil { - queryCmd.AddCommand(cmds.Queries...) - } else { - queryCmd := topLevelCmd(rootCmd.Context(), "query", "Querying subcommands") - queryCmd.Aliases = []string{"q"} - queryCmd.AddCommand(cmds.Queries...) - rootCmd.AddCommand(queryCmd) - } - } - - if len(cmds.Txs) > 0 { - if txCmd := findSubCommand(rootCmd, "tx"); txCmd != nil { - txCmd.AddCommand(cmds.Txs...) - } else { - txCmd := topLevelCmd(rootCmd.Context(), "tx", "Transaction subcommands") - txCmd.AddCommand(cmds.Txs...) - rootCmd.AddCommand(txCmd) - } - } - - return nil } // configHandle writes the default config to the home directory if it does not exist and sets the server context diff --git a/server/v2/server.go b/server/v2/server.go index a8773b7944..5e6c790d0f 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -136,9 +136,19 @@ func (s *Server[AppT, T]) CLICommands() CLIConfig { commands := CLIConfig{} for _, mod := range s.components { if climod, ok := mod.(HasCLICommands); ok { - commands.Commands = append(commands.Commands, compart(mod.Name(), climod.CLICommands().Commands...)) - commands.Txs = append(commands.Txs, compart(mod.Name(), climod.CLICommands().Txs...)) - commands.Queries = append(commands.Queries, compart(mod.Name(), climod.CLICommands().Queries...)) + srvCmd := climod.CLICommands() + + if len(srvCmd.Commands) > 0 { + commands.Commands = append(commands.Commands, compart(mod.Name(), srvCmd.Commands...)) + } + + if len(srvCmd.Txs) > 0 { + commands.Txs = append(commands.Txs, compart(mod.Name(), srvCmd.Txs...)) + } + + if len(srvCmd.Queries) > 0 { + commands.Queries = append(commands.Queries, compart(mod.Name(), srvCmd.Queries...)) + } } } diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 2aeed7aed0..4ac920d4a6 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -99,6 +99,15 @@ func initRootCmd[AppT serverv2.AppI[T], T transaction.Tx]( panic(fmt.Sprintf("failed to create logger: %v", err)) } + // add keybase, auxiliary RPC, query, genesis, and tx child commands + rootCmd.AddCommand( + genesisCommand[T](txConfig, moduleManager, appExport[T]), + queryCommand(), + txCommand(), + keys.Commands(), + offchain.OffChain(), + ) + // Add empty server struct here for writing default config if err = serverv2.AddCommands( rootCmd, @@ -109,16 +118,6 @@ func initRootCmd[AppT serverv2.AppI[T], T transaction.Tx]( ); err != nil { panic(err) } - - // add keybase, auxiliary RPC, query, genesis, and tx child commands - rootCmd.AddCommand( - server.StatusCommand(), - genesisCommand[T](txConfig, moduleManager, appExport[T]), - queryCommand(), - txCommand(), - keys.Commands(), - offchain.OffChain(), - ) } // genesisCommand builds genesis-related `simd genesis` command. Users may provide application specific commands as a parameter