Skip to content

✨ feat(schedule): 实现日程操作记录接口 - #174

Open
HuXiaohui424 wants to merge 1 commit into
1024XEngineer:mainfrom
HuXiaohui424:dev/173-record-operation
Open

✨ feat(schedule): 实现日程操作记录接口#174
HuXiaohui424 wants to merge 1 commit into
1024XEngineer:mainfrom
HuXiaohui424:dev/173-record-operation

Conversation

@HuXiaohui424

Copy link
Copy Markdown
Collaborator

结论

本 PR 实现日程创建、修改、删除操作的独立记录接口,保存操作前的 Schedule 快照,并在 mock 存储中限制最近 10 条记录。请 Reviewer 重点确认 previous 的领域模型边界、操作类型校验和有限历史记录策略。

Refs #173

变更

  • 新增日程操作记录参数校验及错误结果构造逻辑。
  • 新增进程内有界 mock 操作记录存储。
  • 自动生成操作记录 ID 和秒级操作时间。
  • OperationRecord::previousRecordScheduleOperationCommand::previous 改为 std::optional<Schedule>
  • 增加创建、修改、删除操作及容量淘汰测试。
  • 更新组件和主机测试 CMake 配置。

明确未包含:

  • 未将记录操作接入日程创建、修改、删除流程。
  • 未实现最近操作查询和操作撤销。
  • 未接入 SQLite 或真实持久化。
  • 未实现 Schedule 到数据库 JSON 字段的序列化。

架构与兼容

领域数据模型由不透明 JsonDocument 快照改为 std::optional<Schedule>,符合领域层保存完整日程实体、存储适配器负责 JSON 序列化的边界。未新增 Port、Profile 或外部协议,未改变组件依赖方向;现有日程 CRUD 行为不变。

验证

  • ./scripts/run_pre_submit_checks.sh
  • 远端 CI 的工作流、格式、IM Gateway、主机测试、架构、ESP-IDF 和 CodeQL 均通过;依赖图已启用时依赖审查也通过,未启用时已记录跳过原因
  • ESP-IDF 对应 Profile 构建
  • 真机或外部服务验证(如适用)

证据:

  • /usr/local/opt/llvm@18/bin/clang-format 格式检查通过。
  • CLANG_FORMAT=/usr/local/opt/llvm@18/bin/clang-format RUFF=/Users/mac/.local/share/uv/tools/ruff/bin/ruff ./scripts/check_format.sh 通过。
  • ./scripts/run_host_tests.sh 通过,22/22 测试通过。
  • 公共 C++ API 文档检查通过。
  • 架构依赖图检查通过。

TDD 记录

  • RED:新增 schedule_operation_test,在接口未实现时无法完成记录、校验和容量行为。
  • GREEN:实现操作参数校验、mock 存储、自动 ID/时间生成及 10 条记录上限后,测试通过。
  • REFACTOR:将校验逻辑和 mock 存储拆分为独立文件,并将操作前状态改为领域层 Schedule 快照。

风险与回退

  • 当前操作记录仅存在进程内,进程重启后会丢失;接入真实存储前不能作为持久化能力使用。
  • 查询和撤销接口仍未实现,当前提交只提供记录能力。
  • mock 存储为进程内共享状态,未来替换真实适配器时需保持 ID、时间和容量语义兼容。
  • 如需回退,可回退 commit 56c6dcd,不会影响现有日程 CRUD 行为。

@HuXiaohui424 HuXiaohui424 self-assigned this Aug 6, 2026
@HuXiaohui424 HuXiaohui424 added the MiniSpec 规格粒度-小改动的精简规格 label Aug 6, 2026
@HuXiaohui424 HuXiaohui424 linked an issue Aug 6, 2026 that may be closed by this pull request
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found in this review.

Verified with:

  • cmake -S tests/host -B /tmp/voicelife-host-review-174 && cmake --build /tmp/voicelife-host-review-174 --target schedule_operation_test && ctest --test-dir /tmp/voicelife-host-review-174 -R '^schedule_operation_test$' --output-on-failure
  • cmake --build /tmp/voicelife-host-review-174 && ctest --test-dir /tmp/voicelife-host-review-174 -L '^schedule$' --output-on-failure
  • ctest --test-dir /tmp/voicelife-host-review-174 --output-on-failure

