From 5f0b9a24b9f0a4145669a78d5ea95b5a94dd0769 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Wed, 30 Jul 2025 15:52:56 +0700 Subject: [PATCH] refactor: improve ServiceManager initialization with cleaner API - Split initializeServiceManager into two methods: * initializeServiceManager(): Simple method using default configuration * initializeServiceManagerWithServiceConfig(): Advanced method for custom config - Simplify NewServiceCommand() to return *ServiceCommand without error - Update all service command methods to use appropriate initialization: * Start: Uses initializeServiceManagerWithServiceConfig() for custom args * Stop/Restart/Reload/Status/Uninstall: Use simple initializeServiceManager() - Remove direct access to sc.serviceManager.svc/prog in favor of lazy initialization - Improve separation of concerns and reduce code duplication --- cmd/cli/commands_service.go | 31 +++++++++++++++++---------- cmd/cli/commands_service_reload.go | 8 +++++-- cmd/cli/commands_service_restart.go | 7 ++++-- cmd/cli/commands_service_start.go | 8 +++++-- cmd/cli/commands_service_status.go | 6 +++++- cmd/cli/commands_service_stop.go | 8 +++++-- cmd/cli/commands_service_uninstall.go | 8 +++++-- 7 files changed, 54 insertions(+), 22 deletions(-) diff --git a/cmd/cli/commands_service.go b/cmd/cli/commands_service.go index d9cb5f0..e8dc1d8 100644 --- a/cmd/cli/commands_service.go +++ b/cmd/cli/commands_service.go @@ -25,16 +25,28 @@ type ServiceCommand struct { serviceManager *ServiceManager } -// NewServiceCommand creates a new service command handler -func NewServiceCommand() (*ServiceCommand, error) { - sm, err := NewServiceManager() +// initializeServiceManager creates a service manager with default configuration +func (sc *ServiceCommand) initializeServiceManager() (service.Service, *prog, error) { + svcConfig := sc.createServiceConfig() + return sc.initializeServiceManagerWithServiceConfig(svcConfig) +} + +// initializeServiceManagerWithServiceConfig creates a service manager with the given configuration +func (sc *ServiceCommand) initializeServiceManagerWithServiceConfig(svcConfig *service.Config) (service.Service, *prog, error) { + p := &prog{} + + s, err := newService(p, svcConfig) if err != nil { - return nil, err + return nil, nil, fmt.Errorf("failed to create service: %w", err) } - return &ServiceCommand{ - serviceManager: sm, - }, nil + sc.serviceManager = &ServiceManager{prog: p, svc: s} + return s, p, nil +} + +// NewServiceCommand creates a new service command handler +func NewServiceCommand() *ServiceCommand { + return &ServiceCommand{} } // createServiceConfig creates a properly initialized service configuration @@ -50,10 +62,7 @@ func (sc *ServiceCommand) createServiceConfig() *service.Config { // InitServiceCmd creates the service command with proper logic and aliases func InitServiceCmd() *cobra.Command { // Create service command handlers - sc, err := NewServiceCommand() - if err != nil { - panic(fmt.Sprintf("failed to create service command: %v", err)) - } + sc := NewServiceCommand() startCmd, startCmdAlias := createStartCommands(sc) rootCmd.AddCommand(startCmdAlias) diff --git a/cmd/cli/commands_service_reload.go b/cmd/cli/commands_service_reload.go index 9e19068..74a80ac 100644 --- a/cmd/cli/commands_service_reload.go +++ b/cmd/cli/commands_service_reload.go @@ -12,7 +12,11 @@ import ( // Reload implements the logic from cmdReload.Run func (sc *ServiceCommand) Reload(cmd *cobra.Command, args []string) error { - status, err := sc.serviceManager.svc.Status() + s, _, err := sc.initializeServiceManager() + if err != nil { + return err + } + status, err := s.Status() if errors.Is(err, service.ErrNotInstalled) { mainLog.Load().Warn().Msg("service not installed") return nil @@ -37,7 +41,7 @@ func (sc *ServiceCommand) Reload(cmd *cobra.Command, args []string) error { case http.StatusCreated: mainLog.Load().Warn().Msg("Service was reloaded, but new config requires service restart.") mainLog.Load().Warn().Msg("Restarting service") - if _, err := sc.serviceManager.svc.Status(); errors.Is(err, service.ErrNotInstalled) { + if _, err := s.Status(); errors.Is(err, service.ErrNotInstalled) { mainLog.Load().Warn().Msg("Service not installed") return nil } diff --git a/cmd/cli/commands_service_restart.go b/cmd/cli/commands_service_restart.go index 5303ea4..dcad4c1 100644 --- a/cmd/cli/commands_service_restart.go +++ b/cmd/cli/commands_service_restart.go @@ -9,13 +9,16 @@ import ( // Restart implements the logic from cmdRestart.Run func (sc *ServiceCommand) Restart(cmd *cobra.Command, args []string) error { - s := sc.serviceManager.svc - p := sc.serviceManager.prog readConfig(false) v.Unmarshal(&cfg) cdUID = curCdUID() cdMode := cdUID != "" + s, p, err := sc.initializeServiceManager() + if err != nil { + return err + } + p.cfg = &cfg if iface == "" { iface = "auto" diff --git a/cmd/cli/commands_service_start.go b/cmd/cli/commands_service_start.go index d5d5150..ea349ba 100644 --- a/cmd/cli/commands_service_start.go +++ b/cmd/cli/commands_service_start.go @@ -21,8 +21,6 @@ import ( // Start implements the logic from cmdStart.Run func (sc *ServiceCommand) Start(cmd *cobra.Command, args []string) error { - s := sc.serviceManager.svc - p := sc.serviceManager.prog checkStrFlagEmpty(cmd, cdUidFlagName) checkStrFlagEmpty(cmd, cdOrgFlagName) validateCdAndNextDNSFlags() @@ -36,6 +34,12 @@ func (sc *ServiceCommand) Start(cmd *cobra.Command, args []string) error { setDependencies(svcConfig) svcConfig.Arguments = append([]string{"run"}, osArgs...) + // Initialize service manager with proper configuration + s, p, err := sc.initializeServiceManagerWithServiceConfig(svcConfig) + if err != nil { + return err + } + p.cfg = &cfg p.preRun() diff --git a/cmd/cli/commands_service_status.go b/cmd/cli/commands_service_status.go index 190d66d..13b1628 100644 --- a/cmd/cli/commands_service_status.go +++ b/cmd/cli/commands_service_status.go @@ -9,7 +9,11 @@ import ( // Status implements the logic from cmdStatus.Run func (sc *ServiceCommand) Status(cmd *cobra.Command, args []string) error { - status, err := sc.serviceManager.svc.Status() + s, _, err := sc.initializeServiceManager() + if err != nil { + return err + } + status, err := s.Status() if err != nil { mainLog.Load().Error().Msg(err.Error()) os.Exit(1) diff --git a/cmd/cli/commands_service_stop.go b/cmd/cli/commands_service_stop.go index daec9ab..5c71842 100644 --- a/cmd/cli/commands_service_stop.go +++ b/cmd/cli/commands_service_stop.go @@ -10,10 +10,14 @@ import ( // Stop implements the logic from cmdStop.Run func (sc *ServiceCommand) Stop(cmd *cobra.Command, args []string) error { - s := sc.serviceManager.svc - p := sc.serviceManager.prog readConfig(false) v.Unmarshal(&cfg) + + s, p, err := sc.initializeServiceManager() + if err != nil { + return err + } + p.cfg = &cfg p.preRun() if ir := runningIface(s); ir != nil { diff --git a/cmd/cli/commands_service_uninstall.go b/cmd/cli/commands_service_uninstall.go index a14cac3..0f3032a 100644 --- a/cmd/cli/commands_service_uninstall.go +++ b/cmd/cli/commands_service_uninstall.go @@ -12,10 +12,14 @@ import ( // Uninstall implements the logic from cmdUninstall.Run func (sc *ServiceCommand) Uninstall(cmd *cobra.Command, args []string) error { - s := sc.serviceManager.svc - p := sc.serviceManager.prog readConfig(false) v.Unmarshal(&cfg) + + s, p, err := sc.initializeServiceManager() + if err != nil { + return err + } + p.cfg = &cfg if iface == "" { iface = "auto"