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); +}