@JunLang-7

Copy link
Copy Markdown
Collaborator

@fennoai 你是资深后端/全栈架构师 + 嵌入式硬件工程师复合型专家,执行严格、客观、不留情面的代码仓库Review,请遵循下面所有评审规则,逐条输出审查结果,禁止敷衍、禁止只说空话、禁止笼统概括。
硬性约束:最终输出评审条目数量不少于20条,若直观可见问题不足,主动挖掘隐性架构隐患、软硬件兼容风险、长期运行潜在缺陷补足条目,不得简化评审内容。

评审维度

  1. 架构与模块设计
  • 模块职责是否清晰,是否存在循环依赖、职责混杂
  • 分层是否合理,是否违反单一职责原则、开闭原则
  • 接口抽象、依赖注入设计是否规范,有无硬编码耦合
  • 软件与硬件模块边界是否清晰,硬件相关逻辑是否侵入业务层
  1. 代码规范与可读性
  • 命名:变量、函数、类、文件命名是否语义清晰,禁止模糊命名、拼音命名
  • 注释:复杂逻辑、硬件时序、特殊寄存器配置必须注释;冗余注释、无效注释、过期注释需要指出
  • 代码格式、风格是否统一,是否存在大量魔法数字、魔法字符串,硬件参数无常量定义
  1. 性能隐患
  • 循环内IO、数据库重复查询、不必要的内存占用、低效算法
  • 资源是否释放(连接、句柄、定时器、文件流、硬件外设句柄)
  • 嵌入式场景:阻塞轮询、中断处理耗时过长、内存频繁分配释放
  1. 安全性检查【重点】
  • 输入校验、SQL注入、XSS、权限控制、敏感信息明文打印
  • 密钥、token、数据库地址、硬件访问口令是否硬编码提交到仓库
  • 外部指令下发至硬件驱动缺少权限校验,存在设备失控风险
  1. 健壮性 & 异常处理
  • 是否缺少异常捕获、错误分支处理
  • 参数判空、边界条件、失败重试逻辑是否完备
  • 硬件通讯异常(I2C/SPI/UART断线、设备无应答)缺少容错、恢复逻辑
  • 缺少硬件故障状态上报、故障隔离机制
  1. 硬件驱动 & 软硬件协同评审(新增专项)
  • 驱动代码与硬件原理图引脚定义是否匹配,无硬件版本兼容逻辑
  • 外设操作缺少电平保护、超时判断,存在烧毁外设芯片风险
  • 运动控制逻辑(如有)缺少软限位、急停、碰撞检测保护
  • 上下位机通讯协议:缺少校验和、重传、断线重连机制
  • 硬件参数(电流、电压、速度阈值)直接写死代码,无统一配置入口
  1. 测试覆盖
  • 是否缺少单元测试、边界用例
  • 核心业务逻辑、硬件驱动逻辑无测试直接合并风险
  • 缺少硬件异常工况、极限压力场景测试用例
  1. 工程规范
  • 无用代码、注释掉的废弃代码是否大量遗留
  • 依赖包版本风险、冗余依赖、存在已知漏洞依赖
  • 配置区分环境(开发/样机/生产),环境配置是否混写
  • 固件、上位机、3D模型、原理图版本号不统一,无关联管理
  1. 可维护性 & 扩展性
  • 新增需求是否需要大面积修改原有代码
  • 是否存在大量重复代码,缺少公共封装
  • 硬件型号迭代时,软件需要大规模改动,缺少适配抽象层

输出要求

  1. 问题分级:🔴严重阻塞(必须修改才能合并/上机测试) / 🟡建议优化(不阻塞但强烈建议重构) / 🟢可选改进
  2. 每条问题固定格式:【文件路径+代码位置】问题描述 + 风险说明 + 可直接落地的修改方案
  3. 条目数量强制≥20条;问题较少时区分表层问题与隐性长期风险,拆分为独立条目逐条列出,禁止多条问题合并为一条
  4. 最后汇总三部分:
    ① 整体仓库风险总结
    ② 优先级整改清单(阻断项优先)
    ③ 长期架构、软硬件协同优化方案
  5. 如果存在架构层面缺陷,单独提炼顶层设计问题,不要只局限单行代码
  6. 不要美化结论,发现隐患直接指出,客观评估风险等级;不使用“建议考虑”这类温和模糊表述,明确写出故障后果(程序崩溃、硬件烧毁、机构失控、数据泄露等)

