Skip to content

refactor(pkg/aep): Encode/EncodeJSON DRY violation + input Envelope mutation side-effect #525

@hrygo

Description

@hrygo

Background

pkg/aep 是 AEP v1 协议编解码包(1 文件,257 LOC),被 33 个文件跨 gateway/worker/messaging/client 导入。模块职责清晰、耦合单向(仅依赖 pkg/events 数据类型),但编码路径存在 DRY 违规和隐式可变性问题。

Scope: solid, dry, coupling — cycle 170 (模块分析通过 1)
Key files: pkg/aep/codec.go, pkg/aep/codec_test.go


Finding Summary

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

Findings

DRY

encode-encodejson-pipeline-duplication

Severity: Medium | Confidence: High | ROI: Medium
Location: codec.go:26-39, codec.go:176-186

Problem: EncodeEncodeJSON 逐字重复 version-fill + timestamp-fill + json.Marshal + escapeJSTerminators 流水线。唯一区别是输出方式(io.Writer + newline vs []byte)。若一条路径更新而另一条未同步,两个编码输出会静默偏离。

Current Pattern:

// codec.go:26-39 — Encode
func Encode(w io.Writer, env *events.Envelope) error {
    env.Version = events.Version
    if env.Timestamp == 0 {
        env.Timestamp = nowMillis()
    }
    data, err := json.Marshal(env)
    if err != nil {
        return fmt.Errorf("aep: marshal envelope: %w", err)
    }
    data = escapeJSTerminators(data)
    _, err = w.Write(append(data, '\n'))
    return err
}

// codec.go:176-186 — EncodeJSON (same pipeline, different output)
func EncodeJSON(env *events.Envelope) ([]byte, error) {
    env.Version = events.Version
    if env.Timestamp == 0 {
        env.Timestamp = nowMillis()
    }
    data, err := json.Marshal(env)
    // ... identical logic
}

Proposed Fix:

func marshalEnvelope(env *events.Envelope) ([]byte, error) {
    c := *env // shallow copy — see finding below
    c.Version = events.Version
    if c.Timestamp == 0 {
        c.Timestamp = nowMillis()
    }
    data, err := json.Marshal(&c)
    if err != nil {
        return nil, fmt.Errorf("aep: marshal envelope: %w", err)
    }
    return escapeJSTerminators(data), nil
}

func Encode(w io.Writer, env *events.Envelope) error {
    data, err := marshalEnvelope(env)
    if err != nil { return err }
    _, err = w.Write(append(data, '\n'))
    return err
}

func EncodeJSON(env *events.Envelope) ([]byte, error) {
    return marshalEnvelope(env)
}

Estimated Impact: ~12 行减少,单一编码路径,消除双路径偏离风险

Acceptance Criteria:

  • 提取 marshalEnvelope 共享编码流水线,EncodeEncodeJSON 均委托调用
  • 现有测试 TestEncodeTestEncodeJSON 无回归
  • 新增 TestMarshalEnvelope_VersionFill 验证 version + timestamp 填充

Coupling

encode-mutates-input-envelope

Severity: Medium | Confidence: High | ROI: Medium
Location: codec.go:27-29, codec.go:177-179

Problem: EncodeEncodeJSON 原地修改调用者的 *events.Envelope(设置 Version 和 Timestamp)。调用者若对同一 envelope 指针多次编码或编码后检查字段,会观察到非预期副作用。并发场景下共享 envelope 指针可能导致 data race。

Current Pattern:

// codec.go:27-29 — caller's struct is mutated
env.Version = events.Version
if env.Timestamp == 0 {
    env.Timestamp = nowMillis()
}

Proposed Fix: 在 marshalEnvelope 中执行浅拷贝(见上方 DRY fix 的 c := *env),代价极低(Envelope ~200 bytes),完全消除副作用。

Estimated Impact: 消除 33 个调用点的隐式可变性风险,启用并发安全编码

Acceptance Criteria:

  • marshalEnvelope 使用 c := *env 浅拷贝,不修改调用者的结构体
  • TestEncode_NoMutation 验证编码后原始 Envelope 字段不变
  • make test 无回归

Implementation Priority

Finding Priority Effort Risk Impact
encode-encodejson-pipeline-duplication P1 Small Low ~12 行减少,消除双路径偏离
encode-mutates-input-envelope P1 Small Low 消除 33 调用点隐式可变性

Recommended starting point: 两个发现合并为一次 PR — 提取 marshalEnvelope 并在其中使用浅拷贝,一次解决 DRY + 可变性。


Out of Scope

  • Decode/DecodeLine 共享 parse-then-validate 模式:仅 5 行逻辑,提取 ROI 过低
  • IsSessionBusy/IsTerminalEvent 等领域函数混入 codec:3-5 行纯函数,规模可接受
  • nowFunc 包级可变变量:当前无测试覆盖,理论风险

Verification

  • make test 通过,无回归
  • make lint 不产生新警告
  • 新增测试验证编码流水线和不可变性

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