Skip to content

Commit fbbb854

Browse files
author
Ubuntu
committed
feat(plugins): atomic conditional writes for plugin storage (insert / updateIf)
Add two additive, single-statement primitives to the plugin StorageCollection<T> API: - insert(id, data): insert-once via INSERT … ON CONFLICT (pk) DO NOTHING. A same-id collision returns { inserted:false, reason:"exists" }; a declared unique-index collision on a non-id field is classified as { inserted:false, reason:"unique_violation", conflictField? } and any other DB error is re-thrown. - updateIf(id, { where, set?, delta? }): predicate-guarded atomic update via one UPDATE … SET json_set/jsonb_set(…) WHERE <pk> AND <guard> RETURNING data. The guard reuses the numeric-correct where translation from the prior fix, and integer deltas are applied in-SQL with COALESCE(base,0) ± n, so N concurrent guarded decrements cannot oversell. set and delta are separate args; floats are rejected at runtime; an empty in:[] guard short-circuits to applied:false. Declared uniqueIndexes are now materialized as real unique indexes on plugin install (fail-loud on error) and dropped on uninstall. The new methods are delegated through the in-process context, the workerd bridge, and the Cloudflare D1 bridge so all runners behave identically. Proven no-oversell on real Postgres (and the SQLite/D1 dialect); both dialects covered by the contract suite.
1 parent 8b89c14 commit fbbb854

17 files changed

Lines changed: 1116 additions & 2 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@emdash-cms/sandbox-workerd": minor
3+
---
4+
5+
Sandboxed plugins can now call `insert` and `updateIf` on their storage collections through the workerd bridge, matching the in-process storage API.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"emdash": minor
3+
---
4+
5+
Adds `insert` (insert-once) and `updateIf` (predicate-guarded atomic update) to plugin storage collections. `insert` creates a document only if its id is free and no declared unique index is violated; `updateIf` applies a wholesale `set` and/or integer `delta` in a single guarded statement, so concurrent guarded decrements (e.g. inventory) can no longer oversell. A collection's declared `uniqueIndexes` are now materialized as real unique indexes when the plugin installs — installation fails loudly if a unique index cannot be created (for instance because existing data already violates it).

packages/cloudflare/src/sandbox/bridge.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,55 @@ export class PluginBridge extends WorkerEntrypoint<PluginBridgeEnv, PluginBridge
438438
return deleted;
439439
}
440440

441+
/**
442+
* Insert-once. Delegates to PluginStorageRepository (same single-statement
443+
* `INSERT … ON CONFLICT DO NOTHING` + unique-violation classification as the
444+
* in-process and workerd paths) so the D1 production sandbox behaves
445+
* identically.
446+
*/
447+
async storageInsert(collection: string, id: string, data: unknown): Promise<unknown> {
448+
const { storageCollections } = this.ctx.props;
449+
if (!storageCollections.includes(collection)) {
450+
throw new Error(`Storage collection not declared: ${collection}`);
451+
}
452+
return this.getStorageRepo(collection).insert(id, data);
453+
}
454+
455+
/**
456+
* Predicate-guarded atomic update. Delegates to PluginStorageRepository so
457+
* the guarded single-statement `UPDATE … RETURNING` (the no-oversell
458+
* primitive) runs identically on the D1 production path. D1 serializes
459+
* writes, so concurrent guarded decrements can never oversell.
460+
*/
461+
async storageUpdateIf(
462+
collection: string,
463+
id: string,
464+
args: { where?: unknown; set?: unknown; delta?: unknown },
465+
): Promise<unknown> {
466+
const { storageCollections } = this.ctx.props;
467+
if (!storageCollections.includes(collection)) {
468+
throw new Error(`Storage collection not declared: ${collection}`);
469+
}
470+
// Validate the guard/patch shapes up front (symmetric with the workerd
471+
// bridge's requireRecord/optionalRecord) so a malformed call fails with a
472+
// clean error rather than an incidental repo throw.
473+
if (!isJsonObject(args.where)) {
474+
throw new Error("storage/updateIf requires an object `where`");
475+
}
476+
if (args.set !== undefined && !isJsonObject(args.set)) {
477+
throw new Error("storage/updateIf `set` must be an object when provided");
478+
}
479+
if (args.delta !== undefined && !isJsonObject(args.delta)) {
480+
throw new Error("storage/updateIf `delta` must be an object when provided");
481+
}
482+
return this.getStorageRepo(collection).updateIf(id, {
483+
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- WhereClause is structurally Record<string, WhereValue>; validated as an object above and re-validated by the repo.
484+
where: args.where as never,
485+
set: args.set,
486+
delta: args.delta,
487+
});
488+
}
489+
441490
// =========================================================================
442491
// Content Operations - capability-gated
443492
// =========================================================================

