Skip to content

refactor(service): windowsManager dead CommandRunner field + LogDir/logDirForHome DRY #523

@hrygo

Description

@hrygo

Background

internal/service 是跨平台系统服务管理模块(~600 LOC),通过 build tags 隔离 Linux/macOS/Windows 实现,导出 Manager 接口供 cmd/hotplex 调用。模块整体结构良好 — 平台隔离清晰,接口设计合理。

Scope: solid, dry, coupling — cycle 165 (模块分析通过 1)
Key files: service.go, manager_windows.go, templates.go


Finding Summary

Category Critical High Medium Low
SOLID 0 0 1 0
DRY 0 0 1 0
合计 0 0 2 0

Findings

SOLID

windows-manager-unused-command-runner

Severity: Medium | Confidence: High | ROI: High
Location: manager_windows.go:16-18, manager_windows.go:21

Problem: windowsManager 声明了 run CommandRunner 字段并在 NewManager() 中初始化,但从未使用。所有方法直接通过 mgr 包调用 Windows SCM API。该字段是误导性的死代码 — 让读者误以为结构体通过 CommandRunner 注入是可测试的。

Current Pattern:

type windowsManager struct {
	run CommandRunner  // never accessed
}

func NewManager() Manager {
	return &windowsManager{run: realRunner{}}
}

Proposed Fix:

type windowsManager struct{}

func NewManager() Manager {
	return &windowsManager{}
}

// If testability is desired later, inject an scmConnector interface:
// type scmConnector interface {
//     Connect() (*mgr.Mgr, error)
// }

Acceptance Criteria:

  • 移除 windowsManager.run 字段和初始化
  • 确认 Windows 构建仍然通过 (GOOS=windows go build ./...)
  • make test 通过

DRY

logdir-vs-logdirforhome-duplication

Severity: Medium | Confidence: High | ROI: Medium
Location: service.go:97-103, templates.go:107-126

Problem: LogDir()logDirForHome() 计算相同的路径逻辑(system 级别返回 systemLogDir(),user 级别返回 ~/.hotplex/logs),但作为独立函数存在。logDirForHome 仅为避免在 BuildLaunchdPlist 中重复调用 os.UserHomeDir() 而存在。

Current Pattern:

// service.go:97
func LogDir(level Level) string {
	if level == LevelSystem { return systemLogDir() }
	home, _ := os.UserHomeDir()
	return filepath.Join(home, ".hotplex", "logs")
}

// templates.go:121
func logDirForHome(level Level, homeDir string) string {
	if level == LevelSystem { return systemLogDir() }
	return filepath.Join(homeDir, ".hotplex", "logs")
}

Proposed Fix:

// service.go — LogDir delegates to logDirForHome
func LogDir(level Level) string { return logDirForHome(level, homeDir()) }

func logDirForHome(level Level, homeDir string) string {
	if level == LevelSystem { return systemLogDir() }
	return filepath.Join(homeDir, ".hotplex", "logs")
}

func homeDir() string {
	h, _ := os.UserHomeDir()
	return h
}

Acceptance Criteria:

  • LogDir 委托给 logDirForHome,路径逻辑只存在一处
  • 日志目录约定变更只需修改 logDirForHome
  • make test 通过

Implementation Priority

Finding Priority Effort Risk Impact
windows-manager-unused-command-runner P1 Trivial None 消除误导性代码,清晰表达意图
logdir-vs-logdirforhome-duplication P2 Small None 单点路径维护

Out of Scope

  • Logs 方法直接绑定 os.Stdout/os.Stderr — 终端透传命令,非库函数,抽象 ROI 低
  • Manager 7 方法接口 — 所有方法被调用者使用,拆分无实际收益
  • Restart Stop+Start 委托 — 仅 3 行,Linux 正确使用 systemctl restart

Verification

  • make test 通过
  • GOOS=windows go build ./... 通过
  • hotplex service install/start/stop/status/logs 在 macOS 上正常工作

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Medium: tech debt, refactoring, improvementsarchitectureDomain: design patterns, coupling, separation of concerns

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions