refactor: inject default service manager

mlataza · mlataza · commit 6491dde65f47 · 2026-02-28T06:47:58.000+08:00
diff --git a/cmd/agent_smith/config.go b/cmd/agent_smith/config.go
@@ -109,7 +109,7 @@ func runConfig(params *configContext) {
 	name := agent.GetServiceName(params.OrgId)
 
 	// Stop and delete the service if it already exists
-	existingService, err := service.Open(name)
+	existingService, err := params.ServiceManager.Open(name)
 	if err == nil {
 		if existingService.IsActive() {
 			logger.Info("Stopping service", "service", name)
@@ -172,7 +172,7 @@ func runConfig(params *configContext) {
 	// Create the service
 	logger.Info("Creating service", "service", name)
 
-	svc, err := service.Create(service.AgentParams{
+	svc, err := params.ServiceManager.Create(service.AgentParams{
 		Name:                name,
 		AgentExecutablePath: agentExecutablePath,
 		OrgId:               params.OrgId,
diff --git a/cmd/agent_smith/config_context.go b/cmd/agent_smith/config_context.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/RewstApp/agent-smith-go/internal/agent"
+	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 )
 
@@ -21,10 +22,11 @@ type configContext struct {
 	Sys    agent.SystemInfoProvider
 	Domain agent.DomainInfoProvider
 
-	FS utils.FileSystem
+	FS             utils.FileSystem
+	ServiceManager service.ServiceManager
 }
 
-func newConfigContext(args []string, sys agent.SystemInfoProvider, domain agent.DomainInfoProvider, fsys utils.FileSystem) (*configContext, error) {
+func newConfigContext(args []string, sys agent.SystemInfoProvider, domain agent.DomainInfoProvider, fsys utils.FileSystem, svcMgr service.ServiceManager) (*configContext, error) {
 	var params configContext
 
 	fs := flag.NewFlagSet("config", flag.ContinueOnError)
@@ -61,6 +63,7 @@ func newConfigContext(args []string, sys agent.SystemInfoProvider, domain agent.
 	params.Sys = sys
 	params.Domain = domain
 	params.FS = fsys
+	params.ServiceManager = svcMgr
 
 	return &params, nil
 }
diff --git a/cmd/agent_smith/config_context_test.go b/cmd/agent_smith/config_context_test.go
@@ -10,7 +10,7 @@ func TestNewConfigContext(t *testing.T) {
 	configUrl := "https://config.url/"
 	configSecret := "secret123"
 
-	result, _ := newConfigContext([]string{"--org-id", orgId, "--config-url", configUrl, "--config-secret", configSecret}, nil, nil, nil)
+	result, _ := newConfigContext([]string{"--org-id", orgId, "--config-url", configUrl, "--config-secret", configSecret}, nil, nil, nil, nil)
 
 	if result.OrgId != orgId {
 		t.Errorf("expected %v, got %v", orgId, result.OrgId)
@@ -44,7 +44,7 @@ func TestNewConfigContext(t *testing.T) {
 	}
 
 	for _, errorTest := range errorTests {
-		_, err := newConfigContext(errorTest.args, nil, nil, nil)
+		_, err := newConfigContext(errorTest.args, nil, nil, nil, nil)
 
 		if err == nil || !strings.Contains(err.Error(), errorTest.message) {
 			t.Errorf("expected error %s, got %v", errorTest.message, err.Error())
diff --git a/cmd/agent_smith/main.go b/cmd/agent_smith/main.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/RewstApp/agent-smith-go/internal/agent"
 	"github.com/RewstApp/agent-smith-go/internal/interpreter"
+	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 )
 
@@ -39,15 +40,16 @@ func main() {
 	domain := agent.NewDomainInfoProvider()
 	executor := interpreter.NewExecutor()
 	fs := utils.NewFileSystem()
+	svcMgr := service.NewServiceManager()
 
-	uninstallContext, err := newUninstallContext(os.Args[1:])
+	uninstallContext, err := newUninstallContext(os.Args[1:], svcMgr)
 	if err == nil {
 		// Run uninstall routine
 		runUninstall(uninstallContext)
 		return
 	}
 
-	configContext, err := newConfigContext(os.Args[1:], sys, domain, fs)
+	configContext, err := newConfigContext(os.Args[1:], sys, domain, fs, svcMgr)
 	if err == nil {
 		// Run config routine
 		runConfig(configContext)
@@ -61,7 +63,7 @@ func main() {
 		return
 	}
 
-	updateContext, err := newUpdateContext(os.Args[1:], sys, domain)
+	updateContext, err := newUpdateContext(os.Args[1:], sys, domain, svcMgr)
 	if err == nil {
 		// Run update routine
 		runUpdate(updateContext)
diff --git a/cmd/agent_smith/uninstall.go b/cmd/agent_smith/uninstall.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/RewstApp/agent-smith-go/internal/agent"
-	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 	"github.com/RewstApp/agent-smith-go/internal/version"
 )
@@ -21,7 +20,7 @@ func runUninstall(params *uninstallContext) {
 
 	name := agent.GetServiceName(params.OrgId)
 
-	service, err := service.Open(name)
+	service, err := params.ServiceManager.Open(name)
 	if err != nil {
 		logger.Error("Failed to open service", "service", name, "error", err)
 		return
diff --git a/cmd/agent_smith/uninstall_context.go b/cmd/agent_smith/uninstall_context.go
@@ -4,14 +4,18 @@ import (
 	"bytes"
 	"flag"
 	"fmt"
+
+	"github.com/RewstApp/agent-smith-go/internal/service"
 )
 
 type uninstallContext struct {
 	OrgId     string
 	Uninstall bool
+
+	ServiceManager service.ServiceManager
 }
 
-func newUninstallContext(args []string) (*uninstallContext, error) {
+func newUninstallContext(args []string, svcMgr service.ServiceManager) (*uninstallContext, error) {
 	var params uninstallContext
 
 	fs := flag.NewFlagSet("uninstall", flag.ContinueOnError)
@@ -32,5 +36,7 @@ func newUninstallContext(args []string) (*uninstallContext, error) {
 		return nil, fmt.Errorf("missing uninstall")
 	}
 
+	params.ServiceManager = svcMgr
+
 	return &params, nil
 }
diff --git a/cmd/agent_smith/uninstall_context_test.go b/cmd/agent_smith/uninstall_context_test.go
@@ -7,7 +7,7 @@ import (
 
 func TestNewUninstallContext(t *testing.T) {
 	orgId := "test123"
-	result, _ := newUninstallContext([]string{"--org-id", orgId, "--uninstall"})
+	result, _ := newUninstallContext([]string{"--org-id", orgId, "--uninstall"}, nil)
 
 	if result.OrgId != orgId {
 		t.Errorf("expected %v, got %v", orgId, result.OrgId)
@@ -27,7 +27,7 @@ func TestNewUninstallContext(t *testing.T) {
 	}
 
 	for _, errorTest := range errorTests {
-		_, err := newUninstallContext(errorTest.args)
+		_, err := newUninstallContext(errorTest.args, nil)
 
 		if err == nil || !strings.Contains(err.Error(), errorTest.message) {
 			t.Errorf("expected error %s, got %v", errorTest.message, err.Error())
diff --git a/cmd/agent_smith/update.go b/cmd/agent_smith/update.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/RewstApp/agent-smith-go/internal/agent"
-	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 	"github.com/RewstApp/agent-smith-go/internal/version"
 )
@@ -21,7 +20,7 @@ func runUpdate(params *updateContext) {
 
 	// Open the service
 	name := agent.GetServiceName(params.OrgId)
-	svc, err := service.Open(name)
+	svc, err := params.ServiceManager.Open(name)
 	if err != nil {
 		logger.Error("Failed to open service", "name", name, "error", err)
 		return
diff --git a/cmd/agent_smith/update_context.go b/cmd/agent_smith/update_context.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/RewstApp/agent-smith-go/internal/agent"
+	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 )
 
@@ -19,9 +20,11 @@ type updateContext struct {
 
 	Sys    agent.SystemInfoProvider
 	Domain agent.DomainInfoProvider
+
+	ServiceManager service.ServiceManager
 }
 
-func newUpdateContext(args []string, sys agent.SystemInfoProvider, domain agent.DomainInfoProvider) (*updateContext, error) {
+func newUpdateContext(args []string, sys agent.SystemInfoProvider, domain agent.DomainInfoProvider, svcMgr service.ServiceManager) (*updateContext, error) {
 	var params updateContext
 
 	fs := flag.NewFlagSet("config", flag.ContinueOnError)
@@ -52,6 +55,7 @@ func newUpdateContext(args []string, sys agent.SystemInfoProvider, domain agent.
 
 	params.Sys = sys
 	params.Domain = domain
+	params.ServiceManager = svcMgr
 
 	return &params, nil
 }
diff --git a/cmd/agent_smith/update_context_test.go b/cmd/agent_smith/update_context_test.go
@@ -8,7 +8,7 @@ import (
 func TestNewUpdateContext(t *testing.T) {
 	orgId := "test123"
 
-	result, _ := newUpdateContext([]string{"--org-id", orgId, "--update"}, nil, nil)
+	result, _ := newUpdateContext([]string{"--org-id", orgId, "--update"}, nil, nil, nil)
 
 	if result.OrgId != orgId {
 		t.Errorf("expected %v, got %v", orgId, result.OrgId)
@@ -37,7 +37,7 @@ func TestNewUpdateContext(t *testing.T) {
 	}
 
 	for _, errorTest := range errorTests {
-		_, err := newUpdateContext(errorTest.args, nil, nil)
+		_, err := newUpdateContext(errorTest.args, nil, nil, nil)
 
 		if err == nil || !strings.Contains(err.Error(), errorTest.message) {
 			t.Errorf("expected error %s, got %v", errorTest.message, err.Error())
diff --git a/internal/service/service.go b/internal/service/service.go
@@ -28,3 +28,8 @@ type Runner interface {
 	Name() string
 	Execute(stop <-chan struct{}, running chan<- struct{}) ServiceExitCode
 }
+
+type ServiceManager interface {
+	Create(params AgentParams) (Service, error)
+	Open(name string) (Service, error)
+}
diff --git a/internal/service/service_darwin.go b/internal/service/service_darwin.go
@@ -107,11 +107,11 @@ func (svc *darwinService) IsActive() bool {
 	return false
 }
 
-func Create(params AgentParams) (Service, error) {
-	return createWithLaunchCtl(params, &defaultLaunchCtl{})
+type defaultServiceManager struct {
+	system launchCtl
 }
 
-func createWithLaunchCtl(params AgentParams, system launchCtl) (Service, error) {
+func (s *defaultServiceManager) Create(params AgentParams) (Service, error) {
 	serviceConfig := strings.Builder{}
 
 	fmt.Fprintf(&serviceConfig, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\"\n\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n")
@@ -124,7 +124,7 @@ func createWithLaunchCtl(params AgentParams, system launchCtl) (Service, error)
 	fmt.Fprintf(&serviceConfig, "<key>EnvironmentVariables</key>\n<dict>\n<key>PATH</key>\n<string>/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin</string>\n</dict>\n")
 	fmt.Fprintf(&serviceConfig, "</dict>\n</plist>\n")
 
-	svc := &darwinService{name: params.Name, system: system}
+	svc := &darwinService{name: params.Name, system: s.system}
 	err := os.WriteFile(svc.serviceFilePath(), []byte(serviceConfig.String()), utils.DefaultFileMod)
 	if err != nil {
 		return nil, err
@@ -133,22 +133,24 @@ func createWithLaunchCtl(params AgentParams, system launchCtl) (Service, error)
 	return svc, nil
 }
 
-func Open(name string) (Service, error) {
-	return openWithLaunchCtl(name, &defaultLaunchCtl{})
-}
-
-func openWithLaunchCtl(name string, system launchCtl) (Service, error) {
-	_, err := system.Run("print", fmt.Sprintf("system/%s", name))
+func (s *defaultServiceManager) Open(name string) (Service, error) {
+	_, err := s.system.Run("print", fmt.Sprintf("system/%s", name))
 	if err != nil {
 		return nil, err
 	}
 
 	return &darwinService{
 		name:   name,
-		system: system,
+		system: s.system,
 	}, nil
 }
 
+func NewServiceManager() ServiceManager {
+	return &defaultServiceManager{
+		system: &defaultLaunchCtl{},
+	}
+}
+
 func Run(runner Runner) (int, error) {
 	// Create a channel to listen for termination signals
 	signalReceived := make(chan os.Signal, 1)
diff --git a/internal/service/service_darwin_test.go b/internal/service/service_darwin_test.go
@@ -232,11 +232,21 @@ func TestDarwinService_IsActive_NoStateLine(t *testing.T) {
 	}
 }
 
-// createWithLaunchCtl tests
+// NewServiceManager tests
 
-func TestCreateWithLaunchCtl_Success(t *testing.T) {
+func TestNewServiceManager_ReturnsNonNil(t *testing.T) {
+	sm := NewServiceManager()
+	if sm == nil {
+		t.Fatal("expected non-nil ServiceManager")
+	}
+}
+
+// defaultServiceManager.Create tests
+
+func TestDefaultServiceManager_Create_Success(t *testing.T) {
 	tmpFile := newTempPlistPath(t)
 	mock := &mockLaunchCtl{plistPath: tmpFile}
+	sm := &defaultServiceManager{system: mock}
 	params := AgentParams{
 		Name:                "test-svc",
 		AgentExecutablePath: "/usr/bin/agent",
@@ -245,7 +255,7 @@ func TestCreateWithLaunchCtl_Success(t *testing.T) {
 		LogFilePath:         "/var/log/agent.log",
 	}
 
-	svc, err := createWithLaunchCtl(params, mock)
+	svc, err := sm.Create(params)
 
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -255,9 +265,10 @@ func TestCreateWithLaunchCtl_Success(t *testing.T) {
 	}
 }
 
-func TestCreateWithLaunchCtl_PlistContent(t *testing.T) {
+func TestDefaultServiceManager_Create_PlistContent(t *testing.T) {
 	tmpFile := newTempPlistPath(t)
 	mock := &mockLaunchCtl{plistPath: tmpFile}
+	sm := &defaultServiceManager{system: mock}
 	params := AgentParams{
 		Name:                "my-service",
 		AgentExecutablePath: "/usr/bin/agent",
@@ -266,7 +277,7 @@ func TestCreateWithLaunchCtl_PlistContent(t *testing.T) {
 		LogFilePath:         "/var/log/agent.log",
 	}
 
-	if _, err := createWithLaunchCtl(params, mock); err != nil {
+	if _, err := sm.Create(params); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
@@ -299,22 +310,24 @@ func TestCreateWithLaunchCtl_PlistContent(t *testing.T) {
 	}
 }
 
-func TestCreateWithLaunchCtl_WriteFileError(t *testing.T) {
+func TestDefaultServiceManager_Create_WriteFileError(t *testing.T) {
 	mock := &mockLaunchCtl{plistPath: "/nonexistent/dir/file.plist"}
+	sm := &defaultServiceManager{system: mock}
 
-	_, err := createWithLaunchCtl(AgentParams{Name: "test-svc"}, mock)
+	_, err := sm.Create(AgentParams{Name: "test-svc"})
 
 	if err == nil {
 		t.Error("expected error on WriteFile failure, got nil")
 	}
 }
 
-// openWithLaunchCtl tests
+// defaultServiceManager.Open tests
 
-func TestOpenWithLaunchCtl_Success(t *testing.T) {
+func TestDefaultServiceManager_Open_Success(t *testing.T) {
 	mock := &mockLaunchCtl{}
+	sm := &defaultServiceManager{system: mock}
 
-	svc, err := openWithLaunchCtl("test-svc", mock)
+	svc, err := sm.Open("test-svc")
 
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -327,10 +340,11 @@ func TestOpenWithLaunchCtl_Success(t *testing.T) {
 	}
 }
 
-func TestOpenWithLaunchCtl_Error(t *testing.T) {
+func TestDefaultServiceManager_Open_Error(t *testing.T) {
 	mock := &mockLaunchCtl{runErr: errors.New("not found")}
+	sm := &defaultServiceManager{system: mock}
 
-	_, err := openWithLaunchCtl("test-svc", mock)
+	_, err := sm.Open("test-svc")
 
 	if err == nil {
 		t.Error("expected error, got nil")
diff --git a/internal/service/service_linux.go b/internal/service/service_linux.go
diff --git a/internal/service/service_linux_test.go b/internal/service/service_linux_test.go
diff --git a/internal/service/service_windows.go b/internal/service/service_windows.go
diff --git a/internal/service/service_windows_test.go b/internal/service/service_windows_test.go

Original file line number	Diff line number	Diff line change
`@@ -4,14 +4,18 @@ import (`
`4`	`4`	`"bytes"`
`5`	`5`	`"flag"`
`6`	`6`	`"fmt"`
	`7`	`+`
	`8`	`+ "github.com/RewstApp/agent-smith-go/internal/service"`
`7`	`9`	`)`
`8`	`10`
`9`	`11`	`type uninstallContext struct {`
`10`	`12`	`OrgId string`
`11`	`13`	`Uninstall bool`
	`14`	`+`
	`15`	`+ ServiceManager service.ServiceManager`
`12`	`16`	`}`
`13`	`17`
`14`		`-func newUninstallContext(args []string) (*uninstallContext, error) {`
	`18`	`+func newUninstallContext(args []string, svcMgr service.ServiceManager) (*uninstallContext, error) {`
`15`	`19`	`var params uninstallContext`
`16`	`20`
`17`	`21`	`fs := flag.NewFlagSet("uninstall", flag.ContinueOnError)`
`@@ -32,5 +36,7 @@ func newUninstallContext(args []string) (*uninstallContext, error) {`
`32`	`36`	`return nil, fmt.Errorf("missing uninstall")`
`33`	`37`	`}`
`34`	`38`
	`39`	`+ params.ServiceManager = svcMgr`
	`40`	`+`
`35`	`41`	`return &params, nil`
`36`	`42`	`}`