From ba15832fcc1cc5d13fcddfb4433c5f4362ba665b Mon Sep 17 00:00:00 2001 From: David J Parrott Date: Sun, 8 Nov 2026 12:52:07 -0500 Subject: [PATCH] Reconcile select option widening at startup Existing databases kept stale select options after a comptime declaration grew. Changing { "a", "b" } to { "a", "b", "c" } left writes of "c" rejected after a restart, because provisioning only added columns and logged nothing about other field changes. Startup now applies safe widening as a metadata-only write, preserving records, live field ids, migration-owned indexes and triggers. A select may gain values or raise its limit when every live value remains valid, its flags are unchanged and its scalar or array storage stays the same. Narrowing and any other option or flag change keep the live definition and log that an explicit migration is required. Reordering the same values leaves the stored order and cache generation alone. Accepted fields, rules, declared indexes, tenant and search objects and one generation bump now commit together. Provisioning checks the full stored definition under the metadata lock, so a conflicting concurrent edit fails with Conflict instead of overwriting newer fields, rules or OAuth options, while identical concurrent plans converge without another rebuild or generation bump. Every rebuild input, including relation targets, is read after taking that lock. Because the stored definition is now read under the lock, an update validates caller fields and options before transaction setup and the canonical stored name afterwards, covering reserved system names and legacy FTS suffixes. A metadata-only plan applies that check only when it will write, so an unchanged legacy collection keeps starting. An incoming rename stays ignored. The owned result is allocated before commit, so a failed reload rolls the rebuild back. If foreign keys cannot be restored after commit, the connection is logged and quarantined while the committed result is still returned. Search follows the flags actually accepted, so a skipped searchable change neither creates nor removes FTS objects, and disabled FTS rejects an accepted searchable addition before any rebuild. Relation names and ids compare as the same target, and user field ids are treated as opaque even with an underscore prefix. Reserved field names are engine-owned only on auth collections, so a stored one on another collection type stays in the definition, the snapshot comparison and index validation. startup_log.refusal shares the warn-under-test spelling between provisioning and the Postgres connection refusals. The framework guide, scaffold guidance, known limitations and changelog describe these boundaries. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HTAHrkfe1L2hfpLeP1CQAS --- KNOWN_LIMITATIONS.md | 2 +- changelog.d/provision-field-options.md | 3 + docs/framework.md | 25 +- src/backend/postgres/conn.zig | 9 +- src/collections.zig | 581 +++++++++++++++-- src/provision.zig | 867 ++++++++++++++++++++++--- src/scaffold/agents_md.zig | 8 +- src/schema.zig | 50 +- src/startup_log.zig | 7 + 9 files changed, 1373 insertions(+), 179 deletions(-) create mode 100644 changelog.d/provision-field-options.md create mode 100644 src/startup_log.zig diff --git a/KNOWN_LIMITATIONS.md b/KNOWN_LIMITATIONS.md index c468426b..4fcad241 100644 --- a/KNOWN_LIMITATIONS.md +++ b/KNOWN_LIMITATIONS.md @@ -17,7 +17,7 @@ ZigBase v0.13.0 is an early release. The gaps below are known and tracked for fu ## Schema / migrations - **Consumer migration coordination is cooperative.** PostgreSQL apply/rollback batches require a direct or session-pooled connection, not transaction/statement pooling. SQLite batches use a permanent `.migrations.lock` sidecar and fail fast with `MigrationBusy` on contention. Use a writable, trusted local directory with working file locks; never replace/unlink the sidecar or database during use, or access the database through hard-link aliases. Individual commit boundaries and non-transactional callbacks are preserved. Old binaries, external SQL writers, automatic collection provisioning and distributed filesystems still need a single leader. Coordination does not make external callback effects atomic or incompatible rolling schema upgrades safe. -- **Comptime auto-migration is additive-only.** Startup provisioning of a comptime `.collections` schema creates missing collections and adds new fields, preserving data. **Non-additive changes (rename, drop, or type-change a field) are detected, logged, and skipped** — they require an explicit `.migrations` entry. (Access **rules** are the exception: a changed rule *is* re-applied on every startup, as a metadata-only write.) +- **Comptime auto-migration applies only safe additive changes.** Startup provisioning creates missing collections, adds fields, and widens select values or selection limits only when existing values remain valid, flags stay unchanged, and scalar/array storage stays the same. Narrowing, other option/flag changes, and rename/drop/type changes are detected, logged, and skipped; they require an explicit `.migrations` entry. Access rules are reapplied without rebuilding records. Accepted changes, declared indexes, search objects and the cache-generation bump commit atomically; a conflicting concurrent definition change returns `Conflict` before overwriting it, while an already-applied identical plan succeeds without another rewrite. Reordering select values alone is a no-op. - **Schema rule preflight is structural, not a proof of authorization behavior.** `schema apply` (including dry runs) and REST collection mutations reject invalid syntax and unresolved field/relation references against the prospective schema. Request-dependent values and application policy still require runtime tests. Trusted low-level collection writers can stage incomplete graphs; run `zigbase schema check-rules --data-dir PATH` against their final state before cutover. - **A live server picks up another process's schema change within about five seconds, not instantly.** `schema apply`, `migrate`, and `import` run as separate CLI processes; the server notices via a background observer that polls the `_schema_state` generation marker (~5s cadence) and then drops its in-process collection cache. Requests in that window still see the pre-change view. A cutover that must be seen immediately should still restart the server. See [Migrating an existing backend to ZigBase](docs/migration-tools.md). - **Collection rename requires an explicit offline migration.** `m.renameCollection(from, to, .{ .offline = true })` preserves identities, database/auth/search references, and immutable file namespaces; document diffs do not infer renames. Stop all processes and update compiled name references; old URLs/topics are not aliased. Namespace reservations survive deletion, so reserved physical prefixes cannot be claimed by a new collection. See [offline collection rename](docs/framework.md#offline-collection-rename). diff --git a/changelog.d/provision-field-options.md b/changelog.d/provision-field-options.md new file mode 100644 index 00000000..62edee8d --- /dev/null +++ b/changelog.d/provision-field-options.md @@ -0,0 +1,3 @@ +### Fixes +- Startup provisioning now applies safe select-option widening to existing databases without rebuilding tables. Select narrowing, single/multi-value changes, and other field option or flag changes emit an explicit-migration warning instead of being silently ignored. +- Concurrent startup plans for the same definition converge without conflicts or repeated cache invalidation; select-value reordering alone leaves the stored definition unchanged. diff --git a/docs/framework.md b/docs/framework.md index 30489406..0343f69f 100644 --- a/docs/framework.md +++ b/docs/framework.md @@ -4163,6 +4163,21 @@ and applies the **minimal safe change set** (running it twice is a clean no-op): - A field present in the spec but missing from the live collection is **added**, rebuilding the table while **preserving existing data** (the new column is null for old rows). +- An existing **select** field's options are updated when the declared `.values` + contain every live value and `.maxSelect` does not decrease. This is a + metadata-only write, preserving records, indexes, and triggers. The field's + other flags must be unchanged, and it must keep the same single/multi-value + representation: increasing `.maxSelect` from 1 to 2 or more needs an explicit + data migration from scalar strings to JSON arrays. A value of 0 is a zero limit, + not an unlimited sentinel. If another process changes any stored collection definition (including rules or options) between + the startup comparison and schema write, provisioning fails with `Conflict` + rather than overwriting that change, including when widening accompanies a + new field or TTL change; restart against the current schema. Concurrent boots + requesting the same resulting definition succeed without another rewrite. + Reordering the same select values keeps the stored order and does not count as + widening or invalidate caches. Accepted fields, rules, + declared indexes, search objects and one cache-generation bump commit together; + any failure rolls the whole collection update back. - A changed **access rule** (`.list_rule` / `.view_rule` / `.create_rule` / `.update_rule` / `.delete_rule`) is **re-applied to the live collection**. This is a metadata-only write — no table rebuild, no data copy — so tightening a rule in code @@ -4170,12 +4185,20 @@ and applies the **minimal safe change set** (running it twice is a clean no-op): (`null`) in the literal means "unspecified: leave whatever is live alone"; to lock a rule explicitly, spell it `""` (blank and unset are the same rule at evaluation time — superusers only — so switching between them is not treated as a change). -- A **non-additive** change — a field rename, drop, or type/storage-class change — +- A **non-additive** change — a field rename, drop, type/storage-class change, + select narrowing, or any other existing field option or flag change — is **detected, logged, and SKIPPED** (never applied, so no data loss). Relation targets must reference a known collection (a comptime collection or a pre-existing live one such as `_superusers`); an unknown target is a startup error. For the changes auto-migration won't do, use the `.migrations` escape hatch. +Changing a text length/pattern, number scale/bounds, relation target, or required +flag in the declaration alone does not update an existing database; startup logs +a warning. Test schema upgrades against an existing database as well as a fresh +one: a fresh `zigbase.testing.start` harness already has the new options. +Search indexes follow the effective persisted flags: a skipped `.searchable` +change neither enables nor disables an existing field's search index. A newly +added searchable field is indexed after its column is created. Declared `.indexes` are reconciled on every startup, including when no fields change. See [Indexes](#indexes) for ownership, adoption, and failure behavior. diff --git a/src/backend/postgres/conn.zig b/src/backend/postgres/conn.zig index 8a8c4556..893097ba 100644 --- a/src/backend/postgres/conn.zig +++ b/src/backend/postgres/conn.zig @@ -16,14 +16,7 @@ const scram = @import("scram.zig"); const connstr = @import("connstr.zig"); const tls_trust = @import("tls_trust.zig"); -/// A TLS/auth misconfiguration is a hard `.err` in production, but Zig's test runner -/// counts every `std.log.err` as a test failure — so the tests that deliberately -/// exercise these refuse-to-connect paths would fail on the log alone even though -/// their assertions (the returned error) are correct. Under the test runner we emit -/// the identical message at `.warn`; production still logs `.err`. -fn logMisconfig(comptime fmt: []const u8, args: anytype) void { - if (builtin.is_test) std.log.warn(fmt, args) else std.log.err(fmt, args); -} +const logMisconfig = @import("../../startup_log.zig").refusal; pub const ConnError = error{ ConnectFailed, diff --git a/src/collections.zig b/src/collections.zig index b37e4773..c4265fa3 100644 --- a/src/collections.zig +++ b/src/collections.zig @@ -5,6 +5,7 @@ const migrations = @import("migrations.zig"); const ddl = @import("ddl.zig"); const id = @import("id.zig"); const schema_gen = @import("schema_gen.zig"); +const fts = @import("search/fts.zig"); /// The JSON parse helpers in schema.zig return the inferred error set of /// std.json.parseFromSlice, which is wider than schema.ParseError. Capture it so @@ -12,7 +13,7 @@ const schema_gen = @import("schema_gen.zig"); const SchemaJsonError = @typeInfo(@typeInfo(@TypeOf(schema.fieldsFromJson)).@"fn".return_type.?).error_union.error_set || @typeInfo(@typeInfo(@TypeOf(schema.indexesFromJson)).@"fn".return_type.?).error_union.error_set; -pub const EngineError = error{ Validation, NotFound, Conflict, StorageNamespaceMissing } || @import("files/namespace.zig").Error || db.DbError || std.mem.Allocator.Error || schema.ParseError || SchemaJsonError; +pub const EngineError = error{ Validation, NotFound, Conflict, StorageNamespaceMissing } || @import("files/namespace.zig").Error || db.DbError || std.mem.Allocator.Error || schema.ParseError || SchemaJsonError || fts.EnsureIndexError; /// Validation details for the most recent failed create/update (2b surfaces these). pub threadlocal var last_errors: ?[]const schema.ValidationError = null; @@ -225,30 +226,32 @@ fn dupOptText(alloc: std.mem.Allocator, st: *db.Stmt, idx: c_int) !?[]const u8 { /// Must be called before the next step()/finalize() (columnText pointers are transient). fn rowToCollection(alloc: std.mem.Allocator, st: *db.Stmt) EngineError!schema.Collection { if (st.isNull(15)) return error.StorageNamespaceMissing; - const col_id = try alloc.dupe(u8, st.columnText(0)); - const name = try alloc.dupe(u8, st.columnText(1)); - const ctype = std.meta.stringToEnum(schema.CollectionType, st.columnText(2)) orelse .base; - const system = st.columnInt(3) != 0; - const fields = try schema.fieldsFromJson(alloc, st.columnText(4)); - const indexes = try schema.indexesFromJson(alloc, st.columnText(5)); - return .{ - .id = col_id, - .name = name, - .type = ctype, - .system = system, - .fields = fields, - .indexes = indexes, - .listRule = try dupOptText(alloc, st, 6), - .viewRule = try dupOptText(alloc, st, 7), - .createRule = try dupOptText(alloc, st, 8), - .updateRule = try dupOptText(alloc, st, 9), - .deleteRule = try dupOptText(alloc, st, 10), - .created = try alloc.dupe(u8, st.columnText(11)), - .updated = try alloc.dupe(u8, st.columnText(12)), - .options = try schema.optionsFromJson(alloc, st.columnText(13)), - .rename_epoch = st.columnInt(14), - .storage_namespace = try alloc.dupe(u8, st.columnText(15)), + // Start with an empty owned graph; the auth identity default normally points + // at static "email", so override it until optionsFromJson installs ownership. + var result: schema.Collection = .{ + .id = "", + .name = "", + .fields = &.{}, + .options = .{ .auth = .{ .identityFields = &.{} } }, }; + errdefer result.deinit(alloc); + result.id = try alloc.dupe(u8, st.columnText(0)); + result.name = try alloc.dupe(u8, st.columnText(1)); + result.type = std.meta.stringToEnum(schema.CollectionType, st.columnText(2)) orelse .base; + result.system = st.columnInt(3) != 0; + result.fields = try schema.fieldsFromJson(alloc, st.columnText(4)); + result.indexes = try schema.indexesFromJson(alloc, st.columnText(5)); + result.listRule = try dupOptText(alloc, st, 6); + result.viewRule = try dupOptText(alloc, st, 7); + result.createRule = try dupOptText(alloc, st, 8); + result.updateRule = try dupOptText(alloc, st, 9); + result.deleteRule = try dupOptText(alloc, st, 10); + result.created = try alloc.dupe(u8, st.columnText(11)); + result.updated = try alloc.dupe(u8, st.columnText(12)); + result.options = try schema.optionsFromJson(alloc, st.columnText(13)); + result.rename_epoch = st.columnInt(14); + result.storage_namespace = try alloc.dupe(u8, st.columnText(15)); + return result; } /// Load one FULLY-OWNED collection from the current row of `st`: `rowToCollection` plus auth @@ -312,6 +315,18 @@ pub fn list(alloc: std.mem.Allocator, w: *db.Db) EngineError![]schema.Collection } pub fn update(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []const u8, newdef: schema.Collection) EngineError!schema.Collection { + return updateImpl(alloc, io, w, id_or_name, newdef, null); +} + +/// Provisioning may plan an additive rebuild before another process changes the +/// schema. Recheck preflight's full stored definition under the same metadata lock as +/// the rebuild, before issuing DDL. Keep update()'s FK and savepoint ownership: +/// wrapping it in a new outer transaction would defeat SQLite's FK pause. +pub fn updateWithExpectedDefinition(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []const u8, newdef: schema.Collection, expected: schema.Collection) EngineError!schema.Collection { + return updateImpl(alloc, io, w, id_or_name, newdef, expected); +} + +fn updateImpl(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []const u8, newdef: schema.Collection, expected: ?schema.Collection) EngineError!schema.Collection { last_errors = null; // Like create(): everything except the validation errors (via `last_errors`) and the final @@ -322,8 +337,6 @@ pub fn update(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []con defer scratch.deinit(); const sa = scratch.allocator(); - const old = (try get(sa, w, id_or_name)) orelse return error.NotFound; - // assign ids to new fields lacking one; preserve existing ids (all on the scratch arena) const fields = try sa.alloc(schema.Field, newdef.fields.len); for (newdef.fields, 0..) |f, i| { @@ -335,27 +348,18 @@ pub fn update(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []con } var newc = newdef; newc.fields = fields; - // Rename is not supported in SP2, so the id/name are preserved from `old`. - newc.id = try sa.dupe(u8, old.id); - newc.name = try sa.dupe(u8, old.name); + // An update ignores an incoming rename. Validate its fields/options now; + // the canonical stored name is checked after loading metadata under lock. // validate (see create(): last_errors is handed back as an OWNED, freeable slice on `alloc`) var errs: std.ArrayList(schema.ValidationError) = .empty; errdefer errs.deinit(alloc); - try schema.validate(alloc, newc, &errs); + try schema.validateFieldsAndOptions(alloc, newc, &errs); if (errs.items.len > 0) { last_errors = try errs.toOwnedSlice(alloc); return error.Validation; } - // Inject auth system fields (after validation, which only sees user fields) so the rebuild - // preserves the auth columns: `old` is auth-injected (from get), so both sides carry the - // auth field ids and rebuildPlan keeps them. Without this an auth-collection update would - // drop email/passwordHash/tokenKey/etc and destroy all credentials. On `sa`. - const newc_full = try schema.injectAuthFields(sa, newc); - // relation-resolved view for the rebuild's FK generation - const ddl_new = try resolveRelations(sa, w, newc_full); - const d = db.dbDialect(w); // SQLite rebuilds via __new+copy+drop+rename, so FK enforcement must be paused around it; // Postgres uses in-place ALTERs (rebuildPlanPg) and has no such PRAGMA (it would error). @@ -369,28 +373,54 @@ pub fn update(alloc: std.mem.Allocator, io: std.Io, w: *db.Db, id_or_name: []con if (sqlite_rebuild) { try w.exec("PRAGMA foreign_keys=OFF;"); } - errdefer if (sqlite_rebuild) { - w.exec("PRAGMA foreign_keys=ON;") catch {}; + defer if (sqlite_rebuild) { + w.exec("PRAGMA foreign_keys=ON;") catch |err| { + std.log.warn("collection update: could not restore foreign keys ({s}); connection quarantined", .{@errorName(err)}); + const inner = if (comptime @import("build_options").postgres) &w.sqlite else w; + inner.quarantined = true; + }; }; const tx = try Tx.begin(w); errdefer tx.rollback(); - const plan = try ddl.rebuildPlan(sa, old, ddl_new, d); - for (plan) |stmt| try w.exec(try sa.dupeZ(u8, stmt)); - if (newc.type == .auth) { - for (newc.options.auth.identityFields) |idf| { - try w.exec(try sa.dupeZ(u8, try ddl.authIdentityIndexSql(sa, newc.name, idf))); + // Every input to the rebuild plan, including relation target names, must + // come from this locked snapshot. A pre-lock read could observe an interim + // definition even when another writer restores the expected one (ABA). + const old = (try get(sa, w, id_or_name)) orelse return error.NotFound; + try validateStoredName(alloc, old.name); + newc.id = try sa.dupe(u8, old.id); + newc.name = try sa.dupe(u8, old.name); + const unchanged = expected != null and try definitionEql(sa, old, newc); + if (!unchanged) { + if (expected) |snapshot| try checkDefinitionSnapshot(sa, snapshot, old); + + // Inject auth system fields (after validation, which only sees user fields) so the rebuild + // preserves the auth columns: `old` is auth-injected (from get), so both sides carry the + // auth field ids and rebuildPlan keeps them. Without this an auth-collection update would + // drop email/passwordHash/tokenKey/etc and destroy all credentials. On `sa`. + const newc_full = try schema.injectAuthFields(sa, newc); + // relation-resolved view for the rebuild's FK generation + const ddl_new = try resolveRelations(sa, w, newc_full); + + const plan = try ddl.rebuildPlan(sa, old, ddl_new, d); + for (plan) |stmt| try w.exec(try sa.dupeZ(u8, stmt)); + if (newc.type == .auth) { + for (newc.options.auth.identityFields) |idf| { + try w.exec(try sa.dupeZ(u8, try ddl.authIdentityIndexSql(sa, newc.name, idf))); + } } + if (expected != null) { + try ensureTenantIndex(sa, w, newc); + try fts.ensureIndex(sa, w, newc); + } + try updateRow(sa, w, newc.id, newc); // persist user fields only + try schema_gen.bump(w); // inside the tx: a rolled-back rebuild must not advertise a change } - try updateRow(sa, w, newc.id, newc); // persist user fields only - try schema_gen.bump(w); // inside the tx: a rolled-back rebuild must not advertise a change + // Materialize the owned result before committing. An allocator failure in + // get() must roll back the rebuild rather than report failure after a write. + const result = (try get(alloc, w, old.id)).?; + errdefer result.deinit(alloc); try tx.commit(); - if (sqlite_rebuild) { - try w.exec("PRAGMA foreign_keys=ON;"); - } - - // Return a fully-owned reload on `alloc` (name changes require Migrator.renameCollection), matching - // the former hand-assembled `newc_full` but with every string/slice owned. - return (try get(alloc, w, old.id)).?; + return result; } fn updateRow(alloc: std.mem.Allocator, w: *db.Db, col_id: []const u8, col: schema.Collection) EngineError!void { @@ -510,34 +540,110 @@ pub fn updateRules(alloc: std.mem.Allocator, w: *db.Db, col_id: []const u8, rule try tx.commit(); } -/// Reconcile only declared indexes. Migration-owned indexes and table data are untouched. -/// A failed CREATE (e.g. duplicate values for a new UNIQUE constraint) rolls back both the -/// index changes and metadata. No-op declarations do not bump the schema generation. -pub fn updateIndexes(alloc: std.mem.Allocator, w: *db.Db, col_id: []const u8, indexes: []const schema.Index) EngineError!void { +/// Apply a provisioning plan without rebuilding records. All physical index/search +/// changes and metadata share one lock, transaction, and generation bump. +pub fn provisionMetadata(alloc: std.mem.Allocator, w: *db.Db, expected: schema.Collection, desired: schema.Collection) EngineError!void { + last_errors = null; var scratch = std.heap.ArenaAllocator.init(alloc); defer scratch.deinit(); - const sa = scratch.allocator(); + const a = scratch.allocator(); const tx = try Tx.begin(w); errdefer tx.rollback(); - const old = (try get(sa, w, col_id)) orelse return error.NotFound; + const current = (try get(a, w, expected.id)) orelse return error.NotFound; + const unchanged = try definitionEql(a, current, desired); + if (!unchanged) { + try validateStoredName(alloc, current.name); + try checkDefinitionSnapshot(a, expected, current); + } + try reconcileIndexes(alloc, w, current, desired); + try ensureTenantIndex(a, w, desired); + try fts.ensureIndex(a, w, desired); + if (!unchanged) { + try updateRow(a, w, desired.id, desired); + try schema_gen.bump(w); + } + try tx.commit(); +} + +/// Only a caller that will write checks this: a legacy reserved name that needs no +/// change must keep starting. On failure, `last_errors` is an owned slice on `alloc`. +fn validateStoredName(alloc: std.mem.Allocator, name: []const u8) EngineError!void { + var errs: std.ArrayList(schema.ValidationError) = .empty; + errdefer errs.deinit(alloc); + try schema.validateCollectionName(alloc, name, &errs); + if (errs.items.len == 0) return; + last_errors = try errs.toOwnedSlice(alloc); + return error.Validation; +} + +fn definitionEql(alloc: std.mem.Allocator, a: schema.Collection, b: schema.Collection) EngineError!bool { + if (!std.mem.eql(u8, a.id, b.id) or !std.mem.eql(u8, a.name, b.name) or + a.system != b.system or a.rename_epoch != b.rename_epoch or + !std.mem.eql(u8, a.storage_namespace, b.storage_namespace)) return false; + if (!Rules.from(a).eql(Rules.from(b))) return false; + checkFieldSnapshot(alloc, a, b) catch |err| switch (err) { + error.Conflict => return false, + else => return err, + }; + const ai = try schema.indexesToJson(alloc, a.indexes); + defer alloc.free(ai); + const bi = try schema.indexesToJson(alloc, b.indexes); + defer alloc.free(bi); + const ao = try schema.optionsToJson(alloc, a, false); + defer alloc.free(ao); + const bo = try schema.optionsToJson(alloc, b, false); + defer alloc.free(bo); + return a.type == b.type and std.mem.eql(u8, ai, bi) and std.mem.eql(u8, ao, bo); +} + +fn checkDefinitionSnapshot(alloc: std.mem.Allocator, expected: schema.Collection, current: schema.Collection) EngineError!void { + if (!try definitionEql(alloc, expected, current)) return error.Conflict; +} + +fn checkFieldSnapshot(alloc: std.mem.Allocator, expected: schema.Collection, current: schema.Collection) EngineError!void { + const expected_json = try userFieldsJson(alloc, expected); + defer alloc.free(expected_json); + const current_json = try userFieldsJson(alloc, current); + defer alloc.free(current_json); + if (!std.mem.eql(u8, expected_json, current_json)) return error.Conflict; +} + +/// get() injects auth system fields, which have reserved names. User field ids are +/// opaque: an underscore prefix does not identify a system-owned field. +fn userFieldsJson(alloc: std.mem.Allocator, col: schema.Collection) ![]u8 { + var user_fields: std.ArrayList(schema.Field) = .empty; + defer user_fields.deinit(alloc); + for (col.fields) |field| { + if (schema.isInjectedField(col.type, field.name)) continue; + try user_fields.append(alloc, field); + } + return schema.fieldsToJson(alloc, user_fields.items); +} + +/// Caller owns the metadata transaction and generation bump. Reconcile declared +/// indexes without rebuilding records or touching migration-owned objects. +fn reconcileIndexes(alloc: std.mem.Allocator, w: *db.Db, old: schema.Collection, desired: schema.Collection) EngineError!void { + const indexes = desired.indexes; + var scratch = std.heap.ArenaAllocator.init(alloc); + defer scratch.deinit(); + const sa = scratch.allocator(); const old_json = try schema.indexesToJson(sa, old.indexes); const new_json = try schema.indexesToJson(sa, indexes); - if (std.mem.eql(u8, old_json, new_json)) { - try tx.commit(); - return; - } - var newc = old; - newc.indexes = indexes; + if (std.mem.eql(u8, old_json, new_json)) return; + var newc = desired; var user_fields: std.ArrayList(schema.Field) = .empty; - for (old.fields) |field| { - if (field.id.len > 0 and field.id[0] == '_') continue; + for (desired.fields) |field| { if (schema.isSystemFieldName(field.name)) continue; try user_fields.append(sa, field); } newc.fields = user_fields.items; + // Reserved names are hidden from field validation, which would refuse a stored + // legacy field. Index rules still apply to it, so those see every desired field. + newc.indexes = &.{}; var errs: std.ArrayList(schema.ValidationError) = .empty; errdefer errs.deinit(alloc); try schema.validate(alloc, newc, &errs); + try schema.validateIndexes(alloc, desired, &errs); if (errs.items.len > 0) { last_errors = try errs.toOwnedSlice(alloc); return error.Validation; @@ -588,8 +694,6 @@ pub fn updateIndexes(alloc: std.mem.Allocator, w: *db.Db, col_id: []const u8, in try st.bindText(2, new_json); _ = try st.step(); } - try schema_gen.bump(w); - try tx.commit(); } /// Conservative adoption of ordinary migration-created indexes. Compare catalog properties, @@ -1212,3 +1316,340 @@ test "a failed create inside a caller's transaction leaves the caller's transact const still = (try get(a, &conn, "keep")).?; still.deinit(a); } + +/// Auto-create the index backing the `tenant_field` column so the per-request tenant scope +/// predicate (`""."" = ?`) is served from an index, not a scan. Idempotent +/// (`IF NOT EXISTS`); identifiers are gated through `schema.isValidIdentifier` before interpolation. +pub fn ensureTenantIndex(alloc: std.mem.Allocator, w: *db.Db, col: schema.Collection) EngineError!void { + const tf = col.options.tenant_field orelse return; + if (!schema.isValidIdentifier(col.name) or !schema.isValidIdentifier(tf)) return; + // Bound + freed rather than nested inside the allocPrint: `alloc` here is the caller's + // allocator, not an arena (the `sql` free below says so), so a nested temporary would leak. + const qidx = try ddl.quoteComposite(alloc, "idx_{s}_{s}_tenant", .{ col.name, tf }); + defer alloc.free(qidx); + const qtbl = try ddl.quoteIdent(alloc, col.name); + defer alloc.free(qtbl); + const qtf = try ddl.quoteIdent(alloc, tf); + defer alloc.free(qtf); + const sql = try std.fmt.allocPrintSentinel( + alloc, + "CREATE INDEX IF NOT EXISTS {s} ON {s} ({s});", + .{ qidx, qtbl, qtf }, + 0, + ); + defer alloc.free(sql); + try w.exec(sql); + std.log.info("provision: ensured tenant index on '{s}'.'{s}'", .{ col.name, tf }); +} + +test "owned update result allocation failure rolls back rebuild and generation" { + const a = std.testing.allocator; + var allocations: usize = 0; + for ([_]bool{ false, true }) |fail_last| { + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + const original = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "items", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + defer original.deinit(a); + try conn.exec("INSERT INTO items(id,title) VALUES ('old','preserve me');"); + try conn.exec("CREATE INDEX migration_index ON items(title);"); + const generation = try schema_gen.read(&conn); + var desired = original; + desired.fields = &.{ original.fields[0], .{ .id = "extra_id", .name = "extra", .options = .{ .text = .{} } } }; + // Measure the successful path, then fail its last allocation: the owned + // reload after the DDL and metadata writes, rather than an early preflight. + var failing = std.testing.FailingAllocator.init(a, .{ + .fail_index = if (fail_last) allocations - 1 else std.math.maxInt(usize), + }); + const fa = failing.allocator(); + if (fail_last) { + try std.testing.expectError(error.OutOfMemory, updateWithExpectedDefinition(fa, std.testing.io, &conn, original.id, desired, original)); + try std.testing.expectEqual(generation, try schema_gen.read(&conn)); + const preserved = (try get(a, &conn, original.id)).?; + defer preserved.deinit(a); + try std.testing.expect(schema.fieldByName(preserved, "extra") == null); + var st = try conn.prepare("SELECT (SELECT title FROM items WHERE id='old'), (SELECT count(*) FROM pragma_table_info('items') WHERE name='extra'), (SELECT count(*) FROM sqlite_master WHERE name='migration_index');"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqualStrings("preserve me", st.columnText(0)); + try std.testing.expectEqual(@as(i64, 0), st.columnInt(1)); + try std.testing.expectEqual(@as(i64, 1), st.columnInt(2)); + } else { + const updated = try updateWithExpectedDefinition(fa, std.testing.io, &conn, original.id, desired, original); + updated.deinit(fa); + allocations = failing.alloc_index; + try std.testing.expect(allocations > 0); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&conn)); + } + try std.testing.expect(!conn.inTransaction()); + var fk = try conn.prepare("PRAGMA foreign_keys;"); + defer fk.finalize(); + try std.testing.expect(try fk.step()); + try std.testing.expectEqual(@as(i64, 1), fk.columnInt(0)); + } +} + +test "rebuild reads collection and relation metadata only after locking" { + const c = @import("c.zig").c; + const Reads = struct { + handle: ?*c.sqlite3, + locked: bool = false, + reads: usize = 0, + denied: bool = false, + + fn authorize(context: ?*anyopaque, action: c_int, first: [*c]const u8, _: [*c]const u8, _: [*c]const u8, _: [*c]const u8) callconv(.c) c_int { + const self: *@This() = @ptrCast(@alignCast(context.?)); + if (first == null) return c.SQLITE_OK; + const table = std.mem.span(first); + if (action == c.SQLITE_UPDATE and std.mem.eql(u8, table, "_schema_state")) self.locked = true; + if (action == c.SQLITE_READ and std.mem.eql(u8, table, "_collections")) { + self.reads += 1; + // The SQLite authorizer rejects pre-lock reads rather than + // coordinating threads with timing-sensitive sleeps. + if (!self.locked or c.sqlite3_get_autocommit(self.handle) != 0) { + self.denied = true; + return c.SQLITE_DENY; + } + } + return c.SQLITE_OK; + } + }; + const a = std.testing.allocator; + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + const target = try create(a, std.testing.io, &conn, .{ .id = "", .name = "targets", .fields = &.{} }); + defer target.deinit(a); + const original = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "items", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + defer original.deinit(a); + var desired = original; + desired.fields = &.{ original.fields[0], .{ .id = "target_id", .name = "target", .options = .{ .relation = .{ .targetCollectionId = target.id } } } }; + const inner = if (comptime @import("build_options").postgres) &conn.sqlite else &conn; + var reads: Reads = .{ .handle = inner.handle }; + try std.testing.expectEqual(c.SQLITE_OK, c.sqlite3_set_authorizer(inner.handle, Reads.authorize, &reads)); + defer _ = c.sqlite3_set_authorizer(inner.handle, null, null); + const updated = try updateWithExpectedDefinition(a, std.testing.io, &conn, original.id, desired, original); + defer updated.deinit(a); + try std.testing.expect(reads.locked); + try std.testing.expect(reads.reads > 1); + try std.testing.expect(!reads.denied); + try std.testing.expect(schema.fieldByName(updated, "target") != null); +} + +test "committed rebuild survives FK restoration failure and quarantines its connection" { + const c = @import("c.zig").c; + const DenyRestore = struct { + fn authorize(_: ?*anyopaque, action: c_int, first: [*c]const u8, second: [*c]const u8, _: [*c]const u8, _: [*c]const u8) callconv(.c) c_int { + if (action == c.SQLITE_PRAGMA and first != null and second != null and + std.mem.eql(u8, std.mem.span(first), "foreign_keys") and + std.ascii.eqlIgnoreCase(std.mem.span(second), "ON")) return c.SQLITE_DENY; + return c.SQLITE_OK; + } + }; + const a = std.testing.allocator; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir = try tmp.dir.realPathFileAlloc(std.testing.io, ".", a); + defer a.free(dir); + const path = try std.fmt.allocPrintSentinel(a, "{s}/data.db", .{dir}, 0); + defer a.free(path); + var conn = try db.Db.open(path); + defer conn.close(); + try migrations.run(&conn); + const original = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "items", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + defer original.deinit(a); + const generation = try schema_gen.read(&conn); + var desired = original; + desired.fields = &.{ original.fields[0], .{ .id = "extra_id", .name = "extra", .options = .{ .text = .{} } } }; + const inner = if (comptime @import("build_options").postgres) &conn.sqlite else &conn; + try std.testing.expectEqual(c.SQLITE_OK, c.sqlite3_set_authorizer(inner.handle, DenyRestore.authorize, null)); + const result = try updateWithExpectedDefinition(a, std.testing.io, &conn, original.id, desired, original); + defer result.deinit(a); + try std.testing.expect(schema.fieldByName(result, "extra") != null); + try std.testing.expect(!conn.isHealthy()); + try std.testing.expectError(error.WriterUnavailable, conn.exec("SELECT 1;")); + var observer = try db.Db.open(path); + defer observer.close(); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&observer)); + const saved = (try get(a, &observer, original.id)).?; + defer saved.deinit(a); + try std.testing.expect(schema.fieldByName(saved, "extra") != null); +} + +test "invalid update inputs are rejected before foreign key changes or metadata locks" { + const c = @import("c.zig").c; + const RejectMutation = struct { + attempts: usize = 0, + fn authorize(context: ?*anyopaque, action: c_int, _: [*c]const u8, _: [*c]const u8, _: [*c]const u8, _: [*c]const u8) callconv(.c) c_int { + const self: *@This() = @ptrCast(@alignCast(context.?)); + if (action == c.SQLITE_UPDATE or action == c.SQLITE_TRANSACTION or action == c.SQLITE_PRAGMA) { + self.attempts += 1; + return c.SQLITE_DENY; + } + return c.SQLITE_OK; + } + }; + const a = std.testing.allocator; + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + const original = try create(a, std.testing.io, &conn, .{ .id = "", .name = "items", .fields = &.{} }); + defer original.deinit(a); + var invalid = original; + invalid.fields = &.{.{ .id = "bad_id", .name = "bad-name", .options = .{ .text = .{} } }}; + var rejected: RejectMutation = .{}; + const inner = if (comptime @import("build_options").postgres) &conn.sqlite else &conn; + try std.testing.expectEqual(c.SQLITE_OK, c.sqlite3_set_authorizer(inner.handle, RejectMutation.authorize, &rejected)); + defer _ = c.sqlite3_set_authorizer(inner.handle, null, null); + try std.testing.expectError(error.Validation, update(a, std.testing.io, &conn, original.id, invalid)); + const errors = last_errors.?; + defer a.free(errors); + defer last_errors = null; + try std.testing.expectEqual(@as(usize, 0), rejected.attempts); + try std.testing.expect(!conn.inTransaction()); +} + +fn expectStoredNameRejected(a: std.mem.Allocator, reserved_name: []const u8, by_id: bool) !void { + const c = @import("c.zig").c; + const DdlAttempts = struct { + count: usize = 0, + fn authorize(context: ?*anyopaque, action: c_int, _: [*c]const u8, _: [*c]const u8, _: [*c]const u8, _: [*c]const u8) callconv(.c) c_int { + const self: *@This() = @ptrCast(@alignCast(context.?)); + if (action == c.SQLITE_CREATE_TABLE or action == c.SQLITE_DROP_TABLE or + action == c.SQLITE_ALTER_TABLE or action == c.SQLITE_CREATE_INDEX or + action == c.SQLITE_DROP_INDEX) self.count += 1; + return c.SQLITE_OK; + } + }; + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + if (!std.mem.eql(u8, reserved_name, "_superusers")) { + const created = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "legacy", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + created.deinit(a); + // Simulate a legacy table created before the FTS suffix reservation. + try conn.exec("ALTER TABLE legacy RENAME TO legacy_fts;"); + try conn.exec("UPDATE _collections SET name='legacy_fts' WHERE name='legacy';"); + } + const original = (try get(a, &conn, reserved_name)).?; + defer original.deinit(a); + const quoted = try ddl.quoteIdent(a, original.name); + defer a.free(quoted); + const index_sql = try std.fmt.allocPrintSentinel(a, "CREATE INDEX name_guard_marker ON {s}(created);", .{quoted}, 0); + defer a.free(index_sql); + try conn.exec(index_sql); + const generation = try schema_gen.read(&conn); + var desired = original; + var fields: std.ArrayList(schema.Field) = .empty; + defer fields.deinit(a); + for (original.fields) |field| { + if (!schema.isSystemFieldName(field.name)) try fields.append(a, field); + } + try fields.append(a, .{ .id = "extra_id", .name = "extra", .options = .{ .text = .{} } }); + desired.fields = fields.items; + var attempts: DdlAttempts = .{}; + const inner = if (comptime @import("build_options").postgres) &conn.sqlite else &conn; + try std.testing.expectEqual(c.SQLITE_OK, c.sqlite3_set_authorizer(inner.handle, DdlAttempts.authorize, &attempts)); + defer _ = c.sqlite3_set_authorizer(inner.handle, null, null); + const key = if (by_id) original.id else original.name; + if (update(a, std.testing.io, &conn, key, desired)) |saved| { + saved.deinit(a); + return error.TestExpectedStoredNameValidation; + } else |err| try std.testing.expectEqual(error.Validation, err); + const errors = last_errors.?; + defer a.free(errors); + defer last_errors = null; + try std.testing.expectEqualStrings("name", errors[0].field); + try std.testing.expectEqual(@as(usize, 0), attempts.count); + try std.testing.expectEqual(generation, try schema_gen.read(&conn)); + try std.testing.expect(!conn.inTransaction()); + const preserved = (try get(a, &conn, original.id)).?; + defer preserved.deinit(a); + try std.testing.expect(schema.fieldByName(preserved, "extra") == null); + var state = try conn.prepare("SELECT (SELECT count(*) FROM sqlite_master WHERE name='name_guard_marker'), (SELECT foreign_keys FROM pragma_foreign_keys);"); + defer state.finalize(); + try std.testing.expect(try state.step()); + try std.testing.expectEqual(@as(i64, 1), state.columnInt(0)); + try std.testing.expectEqual(@as(i64, 1), state.columnInt(1)); +} + +test "updates reject stored superuser name for name and id lookups" { + for ([_]bool{ false, true }) |by_id| try expectStoredNameRejected(std.testing.allocator, "_superusers", by_id); +} + +test "updates reject stored legacy FTS suffix for name and id lookups" { + for ([_]bool{ false, true }) |by_id| try expectStoredNameRejected(std.testing.allocator, "legacy_fts", by_id); +} + +test "metadata-only provisioning rejects a stored reserved name only when it would write" { + const a = std.testing.allocator; + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + const created = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "legacy", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + created.deinit(a); + // Simulate a legacy table created before the FTS suffix reservation. + try conn.exec("ALTER TABLE legacy RENAME TO legacy_fts;"); + try conn.exec("UPDATE _collections SET name='legacy_fts' WHERE name='legacy';"); + const original = (try get(a, &conn, "legacy_fts")).?; + defer original.deinit(a); + const generation = try schema_gen.read(&conn); + + // An unchanged boot writes nothing, so an existing deployment keeps starting. + try provisionMetadata(a, &conn, original, original); + try std.testing.expectEqual(generation, try schema_gen.read(&conn)); + + var desired = original; + desired.listRule = "@public"; + try std.testing.expectError(error.Validation, provisionMetadata(a, &conn, original, desired)); + const errors = last_errors.?; + defer a.free(errors); + defer last_errors = null; + try std.testing.expectEqualStrings("name", errors[0].field); + try std.testing.expectEqual(generation, try schema_gen.read(&conn)); + try std.testing.expect(!conn.inTransaction()); + const preserved = (try get(a, &conn, original.id)).?; + defer preserved.deinit(a); + try std.testing.expectEqual(@as(?[]const u8, null), preserved.listRule); +} + +test "updates validate stored name while ignoring incoming rename" { + const a = std.testing.allocator; + var conn = try db.Db.openMemory(); + defer conn.close(); + try migrations.run(&conn); + const original = try create(a, std.testing.io, &conn, .{ + .id = "", + .name = "items", + .fields = &.{.{ .id = "title_id", .name = "title", .options = .{ .text = .{} } }}, + }); + defer original.deinit(a); + for ([_][]const u8{ "_ignored", "ignored_fts" }) |incoming_name| { + var desired = original; + desired.name = incoming_name; + const updated = try update(a, std.testing.io, &conn, original.id, desired); + defer updated.deinit(a); + try std.testing.expectEqualStrings("items", updated.name); + try std.testing.expectEqualStrings(original.id, updated.id); + } +} diff --git a/src/provision.zig b/src/provision.zig index 54133886..21134e96 100644 --- a/src/provision.zig +++ b/src/provision.zig @@ -3,7 +3,7 @@ //! Developers describe their schema IN ZIG at comptime via `Schema(.{ ... })`, //! and ZigBase provisions it at startup: creating not-yet-existing collections, //! resolving relation targets BY NAME, and applying safe ADDITIVE auto-migration -//! (new columns). Destructive changes (type change / column drop) are detected, +//! (new columns and select-option widening). Other field changes are detected, //! logged, and SKIPPED — never silently applied. An explicit-migration escape //! hatch (`.migrations`) covers the rest. //! @@ -62,31 +62,6 @@ fn warnTenantOwned(col: schema.Collection) void { } } -/// Auto-create the index backing the `tenant_field` column so the per-request tenant scope -/// predicate (`""."" = ?`) is served from an index, not a scan. Idempotent -/// (`IF NOT EXISTS`); identifiers are gated through `schema.isValidIdentifier` before interpolation. -fn ensureTenantIndex(alloc: std.mem.Allocator, w: *db.Db, col: schema.Collection) ProvisionError!void { - const tf = col.options.tenant_field orelse return; - if (!schema.isValidIdentifier(col.name) or !schema.isValidIdentifier(tf)) return; - // Bound + freed rather than nested inside the allocPrint: `alloc` here is the caller's - // allocator, not an arena (the `sql` free below says so), so a nested temporary would leak. - const qidx = try ddl.quoteComposite(alloc, "idx_{s}_{s}_tenant", .{ col.name, tf }); - defer alloc.free(qidx); - const qtbl = try ddl.quoteIdent(alloc, col.name); - defer alloc.free(qtbl); - const qtf = try ddl.quoteIdent(alloc, tf); - defer alloc.free(qtf); - const sql = try std.fmt.allocPrintSentinel( - alloc, - "CREATE INDEX IF NOT EXISTS {s} ON {s} ({s});", - .{ qidx, qtbl, qtf }, - 0, - ); - defer alloc.free(sql); - try w.exec(sql); - std.log.info("provision: ensured tenant index on '{s}'.'{s}'", .{ col.name, tf }); -} - /// Wrap search validation/reconciliation: a SQLite build with `-Dfts5=false` cannot serve a /// `.searchable` schema. Rather than let that surface as a silent no-op (and a later `?search=` /// 500), fail LOUDLY at startup with an actionable message — the fail-fast-at-boot principle @@ -96,10 +71,8 @@ fn inspectSearchIndex(alloc: std.mem.Allocator, w: *db.Db, spec: schema.Collecti const result = if (reconcile) fts.ensureIndex(alloc, w, spec) else fts.validateIndex(alloc, w, spec); result catch |err| switch (err) { error.SearchDisabled => { - std.log.err( - "refusing to start: collection '{s}' declares .searchable fields but this binary was built with -Dfts5=false — rebuild with -Dfts5 (or its default) to enable full-text search", - .{spec.name}, - ); + const message = "refusing to start: collection '{s}' declares .searchable fields but this binary was built with -Dfts5=false — rebuild with -Dfts5 (or its default) to enable full-text search"; + @import("startup_log.zig").refusal(message, .{spec.name}); return error.FtsDisabled; }, // `err`'s static type is fts.EnsureIndexError (includes SearchDisabled, handled above); @@ -965,6 +938,9 @@ pub fn applySpecs( ) ProvisionError!void { var arena = std.heap.ArenaAllocator.init(alloc); defer arena.deinit(); + // Provisioning's caller receives ProvisionError, not an owned validation list. + // Low-level diagnostics allocated on this arena must not escape through TLS. + defer collections.last_errors = null; const a = arena.allocator(); // Opt-in vector search (#157/#159): on Postgres the `?vector=` KNN lowering needs the pgvector @@ -1013,7 +989,8 @@ pub fn ensureCollection( w: *db.Db, spec_in: schema.Collection, ) ProvisionError!void { - const spec = try resolveTargets(alloc, w, spec_in); + var target_ids: std.StringHashMapUnmanaged([]const u8) = .empty; + const spec = try resolveTargets(alloc, w, spec_in, &target_ids); // F3 lint: surface any allow-all (@public) rule prominently at startup. warnPublicRules(spec); @@ -1027,21 +1004,27 @@ pub fn ensureCollection( if (existing == null) { _ = try collections.create(alloc, io, w, spec); std.log.info("provision: created collection '{s}'", .{spec.name}); - try ensureTenantIndex(alloc, w, spec); + try collections.ensureTenantIndex(alloc, w, spec); try inspectSearchIndex(alloc, w, spec, true); return; } const live = existing.?; - // The physical table exists now; ensure the tenant_field is indexed (idempotent). - try ensureTenantIndex(alloc, w, spec); // Diff user fields. `live.fields` from get() includes injected auth system // fields for auth collections; compare only against non-system field names. // (additions/merged below skip .deinit(): this fn runs under applySpecs' // arena, which reclaims them wholesale.) var additions: std.ArrayList(schema.Field) = .empty; + var merged: std.ArrayList(schema.Field) = .empty; + for (live.fields) |lf| { + if (!schema.isInjectedField(live.type, lf.name)) try merged.append(alloc, lf); + } + var widened_count: usize = 0; var changed = false; for (spec.fields) |sf| { - const lf = schema.fieldByName(live, sf.name); + const field_index = for (merged.items, 0..) |field, i| { + if (std.mem.eql(u8, field.name, sf.name)) break i; + } else null; + const lf = if (field_index) |i| merged.items[i] else null; if (lf == null) { try additions.append(alloc, sf); changed = true; @@ -1055,14 +1038,27 @@ pub fn ensureCollection( "provision: collection '{s}' field '{s}' type changed ({s} -> {s}); SKIPPED — write an explicit migration (auto-migration is additive-only)", .{ spec.name, sf.name, @tagName(lf.?.fieldType()), @tagName(sf.fieldType()) }, ); + continue; + } + switch (try fieldChangeWithRelations(alloc, lf.?, sf, target_ids)) { + .unchanged => {}, + .widen_select => { + var next = lf.?; // preserve the live field's id and other metadata + next.options = sf.options; + merged.items[field_index.?] = next; + widened_count += 1; + }, + .migration => std.log.warn( + "provision: collection '{s}' field '{s}' options or flags changed; SKIPPED — write an explicit migration (auto-migration only widens selects without changing single/multi-value storage)", + .{ spec.name, sf.name }, + ), } } // Detect drops: a live user field absent from the comptime spec. We do NOT // drop it (data loss); just warn so the developer knows the schemas diverge. for (live.fields) |lf| { - if (lf.id.len > 0 and lf.id[0] == '_') continue; // injected auth system field - if (schema.isSystemFieldName(lf.name)) continue; + if (schema.isInjectedField(live.type, lf.name)) continue; if (specFieldByName(spec, lf.name) == null) { std.log.warn( "provision: collection '{s}' has live field '{s}' not in the comptime schema; left in place (auto-migration never drops columns)", @@ -1071,6 +1067,13 @@ pub fn ensureCollection( } } + for (additions.items) |sf| try merged.append(alloc, sf); + // Every downstream schema consumer must see the changes actually accepted + // above. In particular, rejected searchable flags must not remove/create FTS + // objects while _collections continues to expose the old flags. + var effective = live; + effective.fields = merged.items; + // TTL: a `.ttl_field` newly added to (or changed on) an EXISTING collection must // be persisted, else the GC never sees it. Compare the spec's ttl_field against // the live one and, if it differs, force an update (narrowly — we only carry the @@ -1085,54 +1088,74 @@ pub fn ensureCollection( }; if (ttl_differs) changed = true; - // Access rules: a rule-only comptime change carries NO physical-schema consequence, so it - // must not be gated behind `changed` (which only tracks field additions and the ttl_field) — - // and it must not go through collections.update either, whose unconditional table rebuild - // would copy every row and drop unknown indexes just to tighten a rule. Persist it with the - // metadata-only writer instead. This matters for security: policy.zig enforces the rules on - // the collection loaded out of _collections, so a developer who TIGHTENS a rule in code and - // redeploys would otherwise keep enforcing the old, looser rule indefinitely. const merged_rules = mergeRules(spec, live); - if (!changed and !merged_rules.eql(collections.Rules.from(live))) { - try collections.updateRules(alloc, w, live.id, merged_rules); - std.log.info("provision: collection '{s}' access rules updated", .{spec.name}); + merged_rules.applyTo(&effective); + effective.indexes = spec.indexes; + effective.options.ttl_field = spec.options.ttl_field; + // Validate accepted additions even when their columns do not exist yet. + try inspectSearchIndex(alloc, w, effective, false); + if (changed) { + const updated = try collections.updateWithExpectedDefinition(alloc, io, w, live.id, effective, live); + updated.deinit(alloc); + std.log.info("provision: collection '{s}' added {d} field(s)", .{ spec.name, additions.items.len }); + } else { + try collections.provisionMetadata(alloc, w, live, effective); } + if (!merged_rules.eql(collections.Rules.from(live))) + std.log.info("provision: collection '{s}' access rules updated", .{spec.name}); + if (widened_count > 0) + std.log.info("provision: collection '{s}' widened {d} select field(s)", .{ spec.name, widened_count }); +} - if (!changed) { - try inspectSearchIndex(alloc, w, spec, true); - try collections.updateIndexes(alloc, w, live.id, spec.indexes); - return; +/// Relation names and ids identify the same target. Resolve only for comparison; +/// retain the live representation in the metadata that provisioning persists. +fn fieldChangeWithRelations(alloc: std.mem.Allocator, live: schema.Field, resolved: schema.Field, target_ids: std.StringHashMapUnmanaged([]const u8)) ProvisionError!FieldChange { + var normalized = live; + if (live.options == .relation) { + if (target_ids.get(live.options.relation.targetCollectionId)) |target_id| + normalized.options.relation.targetCollectionId = target_id; } + return fieldChange(alloc, normalized, resolved); +} - // Validate the old physical search objects before an additive rebuild can - // remove their triggers. Desired searchable fields may not exist yet. - // The unchanged path above needs only its single reconciliation pass. - try inspectSearchIndex(alloc, w, spec, false); - - // Additive auto-migration: rebuild the table with the union of live + new - // fields, matching existing columns by id (rebuildPlan preserves their data), - // and persist the merged user-field schema. - var merged: std.ArrayList(schema.Field) = .empty; - // keep the live user fields (preserves their ids/order), then append additions - for (live.fields) |lf| { - if (lf.id.len > 0 and lf.id[0] == '_') continue; // skip injected auth fields - if (schema.isSystemFieldName(lf.name)) continue; - try merged.append(alloc, lf); +const FieldChange = enum { unchanged, widen_select, migration }; + +/// Compare persisted field values, not slice addresses. A declaration's stable id +/// can differ from a runtime-created field's id; provisioning matches by name. +fn fieldChange(alloc: std.mem.Allocator, live: schema.Field, spec: schema.Field) ProvisionError!FieldChange { + var normalized = spec; + normalized.id = live.id; + const old = try schema.fieldsToJson(alloc, &.{live}); + defer alloc.free(old); + const next = try schema.fieldsToJson(alloc, &.{normalized}); + defer alloc.free(next); + if (std.mem.eql(u8, old, next)) return .unchanged; + + if (live.options != .select or spec.options != .select) return .migration; + if (live.required != spec.required or live.unique != spec.unique or + live.hidden != spec.hidden or live.encrypted != spec.encrypted or + live.searchable != spec.searchable) return .migration; + const before = live.options.select; + const after = spec.options.select; + if (after.maxSelect < before.maxSelect) return .migration; + // Crossing 1 -> many changes how existing TEXT values are decoded (scalar + // versus JSON array), so it needs a data migration even though the limit grows. + // maxSelect=0 is a zero limit in records.validate, not an unlimited sentinel. + if (live.isMultiValue() != spec.isMultiValue()) return .migration; + for (before.values) |value| { + for (after.values) |candidate| { + if (std.mem.eql(u8, value, candidate)) break; + } else return .migration; } - for (additions.items) |sf| try merged.append(alloc, sf); - - var newdef = live; - newdef.fields = merged.items; - newdef.indexes = spec.indexes; - merged_rules.applyTo(&newdef); - // Carry the (possibly newly-set) ttl_field; keep the rest of live.options - // (e.g. persisted OAuth secrets) untouched by NOT copying spec.options wholesale. - newdef.options.ttl_field = spec.options.ttl_field; - _ = try collections.update(alloc, io, w, live.id, newdef); - std.log.info("provision: collection '{s}' added {d} field(s)", .{ spec.name, additions.items.len }); - // The additive rebuild drops triggers tied to the old table; re-ensure the FTS index so its - // sync triggers (and any newly-searchable column) are restored. - try inspectSearchIndex(alloc, w, spec, true); + if (after.maxSelect == before.maxSelect) { + for (after.values) |value| { + for (before.values) |candidate| { + if (std.mem.eql(u8, value, candidate)) break; + } else return .widen_select; + } + return .unchanged; + } + return .widen_select; } /// `spec`'s access rules overlaid on `live`'s: a `null` spec rule means "unspecified — @@ -1161,7 +1184,7 @@ fn mergeRules(spec: schema.Collection, live: schema.Collection) collections.Rule /// `WHERE id = ?1 OR name = ?1` treats a name exactly like an id, so this is functionally /// identical to the resolved-id form once the collection exists (and is exactly why the /// manifest importer's `manifestCollections` already normalizes id-or-name — see import_manifest.zig). -fn resolveTargets(alloc: std.mem.Allocator, w: *db.Db, col: schema.Collection) ProvisionError!schema.Collection { +fn resolveTargets(alloc: std.mem.Allocator, w: *db.Db, col: schema.Collection, target_ids: *std.StringHashMapUnmanaged([]const u8)) ProvisionError!schema.Collection { var any = false; for (col.fields) |f| if (f.options == .relation) { any = true; @@ -1173,7 +1196,11 @@ fn resolveTargets(alloc: std.mem.Allocator, w: *db.Db, col: schema.Collection) P fields[i] = f; switch (f.options) { .relation => |r| { - if (try collections.get(alloc, w, r.targetCollectionId)) |target| { + if (target_ids.get(r.targetCollectionId)) |target_id| { + fields[i].options.relation.targetCollectionId = target_id; + } else if (try collections.get(alloc, w, r.targetCollectionId)) |target| { + try target_ids.put(alloc, target.name, target.id); + try target_ids.put(alloc, target.id, target.id); var nr = r; nr.targetCollectionId = target.id; fields[i].options = .{ .relation = nr }; @@ -2373,6 +2400,473 @@ test "applySpecs additively adds a new field (auto-migration), preserving data" try std.testing.expect(st.isNull(1)); // new column, null for old row } +test "applySpecs widens persisted selects after reopening without rebuilding or losing data" { + const a = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir = try tmp.dir.realPathFileAlloc(io, ".", a); + defer a.free(dir); + const path = try std.fmt.allocPrintSentinel(a, "{s}/data.db", .{dir}, 0); + defer a.free(path); + // Runtime schemas accept opaque field ids, including an underscore prefix. + // These ids must never be mistaken for ownership of injected system fields. + const runtime_fields = [_]schema.Field{ + .{ .id = "_runtime_kind", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }, + .{ .id = "_runtime_tags", .name = "tags", .options = .{ .select = .{ .values = &.{ "a", "b" }, .maxSelect = 2 } } }, + .{ .id = "_runtime_note", .name = "note", .options = .{ .text = .{} } }, + }; + { + var first = try db.Db.open(path); + defer first.close(); + try migrations.run(&first); + const created = try collections.create(a, io, &first, .{ .id = "", .name = "items", .fields = &runtime_fields }); + defer created.deinit(a); + try first.exec("INSERT INTO items(id,kind,tags,note) VALUES ('old','a','[\"a\",\"b\"]','keep me');"); + try first.exec("CREATE INDEX migration_index ON items(kind);"); + try first.exec("CREATE TRIGGER migration_trigger AFTER INSERT ON items BEGIN SELECT 1; END;"); + } + // A fresh test harness would already have these values and hide the stale-schema bug. + var d = try db.Db.open(path); + defer d.close(); + const after = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "b", "c", "a" } }, + .{ .name = "tags", .type = .select, .values = .{ "a", "b", "c" }, .maxSelect = 3 }, + } } }); + const generation = try schema_gen.read(&d); + // The metadata and cache-generation marker must commit together. + try d.exec("CREATE TRIGGER block_generation BEFORE UPDATE ON _schema_state WHEN NEW.generation != OLD.generation BEGIN SELECT RAISE(ABORT,'blocked'); END;"); + try std.testing.expectError(error.ExecFailed, applySpecs(a, io, &d, after)); + { + const unchanged = (try collections.get(a, &d, "items")).?; + defer unchanged.deinit(a); + try std.testing.expectEqual(@as(usize, 2), schema.fieldByName(unchanged, "kind").?.options.select.values.len); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + } + try d.exec("DROP TRIGGER block_generation;"); + try applySpecs(a, io, &d, after); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + try applySpecs(a, io, &d, after); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + + const live = (try collections.get(a, &d, "items")).?; + defer live.deinit(a); + try std.testing.expectEqual(@as(usize, 3), live.fields.len); + for (runtime_fields) |original| + try std.testing.expectEqualStrings(original.id, schema.fieldByName(live, original.name).?.id); + try std.testing.expectEqual(@as(u32, 3), schema.fieldByName(live, "tags").?.options.select.maxSelect); + // Exercise the actual write validator loaded from _collections, not just the JSON. + const input = try std.json.parseFromSlice(std.json.Value, a, "{\"kind\":\"c\",\"tags\":[\"a\",\"b\",\"c\"]}", .{}); + defer input.deinit(); + const records = @import("records.zig"); + const created = try records.create(a, io, &d, live, input.value); + defer records.freeRecord(a, created); + const old = (try records.get(a, &d, live, "old")).?; + defer records.freeRecord(a, old); + try std.testing.expectEqualStrings("a", old.object.get("kind").?.string); + try std.testing.expectEqual(@as(usize, 2), old.object.get("tags").?.array.items.len); + try std.testing.expectEqualStrings("keep me", old.object.get("note").?.string); + var st = try d.prepare("SELECT count(*) FROM sqlite_master WHERE name IN ('migration_index','migration_trigger');"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(0)); +} + +test "applySpecs keeps a stored reserved-named field on a base collection while widening" { + const a = std.testing.allocator; + const io = std.testing.io; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const created = try collections.create(a, io, &d, .{ .id = "", .name = "contacts", .fields = &.{ + .{ .id = "kind_id", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }, + .{ .id = "contact_id", .name = "contact", .options = .{ .text = .{} } }, + } }); + defer created.deinit(a); + // Creation refuses reserved names, so stage what a migration can leave behind. + try d.exec("ALTER TABLE contacts RENAME COLUMN contact TO email;"); + try d.exec("UPDATE _collections SET \"schema\"=replace(\"schema\",'\"contact\"','\"email\"') WHERE name='contacts';"); + const after = comptime buildCollections(.{ .contacts = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b", "c" } }, + } } }); + const generation = try schema_gen.read(&d); + try applySpecs(a, io, &d, after); + try applySpecs(a, io, &d, after); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + + const live = (try collections.get(a, &d, "contacts")).?; + defer live.deinit(a); + try std.testing.expectEqual(@as(usize, 3), schema.fieldByName(live, "kind").?.options.select.values.len); + try std.testing.expectEqualStrings("contact_id", schema.fieldByName(live, "email").?.id); + + // The snapshot guard must see that field too: a stale plan differing only in it conflicts. + var stale = live; + stale.fields = live.fields[0..1]; + var desired = live; + desired.listRule = "@public"; + try std.testing.expectError(error.Conflict, collections.provisionMetadata(a, &d, stale, desired)); +} + +test "provisioning refuses an index on a stored reserved-named encrypted field" { + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const created = try collections.create(a, std.testing.io, &d, .{ .id = "", .name = "contacts", .fields = &.{ + .{ .id = "contact_id", .name = "contact", .encrypted = true, .options = .{ .text = .{} } }, + } }); + defer created.deinit(a); + // Creation refuses reserved names, so stage what a migration can leave behind. + try d.exec("ALTER TABLE contacts RENAME COLUMN contact TO email;"); + try d.exec("UPDATE _collections SET \"schema\"=replace(\"schema\",'\"contact\"','\"email\"') WHERE name='contacts';"); + const live = (try collections.get(a, &d, "contacts")).?; + defer live.deinit(a); + var desired = live; + desired.indexes = &.{.{ .name = "idx_email", .fields = &.{"email"} }}; + try std.testing.expectError(error.Validation, collections.provisionMetadata(a, &d, live, desired)); + const errors = collections.last_errors.?; + defer a.free(errors); + defer collections.last_errors = null; + try std.testing.expectEqual(@as(usize, 1), errors.len); + try std.testing.expectEqualStrings("email", errors[0].field); +} + +test "applySpecs preserves skipped option changes while adding fields and widening selects" { + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const before = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b" } }, + .{ .name = "tags", .type = .select, .values = .{ "a", "b", "c" }, .maxSelect = 3 }, + .{ .name = "title", .type = .text, .min = 2 }, + .{ .name = "state", .type = .select, .values = .{ "a", "b" } }, + } } }); + try applySpecs(a, std.testing.io, &d, before); + try d.exec("INSERT INTO items(id,kind,tags,title,state) VALUES ('old','b','[\"a\",\"b\",\"c\"]','ok','a');"); + const after = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b", "c" } }, + .{ .name = "tags", .type = .select, .values = .{ "a", "b" }, .maxSelect = 2 }, + .{ .name = "title", .type = .text, .min = 3 }, + .{ .name = "state", .type = .select, .values = .{ "a", "b", "c" }, .required = true }, + .{ .name = "views", .type = .number }, + } } }); + try applySpecs(a, std.testing.io, &d, after); + const live = (try collections.get(a, &d, "items")).?; + defer live.deinit(a); + try std.testing.expectEqual(@as(usize, 3), schema.fieldByName(live, "kind").?.options.select.values.len); + try std.testing.expectEqual(@as(u32, 3), schema.fieldByName(live, "tags").?.options.select.maxSelect); + try std.testing.expectEqual(@as(usize, 3), schema.fieldByName(live, "tags").?.options.select.values.len); + try std.testing.expectEqual(@as(?u32, 2), schema.fieldByName(live, "title").?.options.text.min); + try std.testing.expect(!schema.fieldByName(live, "state").?.required); + try std.testing.expectEqual(@as(usize, 2), schema.fieldByName(live, "state").?.options.select.values.len); + try std.testing.expect(schema.fieldByName(live, "views") != null); + var st = try d.prepare("SELECT kind,tags,title,views FROM items WHERE id='old';"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqualStrings("b", st.columnText(0)); + try std.testing.expectEqualStrings("[\"a\",\"b\",\"c\"]", st.columnText(1)); + try std.testing.expectEqualStrings("ok", st.columnText(2)); + try std.testing.expect(st.isNull(3)); +} + +test "select widening rejects stale field snapshots without overwriting values or flags" { + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const initial = comptime buildCollections(.{ .items = .{ .type = .auth, .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b" } }, + } } }); + var runtime_kind = initial[0].fields[0]; + runtime_kind.id = "_runtime_kind"; + var runtime_initial = initial[0]; + runtime_initial.fields = &.{runtime_kind}; + const runtime_created = try collections.create(a, std.testing.io, &d, runtime_initial); + defer runtime_created.deinit(a); + const stale = (try collections.get(a, &d, "items")).?; + defer stale.deinit(a); + const first = comptime buildCollections(.{ .items = .{ .type = .auth, .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b", "c" } }, + } } }); + // Two boots can both preflight against {a,b}. The first writer adds c. + try applySpecs(a, std.testing.io, &d, first); + const generation = try schema_gen.read(&d); + const other = comptime buildCollections(.{ .items = .{ .type = .auth, .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b", "d" } }, + } } }); + var desired_other = stale; + var other_field = schema.fieldByName(stale, "kind").?; + other_field.options = other[0].fields[0].options; + desired_other.fields = &.{other_field}; + try std.testing.expectError(error.Conflict, collections.provisionMetadata(a, &d, stale, desired_other)); + try std.testing.expect(!d.inTransaction()); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + const current = (try collections.get(a, &d, "items")).?; + defer current.deinit(a); + try std.testing.expectEqualStrings("_runtime_kind", schema.fieldByName(current, "kind").?.id); + try std.testing.expectEqualStrings("c", schema.fieldByName(current, "kind").?.options.select.values[2]); + + // An admin changes a flag after another widening preflight. Preserve that + // change too, even when the selection values themselves have not changed. + var field = schema.fieldByName(current, "kind").?; + field.hidden = true; + var edited = current; + edited.fields = &.{field}; + const saved = try collections.update(a, std.testing.io, &d, current.id, edited); + defer saved.deinit(a); + const flagged_generation = try schema_gen.read(&d); + const wider = comptime buildCollections(.{ .items = .{ .type = .auth, .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b", "c", "d" } }, + } } }); + var desired_wider = current; + var wider_field = schema.fieldByName(current, "kind").?; + wider_field.options = wider[0].fields[0].options; + desired_wider.fields = &.{wider_field}; + try std.testing.expectError(error.Conflict, collections.provisionMetadata(a, &d, current, desired_wider)); + try std.testing.expectEqual(flagged_generation, try schema_gen.read(&d)); + const unchanged = (try collections.get(a, &d, "items")).?; + defer unchanged.deinit(a); + try std.testing.expect(schema.fieldByName(unchanged, "kind").?.hidden); + try std.testing.expectEqual(@as(usize, 3), schema.fieldByName(unchanged, "kind").?.options.select.values.len); + // The auth path compared only custom fields and never persisted injected + // credential fields in _collections.schema. + try std.testing.expect(schema.fieldByName(unchanged, "passwordHash") != null); + { + var st = try d.prepare("SELECT schema FROM _collections WHERE name='items';"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expect(std.mem.indexOf(u8, st.columnText(0), "passwordHash") == null); + } + + try collections.delete(a, &d, unchanged.id); + const deleted_generation = try schema_gen.read(&d); + try std.testing.expectError(error.NotFound, collections.provisionMetadata(a, &d, unchanged, desired_wider)); + try std.testing.expect(!d.inTransaction()); + try std.testing.expectEqual(deleted_generation, try schema_gen.read(&d)); +} + +test "field option diff requires migration for narrowing flags and representation changes" { + const a = std.testing.allocator; + const single: schema.Field = .{ .id = "live_id", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }; + var same = single; + same.id = "comptime_id"; + same.options = .{ .select = .{ .values = &.{ "a", "b" } } }; + try std.testing.expectEqual(FieldChange.unchanged, try fieldChange(a, single, same)); + var multi = single; + multi.options.select.maxSelect = 2; + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, single, multi)); + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, multi, single)); + var zero = single; + zero.options.select.maxSelect = 0; + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, single, zero)); + try std.testing.expectEqual(FieldChange.widen_select, try fieldChange(a, zero, single)); + var narrow = single; + narrow.options.select.values = &.{"a"}; + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, single, narrow)); + inline for (.{ "required", "unique", "hidden", "encrypted", "searchable" }) |flag| { + var flagged = single; + @field(flagged, flag) = true; + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, single, flagged)); + } + // Numeric options can alter interpretation even when storageClass is unchanged. + const fixed: schema.Field = .{ .id = "fixed", .name = "price", .options = .{ .number = .{ .mode = .fixed, .scale = 2 } } }; + var rescaled = fixed; + rescaled.options.number.scale = 3; + try std.testing.expectEqual(FieldChange.migration, try fieldChange(a, fixed, rescaled)); +} + +test "provisioning preserves effective searchable flags while widening selects" { + if (comptime !fts.enabled) return error.SkipZigTest; + const a = std.testing.allocator; + const records = @import("records.zig"); + for ([_]bool{ false, true }) |was_searchable| { + for ([_]bool{ false, true }) |add_field| { + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const fields = [_]schema.Field{ + .{ .id = "_kind", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }, + .{ .id = "_title", .name = "title", .searchable = was_searchable, .options = .{ .text = .{} } }, + }; + const original = try collections.create(a, std.testing.io, &d, .{ .id = "", .name = "items", .fields = &fields }); + defer original.deinit(a); + try fts.ensureIndex(a, &d, original); + try d.exec("INSERT INTO items(id,kind,title) VALUES ('old','a','needle');"); + var desired_fields = [_]schema.Field{ + fields[0], fields[1], .{ .id = "_body", .name = "body", .searchable = true, .options = .{ .text = .{} } }, + }; + desired_fields[0].options.select.values = &.{ "a", "b", "c" }; + desired_fields[1].searchable = !was_searchable; // rejected in either direction + var desired = original; + desired.fields = desired_fields[0..if (add_field) 3 else 2]; + try applySpecs(a, std.testing.io, &d, &.{desired}); + try applySpecs(a, std.testing.io, &d, &.{desired}); // repeat boot must preserve the same FTS + const live = (try collections.get(a, &d, "items")).?; + defer live.deinit(a); + try std.testing.expectEqual(was_searchable, schema.fieldByName(live, "title").?.searchable); + try std.testing.expectEqualStrings("_kind", schema.fieldByName(live, "kind").?.id); + if (!was_searchable and !add_field) { + try std.testing.expectError(error.NotSearchable, records.list(a, &d, live, .{ .search = "needle" })); + var st = try d.prepare("SELECT count(*) FROM sqlite_master WHERE name='items_fts';"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqual(@as(i64, 0), st.columnInt(0)); + } else { + // The actual records search must agree with persisted flags: a rejected + // enable never indexes title, and a rejected disable never loses it. + var found = try records.list(a, &d, live, .{ .search = "needle" }); + defer found.deinit(a); + try std.testing.expectEqual(@as(usize, if (was_searchable) 1 else 0), found.items.len); + if (add_field) { + try d.exec("UPDATE items SET body='freshmarker' WHERE id='old';"); + var fresh = try records.list(a, &d, live, .{ .search = "freshmarker" }); + defer fresh.deinit(a); + try std.testing.expectEqual(@as(usize, 1), fresh.items.len); + } + if (was_searchable) { + try d.exec("UPDATE items SET title='updatedneedle' WHERE id='old';"); + var updated = try records.list(a, &d, live, .{ .search = "updatedneedle" }); + defer updated.deinit(a); + try std.testing.expectEqual(@as(usize, 1), updated.items.len); + } + } + } + } +} + +test "disabled FTS rejects accepted searchable additions before rebuilding" { + if (comptime fts.enabled) return error.SkipZigTest; + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const fields = [_]schema.Field{ + .{ .id = "_title", .name = "title", .options = .{ .text = .{} } }, + .{ .id = "_kind", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }, + }; + const original = try collections.create(a, std.testing.io, &d, .{ .id = "", .name = "items", .fields = &fields }); + defer original.deinit(a); + try d.exec("INSERT INTO items(id,title,kind) VALUES ('old','keep me','a');"); + try d.exec("CREATE INDEX migration_index ON items(kind);"); + try d.exec("CREATE TRIGGER migration_trigger AFTER INSERT ON items BEGIN SELECT 1; END;"); + const generation = try schema_gen.read(&d); + var desired_fields = [_]schema.Field{ + fields[0], fields[1], .{ .id = "_body", .name = "body", .searchable = true, .options = .{ .text = .{} } }, + }; + desired_fields[1].options.select.values = &.{ "a", "b", "c" }; + var desired = original; + desired.fields = &desired_fields; + try std.testing.expectError(error.FtsDisabled, applySpecs(a, std.testing.io, &d, &.{desired})); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + try std.testing.expect(!d.inTransaction()); + const unchanged = (try collections.get(a, &d, "items")).?; + defer unchanged.deinit(a); + try std.testing.expect(schema.fieldByName(unchanged, "body") == null); + try std.testing.expectEqual(@as(usize, 2), schema.fieldByName(unchanged, "kind").?.options.select.values.len); + try std.testing.expectEqualStrings("_title", schema.fieldByName(unchanged, "title").?.id); + { + var st = try d.prepare("SELECT (SELECT title FROM items WHERE id='old'), (SELECT kind FROM items WHERE id='old'), (SELECT count(*) FROM pragma_table_info('items') WHERE name='body'), (SELECT count(*) FROM sqlite_master WHERE name IN ('migration_index','migration_trigger'));"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqualStrings("keep me", st.columnText(0)); + try std.testing.expectEqualStrings("a", st.columnText(1)); + try std.testing.expectEqual(@as(i64, 0), st.columnInt(2)); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(3)); + } + // A rejected change on an existing field is not an accepted search feature: + // disabling search on the addition makes this retry valid even though the + // desired title flag is now true. Preflight must use effective, not raw spec. + desired_fields[0].searchable = true; + desired_fields[2].searchable = false; + try applySpecs(a, std.testing.io, &d, &.{desired}); + const upgraded = (try collections.get(a, &d, "items")).?; + defer upgraded.deinit(a); + try std.testing.expect(!schema.fieldByName(upgraded, "title").?.searchable); + try std.testing.expect(!schema.fieldByName(upgraded, "body").?.searchable); + try std.testing.expectEqual(@as(usize, 3), schema.fieldByName(upgraded, "kind").?.options.select.values.len); +} + +test "additive rebuild rejects stale select snapshots before changing storage" { + const a = std.testing.allocator; + for ([_]bool{ false, true }) |ttl_only| { + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const fields = [_]schema.Field{ + .{ .id = "_kind", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }, + .{ .id = "_expiry", .name = "expiry", .options = .{ .date = .{} } }, + }; + const original = try collections.create(a, std.testing.io, &d, .{ .id = "", .name = "items", .fields = &fields }); + defer original.deinit(a); + try d.exec("INSERT INTO items(id,kind) VALUES ('old','a');"); + try d.exec("CREATE INDEX migration_index ON items(kind);"); + try d.exec("CREATE TRIGGER migration_trigger AFTER INSERT ON items BEGIN SELECT 1; END;"); + try d.exec("CREATE TABLE child (id TEXT PRIMARY KEY, parent TEXT REFERENCES items(id) ON DELETE CASCADE);"); + try d.exec("INSERT INTO child VALUES ('child','old');"); + const stale = (try collections.get(a, &d, "items")).?; + defer stale.deinit(a); + var desired_fields = [_]schema.Field{ + fields[0], fields[1], .{ .id = "_views", .name = "views", .options = .{ .number = .{} } }, + }; + desired_fields[0].options.select.values = &.{ "a", "b", "c" }; + var desired = stale; + desired.fields = desired_fields[0..if (ttl_only) 2 else 3]; + if (ttl_only) desired.options.ttl_field = "expiry"; + + // A second writer commits d after this boot planned c plus an unrelated + // field/TTL change. Its new row must not become invalid under a stale plan. + var intervening_fields = fields; + intervening_fields[0].options.select.values = &.{ "a", "b", "d" }; + var intervening = original; + intervening.fields = &intervening_fields; + try applySpecs(a, std.testing.io, &d, &.{intervening}); + try d.exec("INSERT INTO items(id,kind) VALUES ('other','d');"); + const generation = try schema_gen.read(&d); + for ([_]bool{ false, true }) |nested| { + if (nested) try d.begin(); + try std.testing.expectError(error.Conflict, collections.updateWithExpectedDefinition(a, std.testing.io, &d, stale.id, desired, stale)); + try std.testing.expectEqual(nested, d.inTransaction()); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + if (nested) try d.rollback(); + } + const preserved = (try collections.get(a, &d, "items")).?; + defer preserved.deinit(a); + try std.testing.expectEqualStrings("_kind", schema.fieldByName(preserved, "kind").?.id); + try std.testing.expectEqualStrings("d", schema.fieldByName(preserved, "kind").?.options.select.values[2]); + try std.testing.expect(schema.fieldByName(preserved, "views") == null); + try std.testing.expect(preserved.options.ttl_field == null); + { + var st = try d.prepare("SELECT (SELECT kind FROM items WHERE id='other'), (SELECT count(*) FROM items), (SELECT count(*) FROM child), (SELECT count(*) FROM sqlite_master WHERE name IN ('migration_index','migration_trigger'));"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqualStrings("d", st.columnText(0)); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(1)); + try std.testing.expectEqual(@as(i64, 1), st.columnInt(2)); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(3)); + } + { + var st = try d.prepare("PRAGMA foreign_keys;"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqual(@as(i64, 1), st.columnInt(0)); + } + // Re-plan from the current schema, then exercise the real provisioning + // rebuild. Its FK pause must happen outside BEGIN so child rows survive. + desired_fields[0].options.select.values = &.{ "a", "b", "c", "d" }; + try applySpecs(a, std.testing.io, &d, &.{desired}); + const upgraded = (try collections.get(a, &d, "items")).?; + defer upgraded.deinit(a); + try std.testing.expectEqual(@as(usize, 4), schema.fieldByName(upgraded, "kind").?.options.select.values.len); + try std.testing.expectEqual(ttl_only, upgraded.options.ttl_field != null); + try std.testing.expectEqual(!ttl_only, schema.fieldByName(upgraded, "views") != null); + var child = try d.prepare("SELECT count(*) FROM child;"); + defer child.finalize(); + try std.testing.expect(try child.step()); + try std.testing.expectEqual(@as(i64, 1), child.columnInt(0)); + } +} + test "index-only provisioning reconciles definitions without rebuilding tables" { const a = std.testing.allocator; var d = try db.Db.openMemory(); @@ -2638,8 +3132,24 @@ test "applySpecs provisions a self-relation; idempotent re-provision is a no-op" try d.exec("INSERT INTO \"authors\" (\"id\",\"created\",\"updated\",\"name\") VALUES ('a1','','','Ada');"); try d.exec("INSERT INTO \"authors\" (\"id\",\"created\",\"updated\",\"name\",\"mentor\") VALUES ('a2','','','Grace','a1');"); + var declared_mentor = af[1]; + declared_mentor.options.relation.targetCollectionId = authors.id; + // The actual classifier used at boot must avoid the migration-warning branch. + var target_ids: std.StringHashMapUnmanaged([]const u8) = .empty; + defer target_ids.deinit(a); + try target_ids.put(a, authors.name, authors.id); + try target_ids.put(a, authors.id, authors.id); + try std.testing.expectEqual(FieldChange.unchanged, try fieldChangeWithRelations(a, mentor, declared_mentor, target_ids)); + const generation = try schema_gen.read(&d); // Re-provision: clean no-op (no error, no duplicate collection). try applySpecs(a, std.testing.io, &d, &specs); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + var by_id = af; + by_id[1].options.relation.targetCollectionId = authors.id; + var id_spec = specs[0]; + id_spec.fields = &by_id; + try applySpecs(a, std.testing.io, &d, &.{id_spec}); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); const all = try collections.list(a, &d); defer { for (all) |c| c.deinit(a); @@ -3538,3 +4048,198 @@ test "resolveDiscoveryProviders returns the same slice when no provider uses dis const out = try resolveDiscoveryProviders(a, stub.transport(), &cols); try std.testing.expectEqual(@as(usize, @intFromPtr(&cols)), @as(usize, @intFromPtr(out.ptr))); } + +test "provisioning commits widening rules and indexes atomically once" { + const a = std.testing.allocator; + for ([_]bool{ false, true }) |fts_failure| { + if (fts_failure and !fts.enabled) continue; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const initial = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b" } }, + .{ .name = "title", .type = .text, .searchable = true }, + } } }); + var original = initial[0]; + // Keep this regression runnable in the disabled-FTS configuration. + var fields = [_]schema.Field{ original.fields[0], original.fields[1] }; + fields[1].searchable = fts.enabled; + original.fields = &fields; + try applySpecs(a, std.testing.io, &d, &.{original}); + try d.exec("INSERT INTO items(id,kind,title) VALUES ('one','a','same'),('two','b','same');"); + try d.exec("CREATE INDEX migration_index ON items(kind);"); + try d.exec("CREATE TRIGGER migration_trigger AFTER INSERT ON items BEGIN SELECT 1; END;"); + const before = (try collections.get(a, &d, "items")).?; + defer before.deinit(a); + const generation = try schema_gen.read(&d); + fields[0].options.select.values = &.{ "a", "b", "c" }; + var desired = original; + desired.viewRule = "id != ''"; + desired.indexes = &.{.{ .name = "unique_title", .fields = &.{"title"}, .unique = true }}; + if (fts_failure) { + // Physical ownership conflict must fail before any metadata can commit. + try d.exec("DROP TABLE items_fts;"); + try d.exec("CREATE TABLE items_fts (id TEXT);"); + desired.indexes = &.{.{ .name = "title_index", .fields = &.{"title"} }}; + } + try std.testing.expectError(error.ExecFailed, applySpecs(a, std.testing.io, &d, &.{desired})); + try std.testing.expect(!d.inTransaction()); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + const after = (try collections.get(a, &d, "items")).?; + defer after.deinit(a); + try std.testing.expect(collections.Rules.from(before).eql(collections.Rules.from(after))); + try std.testing.expectEqual(@as(usize, 2), schema.fieldByName(after, "kind").?.options.select.values.len); + try std.testing.expectEqual(@as(usize, 0), after.indexes.len); + { + var st = try d.prepare("SELECT count(*) FROM sqlite_master WHERE name IN ('migration_index','migration_trigger');"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(0)); + } + if (fts_failure) continue; + desired.indexes = &.{.{ .name = "title_index", .fields = &.{"title"} }}; + try applySpecs(a, std.testing.io, &d, &.{desired}); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + try applySpecs(a, std.testing.io, &d, &.{desired}); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + } +} + +test "whole definition guards reject concurrent rules options and TTL before any write" { + const a = std.testing.allocator; + const edits = [_][:0]const u8{ + "UPDATE _collections SET viewRule='id != 0' WHERE name='items';", + "UPDATE _collections SET options=json_set(options,'$.ttl.field','expiry') WHERE name='items';", + "UPDATE _collections SET options=json_set(options,'$.auth.oauth2.enabled',json('true')) WHERE name='items';", + "UPDATE _collections SET options=json_set(options,'$.auth.oauth2.providers',json('[{\"name\":\"custom\",\"clientId\":\"id\",\"clientSecret\":\"new-secret\",\"enabled\":false}]')) WHERE name='items';", + }; + for (edits) |edit| { + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const initial = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b" } }, + .{ .name = "expiry", .type = .date }, + } } }); + try applySpecs(a, std.testing.io, &d, initial); + const stale = (try collections.get(a, &d, "items")).?; + defer stale.deinit(a); + var fields = [_]schema.Field{ stale.fields[0], stale.fields[1], .{ .id = "newfield", .name = "extra", .options = .{ .text = .{} } } }; + fields[0].options.select.values = &.{ "a", "b", "c" }; + var desired = stale; + desired.fields = &fields; + try d.exec(edit); + const generation = try schema_gen.read(&d); + try std.testing.expectError(error.Conflict, collections.updateWithExpectedDefinition(a, std.testing.io, &d, stale.id, desired, stale)); + desired.fields = fields[0..2]; + try std.testing.expectError(error.Conflict, collections.provisionMetadata(a, &d, stale, desired)); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + const after = (try collections.get(a, &d, "items")).?; + defer after.deinit(a); + try std.testing.expectEqual(@as(usize, 2), schema.fieldByName(after, "kind").?.options.select.values.len); + try std.testing.expect(schema.fieldByName(after, "extra") == null); + } +} + +test "index reconciliation accepts opaque underscore field ids and keeps validation errors owned" { + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const original = try collections.create(a, std.testing.io, &d, .{ + .id = "", + .name = "items", + .fields = &.{ + .{ .id = "_runtime_kind", .name = "kind", .options = .{ .text = .{} } }, + .{ .id = "_runtime_secret", .name = "secret", .encrypted = true, .options = .{ .text = .{} } }, + }, + }); + defer original.deinit(a); + var desired = original; + desired.indexes = &.{.{ .name = "idx_kind", .fields = &.{"kind"} }}; + try collections.provisionMetadata(a, &d, original, desired); + const indexed = (try collections.get(a, &d, original.id)).?; + defer indexed.deinit(a); + try std.testing.expectEqual(@as(usize, 1), indexed.indexes.len); + try std.testing.expectEqualStrings("_runtime_kind", schema.fieldByName(indexed, "kind").?.id); + const generation = try schema_gen.read(&d); + desired = indexed; + // Opaque ids must not hide encrypted fields from index validation either. + desired.indexes = &.{.{ .name = "idx_secret", .fields = &.{"secret"} }}; + try std.testing.expectError(error.Validation, collections.provisionMetadata(a, &d, indexed, desired)); + const encrypted_errors = collections.last_errors.?; + try std.testing.expectEqualStrings("secret", encrypted_errors[0].field); + a.free(encrypted_errors); + collections.last_errors = null; + desired.indexes = &.{.{ .name = "bad-index-name", .fields = &.{"kind"} }}; + try std.testing.expectError(error.Validation, collections.provisionMetadata(a, &d, indexed, desired)); + // The helper's scratch arena is gone. This caller-owned diagnostic must still + // be readable and freeable, rather than a dangling threadlocal arena slice. + const errors = collections.last_errors.?; + defer a.free(errors); + defer collections.last_errors = null; + try std.testing.expect(errors.len > 0); + try std.testing.expectEqualStrings("bad-index-name", errors[0].field); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + try std.testing.expectError(error.Validation, applySpecs(a, std.testing.io, &d, &.{desired})); + try std.testing.expect(collections.last_errors == null); +} + +test "identical provisioning plans from two boots converge without conflict or another rebuild" { + const a = std.testing.allocator; + for ([_]bool{ false, true }) |rebuild| { + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const original = try collections.create(a, std.testing.io, &d, .{ + .id = "", + .name = "items", + .fields = &.{.{ .id = "kind_id", .name = "kind", .options = .{ .select = .{ .values = &.{ "a", "b" } } } }}, + }); + defer original.deinit(a); + try d.exec("INSERT INTO items(id,kind) VALUES ('old','a');"); + var fields = [_]schema.Field{ original.fields[0], .{ .id = "extra_id", .name = "extra", .options = .{ .text = .{} } } }; + fields[0].options.select.values = &.{ "a", "b", "c" }; + var desired = original; + desired.fields = fields[0..if (rebuild) 2 else 1]; + desired.viewRule = "id != ''"; + const generation = try schema_gen.read(&d); + for (0..2) |boot| { + if (rebuild) { + const result = try collections.updateWithExpectedDefinition(a, std.testing.io, &d, original.id, desired, original); + result.deinit(a); + } else try collections.provisionMetadata(a, &d, original, desired); + try std.testing.expectEqual(generation + 1, try schema_gen.read(&d)); + if (boot == 0) { + try d.exec("CREATE INDEX after_first_boot ON items(kind);"); + try d.exec("CREATE TRIGGER after_first_boot_trigger AFTER INSERT ON items BEGIN SELECT 1; END;"); + } + } + var st = try d.prepare("SELECT (SELECT kind FROM items WHERE id='old'), (SELECT count(*) FROM sqlite_master WHERE name IN ('after_first_boot','after_first_boot_trigger'));"); + defer st.finalize(); + try std.testing.expect(try st.step()); + try std.testing.expectEqualStrings("a", st.columnText(0)); + try std.testing.expectEqual(@as(i64, 2), st.columnInt(1)); + } +} + +test "reordered select values leave persisted order and generation unchanged" { + const a = std.testing.allocator; + var d = try db.Db.openMemory(); + defer d.close(); + try migrations.run(&d); + const initial = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "b", "a" } }, + } } }); + try applySpecs(a, std.testing.io, &d, initial); + const generation = try schema_gen.read(&d); + const reorder = comptime buildCollections(.{ .items = .{ .fields = .{ + .{ .name = "kind", .type = .select, .values = .{ "a", "b" } }, + } } }); + try std.testing.expectEqual(FieldChange.unchanged, try fieldChange(a, initial[0].fields[0], reorder[0].fields[0])); + try applySpecs(a, std.testing.io, &d, reorder); + try std.testing.expectEqual(generation, try schema_gen.read(&d)); + const live = (try collections.get(a, &d, "items")).?; + defer live.deinit(a); + try std.testing.expectEqualStrings("b", schema.fieldByName(live, "kind").?.options.select.values[0]); +} diff --git a/src/scaffold/agents_md.zig b/src/scaffold/agents_md.zig index bdb4fdcd..699474a3 100644 --- a/src/scaffold/agents_md.zig +++ b/src/scaffold/agents_md.zig @@ -177,9 +177,11 @@ const framework_traps = \\## Trap 8 — schema provisioning is ADDITIVE ONLY \\ \\The comptime `.collections` literal is provisioned at startup: missing - \\collections are created and new fields are added. **Renames, drops, and type - \\changes are detected, logged, and then skipped** — the server starts happily - \\and your change did not happen. Those need an explicit `.migrations` entry. + \\collections are created, new fields are added, and select options may widen + \\without changing single/multi-value storage. **Renames, drops, type changes, + \\and other existing field option/flag changes are logged and skipped** — the + \\server starts happily and your change did not happen. Those need an explicit + \\`.migrations` entry. \\Read the startup log after any schema edit. \\ \\## Trap 9 — hook allocations belong to the request arena diff --git a/src/schema.zig b/src/schema.zig index fbaccd7b..0c339a7a 100644 --- a/src/schema.zig +++ b/src/schema.zig @@ -882,6 +882,12 @@ pub fn isSystemFieldName(name: []const u8) bool { return false; } +/// Only auth collections carry engine-injected fields. A stored field with a reserved +/// name on any other collection type is user data, left behind by a migration. +pub fn isInjectedField(collection_type: CollectionType, name: []const u8) bool { + return collection_type == .auth and isSystemFieldName(name); +} + /// The implicit system fields of an auth collection (beyond id/created/updated). /// passwordHash/tokenKey are hidden (never serialized). Stable ids (leading '_'). pub fn authSystemFields() []const Field { @@ -1000,14 +1006,40 @@ pub fn hasSearchableField(c: Collection) bool { /// Appends any validation problems to `errors`. Self-sizing; messages/codes are static or borrowed from `c`. pub fn validate(alloc: std.mem.Allocator, c: Collection, errors: *std.ArrayList(ValidationError)) std.mem.Allocator.Error!void { - if (!isValidIdentifier(c.name)) + try validateCollectionName(alloc, c.name, errors); + try validateFieldsAndOptions(alloc, c, errors); +} + +/// Check a canonical stored name independently of an update's ignored rename. +pub fn validateCollectionName(alloc: std.mem.Allocator, name: []const u8, errors: *std.ArrayList(ValidationError)) std.mem.Allocator.Error!void { + if (!isValidIdentifier(name)) try errors.append(alloc, .{ .field = "name", .code = codes.s(.validation_invalid_name), .message = "Name must start with a letter and contain only letters, digits, and underscores." }); // `_fts` is reserved: a searchable collection `posts` provisions an FTS5 shadow table named // `posts_fts`, so a user collection literally named `_fts` could collide with ``'s index // (CREATE VIRTUAL TABLE over an existing base table errors). Reserve the suffix outright (#157). - if (std.mem.endsWith(u8, c.name, fts_suffix)) + if (std.mem.endsWith(u8, name, fts_suffix)) try errors.append(alloc, .{ .field = "name", .code = codes.s(.validation_reserved_suffix), .message = "Collection name may not end with '_fts' (reserved for full-text search index tables)." }); +} +/// Check index names and that no index covers an encrypted field of `c`. +pub fn validateIndexes(alloc: std.mem.Allocator, c: Collection, errors: *std.ArrayList(ValidationError)) std.mem.Allocator.Error!void { + for (c.indexes) |idx| { + if (!isValidIdentifier(idx.name)) + try errors.append(alloc, .{ .field = idx.name, .code = codes.s(.validation_invalid_name), .message = "Invalid index name." }); + for (idx.fields) |fname| { + if (!isValidIdentifier(fname)) + try errors.append(alloc, .{ .field = fname, .code = codes.s(.validation_invalid_name), .message = "Invalid index field name." }); + // An encrypted column holds per-row-nonce ciphertext, so an index over it + // is useless (every row's stored bytes differ). Reject rather than build a + // dead index. (Comptime path enforces this with @compileError.) + if (fieldByName(c, fname)) |fl| if (fl.encrypted) + try errors.append(alloc, .{ .field = fname, .code = codes.s(.validation_encrypted_index), .message = "Cannot index an encrypted field." }); + } + } +} + +/// Check caller-controlled fields, indexes and options without loading metadata. +pub fn validateFieldsAndOptions(alloc: std.mem.Allocator, c: Collection, errors: *std.ArrayList(ValidationError)) std.mem.Allocator.Error!void { for (c.fields, 0..) |f, i| { if (!isValidIdentifier(f.name)) { try errors.append(alloc, .{ .field = f.name, .code = codes.s(.validation_invalid_name), .message = "Invalid field name." }); @@ -1073,19 +1105,7 @@ pub fn validate(alloc: std.mem.Allocator, c: Collection, errors: *std.ArrayList( } } - for (c.indexes) |idx| { - if (!isValidIdentifier(idx.name)) - try errors.append(alloc, .{ .field = idx.name, .code = codes.s(.validation_invalid_name), .message = "Invalid index name." }); - for (idx.fields) |fname| { - if (!isValidIdentifier(fname)) - try errors.append(alloc, .{ .field = fname, .code = codes.s(.validation_invalid_name), .message = "Invalid index field name." }); - // An encrypted column holds per-row-nonce ciphertext, so an index over it - // is useless (every row's stored bytes differ). Reject rather than build a - // dead index. (Comptime path enforces this with @compileError.) - if (fieldByName(c, fname)) |fl| if (fl.encrypted) - try errors.append(alloc, .{ .field = fname, .code = codes.s(.validation_encrypted_index), .message = "Cannot index an encrypted field." }); - } - } + try validateIndexes(alloc, c, errors); // Auth identity fields are interpolated into SQL/DDL, so they must be valid identifiers. if (c.type == .auth) { diff --git a/src/startup_log.zig b/src/startup_log.zig new file mode 100644 index 00000000..e7c23035 --- /dev/null +++ b/src/startup_log.zig @@ -0,0 +1,7 @@ +const std = @import("std"); + +/// Expected startup rejection paths retain their diagnostic under the test +/// runner, whose error counter otherwise treats the expected refusal as failure. +pub fn refusal(comptime fmt: []const u8, args: anytype) void { + if (@import("builtin").is_test) std.log.warn(fmt, args) else std.log.err(fmt, args); +}