Skip to content

refactor(messaging): SOLID + DRY — chat access duplication, concrete Bridge dependency, global BotRegistry #500

@hrygo

Description

@hrygo

Background

internal/messaging 是 HotPlex 的消息层共享基础设施,提供平台适配器基座、Bot 注册表、交互管理、以及 SQLite/PG 双存储的 chat access 分类。模块包含 ~12 个 .go 源文件(不含 Slack/Feishu 子模块),在 PR 67 的 SOLID/DRY 重构后整体结构良好。

Scope: solid, dry, coupling — cycle 136 (模块分析通过 1)
Key files: chat_access_store.go, chat_access_pg_store.go, interaction.go, bot_registry.go, bridge.go, platform_types.go


Finding Summary

Category Critical High Medium Low
DRY 0 0 1 1
Coupling 0 0 2 0
合计 0 0 3 1

Findings

DRY

chat-access-classify-duplication

Severity: Medium | Confidence: High | ROI: Medium
Location: chat_access_store.go:81-135, chat_access_pg_store.go:57-115

Problem: Classify 方法在 SQLite 和 PG 实现之间完整复制。唯一差异是 receiver 类型名和 dialect.Rebind() 包装;50 行业务逻辑(冷却检查、飞书快速路径、Slack 回退)完全相同。冷却常量(3600s)在两份拷贝中独立硬编码。

Current Pattern:

// chat_access_store.go:81 & chat_access_pg_store.go:57
func (s *ChatAccessStore) Classify(ctx context.Context, platform, chatID, botID, userID string, lastMessageAtMs int64) ChatAccessType {
    cooldown := int64(3600) // hardcoded in both copies
    var lastCreatedAt int64
    err := s.db.QueryRowContext(ctx, `SELECT created_at ...`, ...).Scan(&lastCreatedAt)
    // ... identical 50-line logic ...
}

Proposed Fix:

type classifyQuerier interface {
    QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row
}

func classifyAccess(ctx context.Context, q classifyQuerier, queryWrapper func(string) string, platform, chatID, botID, userID string, lastMessageAtMs int64) ChatAccessType {
    cooldown := int64(3600)
    // ... single implementation ...
}

func (s *ChatAccessStore) Classify(ctx context.Context, ..._ ChatAccessType {
    return classifyAccess(ctx, s.db, func(q string) string { return q }, ...)
}
func (s *pgStore) Classify(ctx context.Context, ..._ ChatAccessType {
    return classifyAccess(ctx, s.db, s.dialect.Rebind, ...)
}

Acceptance Criteria:

  • Classify 业务逻辑只存在一份(共享函数或方法提取)
  • 冷却常量定义在单一位置
  • SQLite 和 PG 实现只包含方言差异(query wrapper)
  • 现有 TestClassify* 测试全部通过

make-envelope-vs-extract-platform-key-field-mapping

Severity: Low | Confidence: High | ROI: Medium
Location: bridge.go:128-150, platform_types.go:12-44

Problem: MakeEnvelope 手动映射 PlatformContext 字段到 metadata keys(bot_id, team_id, channel_id 等),ExtractPlatformKeys 用反向 switch 做相同映射。添加新平台字段需要编辑两处,存在字段名不一致风险。

Acceptance Criteria:

  • 前向/反向字段映射使用同一数据源(如 per-platform 字段注册表)
  • 添加新平台字段只需修改一处

Coupling

send-response-concrete-bridge-dependency

Severity: Medium | Confidence: High | ROI: High
Location: interaction.go:15, interaction.go:32-33

Problem: NewSendResponseFunc 依赖具体 *Bridge 类型而非接口。函数只调用 bridge.Handle(),但签名直接接受 *Bridge,阻止了单元测试中使用 mock bridge。影响 2 个调用点(feishu/interaction.go, slack/interaction.go)。

Current Pattern:

func NewSendResponseFunc(log *slog.Logger, bridge *Bridge, requestID, sessionID, ownerID string, conn PlatformConn) func(map[string]any) {
    return func(metadata map[string]any) {
        if bridge != nil {
            if err := bridge.Handle(respCtx, env, conn); err != nil { ... }
        }
    }
}

Proposed Fix:

type BridgeHandler interface {
    Handle(ctx context.Context, env *events.Envelope, pc PlatformConn) error
}

func NewSendResponseFunc(log *slog.Logger, bridge BridgeHandler, requestID, sessionID, ownerID string, conn PlatformConn) func(map[string]any) {
    // ... unchanged body ...
}

Acceptance Criteria:

  • NewSendResponseFunc 接受 BridgeHandler 接口而非 *Bridge
  • 飞书和 Slack 调用点适配(*Bridge 已满足接口)
  • 交互超时行为可使用 mock bridge 进行单元测试

global-default-bot-registry

Severity: Medium | Confidence: High | ROI: Medium
Location: bot_registry.go:123-127

Problem: BotRegistry 通过包级全局 defaultRegistryDefaultBotRegistry() 暴露单例。消费者共享可变状态,无法运行隔离测试。违反项目自身的 DIP 约定(全局状态应通过 DI 注入)。

Current Pattern:

var defaultRegistry = newBotRegistry()

func DefaultBotRegistry() *BotRegistry {
    return defaultRegistry
}

Proposed Fix:

// Inject via GatewayDeps instead of global
func NewBotRegistry() *BotRegistry {
    return &BotRegistry{entries: make(map[string]*BotEntry)}
}
// Remove DefaultBotRegistry; callers receive instance via DI

Acceptance Criteria:

  • DefaultBotRegistry() 移除或标记 deprecated
  • BotRegistry 通过 GatewayDeps DI 注入
  • 不依赖全局状态的隔离测试可行

Implementation Priority

Finding Priority Effort Risk Impact
send-response-concrete-bridge-dependency P0 Small Low 解锁交互超时单元测试,窄接口改动
chat-access-classify-duplication P1 Medium Low ~50 行消除,防 divergence
global-default-bot-registry P2 Medium Medium DI 改造需改动 gateway 初始化
make-envelope-field-mapping P3 Medium Low 防字段名不一致,低频改动

Recommended starting point: send-response-concrete-bridge-dependency — 5 分钟修复,添加 BridgeHandler 接口,*Bridge 已隐式满足接口无需改动。


Out of Scope

  • Slack/Feishu 子模块的独立分析(已有各自 findings_dropped 记录)
  • abortTriggers 全局 map(register + RWMutex 模式,安全的插件扩展点)
  • Bridge 依赖 session + worker 包(编排层的固有依赖,非耦合问题)
  • Record 方法 SQLite/PG 差异(dialect 抽象已处理有意义差异)

Verification

  • 所有现有 chat_access_*_test.go 测试通过
  • 飞书/Slack 交互超时行为不受影响
  • make test-race 通过

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Medium: tech debt, refactoring, improvementsarchitectureDomain: design patterns, coupling, separation of concernsarea/messagingScope: Slack/Feishu adapters, platform adapterrefactorRefactor: DRY, SOLID, code quality improvements

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions