diff --git a/.changeset/export-association-partition.md b/.changeset/export-association-partition.md new file mode 100644 index 00000000..c4cec5c1 --- /dev/null +++ b/.changeset/export-association-partition.md @@ -0,0 +1,5 @@ +--- +'@haverstack/core': minor +--- + +Export `isAuthorityAssociation` from `@haverstack/core`, for apps that split a journal entry's association changes between `amendAssociations()` and `amendAccess()`, and `partitionAssociations` and `AUTHORITY_ASSOCIATION_KINDS` from `@haverstack/core/adapter`, for storage engines that keep both halves in one table. diff --git a/.changeset/fts5-not-in-phrase.md b/.changeset/fts5-not-in-phrase.md new file mode 100644 index 00000000..a5c451b4 --- /dev/null +++ b/.changeset/fts5-not-in-phrase.md @@ -0,0 +1,6 @@ +--- +'@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch +--- + +`filter.search` no longer drops a `NOT` written inside a quoted phrase: `"cats (not dogs"` searches for the phrase as typed. diff --git a/.changeset/mutate-fresh-read.md b/.changeset/mutate-fresh-read.md new file mode 100644 index 00000000..1871a2ea --- /dev/null +++ b/.changeset/mutate-fresh-read.md @@ -0,0 +1,6 @@ +--- +'@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch +--- + +Concurrent writes to one record no longer lose updates: `mutateRecord`, `restoreVersion` and `commitMigration` read and check the record inside the transaction that writes it, so a content patch applies to the content as it stands rather than to an earlier read. diff --git a/.changeset/sqlite-shared-dedupe.md b/.changeset/sqlite-shared-dedupe.md new file mode 100644 index 00000000..b6f7120f --- /dev/null +++ b/.changeset/sqlite-shared-dedupe.md @@ -0,0 +1,6 @@ +--- +'@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch +--- + +Remove redundant code from the bundled SQLite record logic: unused barrel exports, local aliases of core's native sort fields, and repeated not-found/version-read/row-hydration code. diff --git a/.changeset/sqlite-shared-indexes.md b/.changeset/sqlite-shared-indexes.md new file mode 100644 index 00000000..7bfe3acd --- /dev/null +++ b/.changeset/sqlite-shared-indexes.md @@ -0,0 +1,6 @@ +--- +'@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch +--- + +Internal restructure of the bundled SQLite record logic: the full-text and content indexes move into their own module. No behavior change. diff --git a/AGENTS.md b/AGENTS.md index 86eb80e8..4d7e8216 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,7 +92,7 @@ pnpm changeset - **`patch`** for anything a consumer cannot observe; **`minor`** for anything they can. Every package is `0.x`, so minor _is_ the breaking slot. - **Never `major`** — that cuts a 1.0 release, which is not a PR-level decision. - **No changeset** for tests, CI, or repo tooling. -- `@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against `@haverstack/record-adapter-sqlite`, which bundles it — a changeset naming only private packages is rejected. If a template hands you a list of every package (changeset-bot's does), trim it to what you changed. +- `@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against both packages that bundle it, `@haverstack/record-adapter-sqlite` and `@haverstack/record-adapter-do-sqlite` — each ships its own copy, so each changelog owes the entry. A changeset naming only private packages is rejected. If a template hands you a list of every package (changeset-bot's does), trim it to what you changed. A `minor` on a package minor-bumps everything that depends on it, transitively; a `patch` bumps nothing else. Preview with `node scripts/expand-changesets.mjs --dry-run`. Never hand-edit a `version` field or a `CHANGELOG.md` — CI owns both. See [CONTRIBUTING.md § Releasing](./CONTRIBUTING.md#releasing). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ad5e9b3..7f81a836 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ Every package is on `0.x`, where semver puts breaking changes in the **minor** s **Don't select `major`.** It cuts a 1.0 release, which is a decision about the install base (see [No backward compatibility yet](#no-backward-compatibility-yet)), not something a single PR makes. -`@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against `@haverstack/record-adapter-sqlite`, which bundles it. Naming a private package beside a public one makes Changesets refuse the whole release plan, so `scripts/expand-changesets.mjs` drops the line first — changeset-bot's template is safe to commit unedited. A changeset naming _only_ private packages fails: it would release nowhere. +`@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against both packages that bundle it, `@haverstack/record-adapter-sqlite` and `@haverstack/record-adapter-do-sqlite`: each inlines its own copy at build time, so a change to sqlite-shared ships in both and belongs in both changelogs. Naming a private package beside a public one makes Changesets refuse the whole release plan, so `scripts/expand-changesets.mjs` drops the line first — changeset-bot's template is safe to commit unedited. A changeset naming _only_ private packages fails: it would release nowhere. ### The ripple rule diff --git a/docs/spec/adapters.md b/docs/spec/adapters.md index bfdc267a..cc9dbfad 100644 --- a/docs/spec/adapters.md +++ b/docs/spec/adapters.md @@ -117,6 +117,8 @@ An adapter that holds a stack's identity has exactly one entry point, an async s The invariants above are easy to state in prose and easy to violate silently — FTS5's external-content index going stale, a content path matching an unfiltered superset, a cursor replayed under the wrong sort. **`@haverstack/adapter-conformance`** is the runnable form of this document: `runRecordAdapterConformance()` and `runBlobAdapterConformance()` take an `open()` for your adapter and its declared `capabilities`, and register a vitest suite checking exactly the invariants named here, gated so a capability you don't declare is never penalized for its absence. `record-adapter-sqlite` and `blob-adapter-disk` both run it against themselves (`tests/conformance.test.ts`), alongside `@haverstack/core`'s own `MemoryAdapter` — three independent implementations passing the same suite is the check that the suite describes the contract rather than one adapter's accidents. +## SQLite-backed adapters + **`@haverstack/sqlite-shared`** is an internal, non-public package holding everything a SQLite-backed record adapter needs that isn't specific to one binding — schema DDL and the function that applies it, `WHERE`/`ORDER` building, the cursor codec, row mappers, the FTS5 sanitizer and indexing strategy, the storage-ownership lock, the `_config` record's reader and writer, the capability declaration every engine running this query builder makes, and (via a small `SqlExecutor` interface normalizing a binding's call convention) the actual CRUD/query/version/type/association/token logic itself. `SharedSqlRecordAdapter` is where the last of those meet: an abstract base implementing all of `StackRecordAdapter` by forwarding to that logic, so an adapter supplies only what's genuinely engine-specific — constructing the database handle, transaction semantics, and its own lifecycle (`flush`/`close`, the two members the base leaves abstract). `record-adapter-sqlite` and `record-adapter-do-sqlite` both consume it; the split exists so a second (and third) SQLite engine inherits the behavior rather than reimplementing it, and so a cursor minted by one is decodable by another. **It exposes two entry points.** The full barrel (`.`) includes the token-store logic (`SharedTokenLogic`, `TOKENS_SCHEMA_SQL`) and the file-lock helpers, both Node-specific (`node:crypto`, `node:fs`) — fine for `record-adapter-sqlite`, but a bare `import` of either survives tree-shaking as a dead-but-still-imported module in a bundle, which throws at load time in a Workers runtime without `nodejs_compat`. A record-only consumer with no token store and no lock file — `record-adapter-do-sqlite`, where the platform's single-writer-per-id model already is the lock — imports the `./record` subpath instead, which never reaches either. diff --git a/docs/spec/data-model.md b/docs/spec/data-model.md index 96dc5b6d..ac569eaa 100644 --- a/docs/spec/data-model.md +++ b/docs/spec/data-model.md @@ -677,7 +677,7 @@ A `search` that sanitizes to nothing (a bare `*`, punctuation-only input) is tre **Search text is repaired, not rejected.** `filter.search` is the one filter carrying a query language, and it holds what a person typed into a box — where an unbalanced quote (`5" nails`), a trailing operator (`cats AND`) or a leading one is ordinary input on the way to a longer query, not a malformed request. The FTS sanitizers close an odd trailing quote, drop operators left without an operand, and reduce everything outside a phrase to letters, digits, marks, whitespace, parens and quotes — so the search runs against the terms actually present rather than failing. Text inside a phrase is left alone: `"cats AND dogs"` is literal to the engine, and rewriting inside it would change what was asked for. -**The reduction is an allow-list, not a list of characters to strip.** FTS5's column-filter syntax is wider than `colname:term` — `-name` and `{a b}` filter columns with no colon in them — so a strip-list written against the colon form still let `-cats` and even the hyphen in `cats-dogs` reach the engine as column names, answered with `no such column`. Naming what may pass means the next piece of syntax nobody anticipated arrives as a separator rather than as an error, and nothing is lost: the default tokenizer already splits terms on those characters, so `cats-dogs` searches for the two tokens it was indexed as. Two grammar rules the same reduction has to respect: a group needs an explicit operator beside it (`cats (dogs)` is an error where `cats AND (dogs)` is not, so the implied `AND` is written back in), and a control character ends the string early for SQLite's C API, so control characters are dropped inside a phrase as well as outside. +**The reduction is an allow-list, not a list of characters to strip.** FTS5's column-filter syntax is wider than `colname:term` — `-name` and `{a b}` filter columns with no colon in them — so a strip-list written against the colon form would let `-cats` and even the hyphen in `cats-dogs` reach the engine as column names, answered with `no such column`. Naming what may pass means the next piece of syntax nobody anticipated arrives as a separator rather than as an error, and nothing is lost: the default tokenizer already splits terms on those characters, so `cats-dogs` searches for the two tokens it was indexed as. Two grammar rules the same reduction has to respect: a group needs an explicit operator beside it (`cats (dogs)` is an error where `cats AND (dogs)` is not, so the implied `AND` is written back in), and a control character ends the string early for SQLite's C API, so control characters are dropped inside a phrase as well as outside. The repair claims no completeness against the engine's grammar, so adapters back it with a rule that does: **whatever reaches the engine as search text and fails to parse surfaces as `StackBadRequestError`** (`bad_request`/400), never as a raw engine error. Membership in the [error taxonomy](./wire-format.md#the-taxonomy-root) is what gives a server a status to answer with — an error outside it has no mapping, and ordinary search input would land as a 500. The rule is scoped to `filter.search` deliberately: every other clause is built from bound parameters, so a parse failure in one is the adapter's own bug and must not be relabelled as the caller's. diff --git a/docs/spec/events.md b/docs/spec/events.md index 41dd18a3..cedbd889 100644 --- a/docs/spec/events.md +++ b/docs/spec/events.md @@ -159,7 +159,7 @@ There is no `previous`. Prior state is already a first-class, addressable thing: **An `async` handler is permitted; it is simply not awaited.** The handler type is `(change: RecordChange) => void`, and a `void` return means the value is ignored, not that the function must be synchronous. Passing an `async` function is the normal way to defer work: it runs to its first `await`, yields, and the emitter moves on. What it does not buy is ordering or completion. -**What "never delays a write" does and does not promise.** It is a guarantee about the emitter: no handler's returned promise is awaited, so a subscriber cannot extend a write by deferring work. It is not a guarantee against a handler's _synchronous_ body. Handlers run inline on the caller's thread, and a record adapter backed by `node:sqlite` has just run its write synchronously on that same thread ([Adapters § Adapter backends](./adapters.md#adapter-backends)), so a handler doing heavy synchronous work extends that occupied window. Handlers should return promptly and defer anything substantial. +**What "never delays a write" does and does not promise.** It is a guarantee about the emitter: no handler's returned promise is awaited, so a subscriber cannot extend a write by deferring work. It is not a guarantee against a handler's _synchronous_ body. Handlers run inline on the caller's thread, and a record adapter backed by `node:sqlite` has just run its write synchronously on that same thread ([Adapters § SQLite-backed adapters](./adapters.md#sqlite-backed-adapters)), so a handler doing heavy synchronous work extends that occupied window. Handlers should return promptly and defer anything substantial. An indexer that must not miss a write does not get that from a blocking hook — it gets it from a resumable cursor and idempotent application. A blocking hook buys a world where every subscriber is a latency and failure dependency of every write. diff --git a/docs/spec/journal.md b/docs/spec/journal.md index b1437c49..2cbf61c3 100644 --- a/docs/spec/journal.md +++ b/docs/spec/journal.md @@ -48,14 +48,16 @@ type AssociationEdit = Exclude; Undoing one entry's association changes is at most two calls, one per half of [the partition](./access-control.md#storage-unifies-the-api-does-not), passing each element's inverse to `amendAssociations()` / `amendAccess()`: ```ts +import { isAuthorityAssociation } from '@haverstack/core'; + const inverse = (change: AssociationChange): AssociationEdit => change.op === 'add' ? { op: 'remove', association: change.association } : { op: 'add', association: change.op === 'repoint' ? change.previous : change.association }; const edits = (entry.associations ?? []).map(inverse); -const data = edits.filter((e) => !isAuthority(e.association)); -const authority = edits.filter((e) => isAuthority(e.association)); +const data = edits.filter((e) => !isAuthorityAssociation(e.association)); +const authority = edits.filter((e) => isAuthorityAssociation(e.association)); if (data.length) await stack.amendAssociations(recordId, data); if (authority.length) await stack.amendAccess(recordId, authority); ``` diff --git a/packages/core/src/adapter/index.ts b/packages/core/src/adapter/index.ts index 37320f70..99e4369d 100644 --- a/packages/core/src/adapter/index.ts +++ b/packages/core/src/adapter/index.ts @@ -33,3 +33,4 @@ export { } from '../query-validation.js'; export { contentSortEntry, contentSortKey, compareSortEntries } from './sort.js'; export type { SortEntry } from './sort.js'; +export { AUTHORITY_ASSOCIATION_KINDS, partitionAssociations } from '../associations/identity.js'; diff --git a/packages/core/src/associations/identity.ts b/packages/core/src/associations/identity.ts index 984467a2..8cdbf4d2 100644 --- a/packages/core/src/associations/identity.ts +++ b/packages/core/src/associations/identity.ts @@ -92,6 +92,16 @@ function stripAssociationAnnotation(association: Association): Association { return identity as Association; } +/** + * The association kinds that carry authority. Every other kind is data. + * Storage that partitions by kind in a query reads this list rather than + * restating it, so a new authority kind can't fall into the data half. + */ +export const AUTHORITY_ASSOCIATION_KINDS: readonly AuthorityAssociation['kind'][] = [ + 'permission', + 'anyone', +]; + /** * Whether an association carries authority rather than data — the partition * `StackRecord.permissions` and `StackRecord.associations` project, and the @@ -99,7 +109,7 @@ function stripAssociationAnnotation(association: Association): Association { * See docs/spec/access-control.md § Record-level permissions. */ export function isAuthorityAssociation(a: Association): a is AuthorityAssociation { - return a?.kind === 'permission' || a?.kind === 'anyone'; + return (AUTHORITY_ASSOCIATION_KINDS as readonly string[]).includes(a.kind); } /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0333413f..c19e47d8 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -162,4 +162,4 @@ export { hashSchema, isCompatible } from './schema.js'; export type { SchemaDriftViolation } from './schema.js'; export type { ValidationError } from './validate.js'; export { applyMergePatch } from './merge.js'; -export { assertOneSurface } from './associations/identity.js'; +export { assertOneSurface, isAuthorityAssociation } from './associations/identity.js'; diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index 9d0fdf5c..4028f9f9 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -52,6 +52,7 @@ import { applyAssociationEdits, assertOneSurface, associationEqual, + partitionAssociations, } from './associations/identity.js'; /** Core resolves the default direction; an adapter only ever sees an explicit one. */ @@ -712,10 +713,6 @@ export class IncapableMemoryAdapter extends MemoryAdapter { } } -/** Whether an association carries authority — mirrors core's own predicate. */ -const isAuthority = (a: Association): a is AuthorityAssociation => - a.kind === 'permission' || a.kind === 'anyone'; - /** * A record's whole association table, both projections rejoined — what * amendAssociations() acts on, since storage keys all kinds alike. @@ -739,8 +736,7 @@ function withAssociationSet(record: StackRecord, associations: Association[]): S [], ); const { associations: _dropA, permissions: _dropP, ...rest } = record; - const data = keyed.filter((a): a is DataAssociation => !isAuthority(a)); - const authority = keyed.filter(isAuthority); + const { associations: data, permissions: authority } = partitionAssociations(keyed); return { ...(rest as StackRecord), ...(data.length && { associations: data }), diff --git a/packages/record-adapter-sqlite/tests/record.test.ts b/packages/record-adapter-sqlite/tests/record.test.ts index 6356c101..fd306d65 100644 --- a/packages/record-adapter-sqlite/tests/record.test.ts +++ b/packages/record-adapter-sqlite/tests/record.test.ts @@ -598,37 +598,33 @@ describe('ifVersion', () => { expect((await adapter.getRecord(record.id))?.version).toBe(1); }); - // mutateRecord's non-bumping path (opts.bumpsVersion: false — what a - // caller passes for an associations-only change set) re-checks - // ifVersion against a fresh read inside its own transaction, - // since there is no version-bumping UPDATE for the guard to ride in. - // That read can come back null if a concurrent purge lands - // between mutateRecord's own read and this re-check; the adapter must - // report that as StackNotFoundError like every other write path here, - // not crash on it. - test('mutateRecord with bumpsVersion: false reports StackNotFoundError, not a crash, for a concurrently deleted record', async () => { + test('a non-bumping mutateRecord racing a purge reports StackNotFoundError', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord()); - const logic = ( - adapter as unknown as { - record: { getRecord: (id: string) => Promise }; - } - ).record; - const originalGetRecord = logic.getRecord.bind(logic); - vi.spyOn(logic, 'getRecord').mockImplementationOnce(async (id: string) => { - const found = await originalGetRecord(id); - await adapter.deleteRecord(id, { purge: true }); // races mutateRecord's own no-bump re-read - return found; - }); - - await expect( + const [, mutated] = await Promise.allSettled([ + adapter.deleteRecord(record.id, { purge: true }), adapter.mutateRecord( record.id, { associations: [{ kind: 'tag', label: 'x' }] }, { bumpsVersion: false, ifVersion: 1 }, ), - ).rejects.toBeInstanceOf(StackNotFoundError); + ]); + expect(mutated.status === 'rejected' && mutated.reason).toBeInstanceOf(StackNotFoundError); + }); + + test('concurrent unguarded patches both land', async () => { + const adapter = await initAdapter(); + const record = await adapter.createRecord(makeRecord({ content: { a: 0, b: 0 } })); + + await Promise.all([ + adapter.mutateRecord(record.id, { contentPatch: { a: 1 } }), + adapter.mutateRecord(record.id, { contentPatch: { b: 1 } }), + ]); + + const after = await adapter.getRecord(record.id); + expect(after?.content).toEqual({ a: 1, b: 1 }); + expect(after?.version).toBe(3); }); test('a permissions change set enforces ifVersion', async () => { @@ -682,37 +678,15 @@ describe('ifVersion', () => { }); // ------------------------------------------------------- -// ifVersion — enforced against the row actually written +// ifVersion — checked inside the transaction that writes // ------------------------------------------------------- /** - * restoreVersion() and commitMigration() settle their precondition against - * a record read before the transaction opens, because fts5Strategy.remove() - * needs the old content still on the row. That read is not the enforcement: - * the UPDATE carries the guard too, so a writer landing in the window loses - * rather than being silently overwritten. - * - * The window is reachable from a test because both methods `await` that - * read: calling one runs its body up to the await and hands control back - * here with the precondition already satisfied, so the next synchronous - * statement below lands squarely between the check and the write. + * Two calls guarded on the same version: whichever commits first wins, and + * the other sees the version it moved to. */ -describe('ifVersion is re-checked by the statement that writes', () => { - /** - * A second writer, on its own connection — the adapter holds a lock - * *file*, which only initialize()/open() consult, so this is the real - * race rather than a stand-in for one. - */ - const bumpVersionBehindTheAdapter = (id: string): void => { - const other = new DatabaseSync(dbPath); - try { - other.prepare('UPDATE records SET version = version + 1 WHERE id = ?').run(id); - } finally { - other.close(); - } - }; - - test('restoreVersion loses to a writer that lands after the precondition check', async () => { +describe('ifVersion holds between concurrent writes', () => { + test('restoreVersion: one of two racing restores wins', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord({ content: { text: 'original' } })); await adapter.saveVersion(record.id, { @@ -723,39 +697,31 @@ describe('ifVersion is re-checked by the statement that writes', () => { }); await adapter.mutateRecord(record.id, { contentPatch: { text: 'v2' } }); // -> v2 - const restoring = adapter.restoreVersion(record.id, 1, { ifVersion: 2 }); - bumpVersionBehindTheAdapter(record.id); // -> v3, after the check, before the write - - const err = await restoring.catch((e: unknown) => e); + const results = await Promise.allSettled([ + adapter.restoreVersion(record.id, 1, { ifVersion: 2 }), + adapter.restoreVersion(record.id, 1, { ifVersion: 2 }), + ]); + expect(results.map((r) => r.status)).toEqual(['fulfilled', 'rejected']); + const err = (results[1] as PromiseRejectedResult).reason; expect(err).toBeInstanceOf(StackVersionConflictError); expect((err as StackVersionConflictError).actualVersion).toBe(3); - - // And the restore left nothing behind: the content the other writer - // found is the content still there. - const after = await adapter.getRecord(record.id); - expect(after?.content).toEqual({ text: 'v2' }); - expect(after?.version).toBe(3); }); - test('commitMigration loses to a writer that lands after the precondition check', async () => { + test('commitMigration: one of two racing migrations wins', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord({ content: { text: 'original' } })); - const migrating = adapter.commitMigration( - record.id, - 'com.example/note@2', - { text: 'migrated' }, - { ifVersion: 1 }, - ); - bumpVersionBehindTheAdapter(record.id); // -> v2, after the check, before the write - - const err = await migrating.catch((e: unknown) => e); + const results = await Promise.allSettled([ + adapter.commitMigration(record.id, 'com.example/note@2', { text: 'a' }, { ifVersion: 1 }), + adapter.commitMigration(record.id, 'com.example/note@2', { text: 'b' }, { ifVersion: 1 }), + ]); + expect(results.map((r) => r.status)).toEqual(['fulfilled', 'rejected']); + const err = (results[1] as PromiseRejectedResult).reason; expect(err).toBeInstanceOf(StackVersionConflictError); expect((err as StackVersionConflictError).actualVersion).toBe(2); const after = await adapter.getRecord(record.id); - expect(after?.typeId).toBe(record.typeId); - expect(after?.content).toEqual({ text: 'original' }); + expect(after?.content).toEqual({ text: 'a' }); }); test('a soft delete stamps deleted_at and updated_at from one timestamp', async () => { @@ -1396,7 +1362,7 @@ describe('file-ref indexing', () => { }); // The fields content_index covers are cached per typeId (keyed off - // saveType()) so that syncContentIndex() doesn't re-query and re-parse the + // saveType()) so that ContentIndex.sync() doesn't re-query and re-parse the // schema on every write — this pins that redefining a type reaches the // cache. test('redefining a type via saveType updates which fields are treated as file-ref', async () => { diff --git a/packages/sqlite-shared/src/cursor.ts b/packages/sqlite-shared/src/cursor.ts index db9de669..f7cebd76 100644 --- a/packages/sqlite-shared/src/cursor.ts +++ b/packages/sqlite-shared/src/cursor.ts @@ -11,16 +11,6 @@ import { NATIVE_SORT_FIELDS, StackBadRequestError } from '@haverstack/core'; import { contentSortEntry } from '@haverstack/core/adapter'; import type { StackRecord, NativeSortField, QuerySort, ScalarFieldKind } from '@haverstack/core'; -export type SortField = NativeSortField; - -/** - * Re-exported under this package's own name rather than re-spelled: the - * set a cursor's `native` field is narrowed against is the same set core - * narrows a wire request's sort field to, and a fourth column added there - * must not need finding here too. - */ -export const SORT_FIELDS: readonly SortField[] = NATIVE_SORT_FIELDS; - /** * Which partition of the ordering a cursor sits in: a native column or a * content field's number, text, or absent value. The three content cases @@ -31,7 +21,7 @@ export const SORT_FIELDS: readonly SortField[] = NATIVE_SORT_FIELDS; export type CursorKind = 'native' | 'num' | 'text' | 'absent'; export type DecodedCursor = - | { kind: 'native'; field: SortField; value: number; id: string } + | { kind: 'native'; field: NativeSortField; value: number; id: string } | { kind: 'num'; field: string; value: number; id: string } | { kind: 'text'; field: string; value: string; id: string } | { kind: 'absent'; field: string; id: string }; @@ -82,20 +72,20 @@ export const decodeCursor = (cursor: string): DecodedCursor => { throw new StackBadRequestError(`Invalid cursor: non-numeric sort value`); } if (k === 'native') { - if (!SORT_FIELDS.includes(f as SortField)) { + if (!NATIVE_SORT_FIELDS.includes(f as NativeSortField)) { throw new StackBadRequestError(`Invalid cursor: unknown sort field "${f}"`); } - return { kind: 'native', field: f as SortField, value: v, id: i }; + return { kind: 'native', field: f as NativeSortField, value: v, id: i }; } return { kind: 'num', field: f, value: v, id: i }; } throw malformed(); }; -export const getSortField = (query: { sort?: QuerySort }): SortField => +export const getSortField = (query: { sort?: QuerySort }): NativeSortField => query.sort?.field ?? 'createdAt'; -export const getSortColumn = (field: SortField): string => +export const getSortColumn = (field: NativeSortField): string => field === 'createdAt' ? 'created_at' : field === 'updatedAt' ? 'updated_at' : 'version'; /** diff --git a/packages/sqlite-shared/src/executor.ts b/packages/sqlite-shared/src/executor.ts index 3e20f396..7c0a7481 100644 --- a/packages/sqlite-shared/src/executor.ts +++ b/packages/sqlite-shared/src/executor.ts @@ -20,14 +20,10 @@ export interface SqlExecutor { /** Run a parameterized statement and return all result rows. */ all>(sql: string, params?: readonly unknown[]): T[]; /** - * Run `fn` atomically: every statement it issues commits together, or - * none do if it throws. `fn` must be synchronous and call back into - * this executor only — not every engine's transaction primitive can - * straddle a suspended call. A binding with a real multi-statement - * transaction (BEGIN/COMMIT/ROLLBACK) implements this with one; a - * binding whose transaction primitive is itself a callback wrapper - * (e.g. a Durable Object's storage.transactionSync) can pass `fn` - * straight through to it. + * Run `fn` atomically: every statement it issues commits, or none do if it + * throws. A callback rather than BEGIN/COMMIT text because some engines + * only offer one (a Durable Object's storage.transactionSync); for the + * same reason `fn` must be synchronous and use only this executor. */ transaction(fn: () => T): T; } diff --git a/packages/sqlite-shared/src/fts5.ts b/packages/sqlite-shared/src/fts5.ts index ccad28d7..b89f688a 100644 --- a/packages/sqlite-shared/src/fts5.ts +++ b/packages/sqlite-shared/src/fts5.ts @@ -1,6 +1,6 @@ /** - * FTS5 query sanitization and indexing — the full-text search half of the - * shared record logic. + * FTS5 query sanitization: turning what a person typed into a search box + * into a query FTS5 will parse. Indexing lives in indexes.ts. * * FTS5's query language supports operators (AND/OR/NOT, phrases, NEAR, * wildcards) that can be expensive or cause parse errors with untrusted @@ -40,44 +40,41 @@ * Bounding execution time belongs to whoever drives the engine under load * — see docs/spec/wire-format.md § Bounding query cost for the server-side * expectation and the `timeout` wire error that goes with it. + * + * Backtracking. The input is untrusted text, so every regex here is + * written not to backtrack quadratically on it: + * + * - Whitespace runs are collapsed before any rule that spells "beside an + * operator or paren" as `\s*`; a `\s*` facing a long run re-tries the + * whole run per starting position. The rewrite is still a little + * worse than linear overall, so bounding a search's length belongs to + * whoever accepts it. + * - The NEAR rule has no `\s*` after its paren (its `[^,()]*` already + * takes a space, and two quantifiers matching the same character give + * the engine an ambiguous split), and neither of its runs crosses a + * `(`, so an unclosed `NEAR(NEAR(NEAR(` doesn't re-scan to the end once + * per occurrence. + * - The no-right-operand rule has no leading `\s*`, for the same reason + * as the collapse. + * - The operand-before-paren rule's lookbehind pins each match to the + * start of a word; without it every position inside a long word is a + * start, each re-scanning the rest of it. + * - Empty phrases and empty paren pairs are removed by one rule, so a run + * of them becomes one space rather than one per construct. */ -import type { SqlExecutor } from './executor.js'; - /** * Outside a phrase, everything but a letter, digit, mark, underscore, - * whitespace, paren or quote becomes a separator. - * - * An allow-list rather than a list of metacharacters to strip, because - * FTS5's column-filter syntax is wider than it looks and an enumeration - * kept missing parts of it: `-name` and `{a b}` filter columns with no - * colon in sight, so `-cats` and even the hyphen inside `cats-dogs` were - * read as column names and answered "no such column". `+` is a bare - * syntax error. Listing what may pass means the next piece of syntax - * nobody here anticipated arrives as a separator instead of as an error. - * - * Nothing is lost by it: FTS5's default tokenizer already splits terms on - * these characters, so `cats-dogs` searches for the same two tokens it was - * indexed as. + * whitespace, paren or quote becomes a separator. An allow-list because + * FTS5's column-filter syntax is wider than any list of metacharacters. + * See docs/spec/data-model.md § Capability-gated filters. */ const OUTSIDE_PHRASE_DISALLOWED = /[^\p{L}\p{N}\p{M}_\s()"]/gu; /** - * Collapse every whitespace run to a single space. - * - * Every rule below that reasons about what sits beside an operator or a - * paren spells "beside" as `\s*`, and a `\s*` with a long run to chew on - * re-tries the whole run once per starting position — quadratic in the - * run, on text a caller types into a box. Keeping runs collapsed bounds - * what any of them can backtrack over to a single character. What it does - * not buy is linearity overall: the rewrite still costs a little more - * than proportionally as the text grows, which is why bounding a search's - * length belongs to whoever accepts it. - * - * Nothing is lost by it: FTS5's tokenizer splits on whitespace, so a - * collapsed run tokenizes exactly as the original did — inside a phrase - * as well as outside, since a phrase matches a sequence of tokens rather - * than the spacing between them. + * Bounds what any `\s*` can backtrack over (see Backtracking above). Safe + * inside a phrase too: FTS5 matches a phrase as a sequence of tokens, not + * the spacing between them. */ const collapseWhitespace = (query: string): string => query.replace(/\s+/g, ' '); @@ -92,21 +89,9 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { clean = clean.replace(/\*/g, ''); - // Replace NEAR(terms, distance) with just its terms — see note above. - // - // Two things here are about what the engine does when the closing paren - // never arrives, on text a caller types into a box. - // - // No `\s*` after the paren, though the terms it captures may open with - // whitespace: `[^,()]*` already matches a space, so a second quantifier - // that also does gives the engine an ambiguous split to backtrack - // through. The captured space is collapsed with all the rest on the way - // out. - // - // And neither run crosses a `(`, which bounds where a failed match gives - // up: without it, `NEAR(NEAR(NEAR(` re-scans to the end of the string - // once per occurrence. A NEAR's terms are words and phrases and its - // distance a number, so neither holds a paren FTS5 would accept anyway. + // NEAR(terms, distance) keeps its terms — see the note above. The + // pattern's shape is about backtracking (see Backtracking); excluding `(` + // costs nothing, since a NEAR's terms and distance never hold one. clean = clean.replace(/\bNEAR\s*\(([^,()]*)(?:,[^()]*)?\)/gi, '$1'); // A NEAR( with no closing paren isn't the function-call form the rule @@ -114,19 +99,19 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { // call it never meant to write. Keep the grouping, drop the keyword. clean = clean.replace(/\bNEAR\s*\(/gi, '('); - // Reduce to the characters that carry meaning outside a phrase — see note - // above. Everything else becomes a separator, which is what FTS5's own - // tokenizer would do with it anyway. + // See OUTSIDE_PHRASE_DISALLOWED. clean = clean.replace(/"[^"]*"|[^"]+/g, (span) => span.startsWith('"') ? span : span.replace(OUTSIDE_PHRASE_DISALLOWED, ' '), ); - // The reduction above turns every disallowed character into a space, so - // this is where the long runs come from. + // The reduction above is where long whitespace runs come from. clean = collapseWhitespace(clean); - // Strip bare NOT with no left operand — FTS5 requires "term NOT term", not "NOT term" - clean = clean.replace(/(?:^|\(\s*)NOT\s+/gi, (m) => m.replace(/NOT\s+/i, '')); + // Strip bare NOT with no left operand — FTS5 requires "term NOT term", not + // "NOT term". A phrase is matched first and kept whole, so a NOT inside one + // stays the word it is; the closing quote is optional because an odd one + // is only closed further down. + clean = clean.replace(/"[^"]*"?|(^|\(\s*)NOT\s+/gi, (m, lead?: string) => lead ?? m); // Enforce max nesting depth; replace excess ( with a space and discard // unmatched ). Parens inside a phrase are literal text to FTS5, so the @@ -161,20 +146,15 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { if (currentDepth > 0) result += ')'.repeat(currentDepth); - // Remove empty paren pairs left behind by NEAR/NOT stripping, and drop - // operators left without an operand. Looped together because each can - // create work for the other: removing an operator can empty a paren pair - // ("(a AND)" -> "(a)" -> ...), and removing a pair can strand the - // operator that pointed at it ("x AND ()" -> "x AND"). + // Looped to a fixpoint because each rule can create work for another: + // dropping an operator can empty a pair ("(a AND)"), dropping a pair can + // strand an operator ("x AND ()"), and an empty phrase's quotes can + // re-pair once text is inserted beside it. let prev: string; do { prev = result; - // Empty phrases and empty paren pairs, each of which matches nothing, - // in one rule so that a run of them collapses to a single space - // rather than to one space apiece — a run of spaces is what the rules - // below would have to backtrack over. An empty phrase is also the one - // construct whose quotes can re-pair differently once a rule inserts - // text beside it, which is why this repeats to a fixpoint. + // Empty phrases and empty paren pairs match nothing. One rule for both + // (see Backtracking). result = result.replace(/(?:""|\(\s*\))+/g, ' '); result = dropDanglingOperators(result); result = restoreImplicitAnd(result); @@ -188,23 +168,17 @@ const OPERATOR = String.raw`(?:AND|OR|NOT)`; /** * Stands in for a phrase while operators are rewritten. A private-use - * character is not a word character, so `\b` still sees the operator - * beside it, while `^`, `$` and the paren anchors see an operand — which - * is what a phrase is. Nothing reaches here holding one: `clean` is - * rewritten from the caller's string, and a literal U+E000 in it would be - * a term character FTS5 tokenizes away regardless. + * character is not a word character, so `\b` still sees an operator beside + * it, while the anchors see an operand. A literal U+E000 in the input is + * one FTS5 tokenizes away regardless. */ const PHRASE_HOLE = '\u{E000}'; /** - * Drop operators left without an operand — the shape a person types on - * their way to a longer query ("cats AND") or lands on once the rules - * above have removed a term. FTS5 rejects each of them outright, so what - * would otherwise be a syntax error becomes the search for the terms that - * are actually there. - * - * Phrases are masked out first: `"cats AND dogs"` is literal text to FTS5, - * and rewriting inside it would change what the user asked for. + * Drop operators left without an operand — typed on the way to a longer + * query ("cats AND"), or left behind once a rule above removed a term. + * FTS5 rejects each outright. Phrases are masked: inside one, an operator + * is a word the user searched for. */ const dropDanglingOperators = (query: string): string => withPhrasesMasked(query, (masked) => { @@ -220,9 +194,8 @@ const dropDanglingOperators = (query: string): string => // No left operand: at the start of the query or straight after "(". out = out.replace(new RegExp(String.raw`(^|\()\s*\b${OPERATOR}\b\s*`, 'gi'), '$1'); // No right operand: at the end of the query or straight before ")". - // No leading `\s*` — whitespace before the operator is left for the - // collapse on the way out, and a `\s*` here would re-scan a run of - // spaces once per position in it before failing to find an operator. + // No leading `\s*` (see Backtracking); the collapse on the way out + // takes that whitespace. out = out.replace(new RegExp(String.raw`\b${OPERATOR}\b\s*($|\))`, 'gi'), '$1'); } while (out !== prev); return out; @@ -244,24 +217,17 @@ const withPhrasesMasked = (query: string, rewrite: (masked: string) => string): return rewrite(masked).replace(new RegExp(PHRASE_HOLE, 'g'), () => phrases[i++]); }; -/** - * Put back the operator FTS5 requires around a group. Two terms side by - * side are an implicit AND (`cats dogs`), but a group is not: `cats (dogs)` - * is a syntax error where `cats AND (dogs)` is fine. Grouping survives a - * rewrite that strips the operator beside it, so this - * names the AND that was always meant rather than dropping the parens. - * - * Both rules test a *whole* word against the operator list. Matching a - * prefix instead reads the `D` of a preceding `AND` as a term and inserts - * a second one beside it. - * - * Phrases are masked for the same reason as elsewhere — a phrase is an - * operand, so `"a b" (c)` needs the operator too. - */ const OPERAND_CHAR = String.raw`[\p{L}\p{N}_${PHRASE_HOLE}]`; +/** A whole word, never a prefix: the `D` of `AND` is not a term. */ const isOperator = (word: string): boolean => /^(?:AND|OR|NOT)$/i.test(word); +/** + * Put back the operator FTS5 requires beside a group: `cats dogs` is an + * implicit AND, but `cats (dogs)` is a syntax error. A rewrite that strips + * the operator beside a group keeps the grouping, so this writes back the + * AND that was meant. A masked phrase is an operand like any other. + */ const restoreImplicitAnd = (query: string): string => withPhrasesMasked(query, (masked) => masked @@ -274,48 +240,10 @@ const restoreImplicitAnd = (query: string): string => return next && isOperator(next[1]) ? match : ') AND '; }, ) - // An operand then "(", unless the operand is an operator. - // The lookbehind pins the match to the start of the operand run. - // Without it every position inside a run is a start position, and - // each one re-scans the rest of the run before the lookahead - // rejects it — quadratic in a long word before a paren. + // An operand then "(", unless the operand is an operator. The + // lookbehind is load-bearing (see Backtracking). .replace( new RegExp(String.raw`(? (isOperator(word) ? match : `${word} AND `), ), ); - -// ------------------------------------------------------- -// Indexing strategy -// ------------------------------------------------------- - -/** - * FTS5's external-content table needs its special `('delete', rowid, - * content)` command — with the *old* content — to unindex a row; a plain - * DELETE leaves stale, still-searchable entries. remove() therefore reads - * the current rowid/content and must run before the caller's change. - */ -export const fts5Strategy = { - /** Index a record that has no FTS entry yet (used right after an INSERT). */ - insert(exec: SqlExecutor, recordId: string, content: string): void { - exec.run(`INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, [ - content, - recordId, - ]); - }, - /** - * Remove a record's FTS entry. MUST be called before the records row's - * content changes or the row is deleted — see the note above. - */ - remove(exec: SqlExecutor, recordId: string): void { - const row = exec.get<{ rowid: number; content: string }>( - 'SELECT rowid, content FROM records WHERE id = ?', - [recordId], - ); - if (!row) return; - exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ - row.rowid, - row.content, - ]); - }, -}; diff --git a/packages/sqlite-shared/src/index.ts b/packages/sqlite-shared/src/index.ts index 90323da4..417a7da2 100644 --- a/packages/sqlite-shared/src/index.ts +++ b/packages/sqlite-shared/src/index.ts @@ -6,6 +6,6 @@ * pieces; see the note there for why that subset has to exist at all. */ export * from './record.js'; -export { TOKENS_SCHEMA_SQL } from './schema.js'; +export { TOKENS_SCHEMA_SQL, PRAGMA_JOURNAL_MODE_WAL } from './schema.js'; export { acquireLock, releaseLock } from './lock.js'; -export { SharedTokenLogic, type SharedTokenLogicDeps } from './token-logic.js'; +export { SharedTokenLogic } from './token-logic.js'; diff --git a/packages/sqlite-shared/src/indexes.ts b/packages/sqlite-shared/src/indexes.ts new file mode 100644 index 00000000..01328872 --- /dev/null +++ b/packages/sqlite-shared/src/indexes.ts @@ -0,0 +1,135 @@ +/** + * The two derived indexes a records database keeps beside its rows: FTS5 + * for `filter.search`, and content_index for `sort.contentField` and the + * file-reference filters. Both can be rebuilt from `records` alone, and + * both must be rewritten in the same transaction as any write that changes + * what they cover, or a query answers from a record that no longer exists. + */ + +import type { StackType, ScalarFieldKind } from '@haverstack/core'; +import { contentSortEntry } from '@haverstack/core/adapter'; +import type { SqlExecutor } from './executor.js'; + +/** + * FTS5's external-content table needs its special `('delete', rowid, + * content)` command — with the *old* content — to unindex a row; a plain + * DELETE leaves stale, still-searchable entries. remove() therefore reads + * the current rowid/content and must run before the caller's change. + */ +export class SearchIndex { + constructor(private readonly exec: SqlExecutor) {} + + /** Index a record that has no FTS entry yet (used right after an INSERT). */ + insert(recordId: string, content: string): void { + this.exec.run( + `INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, + [content, recordId], + ); + } + + /** + * Remove a record's FTS entry. MUST be called before the records row's + * content changes or the row is deleted — see the note above. + */ + remove(recordId: string): void { + const row = this.exec.get<{ rowid: number; content: string }>( + 'SELECT rowid, content FROM records WHERE id = ?', + [recordId], + ); + if (!row) return; + this.exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ + row.rowid, + row.content, + ]); + } +} + +/** + * Top-level scalar fields, by name and declared kind — what content_index + * covers. Arrays and objects are left out: a value inside one has no + * single position to order its record by, and no file reference the + * attachment filter can see. + */ +const indexedFieldKinds = ( + schema: Record, +): Map => { + const kinds = new Map(); + for (const [field, def] of Object.entries(schema)) { + if (!def.kind || def.kind === 'array' || def.kind === 'object') continue; + kinds.set(field, def.kind as ScalarFieldKind); + } + return kinds; +}; + +/** + * One row per top-level scalar field a record holds a value at. Caches each + * type's indexed fields so a write doesn't re-read and re-parse the schema. + */ +export class ContentIndex { + private readonly fieldsByType = new Map>(); + + constructor(private readonly exec: SqlExecutor) {} + + /** Prime the cache from a type as it is saved. */ + rememberType(type: StackType): void { + this.fieldsByType.set( + type.id, + indexedFieldKinds(type.schema as Record), + ); + } + + /** + * Replace a record's rows with what its content currently holds, on every + * write that can change content or typeId, so neither the ordering nor the + * file-reference lookup can drift from the record. A field holding no + * orderable value gets no row at all, which is what puts the record at the + * end of a sort on that field. + */ + sync(recordId: string, typeId: string, content: Record): void { + this.remove(recordId); + + for (const [field, kind] of this.fieldsOf(typeId)) { + const entry = contentSortEntry(kind, content[field]); + if (!entry) continue; + const sql = `INSERT OR IGNORE INTO content_index + (record_id, field, value_rank, num_value, text_key, text_value, file_id) + VALUES (?, ?, ?, ?, ?, ?, ?)`; + if (entry.kind === 'num') { + this.exec.run(sql, [recordId, field, 0, entry.num, null, null, null]); + } else { + // A file-ref field's value is its file id, so the row it already + // needs for ordering also answers the attachment filter — one + // field is one row, whatever a query reads it for. + const fileId = kind === 'file-ref' ? entry.text : null; + this.exec.run(sql, [recordId, field, 1, null, entry.key, entry.text, fileId]); + } + } + } + + remove(recordId: string): void { + this.exec.run('DELETE FROM content_index WHERE record_id = ?', [recordId]); + } + + /** The declared kind of a type's top-level `field`, if it has one. */ + kindOf(typeId: string, field: string): ScalarFieldKind | undefined { + return this.fieldsOf(typeId).get(field); + } + + /** + * From the cache, falling back to a `types` lookup only for a typeId this + * process hasn't seen via rememberType() yet, e.g. right after open(). + */ + private fieldsOf(typeId: string): Map { + const cached = this.fieldsByType.get(typeId); + if (cached) return cached; + + const typeRow = this.exec.get<{ schema: string }>('SELECT schema FROM types WHERE id = ?', [ + typeId, + ]); + const fields = typeRow + ? indexedFieldKinds(JSON.parse(typeRow.schema) as Record) + : new Map(); + this.fieldsByType.set(typeId, fields); + return fields; + } +} diff --git a/packages/sqlite-shared/src/lock.ts b/packages/sqlite-shared/src/lock.ts index 4adebe25..cdb14c14 100644 --- a/packages/sqlite-shared/src/lock.ts +++ b/packages/sqlite-shared/src/lock.ts @@ -32,7 +32,7 @@ const isProcessAlive = (pid: number): boolean => { */ export const acquireLock = (dbPath: string, force?: boolean): void => { const lockPath = lockPathFor(dbPath); - const info = existsSync(lockPath) ? readLock(lockPath) : null; + const info = readLock(lockPath); if (info) { const ownedBySelf = info.pid === process.pid; if (!ownedBySelf && !force && isProcessAlive(info.pid)) { @@ -47,13 +47,10 @@ export const acquireLock = (dbPath: string, force?: boolean): void => { }; /** - * Read the lock, treating an unreadable one as absent. A lock file is - * written non-atomically, so a crash mid-write leaves a torn one — and a - * lock naming no live process is exactly what acquireLock() reclaims. The - * alternative is a database that cannot be opened again by any means the - * error message mentions, which is the worse failure: the lock exists to - * prevent a second writer, not to outrank the data it guards. - * releaseLock() takes the same view. + * Read the lock, treating an unreadable one as absent. A crash mid-write + * leaves a torn lock naming no live process, which is what acquireLock() + * reclaims anyway; refusing it would lock the data out for good. The lock + * exists to stop a second writer, not to outrank the data it guards. */ const readLock = (lockPath: string): LockInfo | null => { try { diff --git a/packages/sqlite-shared/src/mappers.ts b/packages/sqlite-shared/src/mappers.ts index 44fa6ea0..a922999b 100644 --- a/packages/sqlite-shared/src/mappers.ts +++ b/packages/sqlite-shared/src/mappers.ts @@ -6,8 +6,6 @@ import type { Actor, - AuthorityAssociation, - DataAssociation, GroupRole, Grantee, RecordJournalEntry, @@ -17,6 +15,7 @@ import type { Association, RelationshipTarget, } from '@haverstack/core'; +import { partitionAssociations } from '@haverstack/core/adapter'; export const toMs = (d: Date): number => d.getTime(); @@ -33,23 +32,6 @@ const actorFromColumns = (subject: unknown, principal: unknown): Actor | undefin }; export const fromMs = (ms: number): Date => new Date(ms); -/** - * A record's stored edges, split into the two fields it presents. - * Partitioned by kind rather than stored apart: one table, one delta, one - * durability tier, and an app editing tags never sees the authority half. - * See docs/spec/access-control.md § Record-level permissions. - */ -export const partitionAssociations = ( - associations: Association[], -): { associations: DataAssociation[]; permissions: AuthorityAssociation[] } => ({ - associations: associations.filter( - (a): a is DataAssociation => a.kind !== 'permission' && a.kind !== 'anyone', - ), - permissions: associations.filter( - (a): a is AuthorityAssociation => a.kind === 'permission' || a.kind === 'anyone', - ), -}); - export const rowToRecord = ( row: Record, associations: Association[], @@ -75,6 +57,8 @@ export const rowToRecord = ( if (updatedBy) record.updatedBy = updatedBy; if (row.deleted_at != null) record.deletedAt = fromMs(row.deleted_at as number); if (row.unlisted_at != null) record.unlistedAt = fromMs(row.unlisted_at as number); + // One table, partitioned on read. See docs/spec/access-control.md + // § Record-level permissions. const partitioned = partitionAssociations(associations); if (partitioned.associations.length) record.associations = partitioned.associations; if (partitioned.permissions.length) record.permissions = partitioned.permissions; diff --git a/packages/sqlite-shared/src/query.ts b/packages/sqlite-shared/src/query.ts index 04e28a08..0ebeb7dd 100644 --- a/packages/sqlite-shared/src/query.ts +++ b/packages/sqlite-shared/src/query.ts @@ -2,7 +2,7 @@ * StackQuery -> SQL. Engine-independent — shared verbatim between * SQLite-backed record adapters. The one caveat is full-text search: * `f.search` here assumes a `records_fts` virtual table with a MATCH-able - * `content` column (see fts5.ts). + * `content` column (see FTS5_SCHEMA_SQL in schema.ts). * * A query compiles to a plan rather than to a single statement, because a * content-field sort is two orderings end to end: the records that hold a @@ -13,15 +13,9 @@ * fact, which costs a full pass over the table for every page. */ -import { StackBadRequestError, type StackQuery } from '@haverstack/core'; +import { StackBadRequestError, type NativeSortField, type StackQuery } from '@haverstack/core'; import { contentSortKey, parseContentFilterKey } from '@haverstack/core/adapter'; -import { - decodeCursor, - getSortColumn, - getSortField, - type DecodedCursor, - type SortField, -} from './cursor.js'; +import { decodeCursor, getSortColumn, getSortField, type DecodedCursor } from './cursor.js'; import { sanitizeFts5Query } from './fts5.js'; /** One statement plus the parameters it binds, in textual order. */ @@ -40,13 +34,10 @@ export const atBudget = (page: QueryStatement, rows: number): QueryStatement => }); /** - * Reduce `sort.direction` to a keyword this module may interpolate into - * SQL. Core's assertValidSort() already rejects anything else at the - * invariant layer, but this builder writes the value straight into - * `ORDER BY` and the cursor comparison, so it re-checks rather than trust - * an upstream that a future refactor could bypass: an unvalidated direction - * here is a SQL-injection sink. A sort without a direction is refused: core - * resolves the default before any adapter sees the query. + * Reduce `sort.direction` to a keyword safe to interpolate. Core's + * assertValidSort() already checks it, but this is where it reaches SQL + * text, so an unchecked value here would be an injection sink. Core + * resolves the default direction before any adapter sees the query. */ const sqlDirection = (query: StackQuery): 'ASC' | 'DESC' => { // No sort at all is the unsorted default; a sort always carries its direction. @@ -135,7 +126,7 @@ const cursorMismatch = (cursor: DecodedCursor, field: string): StackBadRequestEr `Cursor sort field "${cursor.field}" does not match query sort field "${field}"`, ); -const nativeCursor = (cursor: DecodedCursor, field: SortField) => { +const nativeCursor = (cursor: DecodedCursor, field: NativeSortField) => { if (cursor.kind !== 'native' || cursor.field !== field) throw cursorMismatch(cursor, field); return cursor; }; @@ -212,13 +203,9 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un rangeFilter('updated_at', f.updatedAt); // Every association filter below is a semi-join rather than a correlated - // EXISTS, so the planner drives from the association side — reading the - // matching record ids straight out of idx_assoc_kind_label / - // idx_assoc_kind_file_id / idx_assoc_related / idx_content_index_file, - // each of which ends with record_id and so needs no table read at all — - // instead of scanning every record and probing for each. The work is - // proportional to how many records match, not to how many the stack - // holds. + // EXISTS, so the planner reads matching record ids straight out of an + // index ending in record_id instead of probing once per record: the work + // scales with the matches, not the stack. /** * The semi-join itself. `kind` is one of this module's own literals, never @@ -261,14 +248,9 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un params.push(f.referencesFileId, f.referencesFileId); } - // Relationship filter — each clause is an optional pattern, so a bare - // label matches every target under it and an external target with no - // `id` matches its whole namespace (docs/spec/data-model.md § Filter). - // - // A record or entity target always stores '' in related_ns (see - // associationKeyColumns). Saying so turns idx_assoc_related's - // (related_scope, related_ns, related_id) prefix into a contiguous - // equality run for those kinds instead of a seek on related_scope alone. + // Each clause is an optional pattern (docs/spec/data-model.md § Filter). + // A record or entity target always stores '' in related_ns; saying so + // keeps idx_assoc_related's prefix a contiguous equality run. if (f.relatedTo) { const clauses: string[] = []; const target = f.relatedTo.target; @@ -362,12 +344,10 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un }; /** - * Everything ordered after the cursor's record within the partition that - * holds a value: a later value rank, or the same rank past the cursor's - * value and id. `op` runs the comparison the same way the ORDER BY does, - * so one form covers both directions — in the direction where the - * cursor's rank is the last one, the leading disjunct is simply never - * true. + * Everything after the cursor's record within the partition that holds a + * value. `op` compares the way the ORDER BY does, so one form covers both + * directions; where the cursor's rank is the last one, the leading + * disjunct is simply never true. */ const presentCursorCondition = ( cursor: Extract, @@ -417,9 +397,10 @@ export const buildQueryPlan = (query: StackQuery): QueryStatement[] => { if (contentField === undefined) { const { conditions, params } = recordConditions(query); - const col = getSortColumn(getSortField(query)); + const field = getSortField(query); + const col = getSortColumn(field); if (decoded) { - const cursor = nativeCursor(decoded, getSortField(query)); + const cursor = nativeCursor(decoded, field); conditions.push(`(r.${col} ${op} ? OR (r.${col} = ? AND r.id ${op} ?))`); params.push(cursor.value, cursor.value, cursor.id); } diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index 6afbf334..4e3525c4 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -32,7 +32,6 @@ import type { AssociationEdit, ActorOptions, IfVersionOptions, - ScalarFieldKind, } from '@haverstack/core'; import type { BumpVersionOptions, @@ -40,11 +39,11 @@ import type { JournalOptions, MutateOptions, } from '@haverstack/core/adapter'; -import { contentSortEntry } from '@haverstack/core/adapter'; +import { AUTHORITY_ASSOCIATION_KINDS } from '@haverstack/core/adapter'; import type { SqlExecutor } from './executor.js'; import { isForeignKeyViolation, isUniqueConstraintViolation } from './executor.js'; import { buildQueryPlan, atBudget } from './query.js'; -import { fts5Strategy } from './fts5.js'; +import { ContentIndex, SearchIndex } from './indexes.js'; import { rowToRecord, rowToAssociation, @@ -57,15 +56,10 @@ import { import { makeCursor } from './cursor.js'; /** - * Run a query statement, restating an engine parse error as the wire - * taxonomy's `bad_request` rather than letting it escape as a raw engine - * error (which a server has no code to map, so it becomes a 500). - * - * Scoped to the one filter that carries a query language: `filter.search` - * is FTS5 source text, and sanitizeFts5Query() repairs the common shapes - * but claims no completeness against the grammar. Every other clause here - * is built from parameters, so a parse error in one is this module's bug — - * rethrown untouched, where it belongs. + * Restate an engine parse error as `bad_request` when the query carries + * search text, the one filter with a query language the sanitizer can't + * fully repair. Any other clause is built from parameters, so a parse + * error there is this module's bug and is rethrown untouched. */ const asStackBadRequestError = (query: StackQuery, run: () => T): T => { try { @@ -80,7 +74,7 @@ const asStackBadRequestError = (query: StackQuery, run: () => T): T => { /** * One spelling of the optimistic-concurrency failure, so the message a - * caller sees never depends on which of the three places noticed it. + * caller sees never depends on which check noticed it. */ const versionConflict = (id: string, expected: number, actual: number): StackVersionConflictError => new StackVersionConflictError( @@ -90,6 +84,9 @@ const versionConflict = (id: string, expected: number, actual: number): StackVer actual, ); +const recordNotFound = (id: string): StackNotFoundError => + new StackNotFoundError(`Record not found: "${id}"`); + /** * Every `versions` column beyond the (record_id, version) key, and the * values for them in the same order — one list, so the INSERT that writes a @@ -120,51 +117,21 @@ export type SharedSqlRecordLogicDeps = { exec: SqlExecutor; }; -/** - * Top-level scalar fields, by name and declared kind — what content_index - * covers. Arrays and objects are left out: a value inside one has no - * single position to order its record by, and no file reference the - * attachment filter can see. - */ -const indexedFieldKinds = ( - schema: Record, -): Map => { - const kinds = new Map(); - for (const [field, def] of Object.entries(schema)) { - if (!def.kind || def.kind === 'array' || def.kind === 'object') continue; - kinds.set(field, def.kind as ScalarFieldKind); - } - return kinds; -}; - export class SharedSqlRecordLogic { - /** - * Per-typeId cache of the fields content_index covers and their declared - * kinds, so syncContentIndex() doesn't hit the `types` table and - * re-parse schema JSON on every write. Populated eagerly in saveType() - * and lazily via getIndexedFields(). - */ - private readonly indexedFieldsByType = new Map>(); + private readonly exec: SqlExecutor; + private readonly searchIndex: SearchIndex; + private readonly contentIndex: ContentIndex; - constructor(private readonly deps: SharedSqlRecordLogicDeps) {} - - private get exec(): SqlExecutor { - return this.deps.exec; + constructor({ exec }: SharedSqlRecordLogicDeps) { + this.exec = exec; + this.searchIndex = new SearchIndex(exec); + this.contentIndex = new ContentIndex(exec); } - /** - * Precondition check for a mutation that has already read the record — - * either because it can't fold the check into its own statement - * (purgeRecord: children reference the row, so the DELETE can't - * carry it), or because it has to settle the precondition *before* - * fts5Strategy.remove() runs, which needs the records row still holding - * the old content. In the latter case versionedUpdate() re-checks it - * inside the statement, and that is the enforcement; this is what lets - * the work before it be skipped. - */ - private checkExpectedVersion(record: StackRecord, expectedVersion: number | undefined): void { - if (expectedVersion === undefined || record.version === expectedVersion) return; - throw versionConflict(record.id, expectedVersion, record.version); + /** Check `ifVersion` against a version read inside the caller's transaction. */ + private checkExpectedVersion(id: string, actual: number, expected: number | undefined): void { + if (expected === undefined || actual === expected) return; + throw versionConflict(id, expected, actual); } /** @@ -174,23 +141,39 @@ export class SharedSqlRecordLogic { * the actual current version for the caller to act on). */ private throwVersionConflict(id: string, expectedVersion: number | undefined): never { - const row = this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [ - id, - ]); - if (!row) throw new StackNotFoundError(`Record not found: "${id}"`); - throw versionConflict(id, expectedVersion as number, row.version); + const version = this.currentVersion(id); + if (version === undefined) throw recordNotFound(id); + throw versionConflict(id, expectedVersion as number, version); + } + + private currentVersion(id: string): number | undefined { + return this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [id]) + ?.version; } /** - * The one shape every mutating UPDATE in this class has: whatever columns - * the caller is changing, then the version bump and the actor/timestamp - * stamp that every mutation owes, gated on the opt-in `ifVersion` - * precondition and failing loudly when nothing matched. - * - * The guard rides in the WHERE clause even where the caller has already - * checked the version against a record it read: that read is outside this - * statement, so a writer slipping in between the two is caught here - * rather than silently overwritten. + * What a mutation builds on, read inside its transaction so nothing can + * land between the read and the write: a patch applied to an earlier read + * would overwrite whatever changed since. Throws for a missing record or + * a failed `ifVersion`. + */ + private readForWrite( + id: string, + ifVersion: number | undefined, + ): { typeId: TypeId; content: Record } { + const row = this.exec.get<{ version: number; type_id: string; content: string }>( + 'SELECT version, type_id, content FROM records WHERE id = ?', + [id], + ); + if (!row) throw recordNotFound(id); + this.checkExpectedVersion(id, row.version, ifVersion); + return { typeId: row.type_id, content: JSON.parse(row.content) }; + } + + /** + * Every version-bumping UPDATE. The `ifVersion` guard rides in the WHERE + * clause, so a write that reads nothing first (a soft delete, an + * undelete) enforces it with no separate read. */ private versionedUpdate( id: string, @@ -216,15 +199,9 @@ export class SharedSqlRecordLogic { } /** - * Read a record back after its own mutation committed, for the caller to - * return. Its absence is not a "not found" for the caller to handle — the - * mutation just succeeded against it — so it fails as the invariant - * breach it would be. - * - * Synchronous, via readRecord() rather than getRecord(), so the read - * cannot be separated from the commit by anything: an async caller - * returning this still hands back a record read in the same tick its - * transaction committed. See readRecord() for why that matters. + * Read a record back after its own mutation committed. Absence here is an + * invariant breach, not a "not found". Synchronous, so nothing can run + * between the commit and the read — see readRecord(). */ private reread(id: string, verb: string): StackRecord { const updated = this.readRecord(id); @@ -237,16 +214,10 @@ export class SharedSqlRecordLogic { // ------------------------------------------------------- /** - * A duplicate id hits the `records` PK — mapped to StackConflictError - * instead of surfacing the raw engine exception, mirroring saveVersion's - * collision mapping below. - * - * One transaction, because a record and the four tables that describe it - * are one fact. A half-applied create is worse than a failed one: the - * call raises, so the caller believes nothing landed, while the records - * row survives to fail their retry on the PK — and the row it leaves is - * invisible to `filter.search` and mis-ordered by `sort.contentField` - * until something writes it again. + * One transaction, because a record and the tables that describe it are + * one fact: a half-applied create raises, so the caller believes nothing + * landed, while the surviving row fails their retry on the PK and is + * missing from both indexes. A duplicate id maps to StackConflictError. */ async createRecord(record: StackRecord, opts: JournalOptions = {}): Promise { this.exec.transaction(() => { @@ -284,8 +255,8 @@ export class SharedSqlRecordLogic { const edges = [...(record.associations ?? []), ...(record.permissions ?? [])]; if (edges.length) this.insertAssociations(record.id, edges); - fts5Strategy.insert(this.exec, record.id, JSON.stringify(record.content)); - this.syncContentIndex(record.id, record.typeId, record.content); + this.searchIndex.insert(record.id, JSON.stringify(record.content)); + this.contentIndex.sync(record.id, record.typeId, record.content); this.appendJournal(record.id, opts.journal); }); @@ -297,64 +268,35 @@ export class SharedSqlRecordLogic { } /** - * The synchronous read behind getRecord(). Callers inside exec.transaction() - * use this one: an `await` inside that callback yields the microtask queue - * mid-transaction, and the next operation to run would try to open one of - * its own — transaction() requires a synchronous callback for exactly this - * reason (see SqlExecutor.transaction). - * - * The same synchrony is what lets the post-commit reads below report the - * version their own mutation produced: reread() calls this directly, so - * nothing interleaves between the commit and the read even though the - * method returning it is async. A backend that made these reads genuinely - * asynchronous would open that window, and a concurrent mutation could be - * reported under this one's verb. + * The synchronous read behind getRecord(), for use inside + * exec.transaction(), whose callback can't await. reread() relies on the + * same synchrony: nothing interleaves between a commit and its read, so a + * concurrent mutation is never reported under this one's verb. */ private readRecord(id: string): StackRecord | null { const row = this.exec.get>('SELECT * FROM records WHERE id = ?', [id]); - if (!row) return null; - const associations = this.getAssociationsForRecord(id); - return rowToRecord(row, associations); + return row ? this.hydrate(row) : null; + } + + /** A records row with its association set attached. */ + private hydrate(row: Record): StackRecord { + return rowToRecord(row, this.getAssociationsForRecord(row.id as string)); } /** - * Apply a change set in one transaction: one UPDATE over the record row - * with only the columns the change set names, the association set - * replaced when it carries one, and the snapshot written alongside — so - * one call is one version whatever it moved. - * - * `opts.bumpsVersion: false` is a change set that touches only - * `associations`, `parentId` and/or `unlisted` — Stack computes this from - * which ops the set actually moves. Such a write still lands its columns, - * but leaves `version`, `updatedAt` and the actor stamps exactly as they - * stood and takes no snapshot. `ifVersion`, if given, is re-checked - * synchronously inside the transaction — the CAS guard the bumping path - * gets from its UPDATE's WHERE clause, restated as a read because the - * UPDATE here carries no version predicate. See - * docs/spec/versioning.md § Version history. - * - * The ifVersion check for a bumping write is standalone rather than - * folded into the UPDATE alone, because a content change needs - * fts.remove() to run *before* the records-table content changes. The - * guard is still in the UPDATE's WHERE, so a writer that slipped in - * between the read and the write is caught there rather than overwriting. + * One transaction, so one call is one version whatever it moved, and the + * patch applies to the content as it stands when the write lands. */ async mutateRecord( id: string, changes: RecordChangeSet, opts: MutateOptions & BumpVersionOptions = {}, ): Promise { - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); - const now = toMs(new Date()); - // Assembled rather than written out per aspect: every aspect is the - // same column-and-value pair on one row, and a fixed statement per - // combination would be 2^4 of them. Built before the no-bump branch - // because both paths write the same columns; only `content` is the - // bumping path's alone, since a patch always bumps. + // Assembled, since a fixed statement per combination of aspects would + // be 2^4 of them. Shared by both paths; only `content` is the bumping + // path's alone, since a patch always bumps. const sets: string[] = []; const values: unknown[] = []; if (changes.parentId !== undefined) { @@ -369,39 +311,38 @@ export class SharedSqlRecordLogic { values.push(changes.unlisted ? now : null); } - if (opts.bumpsVersion === false) { - this.exec.transaction(() => { - const current = this.readRecord(id); - if (!current) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(current, opts.ifVersion); + this.exec.transaction(() => { + const current = this.readForWrite(id, opts.ifVersion); + + // No versioned UPDATE to carry `ifVersion`, so readForWrite() is the + // only check. See docs/spec/versioning.md § Version history. + if (opts.bumpsVersion === false) { if (sets.length > 0) { this.exec.run(`UPDATE records SET ${sets.join(', ')} WHERE id = ?`, [...values, id]); } this.replaceAssociationHalves(id, changes); this.appendJournal(id, opts.journal); - }); - return this.reread(id, 'mutateRecord'); - } + return; + } - const merged = changes.contentPatch - ? applyMergePatch(existing.content, changes.contentPatch) - : undefined; - if (merged !== undefined) { - sets.unshift('content = ?'); - values.unshift(JSON.stringify(merged)); - } + const merged = changes.contentPatch + ? applyMergePatch(current.content, changes.contentPatch) + : undefined; + if (merged !== undefined) { + sets.unshift('content = ?'); + values.unshift(JSON.stringify(merged)); + } - this.exec.transaction(() => { if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); - if (merged !== undefined) fts5Strategy.remove(this.exec, id); + if (merged !== undefined) this.searchIndex.remove(id); this.versionedUpdate(id, sets, values, opts, now); this.replaceAssociationHalves(id, changes); if (merged !== undefined) { - fts5Strategy.insert(this.exec, id, JSON.stringify(merged)); - this.syncContentIndex(id, existing.typeId, merged); + this.searchIndex.insert(id, JSON.stringify(merged)); + this.contentIndex.sync(id, current.typeId, merged); } this.appendJournal(id, opts.journal); }); @@ -431,13 +372,9 @@ export class SharedSqlRecordLogic { } /** - * Deletes a record's FTS entry, associations, versions, file-refs, sort - * index rows, and row, returning the record as it stood — the last copy that will ever - * exist, read here so that nothing can overtake it between the read and - * the deletes. Null when there was no such record. No write() call — - * callers batch it. ifVersion is checked first so a lost CAS race - * leaves nothing touched (children reference the row, so the check can't - * fold into the final DELETE). + * Delete a record and everything that refers to it, returning the record + * as it stood (null if there was none). Read here, inside the caller's + * transaction, so nothing can overtake the last copy that will ever exist. */ private purgeRecord(id: string, ifVersion?: number): StackRecord | null { const purged = this.readRecord(id); @@ -445,18 +382,18 @@ export class SharedSqlRecordLogic { // A CAS against a record that isn't there is a failed precondition, // not the "nothing to delete" that an unconditional purge // reports by returning null. - if (ifVersion !== undefined) throw new StackNotFoundError(`Record not found: "${id}"`); + if (ifVersion !== undefined) throw recordNotFound(id); return null; } - this.checkExpectedVersion(purged, ifVersion); - fts5Strategy.remove(this.exec, id); + this.checkExpectedVersion(id, purged.version, ifVersion); + this.searchIndex.remove(id); this.exec.run('DELETE FROM associations WHERE record_id = ?', [id]); this.exec.run('DELETE FROM versions WHERE record_id = ?', [id]); // A purge takes the journal with it, for the reason it takes the // version history: a log naming what a destroyed record once held is // exactly the residue this verb exists to leave nothing of. this.exec.run('DELETE FROM journal WHERE record_id = ?', [id]); - this.exec.run('DELETE FROM content_index WHERE record_id = ?', [id]); + this.contentIndex.remove(id); this.exec.run('DELETE FROM records WHERE id = ?', [id]); return purged; } @@ -476,17 +413,13 @@ export class SharedSqlRecordLogic { version: number, opts: MutateOptions = {}, ): Promise { - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); - - const target = await this.getVersion(id, version); - if (!target) throw new StackNotFoundError(`Version not found: "${id}"@${version}`); - // Only content and its type are restored — a snapshot carries nothing // else, so there is nothing here to roll containment, listing or the // associations table back to. this.exec.transaction(() => { + this.readForWrite(id, opts.ifVersion); + const target = this.readVersion(id, version); + if (!target) throw new StackNotFoundError(`Version not found: "${id}"@${version}`); if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); this.rewriteContent(id, target.typeId, target.content, opts); this.appendJournal(id, opts.journal); @@ -501,17 +434,8 @@ export class SharedSqlRecordLogic { content: Record, opts: MutateOptions = {}, ): Promise { - // Checked against a read record first, rather than left to the - // UPDATE's WHERE clause alone: fts5Strategy.remove() has to run before - // the content changes, so the precondition has to settle before that - // work starts. rewriteContent()'s UPDATE still carries the guard, so a - // writer slipping in between the two is caught rather than overwritten - // — same shape as mutateRecord() and restoreVersion(). - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); - this.exec.transaction(() => { + this.readForWrite(id, opts.ifVersion); if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); this.rewriteContent(id, toTypeId, content, opts); this.appendJournal(id, opts.journal); @@ -521,14 +445,9 @@ export class SharedSqlRecordLogic { } /** - * Replace a record's type and content wholesale — what restoreVersion and - * commitMigration each do, differing only in where the content came from. - * - * The order is the constraint: fts5Strategy.remove() has to read the old - * content off the records row, so it runs before the UPDATE, and the - * re-index runs after. Every caller must already sit inside - * exec.transaction() — a failure between the two halves would otherwise - * leave the record unindexed. + * Replace a record's type and content wholesale. Unindex, write, re-index, + * in that order (docs/spec/adapters.md § The search index is unindexed + * before the row moves); callers must be inside exec.transaction(). */ private rewriteContent( id: string, @@ -537,10 +456,10 @@ export class SharedSqlRecordLogic { opts: IfVersionOptions & ActorOptions, ): void { const json = JSON.stringify(content); - fts5Strategy.remove(this.exec, id); + this.searchIndex.remove(id); this.versionedUpdate(id, ['type_id = ?', 'content = ?'], [typeId, json], opts); - fts5Strategy.insert(this.exec, id, json); - this.syncContentIndex(id, typeId, content); + this.searchIndex.insert(id, json); + this.contentIndex.sync(id, typeId, content); } async queryRecords(query: StackQuery): Promise { @@ -548,11 +467,8 @@ export class SharedSqlRecordLogic { // One extra row, to learn whether a further page follows. const fetch = limit + 1; - // A content-field sort answers from two statements, the records that - // hold a value at the field and then the records that hold none. They - // read in order, each asking only for the rows the ones before it left - // of the page, and the second is skipped once the first has filled it — - // the common case, since a page rarely straddles the boundary. + // A content-field sort is two statements (see buildQueryPlan). Each asks + // only for what the page still lacks, so the second usually never runs. const rows: Record[] = []; for (const page of buildQueryPlan(query)) { const budget = fetch - rows.length; @@ -568,26 +484,22 @@ export class SharedSqlRecordLogic { const hasMore = rows.length > limit; const page = hasMore ? rows.slice(0, limit) : rows; - const records = page.map((row) => { - const associations = this.getAssociationsForRecord(row.id as string); - return rowToRecord(row, associations); - }); + const records = page.map((row) => this.hydrate(row)); const lastRecord = records[records.length - 1]; const cursor = hasMore && lastRecord - ? makeCursor(lastRecord, query.sort, (record, field) => this.sortFieldKind(record, field)) + ? makeCursor(lastRecord, query.sort, (record, field) => + this.contentIndex.kindOf(record.typeId, field), + ) : null; return { records, cursor }; } /** - * Atomically verify fileId is unreferenced, then purge its - * metadata records across every typeId in `metadataTypeIds` (see - * deleteUnreferencedAttachmentRecords on the adapter contract). A real - * SQL transaction of synchronous calls — no `await` between the check - * and the deletes, so nothing interleaves. + * The reference check and the purges share one synchronous transaction, + * so nothing can add a reference between the two. */ async deleteUnreferencedAttachmentRecords( fileId: FileId, @@ -647,6 +559,10 @@ export class SharedSqlRecordLogic { } async getVersion(id: string, version: number): Promise { + return this.readVersion(id, version); + } + + private readVersion(id: string, version: number): RecordVersion | null { const row = this.exec.get>( 'SELECT * FROM versions WHERE record_id = ? AND version = ?', [id, version], @@ -716,10 +632,7 @@ export class SharedSqlRecordLogic { } catch (err) { if (!(err instanceof StackConflictError)) throw err; - const row = this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [ - id, - ]); - if (!row || row.version !== version.version) throw err; + if (this.currentVersion(id) !== version.version) throw err; this.overwriteVersionRow(id, version); } @@ -730,28 +643,22 @@ export class SharedSqlRecordLogic { // ------------------------------------------------------- /** - * Append one entry, stamping the record-derived half off the row as it - * now stands — so every caller must already sit inside the same - * transaction as the write it describes, after that write has landed. - * - * `seq` is allocated here, from the log's own max, rather than computed - * by the caller. That is what spares this table the collision healing - * versions needs: nothing outside this statement ever holds a seq it - * expects to be free. See docs/spec/journal.md § Ordering. + * Append one entry, stamped from the row as it now stands, so callers run + * this inside their write's transaction, after the write. `seq` is + * allocated in the INSERT itself, so no caller ever holds one it expects + * to be free. See docs/spec/journal.md § Ordering. */ private appendJournal(recordId: string, entry: JournalEntryInput | undefined): void { if (!entry) return; - // Three columns, not readRecord(): the stamp needs version/typeId/ - // parentId and nothing else, and this runs inside every mutating - // write — a full row plus its association set, per write, to read - // three values. `parent_id` is used raw so the NULL/''/id spelling - // reaches the journal exactly as the records table holds it. + // Not readRecord(): this runs on every write and needs three columns, + // not the row plus its associations. parent_id stays raw so its + // NULL/''/id spelling reaches the journal unchanged. const row = this.exec.get<{ version: number; type_id: string; parent_id: string | null; }>('SELECT version, type_id, parent_id FROM records WHERE id = ?', [recordId]); - if (!row) throw new StackNotFoundError(`Record not found: "${recordId}"`); + if (!row) throw recordNotFound(recordId); this.exec.run( `INSERT INTO journal @@ -783,7 +690,7 @@ export class SharedSqlRecordLogic { // that isn't there cannot be spelled that way. A purged record is gone, // and gets the same refusal. See docs/spec/journal.md § Reading it. if (!this.exec.get('SELECT 1 FROM records WHERE id = ?', [id])) { - throw new StackNotFoundError(`Record not found: "${id}"`); + throw recordNotFound(id); } const conditions = ['record_id = ?']; const values: unknown[] = [id]; @@ -820,8 +727,7 @@ export class SharedSqlRecordLogic { toMs(type.createdAt), ], ); - const schema = type.schema as Record; - this.indexedFieldsByType.set(type.id, indexedFieldKinds(schema)); + this.contentIndex.rememberType(type); } async getType(id: TypeId): Promise { @@ -842,9 +748,8 @@ export class SharedSqlRecordLogic { /** * Never bumps `version`/`updatedAt` and never snapshots — see - * StackRecordAdapter.amendAssociations(). The records row is untouched - * entirely; only the associations table changes, in one transaction: - * removes first, then adds. + * StackRecordAdapter.amendAssociations(). Removes apply before adds, so + * removing and re-adding one identity in a call leaves it present. */ async amendAssociations( recordId: string, @@ -853,8 +758,7 @@ export class SharedSqlRecordLogic { ): Promise { assertOneSurface(changes); this.exec.transaction(() => { - if (!this.readRecord(recordId)) - throw new StackNotFoundError(`Record not found: "${recordId}"`); + if (this.currentVersion(recordId) === undefined) throw recordNotFound(recordId); for (const change of changes) { if (change.op !== 'remove') continue; const { association } = change; @@ -880,52 +784,42 @@ export class SharedSqlRecordLogic { return this.reread(recordId, 'amendAssociations'); } - /** The kinds each change-set key replaces, and nothing else. */ - private static readonly DATA_KINDS = ['tag', 'attachment', 'relationship']; - private static readonly AUTHORITY_KINDS = ['permission', 'anyone']; - /** - * Apply a change set's two association keys, each replacing only within - * its own half of the partition. That partition is the whole reason a - * tag write cannot drop an ACL: the `associations` key deletes data - * kinds alone, so the authority rows beside them are never in scope. - * Caller-transactional, like every write below. + * Each key replaces only within its own half of the partition, which is + * why a tag write cannot drop an ACL. Caller-transactional. * See docs/spec/access-control.md § Record-level permissions. */ private replaceAssociationHalves(recordId: string, changes: RecordChangeSet): void { if (changes.associations !== undefined) { - this.replaceAssociations(recordId, changes.associations, SharedSqlRecordLogic.DATA_KINDS); + this.replaceAssociations(recordId, changes.associations, 'data'); } if (changes.permissions !== undefined) { - this.replaceAssociations(recordId, changes.permissions, SharedSqlRecordLogic.AUTHORITY_KINDS); + this.replaceAssociations(recordId, changes.permissions, 'authority'); } } /** * Swap one half of a record's association set wholesale — the "replace, - * don't merge" semantics a change set's association list carries. + * don't merge" semantics a change set's association list carries. The data + * half is "not an authority kind", so core's list is the only one. */ private replaceAssociations( recordId: string, associations: Association[], - kinds: string[], + half: 'data' | 'authority', ): void { this.exec.run( - `DELETE FROM associations WHERE record_id = ? AND kind IN (${kinds.map(() => '?').join(', ')})`, - [recordId, ...kinds], + `DELETE FROM associations WHERE record_id = ? AND kind ${half === 'data' ? 'NOT IN' : 'IN'} ` + + `(${AUTHORITY_ASSOCIATION_KINDS.map(() => '?').join(', ')})`, + [recordId, ...AUTHORITY_ASSOCIATION_KINDS], ); if (associations.length) this.insertAssociations(recordId, associations); } /** - * Upserts on association identity: the primary key decides which row a - * write lands on, and `attachment_record_id` — outside it — is whatever - * the incoming association says, so re-pointing one costs no second row. - * - * FK enforcement (PRAGMA_FOREIGN_KEYS_ON) means inserting an - * association against a record that doesn't exist throws — mapped - * here to StackNotFoundError so amendAssociations() on a nonexistent record - * fails loudly instead of silently creating an orphan row. + * Upserts on association identity, so re-pointing `attachment_record_id` + * (outside the key) costs no second row. A missing record trips the + * foreign key, reported as StackNotFoundError rather than an orphan row. */ private insertAssociations(recordId: string, associations: Association[]): void { for (const assoc of associations) { @@ -946,7 +840,7 @@ export class SharedSqlRecordLogic { ); } catch (err) { if (isForeignKeyViolation(err)) { - throw new StackNotFoundError(`Record not found: "${recordId}"`); + throw recordNotFound(recordId); } throw err; } @@ -960,61 +854,4 @@ export class SharedSqlRecordLogic { ); return rows.map(rowToAssociation); } - - /** - * The fields content_index covers for typeId, from the in-memory cache - * — falling back to a `types` lookup (and populating the cache) only for - * a typeId this process hasn't seen via saveType() yet, e.g. right after - * open(). - */ - private getIndexedFields(typeId: string): Map { - const cached = this.indexedFieldsByType.get(typeId); - if (cached) return cached; - - const typeRow = this.exec.get<{ schema: string }>('SELECT schema FROM types WHERE id = ?', [ - typeId, - ]); - const fields = typeRow - ? indexedFieldKinds(JSON.parse(typeRow.schema) as Record) - : new Map(); - this.indexedFieldsByType.set(typeId, fields); - return fields; - } - - /** - * Replace a record's content_index rows with what its content currently - * holds, on every write that can change content or typeId, so neither - * the ordering nor the file-reference lookup can drift from the record. - * A field holding no orderable value gets no row at all, which is what - * puts the record at the end of a sort on that field. - */ - private syncContentIndex( - recordId: string, - typeId: string, - content: Record, - ): void { - this.exec.run('DELETE FROM content_index WHERE record_id = ?', [recordId]); - - for (const [field, kind] of this.getIndexedFields(typeId)) { - const entry = contentSortEntry(kind, content[field]); - if (!entry) continue; - const sql = `INSERT OR IGNORE INTO content_index - (record_id, field, value_rank, num_value, text_key, text_value, file_id) - VALUES (?, ?, ?, ?, ?, ?, ?)`; - if (entry.kind === 'num') { - this.exec.run(sql, [recordId, field, 0, entry.num, null, null, null]); - } else { - // A file-ref field's value is its file id, so the row it already - // needs for ordering also answers the attachment filter — one - // field is one row, whatever a query reads it for. - const fileId = kind === 'file-ref' ? entry.text : null; - this.exec.run(sql, [recordId, field, 1, null, entry.key, entry.text, fileId]); - } - } - } - - /** The declared kind of a record's top-level `field`, if it has one. */ - private sortFieldKind(record: StackRecord, field: string): ScalarFieldKind | undefined { - return this.getIndexedFields(record.typeId).get(field); - } } diff --git a/packages/sqlite-shared/src/record.ts b/packages/sqlite-shared/src/record.ts index b9f6d226..d813b4c4 100644 --- a/packages/sqlite-shared/src/record.ts +++ b/packages/sqlite-shared/src/record.ts @@ -1,8 +1,8 @@ /** * The record-only barrel, and the base the full one (index.ts) builds on: * index.ts re-exports everything here and adds the token-store pieces - * (TOKENS_SCHEMA_SQL, SharedTokenLogic) and the file-lock helpers - * (acquireLock/releaseLock). The two surfaces therefore differ by exactly + * (TOKENS_SCHEMA_SQL, the WAL pragma its file uses, SharedTokenLogic) and + * the file-lock helpers (acquireLock/releaseLock). The two surfaces therefore differ by exactly * those additions, rather than by whatever two hand-maintained lists * happen to say. * @@ -16,37 +16,13 @@ * for lock.ts's plain functions — so avoiding the import (not just the * unused export) has to happen at this file's level. */ -export { - RECORD_SCHEMA_SQL, - FTS5_SCHEMA_SQL, - PRAGMA_FOREIGN_KEYS_ON, - PRAGMA_JOURNAL_MODE_WAL, - applyRecordSchema, - type RecordSchemaOptions, -} from './schema.js'; -export { buildQueryPlan, atBudget, type QueryStatement } from './query.js'; -export { - encodeCursor, - decodeCursor, - makeCursor, - getSortField, - getSortColumn, - SORT_FIELDS, - type SortField, - type DecodedCursor, -} from './cursor.js'; -export { rowToRecord, rowToAssociation, rowToType, rowToVersion, toMs, fromMs } from './mappers.js'; -export { sanitizeFts5Query, fts5Strategy } from './fts5.js'; -export { - type SqlExecutor, - isForeignKeyViolation, - isUniqueConstraintViolation, -} from './executor.js'; +export { applyRecordSchema } from './schema.js'; +export { sanitizeFts5Query } from './fts5.js'; +export { type SqlExecutor } from './executor.js'; export { insertConfigRecord, readStackConfig, tryReadStackConfig, type StackConfig, } from './config.js'; -export { SharedSqlRecordLogic, type SharedSqlRecordLogicDeps } from './record-logic.js'; export { SharedSqlRecordAdapter, SQLITE_RECORD_CAPABILITIES } from './record-adapter.js'; diff --git a/packages/sqlite-shared/src/schema.ts b/packages/sqlite-shared/src/schema.ts index e9af1338..4508dbcf 100644 --- a/packages/sqlite-shared/src/schema.ts +++ b/packages/sqlite-shared/src/schema.ts @@ -27,39 +27,32 @@ export const RECORD_SCHEMA_SQL = ` ) STRICT; -- Every edge a record carries, authority and data alike: a permission - -- entry has element identity and a delta the same shape as a tag's, so - -- it earns the same table and the same durability tier. The permissions - -- and associations fields are projections over it, partitioned by - -- kind. See docs/spec/access-control.md § Record-level permissions. - -- - -- A relationship's target is (related_scope, related_id) plus one - -- qualifier: related_stack for a record in another stack, related_ns for - -- a foreign namespace. All four are in the primary key, so two targets - -- differing only by namespace are two associations. A permission's - -- grantee reuses them — related_scope 'entity' or 'group', related_id the - -- DID or group id — plus related_role, which only a group grantee sets - -- and which is in the key because member and admin name two different - -- sets of people. - -- - -- attachment_record_id stays out of the primary key: it annotates a - -- reference rather than naming one, so re-pointing it updates the row an - -- an amendAssociations() add lands on instead of writing a second reference to the same - -- file. See docs/spec/attachments.md § Naming the upload a reference came from. - -- - -- The primary key leads with record_id, so its implicit index already - -- serves every by-record read and delete; the indexes below exist only - -- for the filter directions it cannot answer. + -- has element identity and a delta shaped like a tag's, so it earns the + -- same table. The permissions and associations fields are projections + -- over it, partitioned by kind. + -- See docs/spec/access-control.md § Record-level permissions. CREATE TABLE IF NOT EXISTS associations ( record_id TEXT NOT NULL REFERENCES records(id), kind TEXT NOT NULL CHECK (kind IN ('tag', 'attachment', 'relationship', 'permission', 'anyone')), label TEXT NOT NULL, file_id TEXT NOT NULL DEFAULT '', + -- A relationship target is (related_scope, related_id) qualified by + -- related_ns (a foreign namespace) or related_stack (another stack); a + -- permission's grantee reuses scope and id. All are in the key, so + -- targets differing only by a qualifier are distinct associations. related_scope TEXT NOT NULL DEFAULT '' CHECK (related_scope IN ('', 'record', 'entity', 'external', 'group')), related_id TEXT NOT NULL DEFAULT '', related_ns TEXT NOT NULL DEFAULT '', related_stack TEXT NOT NULL DEFAULT '', + -- Set only by a group grantee; in the key because member and admin name + -- two different sets of people. related_role TEXT NOT NULL DEFAULT '' CHECK (related_role IN ('', 'member', 'admin')), + -- Out of the key: it annotates a reference rather than naming one, so + -- re-pointing it updates the existing row. See docs/spec/attachments.md + -- § Naming the upload a reference came from. attachment_record_id TEXT NOT NULL DEFAULT '', + -- Leads with record_id, so its implicit index serves every by-record + -- read and delete; the indexes below cover only the filter directions. PRIMARY KEY (record_id, kind, label, file_id, related_scope, related_id, related_ns, related_stack, related_role) ) STRICT; @@ -122,7 +115,6 @@ export const RECORD_SCHEMA_SQL = ` -- One row per top-level scalar content field a record holds a value at: -- what it orders as, and the file it references. See - -- docs/spec/adapters.md § Adapter backends and -- docs/spec/data-model.md § Sorting by a content field. -- -- value_rank is 0 for a numeric value and 1 for a text one — the same @@ -139,14 +131,11 @@ export const RECORD_SCHEMA_SQL = ` PRIMARY KEY (record_id, field) ) STRICT; - -- Indexes. Each filter column continues into (created_at, id) — the - -- default ordering and its tiebreak — because these columns are - -- low-cardinality by design, and a narrow index on one leaves the - -- planner sorting the whole matching set before it can honor LIMIT. - -- - -- deleted_at and unlisted_at deliberately have none: nothing searches - -- for the rows they mark, and an index on a column every query tests - -- for IS NULL only lures the planner off the one answering ORDER BY. + -- Each filter column continues into (created_at, id), the default order + -- and its tiebreak: these columns are low-cardinality, and a narrow index + -- leaves the planner sorting every match before it can honor LIMIT. + -- deleted_at and unlisted_at get none: an index on a column every query + -- tests IS NULL only lures the planner off the one answering ORDER BY. CREATE INDEX IF NOT EXISTS idx_records_type_id ON records(type_id, created_at, id); CREATE INDEX IF NOT EXISTS idx_records_parent_id ON records(parent_id, created_at, id); CREATE INDEX IF NOT EXISTS idx_records_entity_id ON records(entity_id, created_at, id); @@ -192,9 +181,9 @@ export const TOKENS_SCHEMA_SQL = ` `; /** - * The full-text index behind `StackQuery.filter.search` (see fts5.ts). + * The full-text index behind `StackQuery.filter.search` (see indexes.ts). * What `columnsize=0` costs a reader is in docs/spec/adapters.md - * § Adapter backends. `detail` stays at its default: sanitizeFts5Query + * § SQLite-backed adapters. `detail` stays at its default: sanitizeFts5Query * passes phrase queries through, and a phrase needs the token positions * only that setting records. */ diff --git a/packages/sqlite-shared/src/token-logic.ts b/packages/sqlite-shared/src/token-logic.ts index f515b61d..79636b21 100644 --- a/packages/sqlite-shared/src/token-logic.ts +++ b/packages/sqlite-shared/src/token-logic.ts @@ -1,6 +1,6 @@ /** * The actual StackTokenStore logic (createToken/lookupToken/listTokens/ - * revokeToken), parametrized over a SqlExecutor. Currently consumed by + * revokeToken), parametrized over a SqlExecutor. Consumed by * record-adapter-sqlite's NativeTokenStore, which points it at a file * separate from records — see TOKENS_SCHEMA_SQL in schema.ts. */ @@ -23,10 +23,10 @@ export type SharedTokenLogicDeps = { }; export class SharedTokenLogic { - constructor(private readonly deps: SharedTokenLogicDeps) {} + private readonly exec: SqlExecutor; - private get exec(): SqlExecutor { - return this.deps.exec; + constructor({ exec }: SharedTokenLogicDeps) { + this.exec = exec; } async createToken( diff --git a/packages/sqlite-shared/tests/cursor.test.ts b/packages/sqlite-shared/tests/cursor.test.ts index d2b9e269..b0cfde8e 100644 --- a/packages/sqlite-shared/tests/cursor.test.ts +++ b/packages/sqlite-shared/tests/cursor.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from 'vitest'; import { NATIVE_SORT_FIELDS, StackBadRequestError } from '@haverstack/core'; -import { encodeCursor, decodeCursor, makeCursor, SORT_FIELDS } from '../src/cursor.js'; +import { encodeCursor, decodeCursor, makeCursor } from '../src/cursor.js'; import type { ScalarFieldKind, StackRecord } from '@haverstack/core'; const makeRecord = (overrides: Partial = {}): StackRecord => ({ @@ -121,14 +121,7 @@ describe('makeCursor', () => { }); }); -describe('SORT_FIELDS', () => { - // This package's name for the set, not a second copy of it: a cursor's - // `native` field is narrowed against exactly what core admits as a sort - // field, and a fourth column added there must not need finding here too. - test('is exactly the set core admits as a sort field', () => { - expect(SORT_FIELDS).toEqual([...NATIVE_SORT_FIELDS]); - }); - +describe('native sort fields', () => { test('round-trips a cursor on every field it names', () => { const record = makeRecord(); for (const field of NATIVE_SORT_FIELDS) { diff --git a/packages/sqlite-shared/tests/fts5.test.ts b/packages/sqlite-shared/tests/fts5.test.ts index f8ca80b4..41335e3f 100644 --- a/packages/sqlite-shared/tests/fts5.test.ts +++ b/packages/sqlite-shared/tests/fts5.test.ts @@ -150,6 +150,12 @@ describe('sanitizeFts5Query', () => { expect(sanitizeFts5Query('"cats AND"')).toBe('"cats AND"'); }); + test('leaves a NOT inside a phrase alone, even after a paren', () => { + expect(sanitizeFts5Query('"NOT cats"')).toBe('"NOT cats"'); + expect(sanitizeFts5Query('"cats (not dogs"')).toBe('"cats (not dogs"'); + expect(sanitizeFts5Query('x "a (NOT b')).toBe('x "a (NOT b"'); + }); + // The mask a phrase leaves behind is an operand, so neither of these // operators is dangling. test('a phrase counts as an operand on either side', () => { @@ -173,14 +179,9 @@ describe('sanitizeFts5Query', () => { }); /** - * Search text is whatever a caller typed, so no rule may re-scan the - * same run of it once per starting position inside that run. Each shape - * below did: at this size the cost ran from seven seconds to four - * minutes, against the tens of milliseconds each takes now. - * - * The bound is against that collapse, not a linearity claim — cost - * still grows a little faster than length here, as it does on ordinary - * input. + * No rule may re-scan a run of the input once per starting position in + * it; at this size a quadratic rule takes seconds to minutes. The bound + * catches that, not non-linearity in general. See Backtracking in fts5.ts. */ describe('adversarial input does not blow up', () => { test.each([ diff --git a/scripts/expand-changesets.mjs b/scripts/expand-changesets.mjs index 5d5d7541..fd63c2e9 100644 --- a/scripts/expand-changesets.mjs +++ b/scripts/expand-changesets.mjs @@ -115,8 +115,9 @@ for (const file of readdirSync(changesetDir)) { if (releasable.length === 0) { console.error( `${file} names only packages that are never published (${dropped.join(', ')}).\n` + - 'Name the package that ships the change instead — for sqlite-shared that is ' + - '@haverstack/record-adapter-sqlite, which bundles it.', + 'Name the packages that ship the change instead — for sqlite-shared that is ' + + '@haverstack/record-adapter-sqlite and @haverstack/record-adapter-do-sqlite, ' + + 'which both bundle it.', ); process.exit(1); }