diff --git a/.changeset/22062-email-template-boot-sweep.md b/.changeset/22062-email-template-boot-sweep.md new file mode 100644 index 00000000000..2a094c70c53 --- /dev/null +++ b/.changeset/22062-email-template-boot-sweep.md @@ -0,0 +1,27 @@ +--- +'@objectstack/plugin-email': patch +--- + +The declared-email-template boot sweep reads the stored rows in bulk and no longer rewrites a template that has not changed + +Clause-②: no + +Every boot used to look each effective email template up on its own and rewrite its +`sys_email_template` row unconditionally: one lookup, one UPDATE and the engine's two read-backs +per template, whether or not anything had changed. Measured on `ObjectQL` over `SqlDriver` +(better-sqlite3), a steady boot over 450 unchanged templates sent 1,800 statements; it now sends 3 +reads and no write (79 templates: 316 statements, now 1). + +- The sweep reads the stored rows for the declared names in `$in` pages of 200 names. Each + `(name, locale)` takes the first row the read returns, in the order the driver gives the + per-template lookup, so a slot several organizations hold resolves to the same row as before. +- A template the bulk read did not answer (new since the last boot, a read cut short at its row + bound, or a failed read) is looked up on its own before anything is inserted, as before. +- A `managed_by: 'package'` row that already holds every column the template projects is not + rewritten. `upsertDeclaredEmailTemplate` now returns `false` for it, and + `bootstrapDeclaredEmailTemplates` counts it under `skipped`, the value both already document for + a row deliberately not written. A row with any other provenance is handled exactly as before: + admin-owned and customized rows are never written, and a legacy row with no `managed_by` is + rewritten and adopted. + +An org-scoped template edit still survives the next boot. Signatures and accepted input are unchanged. diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 7afde0e2bf5..96cfa145fef 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -246,8 +246,8 @@ cannot read, and they are neither in nor out. | object name spelled inline | 104 | | object name spelled through a `const` | 51 | -| object name is an `object: string` parameter | 17 | -| object name is some other run-time expression | 61 | +| object name is an `object: string` parameter | 19 | +| object name is some other run-time expression | 59 | ### Subtractions the census could NOT defend — enforced @@ -297,13 +297,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-06 at `2bea8b684`. +Measured on 2026-10-07 at `e1171ca48`. | corpus scale (not enforced) | count | | :--- | ---: | | tracked non-test sources scanned | 612 | | engine-shaped types recognised | 70 | | declared objects in the registry | 116 | -| same-named calls subtracted as non-engine | 159 | +| same-named calls subtracted as non-engine | 160 | {/* END GENERATED: tenant-audit-census */} diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 598d6061259..abbf102cbbb 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -90,14 +90,14 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-06 at `2bea8b684`. +Measured on 2026-10-07 at `e1171ca48`. | corpus scale (not enforced) | count | | :--- | ---: | | tracked non-test sources scanned | 612 | | engine-shaped types recognised | 70 | | declared objects in the registry | 116 | -| same-named calls subtracted as non-engine | 159 | +| same-named calls subtracted as non-engine | 160 | ## Every site diff --git a/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.test.ts b/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.test.ts index 5108328b6dd..c51775ad499 100644 --- a/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.test.ts +++ b/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.test.ts @@ -16,6 +16,8 @@ import { upsertDeclaredEmailTemplate, deactivateDeclaredEmailTemplate, mapTemplateToRow, + SWEEP_NAMES_PER_READ, + SWEEP_ROWS_PER_READ, } from './bootstrap-declared-email-templates.js'; import { bindEmailTemplateProvenanceStamp } from './email-template-provenance.js'; @@ -54,14 +56,20 @@ class FakeEngine { private matches(row: any, cond?: Record): boolean { if (!cond) return true; - return Object.entries(cond).every(([k, v]) => row[k] === v); + // [#22062] `$in` as the real engine reads it: the boot sweep's bulk read. + return Object.entries(cond).every(([k, v]) => + v && typeof v === 'object' && Array.isArray(v.$in) ? v.$in.includes(row[k]) : row[k] === v); } async find(name: string, q?: any): Promise { const all = this.rows[name] ?? []; const cond = q?.filter ?? q?.where; const out = all.filter((r) => this.matches(r, cond)); - return typeof q?.limit === 'number' ? out.slice(0, q.limit) : out; + // [#22062] Rows leave as COPIES, as a real driver's do. Handing out the + // stored objects let a later write reach into a row the caller had + // already read, which no real read does — and it hid a stale copy in the + // sweep's bulk read from every pin here. + return (typeof q?.limit === 'number' ? out.slice(0, q.limit) : out).map((r) => ({ ...r })); } async insert(name: string, data: any): Promise { const arr = (this.rows[name] = this.rows[name] ?? []); @@ -573,3 +581,206 @@ describe('bootstrapDeclaredEmailTemplates — the effective template (#21785)', expect(warn).toHaveBeenCalledTimes(1); }); }); + +// --------------------------------------------------------------------------- +// [#22062] What a boot costs: a bulk read, and no write for an unchanged row +// --------------------------------------------------------------------------- + +/** + * Before, the sweep looked every template up on its own and rewrote its row + * unconditionally: on a steady boot, one lookup, one UPDATE and the engine's two + * read-backs per template. These pin the three halves of the fix: the bulk read + * (and what happens to a key it does not answer), the compare before the write + * (and the controls that must still be written), and the provenance rules the + * compare must not touch. The row-choice pin over several organizations runs on + * the real engine and SQL driver, in `packages/qa/dogfood` + * (`email-template-boot-sweep.test.ts`): this package cannot import a driver. + */ +describe('[#22062] the boot sweep reads stored rows in bulk and rewrites only what changed', () => { + const isBulkRead = (q: any) => Array.isArray(q?.where?.name?.$in); + + /** One template carrying a string, a boolean, an optional column and `variables_json`. */ + const fullTemplate = () => declaredTemplate({ bodyText: 'Plain body', description: 'Reset mail' }); + + /** Boot once, then change the stored row the way a case says. `before` is the row as the boot left it. */ + async function steadyWith(mutate: (row: any) => void) { + const engine = new FakeEngine({ declared: { email_template: [fullTemplate()] } }); + await bootstrapDeclaredEmailTemplates(engine as any, undefined); + const before = { ...rowsOf(engine)[0] }; + mutate(rowsOf(engine)[0]); + return { engine, before }; + } + + it('a boot over unchanged templates writes nothing and reads once per 200 names', async () => { + const declared = Array.from({ length: 450 }, (_, i) => declaredTemplate({ name: `tpl.n${i}` })); + const engine = new FakeEngine({ declared: { email_template: declared } }); + await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + const find = vi.spyOn(engine, 'find'); + const update = vi.spyOn(engine, 'update'); + const insert = vi.spyOn(engine, 'insert'); + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 0, skipped: 450 }); + expect(update).not.toHaveBeenCalled(); + expect(insert).not.toHaveBeenCalled(); + // ceil(450 / 200): the bulk reads only. A steady boot has no key the bulk + // read leaves unanswered, so no per-template lookup is added. + expect(SWEEP_NAMES_PER_READ).toBe(200); + expect(find).toHaveBeenCalledTimes(3); + expect(find.mock.calls.every(([, q]) => isBulkRead(q))).toBe(true); + expect(rowsOf(engine)).toHaveLength(450); + }); + + it.each([ + ['a string column (`subject`)', (r: any) => { r.subject = 'Stale wording'; }, 'subject'], + ['a boolean column holding the other boolean (`active`)', (r: any) => { r.active = false; }, 'active'], + ['a boolean column holding the other 0/1 (`active`)', (r: any) => { r.active = 0; }, 'active'], + ['an optional column that is null (`body_text`)', (r: any) => { r.body_text = null; }, 'body_text'], + ['an optional column that is absent (`body_text`)', (r: any) => { delete r.body_text; }, 'body_text'], + ['`variables_json` holding other text', (r: any) => { r.variables_json = '[]'; }, 'variables_json'], + [ + '`variables_json` handed back parsed, and different', + (r: any) => { r.variables_json = [{ name: 'other', type: 'string', required: false }]; }, + 'variables_json', + ], + ] as const)('control: rewrites a package row whose projected column differs: %s', async (_case, mutate, column) => { + const { engine, before } = await steadyWith(mutate); + const update = vi.spyOn(engine, 'update'); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 1, skipped: 0 }); + expect(update).toHaveBeenCalledTimes(1); + expect(rowsOf(engine)[0][column]).toEqual(before[column]); + }); + + it.each([ + ['a boolean stored as 1 (`active: true`)', (r: any) => { r.active = 1; }], + ['a boolean stored as 0 (`is_system: false`)', (r: any) => { r.is_system = 0; }], + ['`variables_json` handed back parsed', (r: any) => { r.variables_json = JSON.parse(r.variables_json); }], + ['a column the projection omits, holding a value (`from_address`)', (r: any) => { r.from_address = 'ops@example.com'; }], + ] as const)('leaves a package row that already holds the projection unwritten: %s', async (_case, mutate) => { + const { engine } = await steadyWith(mutate); + const update = vi.spyOn(engine, 'update'); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 0, skipped: 1 }); + expect(update).not.toHaveBeenCalled(); + }); + + it('still adopts a legacy row with no `managed_by`, even one that already holds the projection', async () => { + const { engine } = await steadyWith((r) => { delete r.managed_by; }); + const update = vi.spyOn(engine, 'update'); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 1, skipped: 0 }); + expect(update).toHaveBeenCalledTimes(1); + expect(rowsOf(engine)[0].managed_by).toBe('package'); + }); + + it('keeps the first row a key meets in the bulk read: the row the per-template lookup returns', async () => { + const stale = (id: string, organization_id: string) => ({ + id, organization_id, name: 'auth.password_reset', locale: 'en-US', subject: `stale ${id}`, managed_by: 'package', + }); + const engine = new FakeEngine({ + rows: { [TABLE]: [stale('etpl_first', 'org_b'), stale('etpl_second', 'org_a')] }, + declared: { email_template: [declaredTemplate()] }, + }); + const [chosen] = await engine.find(TABLE, { where: { name: 'auth.password_reset', locale: 'en-US' }, limit: 1 }); + expect(chosen.id).toBe('etpl_first'); + + await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(rowsOf(engine).map((r) => [r.id, r.subject])).toEqual([ + ['etpl_first', PACKAGE_WORDING], + ['etpl_second', 'stale etpl_second'], + ]); + }); + + it('looks a key the bulk read did not answer up on its own BEFORE inserting anything', async () => { + const engine = new FakeEngine({ declared: { email_template: [declaredTemplate({ name: 'ops.new' })] } }); + const find = vi.spyOn(engine, 'find'); + const insert = vi.spyOn(engine, 'insert'); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 1, skipped: 0 }); + expect(find).toHaveBeenCalledTimes(2); + expect(isBulkRead(find.mock.calls[0][1])).toBe(true); + expect(find.mock.calls[1][1]).toMatchObject({ where: { name: 'ops.new', locale: 'en-US' }, limit: 1 }); + expect(insert).toHaveBeenCalledTimes(1); + expect(find.mock.invocationCallOrder[1]).toBeLessThan(insert.mock.invocationCallOrder[0]); + }); + + it('never inserts a row twice when a bulk read is cut short at its bound', async () => { + // Other locales of the same name fill the bound, stored BEFORE the declared + // slot's own row, so the bulk read stops exactly where that row would come. + const filler = Array.from({ length: SWEEP_ROWS_PER_READ }, (_, i) => ({ + id: `etpl_fill_${i}`, name: 'auth.password_reset', locale: `x-${i}`, subject: 'Other locale', managed_by: 'package', + })); + const engine = new FakeEngine({ rows: { [TABLE]: filler }, declared: { email_template: [declaredTemplate()] } }); + expect(await bootstrapDeclaredEmailTemplates(engine as any, undefined)).toEqual({ seeded: 1, skipped: 0 }); + // ANTI-VACUITY: the bulk read really does not reach the slot's row. + const page = await engine.find(TABLE, { + where: { name: { $in: ['auth.password_reset'] } }, + limit: SWEEP_ROWS_PER_READ, + }); + expect(page.some((r) => r.locale === 'en-US')).toBe(false); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(result).toEqual({ seeded: 0, skipped: 1 }); + expect(rowsOf(engine).filter((r) => r.locale === 'en-US')).toHaveLength(1); + expect(rowsOf(engine)).toHaveLength(SWEEP_ROWS_PER_READ + 1); + }); + + it('looks every template up on its own when the bulk read fails, inserting nothing twice', async () => { + class FailingBulkRead extends FakeEngine { + override async find(name: string, q?: any): Promise { + if (Array.isArray(q?.where?.name?.$in)) throw new Error('bulk read failed'); + return super.find(name, q); + } + } + const declared = [declaredTemplate(), declaredTemplate({ name: 'ops.digest', category: 'notification' })]; + const engine = new FailingBulkRead({ declared: { email_template: declared } }); + const warn = vi.fn(); + expect(await bootstrapDeclaredEmailTemplates(engine as any, undefined, { warn })).toEqual({ seeded: 2, skipped: 0 }); + + const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined, { warn }); + + expect(result).toEqual({ seeded: 0, skipped: 2 }); + expect(rowsOf(engine)).toHaveLength(2); + // Said once per boot, never once per template. + expect(warn).toHaveBeenCalledTimes(2); + expect(warn.mock.calls[1][1]).toEqual({ error: 'bulk read failed' }); + }); + + it('meets the row as it now is when the list names one slot twice (a package entry, then its env-wide overlay)', async () => { + const overlay = declaredTemplate({ subject: OVERLAY_WORDING }); + const engine = new FakeEngine({ declared: { email_template: [overlay] } }); + await bootstrapDeclaredEmailTemplates(engine as any, undefined); + // The registry lists an env-wide overlay AFTER the package entry it overrides. + (engine as any).declared = { email_template: [declaredTemplate(), overlay] }; + + await bootstrapDeclaredEmailTemplates(engine as any, undefined); + + expect(rowsOf(engine)).toHaveLength(1); + expect(rowsOf(engine)[0].subject).toBe(OVERLAY_WORDING); + }); + + it('the live door shares the write step: an unchanged save writes nothing, a changed one writes', async () => { + const engine = new FakeEngine(); + expect(await upsertDeclaredEmailTemplate(engine as any, declaredTemplate())).toBe(true); + const update = vi.spyOn(engine, 'update'); + + expect(await upsertDeclaredEmailTemplate(engine as any, declaredTemplate())).toBe(false); + expect(update).not.toHaveBeenCalled(); + + expect(await upsertDeclaredEmailTemplate(engine as any, declaredTemplate({ subject: 'Saved again' }))).toBe(true); + expect(update).toHaveBeenCalledTimes(1); + expect(rowsOf(engine)[0].subject).toBe('Saved again'); + }); +}); diff --git a/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts b/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts index ea634fbebbe..04568c89c62 100644 --- a/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts +++ b/packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts @@ -265,11 +265,141 @@ export interface BootstrapDeclaredEmailTemplatesResult { skipped: number; } +/** + * [#22062] How many declared names one bulk read of the boot sweep carries in + * its `$in` list. Module-internal; exported for this package's own tests. + */ +export const SWEEP_NAMES_PER_READ = 200; + +/** + * [#22062] The row bound of one bulk read. Module-internal; exported for this + * package's own tests. + * + * The bound is not a completeness claim. A read that comes back holding this + * many rows may have been cut short, and a key the read did not answer is never + * taken as "no row": the sweep looks that template up on its own before it + * inserts anything (see {@link bootstrapEffectiveEmailTemplates}). Five rows per + * name is room for the usual locale and organization spread, so a steady boot + * pays that fallback only where a name really carries more rows than that. + * + * The bound also does a second job: it makes the read PAGED. A paged read with + * no `orderBy` is the very shape of the per-template lookup + * (`find(…, { where: { name, locale }, limit: 1 })`), so a driver orders both + * the same way, and the first row a key meets in the bulk read is the row that + * lookup returns. Measured on `ObjectQL` over `SqlDriver` (better-sqlite3): both + * statements go out as `… order by id asc limit ?`; the same read with no + * `limit` gets no ORDER BY at all and walks rows in storage order, which chose + * a different row for a `(name, locale)` held by several organizations. + */ +export const SWEEP_ROWS_PER_READ = SWEEP_NAMES_PER_READ * 5; + +/** [#22062] A stored template's key: a template is unique per `(name, locale)`. */ +function templateRowKey(name: unknown, locale: unknown): string { + return JSON.stringify([name, locale]); +} + +/** + * The per-template lookup: the stored row for one `(name, locale)`, or + * `undefined`. The live doors read through it, and so does the boot sweep for + * every key its bulk read did not answer. + */ +async function findTemplateRow( + engine: IDataEngine, + object: string, + name: string, + locale: string | undefined, +): Promise { + const existing = await (engine as any).find(object, { + where: { name, locale }, + limit: 1, + context: SYSTEM_CTX, + }); + return Array.isArray(existing) ? existing[0] : (existing as any)?.data?.[0]; +} + +/** + * [#22062] Read the stored rows for the declared names in bulk, one read per + * {@link SWEEP_NAMES_PER_READ} names, instead of one lookup per template. + * + * Each key keeps the FIRST row it meets, in the read's own order — the order + * the per-template lookup is answered in (see {@link SWEEP_ROWS_PER_READ}). A + * read cut short at its bound keeps that property for every key it answered: + * the rows it returned are a prefix of one order, so a key's first row is in + * it whenever any of its rows is. + * + * Answers `undefined` when a read fails or does not answer a row list. The + * sweep then looks every template up on its own, exactly as it did before this + * read existed — the read is an optimization, and no answer is made up for it. + */ +async function readStoredTemplateRows( + engine: IDataEngine, + declared: unknown[], + object: string, + logger: Logger | undefined, +): Promise | undefined> { + const names = [...new Set(declared.map((item) => (item as { name?: unknown } | null)?.name))] + .filter((name): name is string => typeof name === 'string'); + const byKey = new Map(); + try { + for (let start = 0; start < names.length; start += SWEEP_NAMES_PER_READ) { + const found = await (engine as any).find(object, { + where: { name: { $in: names.slice(start, start + SWEEP_NAMES_PER_READ) } }, + limit: SWEEP_ROWS_PER_READ, + context: SYSTEM_CTX, + }); + if (!Array.isArray(found)) throw new Error('the read answered no row list'); + for (const row of found) { + const key = templateRowKey(row?.name, row?.locale); + if (!byKey.has(key)) byKey.set(key, row); + } + } + } catch (err: any) { + logger?.warn?.( + '[email] bulk read of stored email templates failed — each declared template is looked up on its own instead', + { error: err?.message ?? String(err) }, + ); + return undefined; + } + return byKey; +} + +/** + * [#22062] Does a stored value already hold what the projection would write? + * + * Errs toward "no": a false "yes" keeps a stale template silently, while a + * false "no" costs one write. So a value is held only when it is the projected + * value itself, or one of the two storage spellings a driver may hand back for + * it: `1`/`0` for a boolean column, and — for `variables_json`, a JSON text + * column — the parsed value whose `JSON.stringify` is the projected text + * exactly. Anything else, an absent or `null` column included, is not held. + */ +function storedValueHolds(column: string, stored: unknown, projected: unknown): boolean { + if (stored === projected) return true; + if (typeof projected === 'boolean') return stored === (projected ? 1 : 0); + if (column === 'variables_json' && typeof projected === 'string' && stored !== null && typeof stored === 'object') { + return JSON.stringify(stored) === projected; + } + return false; +} + +/** + * [#22062] True when the row already holds EVERY column the projection writes — + * exactly the column set the update below would write, so "held" means the + * update would change nothing the sender reads. A column the projection omits + * (an optional key the author left unset) is not compared, as the update does + * not write it either. + */ +function rowHoldsProjection(row: Record, projected: Record): boolean { + return Object.entries(projected).every(([column, value]) => storedValueHolds(column, row[column], value)); +} + /** * Materialize ONE declared template into `sys_email_template`, honouring * seed-not-clobber. Shared by the boot sweep and the live subscribe path. * - * @returns `true` when the row was written, `false` when deliberately skipped. + * @returns `true` when the row was written, `false` when deliberately not + * written: an admin-owned or customized row, or a package-managed row that + * already holds the template's projection. * @throws when the raw item fails schema validation, or the write itself fails * — callers decide whether that warns or propagates. */ @@ -280,14 +410,30 @@ export async function upsertDeclaredEmailTemplate( logger?: Logger, ): Promise { const tpl: EmailTemplateDefinition = EmailTemplateDefinitionSchema.parse(raw); - const now = new Date().toISOString(); + const row = await findTemplateRow(engine, object, tpl.name, tpl.locale); + return writeTemplateRow(engine, tpl, row, object, logger); +} - const existing = await (engine as any).find(object, { - where: { name: tpl.name, locale: tpl.locale }, - limit: 1, - context: SYSTEM_CTX, - }); - const row: any = Array.isArray(existing) ? existing[0] : (existing as any)?.data?.[0]; +/** + * The one write step behind both doors: write a validated template against the + * row already read for its `(name, locale)` (`undefined` when there is none). + * + * [#22062] A `managed_by: 'package'` row that already holds the projection is + * not rewritten. Before, every boot rewrote every effective template — one + * UPDATE and the engine's read-backs each — whether or not anything changed. + * A row with any other provenance takes exactly the path it took before: an + * admin-owned or customized row is never written, and a legacy row with no + * `managed_by` (or a `platform` one) is rewritten and adopted. + */ +async function writeTemplateRow( + engine: IDataEngine, + tpl: EmailTemplateDefinition, + row: any, + object: string, + logger: Logger | undefined, +): Promise { + const now = new Date().toISOString(); + const projected = mapTemplateToRow(tpl); if (row?.id) { // Admin owns a same-named row, or has edited this seeded one — never @@ -300,9 +446,10 @@ export async function upsertDeclaredEmailTemplate( return false; } if (row.customized === true) return false; + if (row.managed_by === 'package' && rowHoldsProjection(row, projected)) return false; await (engine as any).update(object, { id: row.id, - ...mapTemplateToRow(tpl), + ...projected, // Adopt pristine/legacy (pre-provenance) rows so future boots recognize // them as package-managed. managed_by: 'package', @@ -313,7 +460,7 @@ export async function upsertDeclaredEmailTemplate( await (engine as any).insert(object, { id: uid('etpl'), - ...mapTemplateToRow(tpl), + ...projected, managed_by: 'package', customized: false, created_at: now, @@ -395,6 +542,20 @@ export async function bootstrapDeclaredEmailTemplates( * and the package's `exports` map names only that entry, so this function and * {@link EffectiveEmailTemplateSources} add nothing to the published surface. * No caller outside this package needs them. + * + * ## [#22062] What a boot costs + * + * The stored rows are read in bulk ({@link readStoredTemplateRows}), and a + * package-managed row that already holds its projection is not rewritten (see + * `writeTemplateRow`). Measured on `ObjectQL` over `SqlDriver` (better-sqlite3), + * a steady boot over 450 unchanged templates went from 1,800 statements (a + * lookup, an UPDATE and its two read-backs per template) to 3 reads and no + * write. + * + * A key the bulk read did not answer — a template new since the last boot, a + * read cut short at its bound, or a failed read — is looked up on its own + * BEFORE anything is inserted, so no row the bulk read missed is ever inserted + * twice. A steady boot has no such key and pays nothing for the lookup. */ export async function bootstrapEffectiveEmailTemplates( engine: IDataEngine, @@ -409,9 +570,21 @@ export async function bootstrapEffectiveEmailTemplates( let seeded = 0; let skipped = 0; + const stored = await readStoredTemplateRows(engine, declared, object, logger); + for (const raw of declared) { try { - const written = await upsertDeclaredEmailTemplate(engine, raw, object, logger); + const tpl: EmailTemplateDefinition = EmailTemplateDefinitionSchema.parse(raw); + const key = templateRowKey(tpl.name, tpl.locale); + const row = stored?.has(key) + ? stored.get(key) + : await findTemplateRow(engine, object, tpl.name, tpl.locale); + const written = await writeTemplateRow(engine, tpl, row, object, logger); + // A write leaves the bulk read's copy of this key stale. The list can + // name one slot twice — the registry lists an env-wide overlay after the + // package entry it overrides — and the later item must meet the row as + // it is now, so it is looked up again rather than compared to the copy. + if (written) stored?.delete(key); if (written) seeded += 1; else skipped += 1; } catch (err: any) { diff --git a/packages/qa/dogfood/test/email-template-boot-sweep.test.ts b/packages/qa/dogfood/test/email-template-boot-sweep.test.ts new file mode 100644 index 00000000000..833f3ea002f --- /dev/null +++ b/packages/qa/dogfood/test/email-template-boot-sweep.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #22062 — plugin-email's declared-template boot sweep on the REAL engine and + * the REAL SQL driver: which stored row a `(name, locale)` held by several + * organizations resolves to, and what a steady boot costs. + * + * ## Why here and not beside the sweep + * + * Both answers belong to the driver: which row comes first is whatever ORDER BY + * the driver gives the read, and a statement count is a count of what reaches + * the database. `@objectstack/plugin-email` cannot import a driver for its own + * suite, so these run where `ObjectQL` and `SqlDriver` already are, against the + * sweep's published entry points. + * + * ## What was measured before the fix (`ObjectQL` over better-sqlite3) + * + * - The per-template lookup, `find(…, { where: { name, locale }, limit: 1 })`, + * goes out as `… order by id asc limit ?`: the SQL driver orders every + * paged read by `id`. So its row is the smallest id, not the oldest row. + * - The same read in bulk with no `limit` gets NO ORDER BY and walks rows in + * storage order. Its first row for the slot below is `etpl_m`; the lookup's + * is `etpl_c`. A bulk read must therefore be paged too, or it picks another + * row than the lookup it replaces. The first case asserts that difference + * on the fixture, so the pin cannot drift into a fixture where both orders + * agree. + * - A steady boot over 450 unchanged templates issued 1,800 statements: per + * template one lookup, one UPDATE and the engine's two read-backs. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysEmailTemplate } from '@objectstack/platform-objects/audit'; +import { bootstrapDeclaredEmailTemplates, upsertDeclaredEmailTemplate } from '@objectstack/plugin-email'; + +const SYS = { isSystem: true, positions: [], permissions: [] }; +const TABLE = 'sys_email_template'; +const NAME = 'auth.password_reset'; +const DECLARED_SUBJECT = 'Reset your password, {{user.name}}'; + +const engines: ObjectQL[] = []; + +afterEach(async () => { + while (engines.length) await engines.pop()?.destroy(); +}); + +/** A fresh engine on its own `:memory:` database, every statement it sends recorded. */ +async function boot(): Promise<{ engine: ObjectQL; statements: string[] }> { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.dogfood.email-template-boot-sweep', + name: 'Email template boot sweep', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysEmailTemplate], + } as never); + await engine.syncSchemas(); + engines.push(engine); + const statements: string[] = []; + driver.getKnex().on('query', (q: { sql: string }) => statements.push(q.sql)); + return { engine, statements }; +} + +function declared(over: Record = {}) { + return { + name: NAME, + label: 'Password Reset', + category: 'auth', + subject: DECLARED_SUBJECT, + bodyHtml: '

Click here

', + variables: [{ name: 'url', type: 'string', required: true }], + ...over, + }; +} + +/** The sweep's declared list, through the metadata-service fallback it reads when the registry holds none. */ +const listing = (items: unknown[]) => ({ list: () => items }); + +/** + * One `(name, locale)` held by three organizations and the global row, plus + * another locale of the same name. Stored in an order that is neither id order + * nor its reverse, every row stale, so whichever row the sweep picks is the one + * it rewrites. + */ +async function seedSharedSlot(engine: ObjectQL): Promise { + const rows: Array<[string, string | null, string]> = [ + ['etpl_m', 'org_b', 'en-US'], + ['etpl_z', null, 'en-US'], + ['etpl_c', 'org_a', 'en-US'], + ['etpl_q', 'org_c', 'en-US'], + ['etpl_0', 'org_a', 'zh-CN'], + ]; + for (const [id, organization_id, locale] of rows) { + await engine.insert(TABLE, { + id, organization_id, name: NAME, label: 'Stale', category: 'auth', locale, + subject: `stale ${id}`, body_html: '

stale

', active: true, managed_by: 'package', customized: false, + }, { context: SYS } as never); + } +} + +async function subjects(engine: ObjectQL): Promise> { + const rows = (await engine.find(TABLE, { where: { name: NAME }, context: SYS } as never)) as any[]; + return Object.fromEntries(rows.map((r) => [r.id, r.subject])); +} + +describe('[#22062] the declared-template boot sweep on the real engine and SQL driver', () => { + it('rewrites the row the per-template lookup chooses for a (name, locale) several organizations hold', async () => { + const { engine, statements } = await boot(); + await seedSharedSlot(engine); + + // The per-template lookup's choice, measured live rather than assumed. + statements.length = 0; + const [chosen] = (await engine.find(TABLE, { where: { name: NAME, locale: 'en-US' }, limit: 1, context: SYS } as never)) as any[]; + expect(statements).toEqual(['select * from `sys_email_template` where `name` = ? and `locale` = ? order by `id` asc limit ?']); + expect(chosen.id).toBe('etpl_c'); + // ANTI-VACUITY: an unpaged bulk read meets another row of this slot first. + const unpaged = (await engine.find(TABLE, { where: { name: { $in: [NAME] } }, context: SYS } as never)) as any[]; + expect(unpaged.find((r) => r.locale === 'en-US').id).not.toBe(chosen.id); + + const result = await bootstrapDeclaredEmailTemplates(engine as never, listing([declared()])); + + expect(result).toEqual({ seeded: 1, skipped: 0 }); + expect(await subjects(engine)).toEqual({ + etpl_c: DECLARED_SUBJECT, + etpl_m: 'stale etpl_m', + etpl_z: 'stale etpl_z', + etpl_q: 'stale etpl_q', + etpl_0: 'stale etpl_0', + }); + }); + + it('the live door rewrites the same row over the same rows', async () => { + const { engine } = await boot(); + await seedSharedSlot(engine); + + expect(await upsertDeclaredEmailTemplate(engine as never, declared())).toBe(true); + + expect((await subjects(engine)).etpl_c).toBe(DECLARED_SUBJECT); + expect(Object.values(await subjects(engine)).filter((s) => s === DECLARED_SUBJECT)).toHaveLength(1); + }); + + it('a steady boot over 450 unchanged templates sends three reads and no write', async () => { + const { engine, statements } = await boot(); + const items = Array.from({ length: 450 }, (_, i) => declared({ name: `tpl.n${i}`, bodyText: i % 3 ? undefined : 'Plain' })); + expect(await bootstrapDeclaredEmailTemplates(engine as never, listing(items))).toEqual({ seeded: 450, skipped: 0 }); + + statements.length = 0; + const result = await bootstrapDeclaredEmailTemplates(engine as never, listing(items)); + + expect(result).toEqual({ seeded: 0, skipped: 450 }); + expect(statements.filter((s) => !/^select /i.test(s))).toEqual([]); + expect(statements).toHaveLength(3); + expect(statements.every((s) => /^select \* from `sys_email_template` where `name` in \(.*\) order by `id` asc limit \?$/.test(s))).toBe(true); + }); +});