packages/cloudflare/src/sandbox/wrapper.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ function createContext(env) {
8282
count: (where) => bridge.storageCount(collectionName, where),
8383
getMany: (ids) => bridge.storageGetMany(collectionName, ids),
8484
putMany: (items) => bridge.storagePutMany(collectionName, items),
85-
deleteMany: (ids) => bridge.storageDeleteMany(collectionName, ids)
85+
deleteMany: (ids) => bridge.storageDeleteMany(collectionName, ids),
86+
insert: (id, data) => bridge.storageInsert(collectionName, id, data),
87+
updateIf: (id, args) => bridge.storageUpdateIf(collectionName, id, args)
8688
};
8789
}
8890

packages/core/src/database/dialect-helpers.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,71 @@ export function pluginDataExtractExpr(
277277
return `CASE WHEN json_type(data, '$.${field}') IN ('integer', 'real') THEN ${extract} END`;
278278
}
279279

280+
/**
281+
* Build the new value of the `_plugin_storage.data` (text-JSON) column for a
282+
* guarded `updateIf`, composing wholesale `set` fields and integer `delta`
283+
* fields into a SINGLE dialect-correct expression.
284+
*
285+
* Both branches go through `json_set` / `jsonb_set` so the write never rewrites
286+
* the whole column from JS (which would require a read-then-write and break the
287+
* single-statement atomicity that makes no-oversell hold):
288+
*
289+
* - **set** field → the value is stored via `json(?)` (SQLite) / `?::jsonb`
290+
* (Postgres) with `JSON.stringify(value)`, uniformly handling scalars,
291+
* objects, arrays, and `null` (stored as JSON `null`, never SQL `NULL` — a
292+
* SQL `NULL` in `jsonb_set` would null the entire `data` column and hit the
293+
* `NOT NULL` constraint).
294+
* - **delta** field → `COALESCE(<numeric extract>, 0) + n`, where the extract
295+
* is the type-guarded numeric form from {@link pluginDataExtractExpr} so a
296+
* missing / null / non-number stored value coalesces to `0` on BOTH dialects
297+
* instead of throwing (Postgres) or coercing oddly. Integer arithmetic stays
298+
* integer (Postgres `to_jsonb(numeric)` and SQLite integer `+` both round-trip
299+
* without a spurious `.0`).
300+
*
301+
* Field names are validated (`validateJsonFieldName` / `pluginDataExtractExpr`)
302+
* before interpolation, so the JSON path is a safe identifier and values are
303+
* bound parameters — no injection surface.
304+
*
305+
* SQLite: json_set(json_set(data, '$.f1', json(?)), '$.f2', COALESCE(json_extract(...), 0) + ?)
306+
* Postgres: (jsonb_set(jsonb_set(data::jsonb, '{f1}', ?::jsonb), '{f2}', to_jsonb(COALESCE(..., 0) + ?)))::text
307+
*/
308+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- accepts any Kysely instance
309+
export function pluginDataWriteExpr(
310+
db: Kysely<any>,
311+
setEntries: Array<[string, unknown]>,
312+
deltaEntries: Array<[string, number]>,
313+
): RawBuilder<string> {
314+
const pg = isPostgres(db);
315+
let expr: RawBuilder<unknown> = pg ? sql`data::jsonb` : sql`data`;
316+
317+
for (const [field, value] of setEntries) {
318+
validateJsonFieldName(field, "plugin storage set field name");
319+
const json = JSON.stringify(value ?? null);
320+
if (pg) {
321+
expr = sql`jsonb_set(${expr}, ${sql.lit(`{${field}}`)}, ${json}::jsonb)`;
322+
} else {
323+
expr = sql`json_set(${expr}, ${sql.lit(`$.${field}`)}, json(${json}))`;
324+
}
325+
}
326+
327+
for (const [field, n] of deltaEntries) {
328+
// Type-guarded numeric extract over the ORIGINAL `data` column so the
329+
// arithmetic is total (non-number → NULL → COALESCE 0), matching PR A's
330+
// numeric-correctness posture on both dialects.
331+
const numericExtract = pluginDataExtractExpr(db, field, { numeric: true });
332+
if (pg) {
333+
expr = sql`jsonb_set(${expr}, ${sql.lit(`{${field}}`)}, to_jsonb(COALESCE(${sql.raw(numericExtract)}, 0) + ${n}))`;
334+
} else {
335+
expr = sql`json_set(${expr}, ${sql.lit(`$.${field}`)}, COALESCE(${sql.raw(numericExtract)}, 0) + ${n})`;
336+
}
337+
}
338+
339+
if (pg) {
340+
return sql<string>`(${expr})::text`;
341+
}
342+
return sql<string>`${expr}`;
343+
}
344+
280345
/**
281346
* SQL expression for ordering plugin-storage rows by a `data` field.
282347
*

packages/core/src/database/repositories/plugin-storage.ts

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,58 @@ import {
1616
validateOrderByClause,
1717
getIndexedFields,
1818
jsonOrderExtract,
19+
isInFilter,
1920
} from "../../plugins/storage-query.js";
2021
import type {
2122
StorageCollection,
2223
QueryOptions,
2324
PaginatedResult,
2425
WhereClause,
26+
InsertResult,
27+
UpdateIfArgs,
28+
UpdateIfResult,
2529
} from "../../plugins/types.js";
30+
import { pluginDataWriteExpr } from "../dialect-helpers.js";
2631
import { withTransaction } from "../transaction.js";
2732
import type { Database } from "../types.js";
2833
import { encodeCursor, decodeCursor } from "./types.js";
2934

35+
/**
36+
* Classify a thrown DB error as a UNIQUE-constraint violation, returning the
37+
* offending index name when the driver exposes it.
38+
*
39+
* - **Postgres** (`pg`): SQLSTATE `23505`; `error.constraint` carries the index
40+
* name.
41+
* - **SQLite / D1** (better-sqlite3): `error.code === "SQLITE_CONSTRAINT_UNIQUE"`
42+
* or a message `UNIQUE constraint failed: index 'uidx_…'`. Our unique indexes
43+
* are partial EXPRESSION indexes, so the message names the index, not a column.
44+
*
45+
* Returns `null` for anything that is not a unique violation (the caller
46+
* re-throws those — raw DB errors are never swallowed).
47+
*/
48+
const UNIQUE_CONSTRAINT_MESSAGE_RE = /UNIQUE constraint failed/i;
49+
const SQLITE_INDEX_NAME_RE = /index ['"]([^'"]+)['"]/;
50+
const SAFE_FIELD_NAME_RE = /^[a-zA-Z][a-zA-Z0-9_]*$/;
51+
52+
function classifyUniqueViolation(error: unknown): { indexName?: string } | null {
53+
if (typeof error !== "object" || error === null) return null;
54+
const e = error as { code?: unknown; constraint?: unknown; message?: unknown };
55+
if (e.code === "23505") {
56+
return { indexName: typeof e.constraint === "string" ? e.constraint : undefined };
57+
}
58+
const message = typeof e.message === "string" ? e.message : "";
59+
if (e.code === "SQLITE_CONSTRAINT_UNIQUE" || UNIQUE_CONSTRAINT_MESSAGE_RE.test(message)) {
60+
const match = SQLITE_INDEX_NAME_RE.exec(message);
61+
return { indexName: match?.[1] };
62+
}
63+
return null;
64+
}
65+
66+
/** True for any non-null object that may carry `inc`/`dec` delta keys. */
67+
function isDeltaLike(value: unknown): value is { inc?: unknown; dec?: unknown } {
68+
return typeof value === "object" && value !== null;
69+
}
70+
3071
/**
3172
* Turn a `buildWhereClause` result (`?`-placeholder SQL + ordered params) into a
3273
* single boolean expression suitable for Kysely's `.where()`.
@@ -319,6 +360,176 @@ export class PluginStorageRepository<T = unknown> implements StorageCollection<T
319360
// so this always satisfies its Promise<number> contract on both dialects.
320361
return Number(result?.count ?? 0);
321362
}
363+
364+
/**
365+
* Insert-once (see {@link StorageCollection.insert}).
366+
*
367+
* Single `INSERT … ON CONFLICT (plugin_id, collection, id) DO NOTHING`. The
368+
* conflict target is the primary key, so a same-`id` collision is swallowed
369+
* (0 rows affected → `{ inserted: false, reason: "exists" }`). A collision on
370+
* a declared partial UNIQUE expression index is NOT the conflict target, so
371+
* the statement throws — we classify that as `unique_violation` and re-throw
372+
* anything else.
373+
*/
374+
async insert(id: string, data: T): Promise<InsertResult> {
375+
const now = new Date().toISOString();
376+
const jsonData = JSON.stringify(data);
377+
378+
try {
379+
const result = await this.db
380+
.insertInto("_plugin_storage")
381+
.values({
382+
plugin_id: this.pluginId,
383+
collection: this.collection,
384+
id,
385+
data: jsonData,
386+
created_at: now,
387+
updated_at: now,
388+
})
389+
.onConflict((oc) => oc.columns(["plugin_id", "collection", "id"]).doNothing())
390+
.executeTakeFirst();
391+
392+
const inserted = (result.numInsertedOrUpdatedRows ?? 0n) > 0n;
393+
if (inserted) return { inserted: true };
394+
return { inserted: false, reason: "exists" };
395+
} catch (error) {
396+
const classified = classifyUniqueViolation(error);
397+
if (!classified) throw error;
398+
const conflictField = await this.recoverConflictField(classified.indexName);
399+
return conflictField
400+
? { inserted: false, reason: "unique_violation", conflictField }
401+
: { inserted: false, reason: "unique_violation" };
402+
}
403+
}
404+
405+
/**
406+
* Predicate-guarded atomic update (see {@link StorageCollection.updateIf}).
407+
*
408+
* One guarded `UPDATE _plugin_storage SET data = <json_set/jsonb_set expr>,
409+
* updated_at = ? WHERE <pk> AND <guard> RETURNING data`. The guard reuses
410+
* PR A's numeric-correct `buildWhereClause` translation verbatim, and the
411+
* `set`/`delta` arithmetic is computed in-SQL — no read-then-write — which is
412+
* what makes N concurrent guarded decrements correct (no oversell).
413+
*
414+
* `applied` is derived from whether a `RETURNING` row came back (equivalently
415+
* rows-affected > 0). A missing row and a failed guard both yield 0 rows →
416+
* `{ applied: false }`; the two are intentionally indistinguishable. Never
417+
* inserts.
418+
*/
419+
async updateIf(id: string, args: UpdateIfArgs<T>): Promise<UpdateIfResult<T>> {
420+
const { where, set, delta } = args;
421+
422+
const setEntries: Array<[string, unknown]> = set ? Object.entries(set) : [];
423+
const hasSet = setEntries.length > 0;
424+
const hasDelta = delta !== undefined && Object.keys(delta).length > 0;
425+
426+
if (!hasSet && !hasDelta) {
427+
throw new Error("updateIf requires at least one of `set` or `delta`.");
428+
}
429+
430+
// Build the signed integer deltas, enforcing integer-only at runtime.
431+
const deltaEntries: Array<[string, number]> = [];
432+
if (hasDelta) {
433+
const setFieldSet = new Set(setEntries.map(([field]) => field));
434+
for (const [field, spec] of Object.entries(delta)) {
435+
if (spec === undefined) continue;
436+
if (setFieldSet.has(field)) {
437+
throw new Error(`updateIf: field "${field}" appears in both \`set\` and \`delta\`.`);
438+
}
439+
if (!isDeltaLike(spec)) {
440+
throw new TypeError(
441+
`updateIf: delta for "${field}" must be exactly one of { inc: number } or { dec: number }.`,
442+
);
443+
}
444+
const { inc, dec } = spec;
445+
let signed: number;
446+
if (typeof inc === "number" && typeof dec !== "number") {
447+
signed = inc;
448+
} else if (typeof dec === "number" && typeof inc !== "number") {
449+
signed = -dec;
450+
} else {
451+
// Both present or neither present/numeric → ambiguous or invalid.
452+
throw new TypeError(
453+
`updateIf: delta for "${field}" must be exactly one of { inc: number } or { dec: number }.`,
454+
);
455+
}
456+
if (!Number.isInteger(signed)) {
457+
throw new TypeError(
458+
`updateIf: delta for "${field}" must be an integer (got ${String(inc ?? dec)}).`,
459+
);
460+
}
461+
deltaEntries.push([field, signed]);
462+
}
463+
}
464+
465+
// Defensive empty-`in` guard: an empty `in: []` matches nothing. The shared
466+
// where-translation would emit invalid `IN ()`; short-circuit to a no-op
467+
// (matches nothing → applied:false) BEFORE building any SQL.
468+
for (const value of Object.values(where)) {
469+
if (isInFilter(value) && value.in.length === 0) {
470+
return { applied: false };
471+
}
472+
}
473+
474+
const now = new Date().toISOString();
475+
const dataExpr = pluginDataWriteExpr(this.db, setEntries, deltaEntries);
476+
477+
let query = this.db
478+
.updateTable("_plugin_storage")
479+
.set({ data: dataExpr, updated_at: now })
480+
.where("plugin_id", "=", this.pluginId)
481+
.where("collection", "=", this.collection)
482+
.where("id", "=", id);
483+
484+
const whereResult = buildWhereClause(this.db, where);
485+
if (whereResult.sql) {
486+
query = query.where(buildRawWhereExpression(whereResult));
487+
}
488+
489+
const row = await query.returning("data").executeTakeFirst();
490+
if (!row) return { applied: false };
491+
// JSON.parse returns any; it flows into the T-typed `data` field directly.
492+
const data: T = JSON.parse(row.data);
493+
return { applied: true, data };
494+
}
495+
496+
/**
497+
* Best-effort recovery of the single field behind a unique-index violation.
498+
* Prefers the `_plugin_indexes` tracking row (authoritative field list);
499+
* falls back to parsing the `generateIndexName` format. Composite indexes
500+
* yield `undefined`.
501+
*/
502+
private async recoverConflictField(indexName?: string): Promise<string | undefined> {
503+
if (!indexName) return undefined;
504+
505+
const row = await this.db
506+
.selectFrom("_plugin_indexes")
507+
.select("fields")
508+
.where("plugin_id", "=", this.pluginId)
509+
.where("collection", "=", this.collection)
510+
.where("index_name", "=", indexName)
511+
.executeTakeFirst();
512+
513+
if (row) {
514+
try {
515+
const parsed: unknown = JSON.parse(row.fields);
516+
if (Array.isArray(parsed) && parsed.length === 1 && typeof parsed[0] === "string") {
517+
return parsed[0];
518+
}
519+
} catch {
520+
// fall through to name parsing
521+
}
522+
return undefined;
523+
}
524+
525+
// Fallback: uidx_plugin_<pluginId>_<collection>_<field>
526+
const prefix = `uidx_plugin_${this.pluginId}_${this.collection}_`;
527+
if (indexName.startsWith(prefix)) {
528+
const field = indexName.slice(prefix.length);
529+
if (SAFE_FIELD_NAME_RE.test(field)) return field;
530+
}
531+
return undefined;
532+
}
322533
}
323534

324535
/**

packages/core/src/plugins/context.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ function createStorageCollection<T>(
123123
putMany: (items) => repo.putMany(items),
124124
deleteMany: (ids) => repo.deleteMany(ids),
125125
count: (where) => repo.count(where),
126+
insert: (id, data) => repo.insert(id, data),
127+
updateIf: (id, updateArgs) => repo.updateIf(id, updateArgs),
126128

127129
// Query returns PaginatedResult instead of the old format
128130
async query(options?: QueryOptions): Promise<PaginatedResult<{ id: string; data: T }>> {

0 commit comments

Comments
 (0)