fix(economy): 表名不再走 ? 绑定 + tx.call 自调用契约#19
Conversation
Cherry-picked db-server fixes from closed PR #14 onto main@522d799. Modules already externalized; only platform handlers/domain. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
SOLID 审查(push 3d81a3e = merge #19)
ootb CI:通过(run 29898279376)。功能修复(表名不再 ? 绑定、dailyTasks 信封、DispatchError 保留 status、tx 自调用)方向正确。
跟进补丁已开:见本轮 fix PR(sql-helpers / tx requires 单点 / HttpDB.withModuleId)。
BLOCKER
无(本推送无新的运行时阻断;原先 near "?" 已由本 PR 消除)。
MAJOR
-
DRY —
domain/economy.ts本地sql()与lib/sql-helpers.raw双轨
#19正确避开了SQL\${TABLE}`,但未复用/扩展已有 sql-helpers。且raw()若写成SQL`FROM ${raw(t)}`**实测仍是FROM ?**(库只在.append时嵌套)。 → 已在 follow-up PR 把sql()` 提升为权威入口并加回归测试。 -
DRY / LSP —
TxRunner.doService与ServiceRegistry.dispatch双重 requires 门
自调用对齐是对的,但预检在「服务未注册」时把错误收成not_in_requires,HTTP 路径是no_such_service。
→ follow-up:requires 只走 dispatch;权限门仍留在 doService / service-routes。 -
DRY — SDK
withModuleId三抽屉复制(#13 引入 config 第三份)
→ follow-up:HttpDB.withModuleId。
MINOR
| 原则 | 项 |
|---|---|
| LSP | dailyTasks.submit 业务失败返 {ok:false},debit/credit 抛 DispatchError — 有意对齐消费方,但服务间错误契约不一致,建议文档化 |
| OCP | index.ts 仍 if (enabledSet.has("feature-economy")) register...(已知 S-M2) |
| LoD | handlers 挖 p.meta?.type / redpacketId |
原则速查(本轮)
| 原则 | 信号 |
|---|---|
| DRY | 本地 sql()、双重 requires、三份 withModuleId |
| LSP | tx vs HTTP 错误码;raw 插值脚枪 |
| OCP / DIP / LoD | 无本轮新增阻断;已知债见 docs/reviews/v2-solid-principles-review.md |
Sent by Cursor Automation: SFMC-Review
| * 表名是信任常量,必须嵌入 SQL 文本;值走 ? 绑定。 | ||
| * 不可对 sql-template-strings 插值表名(会变成 FROM ? → near "?": syntax error)。 | ||
| */ | ||
| function sql(text: string, values: unknown[] = []): { sql: string; values: unknown[] } { |
There was a problem hiding this comment.
MAJOR · DRY:本地 sql() 应落到 lib/sql-helpers 作为唯一权威入口。同目录已有 raw(),但 SQL\...${raw(table)}...`实测仍生成FROM ?(脚枪,与本 PR 要修的 bug 同类)。follow-up PR 已提升 sql()并加回归测试钉死 raw 只能.append`。
| step: TxStepService | ||
| ): Promise<TxStepResult> { | ||
| // 与 ServiceRegistry.dispatch 对齐:提供方自调用跳过 requires(DRY/LSP) | ||
| const providerId = this.deps.serviceRegistry.getProvider(step.name); |
There was a problem hiding this comment.
MAJOR · DRY / LSP:这里用 getProvider() 再预检 requires,与 ServiceRegistry.dispatch 重复。未注册 service 时本路径会先抛 not_in_requires,而 HTTP/dispatch 为 no_such_service。follow-up 已删除预检,requires/自调用只以 dispatch 为准;权限门仍保留。


Summary
Cherry-pick db-server fixes from closed PR #14 onto current main (
522d799).业务模块已外置到
Tanya7z/sfmc-modules,本 PR 只改平台侧 db-server handler/domain。Changes
economy.ts/economy-handlers.ts: 表名改为信任常量嵌入 SQL,值继续参数化(修复near "?": syntax error)tx-runner.ts/service-registry.ts:tx.call自调用跳过 requires,与 ServiceRegistry 对齐;保留领域错误 statusTest plan
npm run build --workspace db-servernode tools/check-ootb.mjs