Skip to content

Commit 9da4ca6

Browse files
committed
fix(metadata-protocol): deleteMany/updateMany 的 atomic 要么为真、要么拒绝 (#4620)
ADR-0119 D4 把 `batchData` 的 `atomic` 修成了真承诺,同一文件里的两个同胞 当时不在那次 PR 的确认范围内,缺陷原样留着: - `deleteManyData` 是假原子:`if (options?.atomic) break;` 没开任何事务, 失败前已删掉的行保持已删除,响应却以原子自居并把它们报成 success。比 `batchData` 那次更糟 —— 部分删除没有自然的撤销手段。 - `updateManyData` 根本不读 `atomic`:选项被接受、被声明成全有或全无,却从 未被读取,调用方拿到的是无任何信号的 best-effort。 两者现在走与 `batchData` 完全相同的原子臂 —— 抽成一个共享 runner (`runAtomicBatch`),而不是把事务处理再抄两份(抄写正是下一个同胞漂移回 谎言的路径): - `atomic: true` 时整批跑在一个 `engine.transaction()` 里,首个失败回滚此前 全部写入; - 回滚批次报告零成功,行分别标记 `ROLLED_BACK:` / `NOT_ATTEMPTED:`,致因行 保留原始错误,客户端可区分「已尝试但被撤销」与「从未执行」; - `atomic` 优先于 `continueOnError`; - 无法回滚的运行时(无 `engine.transaction()`,或默认 driver 无 `beginTransaction`)对 `atomic: true` 返回 501 NOT_IMPLEMENTED,而不是静默 降级成 best-effort —— 静默降级正是本 issue 的缺陷类别。 非原子路径(含 `continueOnError` 交互与响应形状)保持不变;`batchData` 的 既有测试未作任何改动仍全部通过。 逐行结果形状与 `BatchOperationResultSchema` 的分歧(issue 第 3 节)刻意未动: 那是公开 wire 契约决定,单独跟踪。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ
1 parent 941dec4 commit 9da4ca6

4 files changed

Lines changed: 556 additions & 15 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): `deleteMany` / `updateMany` honour `atomic` for real, or refuse it (#4620)
6+
7+
ADR-0119 D4 made `batchData`'s `atomic` flag a real guarantee. Its two siblings
8+
in the same file were out of that PR's confirmed scope and kept the defect:
9+
10+
- **`deleteManyData` was fake-atomic.** `atomic: true` opened no transaction; it
11+
only `break`-ed the loop, so every row deleted before the failure stayed
12+
**deleted** while the response called itself atomic and reported those rows
13+
`success: true`. Worse than the `batchData` case it was copied from, because a
14+
partial delete has no natural undo — a client cannot reconstruct the rows from
15+
its own request.
16+
- **`updateManyData` ignored `atomic` entirely.** The option was accepted,
17+
declared in `BatchOptionsSchema` with an all-or-nothing contract, and never
18+
read: a caller asking for atomicity silently got best-effort, with no signal.
19+
20+
Both now run the **same** atomic arm as `batchData`, extracted into one shared
21+
runner so a fourth copy of transaction handling cannot drift into a fourth lie:
22+
23+
- `atomic: true` runs the whole batch inside ONE `engine.transaction()`; the
24+
first failure rolls back every prior write.
25+
- A rolled-back batch reports **zero successes**. Rows that had succeeded are
26+
marked `ROLLED_BACK: record <i> failed — <cause>`, rows never reached are
27+
`NOT_ATTEMPTED: atomic batch aborted by record <i>`, and the causal row keeps
28+
its own error — so a client can tell "attempted, undone" from "never ran".
29+
- `atomic` outranks `continueOnError`, whose contract text already scoped it to
30+
`atomic=false`.
31+
32+
**Behaviour change to be aware of:** a runtime that cannot roll back (no
33+
`engine.transaction()`, or a default driver without `beginTransaction`) now
34+
**refuses** an `atomic: true` `deleteMany` / `updateMany` with `501
35+
NOT_IMPLEMENTED` instead of silently running best-effort — the same fail-closed
36+
gate `batchData` uses. That silent downgrade is the defect class this fixes; if
37+
you want best-effort, ask for it (`atomic: false`, or omit the option), or probe
38+
the runtime's transaction support before sending. Non-atomic behaviour of both
39+
endpoints — including the `continueOnError` interaction and their response
40+
shapes — is unchanged.

packages/metadata-protocol/src/protocol.delete-many.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,19 @@ describe('deleteManyData — partial-failure semantics (#3897)', () => {
145145
expect(res).toMatchObject({ success: false, total: 3, succeeded: 2, failed: 1 });
146146
});
147147

148-
it('atomic aborts the remaining ids on the first failure', async () => {
148+
// [#4620] This used to pin the fake-atomic: `atomic: true` merely broke the
149+
// loop, so `a` stayed DELETED and the response reported `succeeded: 1` under
150+
// a flag whose one job is to guarantee it was undone. On this engine — no
151+
// `transaction()` at all — the honest answer is a refusal, not a half-batch.
152+
// Real rollback is pinned in protocol.many-data-atomic.test.ts.
153+
it('atomic REFUSES on an engine that cannot roll back, deleting nothing (#4620)', async () => {
149154
const { p, del } = failOn('b');
150-
const res: any = await p.deleteManyData({
155+
await expect(p.deleteManyData({
151156
object: 'invoice',
152157
ids: ['a', 'b', 'c'],
153158
options: { atomic: true, continueOnError: true },
154-
} as any);
159+
} as any)).rejects.toMatchObject({ status: 501, code: 'NOT_IMPLEMENTED' });
155160

156-
expect(del).toHaveBeenCalledTimes(2);
157-
expect(res.succeeded).toBe(1);
161+
expect(del).not.toHaveBeenCalled();
158162
});
159163
});

0 commit comments

Comments
 (0)