Skip to content

Commit f2a1c0b

Browse files
os-zhuangclaude
andauthored
test(service-queue): pin db-queue-adapter's engine double to ObjectQL.delete's dispatch predicate (#5198) (#5533)
* test(service-queue): pin db-queue-adapter's engine double to ObjectQL.delete's dispatch predicate (#5198) The DEBT entry for `db-queue-adapter.test.ts` in `scripts/engine-double-contract.baseline.json` said pinning was blocked on a devDependency change. #5192 already made that change (it needed `@objectstack/objectql` to pin the new `job-queue-retention.test.ts` fake), so the entry's stated blocker no longer existed and its `why` would have misled the next reader. Replace the hand-mirrored `if (opts?.where?.id == null) throw` in the fake engine with `assertEngineDeleteDispatch(opts)` and route on the returned verdict, then delete the ledger entry. The mirror was looser than the producer in BOTH directions, which is why it is replaced rather than corrected in place: - it ACCEPTED `where: { id: { $in: [...] } }` and `where: { id: ['a','b'] }` without `multi` — multi-row predicates the real engine rejects; - it THREW on `{ multi: true }`, which the real engine accepts. Also drive the fake against the producer's own published case-set (`ENGINE_DELETE_DISPATCH_CASES`) so the double's fidelity is measured, not assumed: the gate proves the predicate is CALLED, these tests prove the call is answered. Restoring the mirror turns 5 of the 14 new tests red and the gate red with a PINNED error — both directions verified. Refs #5192, #5179, #4550, #4434 * test(service-queue): keep #4371's top-level `{ id }` rejection explicitly pinned The hand-mirrored `if` this PR removed was originally written for #4371 — a mock that accepted `delete(table, { id })` when the real engine reads `where.id`. `assertEngineDeleteDispatch` still refuses that bag, but it is not one of ENGINE_DELETE_DISPATCH_CASES, so nothing in the file asserted it any more. Assert it directly rather than let the property lapse with the code that used to carry it. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f0d98e1 commit f2a1c0b

2 files changed

Lines changed: 103 additions & 15 deletions

File tree

packages/services/service-queue/src/db-queue-adapter.test.ts

Lines changed: 103 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect, beforeEach } from 'vitest';
4+
import {
5+
ENGINE_DELETE_DISPATCH_CASES,
6+
ENGINE_DELETE_REJECT_MESSAGE,
7+
assertEngineDeleteDispatch,
8+
} from '@objectstack/objectql';
49
import { DbQueueAdapter } from './db-queue-adapter.js';
510

611
/**
@@ -54,15 +59,26 @@ function makeFakeEngine() {
5459
return r;
5560
},
5661
async delete(table: string, opts: any) {
57-
// Real-engine contract: the target id lives at `where.id` — there is no
58-
// top-level `id` option. The mock used to accept `opts.id`, a signature
59-
// the real engine rejects, which is exactly how the adapter's broken
60-
// `{ id }` bags stayed green (#4371 option-2 survey).
61-
const id = opts?.where?.id;
62-
if (id == null) throw new Error('Delete requires an ID or options.multi=true');
62+
// [#4550/#5198] Opened with ObjectQL.delete's OWN dispatch predicate.
63+
// What stood here was a hand-mirrored `if (opts?.where?.id == null)` —
64+
// written for #4371 to stop the mock accepting the top-level `{ id }`
65+
// bags the real engine rejects, and correct about that. But a mirror is a
66+
// second copy of the contract, and it was looser than the producer in
67+
// both directions: `where: { id: { $in: [...] } }` only LOOKS like an id
68+
// (a multi-row predicate the engine refuses without `multi`) and the
69+
// mirror waved it through, while `{ multi: true }` with no `where` is a
70+
// shape the engine ACCEPTS and the mirror threw on. A double that imports
71+
// the decision cannot drift from it; the same predicate already opens the
72+
// sibling `job-queue-retention.test.ts` fake in this package.
73+
const dispatch = assertEngineDeleteDispatch(opts);
6374
const t = tables.get(table) ?? [];
64-
tables.set(table, t.filter((r) => r.id !== id));
65-
return { id };
75+
if (dispatch.kind === 'multi') {
76+
const keep = t.filter((r) => !matches(r, opts?.where ?? {}));
77+
tables.set(table, keep);
78+
return t.length - keep.length; // drivers report a deleted count
79+
}
80+
tables.set(table, t.filter((r) => r.id !== dispatch.id));
81+
return { id: dispatch.id };
6682
},
6783
};
6884
}
@@ -232,3 +248,82 @@ describe('DbQueueAdapter', () => {
232248
expect(await adapter.getQueueSize('mix')).toBe(0);
233249
});
234250
});
251+
252+
// ---------------------------------------------------------------------------
253+
// The double itself, measured against the producer (#4550 / #5198)
254+
// ---------------------------------------------------------------------------
255+
//
256+
// Every assertion above is only worth what this fake's fidelity is worth: a
257+
// double that accepts a call the real engine refuses turns a green suite into
258+
// no suite at all, on exactly the path the double was introduced for (#4434).
259+
// So the fake is driven against ObjectQL.delete's OWN published case-set here,
260+
// rather than trusted because its `delete` now names the right function.
261+
//
262+
// This is what the deleted `scripts/engine-double-contract.baseline.json` DEBT
263+
// entry bought, and why the entry could go: the gate proves the predicate is
264+
// CALLED, and these two tests prove the call is answered — the by-id and multi
265+
// branches route by verdict, and every shape the engine rejects the fake
266+
// rejects, with the producer's own message.
267+
268+
describe('makeFakeEngine().delete conforms to ObjectQL.delete (#4550)', () => {
269+
it.each(ENGINE_DELETE_DISPATCH_CASES.map((c) => [c.what, c] as const))(
270+
'agrees with the engine on %s',
271+
async (_what, c) => {
272+
const engine = makeFakeEngine();
273+
engine.tables.set('sys_job_queue', [{ id: 'rec_1', rule_id: 'r1' }]);
274+
const call = engine.delete('sys_job_queue', c.options as any);
275+
if (c.expect === 'reject') {
276+
// Not merely "throws": the same message a real server answers with, so
277+
// the fake's rejection surface cannot drift from the producer's.
278+
await expect(call).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE);
279+
// …and a refused call must not have deleted anything on its way out.
280+
expect(engine.tables.get('sys_job_queue')).toHaveLength(1);
281+
return;
282+
}
283+
await expect(call).resolves.toBeDefined();
284+
},
285+
);
286+
287+
it('routes by the verdict, not by guessing at `where`', async () => {
288+
const engine = makeFakeEngine();
289+
const rows = () => engine.tables.get('sys_job_queue') ?? [];
290+
291+
// by-id: the scalar id is the ONLY row removed, siblings survive.
292+
engine.tables.set('sys_job_queue', [{ id: 'a' }, { id: 'b' }]);
293+
expect(await engine.delete('sys_job_queue', { where: { id: 'a' } })).toEqual({ id: 'a' });
294+
expect(rows().map((r: any) => r.id)).toEqual(['b']);
295+
296+
// multi: the predicate matches many, and the fake reports the count a
297+
// driver's `deleteMany` reports.
298+
engine.tables.set('sys_job_queue', [
299+
{ id: 'a', status: 'completed' },
300+
{ id: 'b', status: 'completed' },
301+
{ id: 'c', status: 'pending' },
302+
]);
303+
expect(
304+
await engine.delete('sys_job_queue', { where: { status: 'completed' }, multi: true }),
305+
).toBe(2);
306+
expect(rows().map((r: any) => r.id)).toEqual(['c']);
307+
308+
// `where: { id: { $in: […] } }` only LOOKS like an id: it is a multi-row
309+
// predicate, so without `multi` the engine rejects it — the exact case a
310+
// hand-mirrored `if (opts?.where?.id == null)` waves through, and the reason
311+
// the mirror had to go rather than be corrected in place. (This fake's
312+
// `matches` is equality-only by design — no fixture here sends an operator
313+
// predicate — so what is pinned is the dispatch verdict, not `$in` matching.)
314+
engine.tables.set('sys_job_queue', [{ id: 'a' }, { id: 'b' }, { id: 'c' }]);
315+
await expect(
316+
engine.delete('sys_job_queue', { where: { id: { $in: ['a', 'b'] } } }),
317+
).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE);
318+
expect(rows()).toHaveLength(3);
319+
320+
// The property the removed mirror was ORIGINALLY written for (#4371): a
321+
// top-level `{ id }` bag is not an address — the id lives at `where.id`.
322+
// It is not one of ENGINE_DELETE_DISPATCH_CASES, so it is asserted here
323+
// rather than left to lapse with the code that used to carry it.
324+
await expect(
325+
engine.delete('sys_job_queue', { id: 'a' } as any),
326+
).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE);
327+
expect(rows()).toHaveLength(3);
328+
});
329+
});

scripts/engine-double-contract.baseline.json

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,6 @@
241241
"why": "@objectstack/service-datasource does not depend on @objectstack/objectql. Pinning needs a devDependency + lockfile change, which is a separate reviewable act.",
242242
"closes": "add @objectstack/objectql to devDependencies, then open the fake's delete with assertEngineDeleteDispatch(opts)"
243243
},
244-
{
245-
"file": "packages/services/service-queue/src/db-queue-adapter.test.ts",
246-
"unguarded": 1,
247-
"kind": "DEBT",
248-
"why": "HAND-MIRRORS the guard already, which is the second copy of the contract this gate exists to remove — but @objectstack/service-queue does not depend on @objectstack/objectql, so replacing the copy with the producer's predicate needs a devDependency change.",
249-
"closes": "add @objectstack/objectql to devDependencies, then replace the mirrored `if` with assertEngineDeleteDispatch(options)"
250-
},
251244
{
252245
"file": "packages/spec/src/contracts/data-engine.test.ts",
253246
"unguarded": 1,

0 commit comments

Comments
 (0)