现在开始对提供的仓库代码/PR变更、硬件资料、结构模型进行评审

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Findings

  1. 🔴 [components/voicelife_schedule/src/schedule_service.cc:241-271] record_schedule_operation 只提供了一个独立 API,没有接入 create_scheduleupdate_scheduledelete_schedule 的成功分支。风险:真实日程变更不会自动落操作记录,后续最近操作查询和撤销链路会是空的。修改方案:把记录写入放进三类 CRUD 的成功路径,或者把“变更 + 记录”合并成一个原子服务流程。

  2. 🔴 [components/voicelife_schedule/src/schedule_operation_helpers.cc:46-50] 只校验 previous 是否存在,没有校验 previous->id == command.schedule_id。风险:调用方可以把别的日程快照挂到当前记录上,审计日志和撤销目标会直接错位。修改方案:把 previous.idschedule_id 做强一致校验。

  3. 🔴 [components/voicelife_schedule/src/schedule_operation_helpers.cc:46-54] 对 previous 只检查“有无”,没有检查快照内容是否完整、是否符合 Schedule 自身约束。风险:空 event、非法状态、错误时间区间都能进入历史记录,后续回放和撤销会污染数据。修改方案:复用日程实体校验,或者对 previous 单独做完整快照校验。

  4. 🔴 [components/voicelife_schedule/src/schedule_service.cc:248-255] previous 从命令里原样写入记录,没有从权威存储重新读取。风险:上层调用者可以伪造“操作前状态”,操作日志失真,审计链失去可信度。修改方案:记录层只接收 schedule_id 和操作类型,快照由服务内部从当前存储读取。

  5. 🟡 [components/voicelife_schedule/include/voicelife/schedule/schedule_types.h:45-52] OperationRecord 直接嵌入完整 Schedule,把审计模型和业务实体强绑定。风险:Schedule 任意字段演进都会外溢到操作记录迁移,后续持久化适配器成本会持续放大。修改方案:拆成专用的历史快照 DTO,或者把可撤销所需字段收敛到最小集合。

  6. 🟡 [components/voicelife_schedule/include/voicelife/schedule/schedule_commands.h:56-62] RecordScheduleOperationCommand 让外部调用方直接构造完整 Schedule。风险:created_atupdated_at 这类存储态字段会泄漏到边界层,HTTP/IM/工具调用端很难正确组装。修改方案:把快照构造留在服务内,命令只保留外部可提供的最小输入。

  7. 🔴 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:16-24] 模拟存储和自增 ID 都是文件级 static 状态。风险:所有 ScheduleService 实例共享同一份隐藏日志,测试互相污染,运行时多实例也无法隔离。修改方案:把存储对象显式注入,或者至少提供测试专用的清理/重置接口。

  8. 🔴 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:29-37] MockOperations()NextOperationId() 没有任何同步保护。风险:并发调用会产生重复 ID、deque 竞态甚至内存破坏。修改方案:用互斥锁保护整段写入,或者把实现替换成串行化的真实存储适配器。

  9. 🟡 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:12-13,29-32] 操作时间和 ID 直接绑定系统时钟,且没有注入点。风险:回放、重放、确定性测试都不可控,设备时间异常时记录顺序也会漂移。修改方案:把 clock/id generator 抽象成可注入依赖。

  10. 🟡 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:29-37] 时间粒度只有秒级。风险:同一秒内的多条操作会得到相同 operated_at,后续最近操作排序如果只看时间,会出现不稳定结果。修改方案:增加单调序号作为二级排序键,或提升时间分辨率。

  11. 🟢 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:40-42] LoadMockScheduleOperations() 每次都拷贝整份 deque。风险:当前上限虽小,但这是无必要的堆分配和拷贝开销。修改方案:若只是调试或测试读取,返回只读视图或迭代范围;如果要保留拷贝,明确注释这是有意的。

  12. 🔴 [components/voicelife_schedule/src/schedule_operation_mock_data.cc:29-37] 这个实现本质上还是进程内 mock,没有任何持久化恢复。风险:重启后操作记录全部丢失,不能支撑“最近操作”和“撤销”这类语义。修改方案:把这层替换成真实存储适配器,或者把 mock 严格限制在 host 测试编译目标。

  13. 🟡 [components/voicelife_schedule/include/voicelife/schedule/schedule_commands.h:56-62] 这个命令把完整 Schedule 暴露到调用边界,导致上层必须知道领域对象的内部字段。风险:接口难以从外部协议直接映射,后续新增字段会持续破坏调用端。修改方案:把记录命令设计成“最小输入 + 服务内补齐快照”。

  14. 🟡 [components/voicelife_schedule/test/schedule_operation_test.cc:45-87] 只验证了一个正常删除快照,没有覆盖 previous.id 错配、previous 内容不完整、或 previousschedule_id 不一致的失败路径。风险:核心审计边界回归时,这类脏数据会静默穿透。修改方案:补负例测试,把快照错配当成一等校验场景。

  15. 🟡 [components/voicelife_schedule/test/schedule_operation_test.cc:94-131] 没有覆盖多字节 UTF-8 的长度边界。风险:当前长度逻辑按 UTF-8 字符计数,中文/Emoji 场景一旦回归,单字节测试看不出来。修改方案:补一个 100/101 字符的多字节用例。

  16. 🟡 [components/voicelife_schedule/test/schedule_operation_test.cc:139-157] 退避/容量测试依赖进程内共享状态,没有提供 reset,测试顺序变化时会直接变脏。风险:以后并行跑例或复用同一进程时会出现不可复现的失败。修改方案:给 mock 存储加清理接口,测试 fixture 显式初始化和清空。

  17. 🟡 [components/voicelife_schedule/CMakeLists.txt:3-8, tests/host/CMakeLists.txt:47-56] mock 操作记录实现被直接编进了组件目标,生产侧也会携带这份进程内状态。风险:设备重启后历史全丢,而且生产二进制里混入了只适合测试的全局状态。修改方案:把 mock 实现放到 host-only target,生产目标切换到真实 adapter。

  18. 🟢 [components/voicelife_schedule/src/schedule_service.cc:244-253] 这里先做 ValidateRecordScheduleOperationCommand,后面又再 TrimScheduleText(command.schedule_event) 一次。风险:规范化逻辑复制了两份,后续任何改动都容易只改一处。修改方案:验证函数返回规范化后的文本,服务层只消费一次结果。

  19. 🟡 [components/voicelife_schedule/include/voicelife/schedule/schedule_types.h:45-52] OperationRecord 没有版本号或快照 schema 标识。风险:一旦 Schedule 字段发生演进,历史记录的解释方式会变得不可追踪。修改方案:给快照加版本字段,或者单独定义 versioned snapshot 结构。

  20. 🟡 [components/voicelife_schedule/src/schedule_operation_mock_data.h:10-21] 这个 mock 存储没有 reset API。风险:测试和未来的回放工具都只能依赖隐式全局状态,无法做确定性初始化。修改方案:补一个 ResetMockScheduleOperations(),并限制在测试构建中可见。

Overall Risk

① 整体仓库风险总结:这次变更能通过编译和当前新增 host 测试,但核心风险不在语法,而在边界设计。现在的实现仍然是进程内 mock + 全局静态状态,缺少真实持久化、并发保护和快照可信性,放到实际运行环境里,历史会丢、记录会串、撤销会失真。

② 优先级整改清单:先修 1/2/3/4/7/8/12 这几项,直接关系到记录是否真实、是否可恢复、是否会在并发下坏掉。然后补 14/15/16 的测试缺口,最后再清理 5/6/9/10/11/17/18/19/20 这些长期维护成本。

③ 长期架构、软硬件协同优化方案:把“操作记录”从业务服务里拆成独立的仓储端口,服务层只产生日程快照和意图,适配器负责落库、版本化和恢复;同时把时钟、ID 生成和存储对象注入化,避免全局静态状态进入固件主路径。若后面要接 SQLite 或其他本地存储,记录模型必须先定 versioned snapshot schema,再做 CRUD/undo 的原子事务。

验证:./scripts/run_host_tests.sh -R schedule_operation_test 通过。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MiniSpec 规格粒度-小改动的精简规格

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Record] 实现日程模块操作记录 [Schedule] 完善日程模块核心业务能力

2 participants