fix(server/v2): remove dup commands + reduce api surface (#20810)

cosmos · Jul 4, 2024 · 9df6019 · 9df6019
1 parent e75f7b5
commit 9df6019
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 87 deletions.
diff --git 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
@@ -33,72 +33,6 @@ 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](
-	rootCmd *cobra.Command,
-	newApp AppCreator[AppT, T],
-	logger log.Logger,
-	components ...ServerComponent[AppT, T],
-) (CLIConfig, error) {
-	if len(components) == 0 {
-		return CLIConfig{}, errors.New("no components provided")
-	}
-
-	server := NewServer(logger, components...)
-	flags := server.StartFlags()
-
-	startCmd := &cobra.Command{
-		Use:   "start",
-		Short: "Run the application",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			v := GetViperFromCmd(cmd)
-			l := GetLoggerFromCmd(cmd)
-
-			for _, startFlags := range flags {
-				if err := v.BindPFlags(startFlags); err != nil {
-					return err
-				}
-			}
-
-			if err := v.BindPFlags(cmd.Flags()); err != nil {
-				return err
-			}
-
-			app := newApp(l, v)
-
-			if err := server.Init(app, v, l); err != nil {
-				return err
-			}
-
-			ctx, cancelFn := context.WithCancel(cmd.Context())
-			go func() {
-				sigCh := make(chan os.Signal, 1)
-				signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
-				sig := <-sigCh
-				cancelFn()
-				cmd.Printf("caught %s signal\n", sig.String())
-
-				if err := server.Stop(ctx); err != nil {
-					cmd.PrintErrln("failed to stop servers:", err)
-				}
-			}()
-
-			if err := server.Start(ctx); err != nil {
-				return fmt.Errorf("failed to start servers: %w", err)
-			}
-
-			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](
@@ -107,22 +41,22 @@ func AddCommands[AppT AppI[T], T transaction.Tx](
 	logger log.Logger,
 	components ...ServerComponent[AppT, T],
 ) error {
-	cmds, err := Commands(rootCmd, newApp, logger, components...)
-	if err != nil {
-		return err
+	if len(components) == 0 {
+		return errors.New("no components provided")
 	}
 
-	srv := NewServer(logger, components...)
+	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(srv, cmd); err != nil {
+		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
@@ -131,6 +65,10 @@ func AddCommands[AppT AppI[T], T transaction.Tx](
 		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 {
@@ -148,7 +86,7 @@ func AddCommands[AppT AppI[T], T transaction.Tx](
 		if txCmd := findSubCommand(rootCmd, "tx"); txCmd != nil {
 			txCmd.AddCommand(cmds.Txs...)
 		} else {
-			txCmd := topLevelCmd(rootCmd.Context(), "tx", "Transaction subcommands")
+			txCmd := topLevelCmd(rootCmd.Context(), "tx", "Transactions subcommands")
 			txCmd.AddCommand(cmds.Txs...)
 			rootCmd.AddCommand(txCmd)
 		}
@@ -157,6 +95,56 @@ func AddCommands[AppT AppI[T], T transaction.Tx](
 	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()
+
+	return &cobra.Command{
+		Use:   "start",
+		Short: "Run the application",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			v := GetViperFromCmd(cmd)
+			l := GetLoggerFromCmd(cmd)
+
+			for _, startFlags := range flags {
+				if err := v.BindPFlags(startFlags); err != nil {
+					return err
+				}
+			}
+
+			if err := v.BindPFlags(cmd.Flags()); err != nil {
+				return err
+			}
+
+			if err := server.Init(newApp(l, v), v, l); err != nil {
+				return err
+			}
+
+			ctx, cancelFn := context.WithCancel(cmd.Context())
+			go func() {
+				sigCh := make(chan os.Signal, 1)
+				signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
+				sig := <-sigCh
+				cancelFn()
+				cmd.Printf("caught %s signal\n", sig.String())
+
+				if err := server.Stop(ctx); err != nil {
+					cmd.PrintErrln("failed to stop servers:", err)
+				}
+			}()
+
+			if err := server.Start(ctx); err != nil {
+				return fmt.Errorf("failed to start servers: %w", err)
+			}
+
+			return nil
+		},
+	}
+}
+
 // configHandle writes the default config to the home directory if it does not exist and sets the server context
 func configHandle[AppT AppI[T], T transaction.Tx](s *Server[AppT, T], cmd *cobra.Command) error {
 	home, err := cmd.Flags().GetString(FlagHome)

diff --git 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
@@ -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