Skip to content

Commit 39d8b9d

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-24583-7b12-index-plain-form
2 parents 4e5e331 + 81e6edd commit 39d8b9d

15 files changed

Lines changed: 2565 additions & 22 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
---
4+
5+
fix(driver-sql): `update()` no longer writes its `updated_at` stamp into the caller's `data`, so webhook deliveries for record updates are stored again on PostgreSQL and MySQL
6+
7+
Clause-②: no
8+
9+
- **What was lost.** On PostgreSQL and MySQL, every webhook delivery for a record update was lost. `SqlDriver.update()` assigned its `updated_at` stamp onto the patch object the caller passed in. ObjectQL publishes that same object as the update event's `changes`. On those two dialects the stamp is a knex `Raw` (`CURRENT_TIMESTAMP`) that holds the database client, its pool and a timer. The webhook outbox could not serialise the event (`Converting circular structure to JSON`), so no `sys_http_delivery` row was written. The record write still answered 200, and the webhook still read active. Every other reader of the update event's `changes`, and of the patch after the driver call, saw the same `Raw`. On SQLite the stamp is an ISO string, so the delivery was stored, but the caller's patch and the event's `changes` still carried an `updated_at` the caller never wrote.
10+
- **What changed.** `update()`, including its rotated-table path, stamps `updated_at` into a fresh object, as `updateMany()` already did. The caller's `data` is left exactly as it was passed.
11+
- **Unchanged.** The stored `updated_at` value on every dialect, `preserveAudit` imports, tables without an `updated_at` column, `upsert()` (it already stamps a copy of its input), and every export and option.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24835] `update()` leaves the caller's `data` argument exactly as it was
5+
* handed in — on every path that stamps `updated_at`.
6+
*
7+
* ## What was wrong
8+
*
9+
* `formatInput` and `applyWriteColumnMap` are copy-on-write: a patch that needs
10+
* no coercion comes back as the CALLER'S OWN OBJECT. `update()` and
11+
* `rotatedUpdateById()` then assigned the `updated_at` stamp onto it, so the
12+
* stamp landed in the caller's patch. ObjectQL hands `driver.update` the same
13+
* object it later publishes as `DataEvent.changes`, so every reader of the
14+
* update event saw a value the caller never wrote:
15+
*
16+
* - on PostgreSQL / MySQL the stamp is `knex.fn.now()`, a knex `Raw` that
17+
* holds the client, its pool and a Node `Timeout`. `JSON.stringify` of the
18+
* event payload throws ("Converting circular structure to JSON"), so the
19+
* webhook outbox could not store the delivery and EVERY webhook delivery for
20+
* a record update was lost there;
21+
* - on SQLite it is an ISO-8601 string — serialisable, which is why the loss
22+
* never showed there, and just as foreign to the caller's patch.
23+
*
24+
* `updateMany()` already stamped into a fresh object; these two doors now do
25+
* the same.
26+
*
27+
* ## What is pinned
28+
*
29+
* The caller's patch is deep-equal to a clone taken before the call, on each
30+
* path that stamps: the plain path (a table this driver's DDL created), the
31+
* presumed-timestamp path (`skipSchemaSync`-style registration, both when the
32+
* column exists and when the fallback has to drop the stamp), and the rotated
33+
* path. Each case also proves the stamp really ran on that path, so a pin
34+
* cannot pass because nothing was stamped at all. SQLite only: the mutation is
35+
* dialect-independent (only the stamped VALUE differs), and the live-PG/MySQL
36+
* driver lane does not run on pull requests.
37+
*/
38+
39+
import { describe, it, expect, afterEach } from 'vitest';
40+
import { SqlDriver } from './index.js';
41+
42+
/** These fixtures are not tenant-scoped. */
43+
const OPTS = { bypassTenantAudit: true } as const;
44+
const BACKDATED = '2020-01-01T00:00:00.000Z';
45+
46+
function sqlite(): SqlDriver {
47+
return new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
48+
}
49+
50+
/** Read `updated_at` straight out of storage, past every read-side coercion. */
51+
async function storedUpdatedAt(driver: SqlDriver, table: string, id: string): Promise<unknown> {
52+
const row: any = await (driver as any).knex(table).where('id', id).first();
53+
return row?.updated_at;
54+
}
55+
56+
/** Update with `patch`, and return what the caller's object looked like before and after. */
57+
async function updateAndCompare(driver: SqlDriver, object: string, id: string, patch: Record<string, unknown>) {
58+
const before = structuredClone(patch);
59+
await driver.update(object, id, patch, OPTS);
60+
return { before, after: patch };
61+
}
62+
63+
describe('#24835 — SqlDriver.update() never writes into the caller\'s data', () => {
64+
let driver: SqlDriver | undefined;
65+
afterEach(async () => {
66+
await driver?.disconnect();
67+
driver = undefined;
68+
});
69+
70+
it('plain path: a table the driver created stamps updated_at, and the patch is untouched', async () => {
71+
driver = sqlite();
72+
await driver.initObjects([{ name: 'n24835_plain', fields: { title: { type: 'text' } } } as any]);
73+
await driver.create('n24835_plain', { id: 'p1', title: 'seed' }, OPTS);
74+
await (driver as any).knex('n24835_plain').where('id', 'p1').update({ updated_at: BACKDATED });
75+
76+
const { before, after } = await updateAndCompare(driver, 'n24835_plain', 'p1', { title: 'renamed' });
77+
78+
expect(after).toStrictEqual(before);
79+
expect(Object.keys(after)).toEqual(['title']);
80+
// The stamp ran on this path — it landed in storage, not in the patch.
81+
expect(await storedUpdatedAt(driver, 'n24835_plain', 'p1')).not.toBe(BACKDATED);
82+
});
83+
84+
it('presumed path, column present: the speculative stamped attempt leaves the patch untouched', async () => {
85+
driver = sqlite();
86+
const knex = (driver as any).knex;
87+
await knex.raw(
88+
`create table "n24835_presumed" (id text primary key, title text, `
89+
+ `created_at text default (strftime('%Y-%m-%dT%H:%M:%fZ','now')), `
90+
+ `updated_at text default (strftime('%Y-%m-%dT%H:%M:%fZ','now')))`,
91+
);
92+
driver.registerObjectMetadata([{ name: 'n24835_presumed', fields: { title: { type: 'text' } } } as any]);
93+
await driver.create('n24835_presumed', { id: 'q1', title: 'seed' }, OPTS);
94+
await knex('n24835_presumed').where('id', 'q1').update({ updated_at: BACKDATED });
95+
expect((driver as any).updatedAtStampIsPresumed('n24835_presumed')).toBe(true);
96+
97+
const { before, after } = await updateAndCompare(driver, 'n24835_presumed', 'q1', { title: 'renamed' });
98+
99+
expect(after).toStrictEqual(before);
100+
expect(await storedUpdatedAt(driver, 'n24835_presumed', 'q1')).not.toBe(BACKDATED);
101+
// The presumption was settled by this very call.
102+
expect((driver as any).updatedAtStampIsPresumed('n24835_presumed')).toBe(false);
103+
});
104+
105+
it('presumed path, column absent: the fallback that drops the stamp leaves the patch untouched', async () => {
106+
driver = sqlite();
107+
const knex = (driver as any).knex;
108+
await knex.raw(`create table "n24835_nocol" (id text primary key, title text)`);
109+
driver.registerObjectMetadata([{ name: 'n24835_nocol', fields: { title: { type: 'text' } } } as any]);
110+
await knex('n24835_nocol').insert({ id: 'r1', title: 'seed' });
111+
expect((driver as any).updatedAtStampIsPresumed('n24835_nocol')).toBe(true);
112+
113+
const { before, after } = await updateAndCompare(driver, 'n24835_nocol', 'r1', { title: 'renamed' });
114+
115+
expect(after).toStrictEqual(before);
116+
expect((await knex('n24835_nocol').where('id', 'r1').first()).title).toBe('renamed');
117+
// The fallback path really ran: the driver now knows the column is absent.
118+
expect((driver as any).updatedAtColumnState.get('n24835_nocol')).toBe('absent');
119+
});
120+
121+
it('rotated path: a rotation-managed object stamps updated_at in its shard, and the patch is untouched', async () => {
122+
driver = sqlite();
123+
const rotated = {
124+
name: 'n24835_rot',
125+
fields: { payload: { type: 'text' }, created_at: { type: 'datetime' } },
126+
lifecycle: { class: 'telemetry', storage: { strategy: 'rotation', shards: 3, unit: 'day' } },
127+
};
128+
await driver.initObjects([rotated as any]);
129+
const shards: string[] | undefined = (driver as any).rotationShardsOf('n24835_rot');
130+
expect(shards?.length).toBeGreaterThan(0);
131+
await driver.create('n24835_rot', { id: 's1', payload: 'seed' }, OPTS);
132+
await (driver as any).knex(shards![0]).where('id', 's1').update({ updated_at: BACKDATED });
133+
134+
const { before, after } = await updateAndCompare(driver, 'n24835_rot', 's1', { payload: 'patched' });
135+
136+
expect(after).toStrictEqual(before);
137+
expect(await storedUpdatedAt(driver, shards![0], 's1')).not.toBe(BACKDATED);
138+
});
139+
});

‎packages/drivers/driver-sql/src/sql-driver.ts‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8834,11 +8834,19 @@ export class SqlDriver implements IDataDriver {
88348834
};
88358835

88368836
if (this.stampsUpdatedAt(object) && !this.keepSuppliedUpdatedAt(formatted, options)) {
8837-
formatted.updated_at = this.updatedAtStamp();
8837+
// [#24835] A FRESH object, never `formatted`: `formatInput` and
8838+
// `applyWriteColumnMap` are copy-on-write, so for a patch that needs no
8839+
// coercion `formatted` IS the caller's `data`. Assigning the stamp onto it
8840+
// put `updated_at` into the caller's patch — which ObjectQL publishes as
8841+
// `DataEvent.changes` — and on Postgres/MySQL that value is a knex `Raw`
8842+
// holding the client, its pool and a timer, so every reader that
8843+
// serialises the event (the webhook outbox first) threw. `updateMany`'s
8844+
// shape, below.
8845+
const stamped = { ...formatted, updated_at: this.updatedAtStamp() };
88388846
if (this.updatedAtStampIsPresumed(object)) {
8839-
await this.updateWithPresumedTimestamp(object, formatted, options, issue);
8847+
await this.updateWithPresumedTimestamp(object, stamped, options, issue);
88408848
} else {
8841-
await issue(formatted, options);
8849+
await issue(stamped, options);
88428850
}
88438851
} else {
88448852
await issue(formatted, options);
@@ -10262,13 +10270,16 @@ export class SqlDriver implements IDataDriver {
1026210270
// fact on the line immediately before that call — so on this path the
1026310271
// answer can never come from the declared-shape presumption, and there is
1026410272
// nothing speculative to recover from.
10265-
if (this.stampsUpdatedAt(object) && !this.keepSuppliedUpdatedAt(formatted, options)) {
10266-
formatted.updated_at = this.updatedAtStamp();
10267-
}
10273+
//
10274+
// [#24835] Stamped into a FRESH object, for the reason {@link update} gives:
10275+
// `formatted` may be the caller's own `data`.
10276+
const payload = this.stampsUpdatedAt(object) && !this.keepSuppliedUpdatedAt(formatted, options)
10277+
? { ...formatted, updated_at: this.updatedAtStamp() }
10278+
: formatted;
1026810279
for (const shard of shards) {
1026910280
const builder = this.getBuilder(shard, options).where('id', id);
1027010281
this.applyTenantScope(builder, object, options);
10271-
const count = await builder.update(formatted);
10282+
const count = await builder.update(payload);
1027210283
if (count > 0) {
1027310284
const readback = this.getBuilder(shard, options).where('id', id);
1027410285
this.applyTenantScope(readback, object, options);

‎packages/plugins/plugin-webhooks/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
"@objectstack/types": "workspace:*"
4343
},
4444
"devDependencies": {
45+
"@objectstack/driver-sql": "workspace:*",
4546
"@objectstack/metadata-core": "workspace:*",
4647
"@objectstack/objectql": "workspace:*",
4748
"@types/node": "^26.6.3",
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24835] An insert and a by-id update, on a REAL `SqlDriver` under a REAL
5+
* `ObjectQL` engine, with an active declared webhook, each store one
6+
* `sys_http_delivery` row whose `payload_json` is plain JSON.
7+
*
8+
* ## Why the whole chain on a real driver
9+
*
10+
* The loss this pins was invisible to every fake in this package. The driver's
11+
* `update()` used to stamp `updated_at` into the CALLER'S patch object, and
12+
* ObjectQL publishes that same object as `DataEvent.changes`. On PostgreSQL and
13+
* MySQL the stamp is a knex `Raw` holding the client, its pool and a Node
14+
* `Timeout`, so the outbox's `JSON.stringify(payload)` threw and no delivery
15+
* row was written for ANY record update — the record write still answered 200
16+
* and the webhook still read active. A fake driver never stamps, and a fake
17+
* outbox never serialises, so only this composition can see it:
18+
* `ObjectQL` → `SqlDriver` → realtime → `AutoEnqueuer` →
19+
* `MessagingService.enqueueHttp` → `SqlHttpOutbox` → `sys_http_delivery`.
20+
*
21+
* SQLite here. On SQLite the stamp was an ISO string, so the row WAS written,
22+
* and the assertion that would have caught the PG/MySQL loss is the plain-JSON
23+
* round trip below. The same body was run against a throwaway local
24+
* PostgreSQL 16 cluster when this landed (before: 1 row, the update lost;
25+
* after: 2 rows); no live-PG lane runs on pull requests, so that run is not
26+
* committed.
27+
*/
28+
29+
import { describe, expect, it } from 'vitest';
30+
import { ObjectQL } from '@objectstack/objectql';
31+
import { SqlDriver } from '@objectstack/driver-sql';
32+
import type { IRealtimeService, RealtimeEventHandler, RealtimeEventPayload } from '@objectstack/spec/contracts';
33+
import { HttpDelivery, MessagingService, SqlHttpOutbox, type EnqueueHttpInput } from '@objectstack/service-messaging';
34+
import { AutoEnqueuer } from './auto-enqueuer.js';
35+
import { DeclaredWebhookSource, declaredWebhookRefId } from './declared-webhooks.js';
36+
import { SysWebhook } from './sys-webhook.object.js';
37+
38+
const SYSTEM = { context: { isSystem: true } };
39+
40+
function driverUnderTest(): SqlDriver {
41+
return new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
42+
}
43+
44+
class FakeRealtime implements IRealtimeService {
45+
private subs = new Map<string, { handler: RealtimeEventHandler; opts?: any }>();
46+
private n = 0;
47+
async publish(event: RealtimeEventPayload): Promise<void> {
48+
for (const sub of this.subs.values()) {
49+
if (sub.opts?.object && event.object !== sub.opts.object) continue;
50+
await sub.handler(event);
51+
}
52+
}
53+
async subscribe(_channel: string, handler: any, opts?: any): Promise<string> {
54+
const id = `s-${++this.n}`;
55+
this.subs.set(id, { handler, opts });
56+
return id;
57+
}
58+
async unsubscribe(id: string): Promise<void> { this.subs.delete(id); }
59+
}
60+
61+
/** A declared webhook, as `defineStack({ webhooks })` would author it — active by default. */
62+
const DECLARED = {
63+
name: 'probe_task_changed',
64+
object: 'probe_task',
65+
triggers: ['create', 'update'],
66+
url: 'https://receiver.invalid/hook',
67+
method: 'POST',
68+
};
69+
70+
/** True when `value` is made only of what JSON can carry. */
71+
function isPlainJson(value: unknown): boolean {
72+
if (value === null || ['string', 'number', 'boolean'].includes(typeof value)) return true;
73+
if (Array.isArray(value)) return value.every(isPlainJson);
74+
if (typeof value !== 'object') return false;
75+
if (Object.getPrototypeOf(value) !== Object.prototype) return false;
76+
return Object.values(value as Record<string, unknown>).every(isPlainJson);
77+
}
78+
79+
async function waitFor(check: () => Promise<boolean>, ms = 3000): Promise<void> {
80+
const end = Date.now() + ms;
81+
while (Date.now() < end) {
82+
if (await check()) return;
83+
await new Promise((r) => setTimeout(r, 25));
84+
}
85+
}
86+
87+
describe('#24835 — a webhook delivery is stored for an update on a real SqlDriver', () => {
88+
it('an insert and a by-id update each store one delivery row whose payload_json is plain JSON', async () => {
89+
const driver = driverUnderTest();
90+
const engine = new ObjectQL();
91+
engine.registerDriver(driver, true);
92+
await engine.init();
93+
engine.registry.registerObject({ name: 'probe_task', label: 'Probe Task', fields: { title: { type: 'text', label: 'Title' } } } as any, 'test');
94+
engine.registry.registerObject(HttpDelivery as any, 'test');
95+
engine.registry.registerObject(SysWebhook as any, 'test');
96+
await engine.syncSchemas();
97+
98+
const realtime = new FakeRealtime();
99+
engine.setRealtimeService(realtime);
100+
const lost: string[] = [];
101+
const logger = {
102+
info() {}, debug() {},
103+
warn: (m: string) => { lost.push(m); },
104+
error: (m: string) => { lost.push(m); },
105+
};
106+
const messaging = new MessagingService({ logger: { info() {}, warn() {}, error() {}, debug() {} } as any });
107+
messaging.setHttpOutbox(new SqlHttpOutbox(engine as any, { partitionCount: 1 }));
108+
const enqueuer = new AutoEnqueuer(engine as any, realtime, (input: EnqueueHttpInput) => messaging.enqueueHttp(input), {
109+
refreshIntervalMs: 0,
110+
declaredWebhooks: new DeclaredWebhookSource(engine, { list: (type: string) => (type === 'webhook' ? [DECLARED] : []) }),
111+
logger,
112+
});
113+
const deliveries = async () => (await engine.find('sys_http_delivery', {}, SYSTEM)) as any[];
114+
115+
try {
116+
await enqueuer.start();
117+
const created: any = await engine.insert('probe_task', { title: 'seed' });
118+
await waitFor(async () => (await deliveries()).length >= 1);
119+
const patch = { title: 'webhook probe' };
120+
await engine.update('probe_task', patch, { where: { id: created.id } });
121+
await waitFor(async () => (await deliveries()).length >= 2);
122+
123+
const rows = await deliveries();
124+
expect(rows.map((r) => r.label).sort()).toEqual(['data.record.created', 'data.record.updated']);
125+
for (const row of rows) {
126+
expect(row.ref_id).toBe(declaredWebhookRefId('probe_task_changed'));
127+
expect(typeof row.payload_json).toBe('string');
128+
const parsed = JSON.parse(row.payload_json);
129+
expect(isPlainJson(parsed)).toBe(true);
130+
expect(JSON.stringify(parsed)).toBe(row.payload_json);
131+
expect(parsed).toMatchObject({ object: 'probe_task', recordId: created.id });
132+
}
133+
const updated = JSON.parse(rows.find((r) => r.label === 'data.record.updated').payload_json);
134+
expect(updated.action).toBe('updated');
135+
expect(updated.changes.title).toBe('webhook probe');
136+
expect(lost).toEqual([]);
137+
} finally {
138+
await enqueuer.stop();
139+
await driver.disconnect();
140+
}
141+
});
142+
});

‎packages/plugins/plugin-webhooks/vitest.config.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ export default defineConfig({
5454
// session-read rule). Same reason and the same anchoring as the
5555
// `core` entry above.
5656
{ find: /^@objectstack\/types$/, replacement: path.resolve(__dirname, '../../types/src/index.ts') },
57+
// [#24835] `webhook-delivery-real-sql-driver.test.ts` takes a VALUE
58+
// import on `@objectstack/driver-sql` (a real `SqlDriver` under the
59+
// engine), so the delivery verdict is about the driver source in the
60+
// checkout, not `driver-sql/dist`. Same reason and anchoring as the
61+
// `core` entry above.
62+
{ find: /^@objectstack\/driver-sql$/, replacement: path.resolve(__dirname, '../../drivers/driver-sql/src/index.ts') },
5763
],
5864
},
5965
});

0 commit comments

Comments
 (0)