diff --git a/.changeset/localized-write-companion-guard.md b/.changeset/localized-write-companion-guard.md new file mode 100644 index 000000000..de454ff6d --- /dev/null +++ b/.changeset/localized-write-companion-guard.md @@ -0,0 +1,31 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +--- + +Saving a translation could overwrite the original language. `nextly db:sync` marks a collection as localized in a separate process from the running app, so the app could show the language switcher before its translations table existed — and a translation saved in that window wrote over the original-language values and changed the entry's URL, while reporting success. + +The translations table is now prepared during `db:sync` and during a dev config reload, for collections, singles and field groups alike. If it is still missing, a write in a non-default language is refused with a clear message instead of overwriting anything, and the same refusal now covers singles and embedded field groups rather than only collections. + +Writing the default language before the table exists still goes to the main table as before. The one exception is content that was localized from the start, whose translatable values have never had a main-table column to fall back to: saving that while the translations table is missing used to fail with a database error, and now reports the same clear message as the case above. + +Collections and singles that set a custom `dbName` are handled correctly here too; previously their translations table could be created against a table name that does not exist. And a database that is unreachable or refusing connections is no longer reported as a missing translations table. diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts new file mode 100644 index 000000000..fd7e03523 --- /dev/null +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -0,0 +1,200 @@ +/** + * `nextly db:sync` must create the companion `_locales` table in the SAME run. + * + * The push pipeline deliberately does not manage companion tables + * (`managed-tables.isCompanionTable`), and `ensureCompanionTable` — written as + * the db:sync/dev-boot counterpart to migration-owned creation — used to be + * called only at boot. Because `db:sync` runs in its own CLI process, it flipped + * the registry's `localized` flag and left companion creation to whenever the app + * next started. A server already running then read `localized: 1`, registered the + * companion in its runtime registry, and rendered the whole localization UI over + * a table that did not exist. Writes in that window overwrote the default + * language. + * + * So the assertion is specifically that the table exists when the sync sequence + * returns, not that it exists eventually. + * + * This drives the real `syncCollections` → `syncSingles` → `syncComponents` → + * `ensureLocalizedCompanions` sequence against a real SQLite file, because + * ORDER is the thing that broke: a companion carries a foreign key to its main + * table, so running the hook before singles and components are pushed creates + * nothing for them. What it does NOT prove is that `db-sync.ts` and + * `dev-watcher.ts` still call the hook — that wiring is a single line in each, + * checked by reading them. + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + defineCollection, + defineConfig, + defineSingle, + text, +} from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { getDialectTables } from "../../../database/index"; +import { SchemaRegistry } from "../../../database/schema-registry"; +import { createLogger } from "../../utils/logger"; +import { + ensureLocalizedCompanions, + syncCollections, + syncComponents, + syncSingles, +} from "../dev-build"; +import { ensureCoreTables } from "../dev-server"; + +import type { DrizzleAdapter } from "@nextlyhq/adapter-drizzle/types"; +import type { CLIDatabaseAdapter } from "../../utils/adapter"; +import type { CommandContext } from "../../program"; +import type { LoadConfigResult } from "../../utils/config-loader"; +import type { ResolvedDevOptions } from "../db-sync"; + +let dir: string; +let adapter: Awaited> | undefined; +let previousDialect: string | undefined; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "nextly-dbsync-companion-")); + previousDialect = process.env.DB_DIALECT; +}); + +afterEach(async () => { + await adapter?.disconnect(); + adapter = undefined; + // Integration files share one fork, so a dialect left set here would be read + // by every file that follows. + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; + rmSync(dir, { recursive: true, force: true }); +}); + +/** Physical presence, probed the same way the write path probes it. */ +async function tableExists(name: string): Promise { + try { + await adapter?.executeQuery(`SELECT 1 FROM "${name}" LIMIT 0`); + return true; + } catch { + return false; + } +} + +/** The command's own sequence, minus the parts that read from disk. */ +async function runSync(config: LoadConfigResult["config"]): Promise { + process.env.DB_DIALECT = "sqlite"; + adapter = await createAdapter({ + type: "sqlite", + url: `file:${join(dir, "test.db")}`, + } as Parameters[0]); + + const logger = createLogger({ quiet: true }); + const options = { cwd: dir, autoSync: true } as ResolvedDevOptions; + const context = { logger, options: {}, cwd: dir } as CommandContext; + // The real adapter is a DrizzleAdapter; `CLIDatabaseAdapter` declares only the + // handful of methods the command signatures need, so the command file converts + // between them exactly like this. + const cli = adapter as unknown as CLIDatabaseAdapter; + const drizzleAdapter = adapter as unknown as DrizzleAdapter; + + // Without a resolver the ORM cannot address `dynamic_collections`, so the sync + // fails before it reaches anything this test is about. + const registry = new SchemaRegistry("sqlite"); + registry.registerStaticSchemas(getDialectTables("sqlite")); + drizzleAdapter.setTableResolver(registry); + await ensureCoreTables(cli, options, context); + + const configResult = { config } as LoadConfigResult; + await syncCollections(configResult, cli, options, context); + await syncSingles(configResult, cli, options, context); + await syncComponents(configResult, cli, options, context); + await ensureLocalizedCompanions(config, cli, context); +} + +describe("db:sync creates localized companion tables in-process (integration)", () => { + it("creates a localized collection's companion in the same run", async () => { + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + collections: [ + defineCollection({ + slug: "dbsync_posts", + localized: true, + fields: [text({ name: "title", localized: true })], + }), + ], + }) + ); + + expect(await tableExists("dc_dbsync_posts")).toBe(true); + // The assertion that fails without the fix: the push pipeline creates the + // main table and nothing in this process creates the companion. + expect(await tableExists("dc_dbsync_posts_locales")).toBe(true); + }); + + it("creates a localized single's companion, which needs singles synced first", async () => { + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + singles: [ + defineSingle({ + slug: "dbsync_homepage", + localized: true, + fields: [text({ name: "headline", localized: true })], + }), + ], + }) + ); + + // Singles are force-prefixed with `single_` by `resolveSingleTableName`. + // Asserting the main table too means a prefix change cannot make the companion + // check vacuously pass against a name nothing ever creates. + expect(await tableExists("single_dbsync_homepage")).toBe(true); + expect(await tableExists("single_dbsync_homepage_locales")).toBe(true); + }); + + it("resolves a custom dbName the way the runtime does", async () => { + // A collection's `dbName` is force-prefixed with `dc_` by the canonical + // resolver, so `dbName: "dbsync_notes"` lives at `dc_dbsync_notes`. Taking + // `dbName` verbatim instead builds `dbsync_notes_locales` with a foreign key to + // a `dbsync_notes` table that does not exist — the create fails, the warning is + // swallowed, and the entity is left marked localized with nowhere to put + // translations. + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + collections: [ + defineCollection({ + slug: "dbsync_field_notes", + dbName: "dbsync_notes", + localized: true, + fields: [text({ name: "title", localized: true })], + }), + ], + }) + ); + + expect(await tableExists("dc_dbsync_notes")).toBe(true); + expect(await tableExists("dc_dbsync_notes_locales")).toBe(true); + expect(await tableExists("dbsync_notes_locales")).toBe(false); + }); + + it("leaves a non-localized collection with no companion", async () => { + await runSync( + defineConfig({ + collections: [ + defineCollection({ + slug: "dbsync_logs", + fields: [text({ name: "title" })], + }), + ], + }) + ); + + expect(await tableExists("dc_dbsync_logs")).toBe(true); + // Creating companions unconditionally would strand a dead table in every + // project that does not use localization. + expect(await tableExists("dc_dbsync_logs_locales")).toBe(false); + }); +}); diff --git a/packages/nextly/src/cli/commands/db-sync.ts b/packages/nextly/src/cli/commands/db-sync.ts index abc965a94..4ac791ce5 100644 --- a/packages/nextly/src/cli/commands/db-sync.ts +++ b/packages/nextly/src/cli/commands/db-sync.ts @@ -85,6 +85,7 @@ import { import { formatDuration } from "../utils/logger"; import { + ensureLocalizedCompanions, performPermissionSeeding, syncCollections, syncComponents, @@ -302,6 +303,19 @@ export async function runDbSync( await syncSingles(configResult, adapter, options, context); await syncComponents(configResult, adapter, options, context); + // Step 5.6: Create the `_locales` companion of every localized entity, which + // the push pipeline does not manage. Runs after all three syncs because the + // companion references its main table. Without this, `db:sync` left the + // registry saying "localized" with no table to hold translations until the + // app next booted, and writes in that window overwrote the default language. + // + // Gated on the same flag as the rest of the schema push: this issues DDL and + // can copy rows, so `--no-auto-sync` — chosen precisely to keep physical + // schema changes in migration files — must suppress it too. + if (options.autoSync !== false) { + await ensureLocalizedCompanions(configResult.config, adapter, context); + } + if (collectionCount === 0) { logger.warn("No collections defined in config"); logger.info("Add collections to your nextly.config.ts to get started."); diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index a5fae2e5c..c139efd1e 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -16,7 +16,11 @@ import { teardownEntityComponentData } from "../../domains/field-groups/services import { teardownEntityI18n } from "../../domains/i18n/migration/teardown-entity-i18n"; // Resolve the versioning config so `db:sync` persists it (parity with boot/HMR). import { resolveVersionsConfig } from "../../domains/versions/resolve-config"; -import { describeError, immediateMessage } from "../../errors/index"; +import { + describeError, + immediateMessage, + NextlyError, +} from "../../errors/index"; import { STORAGE_FORMAT } from "../../schemas/storage-format"; import { CollectionSyncService } from "../../services/collections/collection-sync-service"; import type { CollectionSyncResultWithValidation } from "../../services/collections/collection-sync-service"; @@ -301,6 +305,12 @@ export async function syncComponents( fields: component.fields, description: component.description, admin: component.admin, + // Persist i18n through db:sync, as the singles mapping above already does. + // Runtime field-group writes gate on the stored flag, so leaving it unset + // made `db:sync` record a localized field group as non-localized: + // translatable values kept being treated as shared main-table fields, and a + // save in a non-default locale overwrote the default one. + localized: component.localized === true, configPath: `${STORAGE_FORMAT.configPathDir}/${component.slug}.ts`, }) ); @@ -761,3 +771,149 @@ async function handleRemovedComponents( } } } + +/** + * Create the companion `_locales` table for every localized collection, single + * and component in the config. + * + * The push pipeline deliberately does not manage companion tables (see + * `managed-tables.isCompanionTable`), and `ensureCompanionTable` — the intended + * db:sync/dev-boot counterpart to migration-owned creation — was only ever called + * at boot. Because `db:sync` runs in its own CLI process, it flipped the + * registry's `localized` flag and left creation to whenever the app next started; + * a server already running then rendered the whole localization UI over a table + * that did not exist. + * + * MUST be called after `syncCollections`, `syncSingles` AND `syncComponents`: + * the companion carries a foreign key to its main table, so running it earlier + * fails for whichever entity types have not been pushed yet. Idempotent — + * `ensureCompanionTable` returns early when the table is already there — so + * running it on every sync costs one probe per localized entity. + */ +export async function ensureLocalizedCompanions( + config: LoadConfigResult["config"], + adapter: CLIDatabaseAdapter, + context: CommandContext +): Promise { + const { logger } = context; + // Same policy `performAutoSync` applies: production never gets unattended schema + // changes, and this issues DDL and can copy rows. Enforced here rather than at + // each call site so no caller can reintroduce the hole. In production the + // companion is `nextly migrate`'s job, and the write guard keeps a non-default + // write from destroying content until it runs. + if (process.env.NODE_ENV === "production") return; + const dialect = adapter.getCapabilities().dialect; + const { ensureCompanionTable, reconcileCompanionColumns } = await import( + "../../domains/i18n/runtime/companion-io" + ); + // Each entity kind resolves its physical table differently, and a custom + // `dbName` is where they diverge: collections and singles force their `dc_` / + // `single_` prefix onto it (and singles normalize the identifier), while field + // groups take no override at all. A single shared "prefix + slug" rule would + // point at a table the runtime never created — `dbName: "forms"` on a + // collection lives at `dc_forms`, so the companion would be built as + // `forms_locales` with a foreign key to a table that does not exist. + const { resolveCollectionTableName, resolveComponentTableName } = + await import("../../domains/schema/utils/resolve-table-name"); + const { resolveSingleTableName } = await import( + "../../domains/singles/services/resolve-single-table-name" + ); + + interface LocalizableEntity { + slug?: string; + dbName?: string; + localized?: boolean; + status?: boolean; + fields?: { name: string; type: string; localized?: boolean }[]; + } + + const groups: [LocalizableEntity[], (e: LocalizableEntity) => string][] = [ + [ + (config.collections ?? []) as LocalizableEntity[], + e => resolveCollectionTableName(e.slug!, e.dbName), + ], + [ + (config.singles ?? []) as LocalizableEntity[], + e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), + ], + [ + (config.fieldGroups ?? []) as LocalizableEntity[], + // Field groups derive their table from the slug alone — unlike collections + // and singles they carry no `dbName` override. + e => resolveComponentTableName(e.slug!), + ], + ]; + + const failures: string[] = []; + for (const [group, resolveTableName] of groups) { + for (const entity of group) { + if (!entity.slug || entity.localized !== true) continue; + const tableName = resolveTableName(entity); + // `ensureCompanionTable` resolves even on failure (a first boot may not have + // the main table yet), so the reporter — not a try/catch — is what surfaces + // a real problem. + // `CLIDatabaseAdapter` under-declares the runtime object, which is a real + // DrizzleAdapter — the same conversion this file already uses for the sync + // service above. `ensureCompanionTable` needs `executeQuery`, which the + // declared CLI interface omits. + await ensureCompanionTable( + adapter as unknown as DrizzleAdapter, + { + slug: entity.slug, + tableName, + fields: entity.fields ?? [], + dialect, + status: entity.status === true, + }, + error => { + logger.error( + `Could not prepare the translations table for "${entity.slug}" (${tableName}_locales). ` + + `Writes in a non-default locale will be refused until it exists: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + failures.push(entity.slug!); + } + ); + // Creating the companion is not enough on its own. `ensureCompanionTable` returns + // immediately once the table is there, so marking a FURTHER field localized on an + // entity that is already localized leaves the companion a column short — while the + // main-table sync above has already added that column to `comp_*` / `dc_*`. The write + // then splits the value into a companion column that does not exist. Additive only, + // Additive only. The reload path reconciles too — it is dev-only, behind its own + // production return — so the two stay in step. + await reconcileCompanionColumns( + adapter as unknown as DrizzleAdapter, + { + slug: entity.slug, + tableName, + fields: entity.fields ?? [], + dialect, + status: entity.status === true, + }, + error => { + logger.error( + `Could not update the translations table for "${entity.slug}" (${tableName}_locales). ` + + `Newly translatable fields will fail to save until it matches: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + failures.push(entity.slug!); + } + ); + } + } + // Fail the command. Exiting 0 here would tell deployment automation the schema is + // in step while the registry advertises localization the database cannot store, so + // every translation write is refused — unlike a failure from the main auto-sync + // pipeline, which does stop the run. Boot and the HMR reload stay best-effort by + // passing no reporter: they must not refuse to start over a companion. + if (failures.length > 0) { + throw NextlyError.conflict({ + reason: "state", + message: `Could not prepare the translations table for: ${failures.join(", ")}. Writes in a non-default locale will be refused until it exists.`, + logContext: { + cause: "companion-provisioning-failed", + entities: failures, + }, + }); + } +} diff --git a/packages/nextly/src/cli/commands/dev-watcher.ts b/packages/nextly/src/cli/commands/dev-watcher.ts index f2f64ff10..b7586911b 100644 --- a/packages/nextly/src/cli/commands/dev-watcher.ts +++ b/packages/nextly/src/cli/commands/dev-watcher.ts @@ -16,6 +16,7 @@ import type { LoadConfigResult } from "../utils/config-loader"; import type { ResolvedDevOptions } from "./db-sync"; import { + ensureLocalizedCompanions, performPermissionSeeding, syncCollections, syncComponents, @@ -72,6 +73,16 @@ export function createDebouncedSync( await syncSingles(configToSync, adapter, options, context); await syncComponents(configToSync, adapter, options, context); + // Turning on localization is a config edit, so it arrives through this + // watcher as often as through `db:sync`. The companion table is not part of + // the push pipeline, and creating it here rather than at the next boot is + // what keeps a running server from advertising localization it cannot store. + // Suppressed under `--no-auto-sync` for the same reason the rest of the + // push is: it issues DDL and can copy rows. + if (options.autoSync !== false) { + await ensureLocalizedCompanions(configToSync.config, adapter, context); + } + // Sync user_ext table (always — handles both code and UI fields) await syncUserFields(configToSync, adapter, options, context); diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index ca2c63395..9967900fc 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -89,6 +89,10 @@ import { isValidLocale, resolveRequestedLocale, } from "../../i18n/resolve-locale"; +import { + companionTableExists as sharedCompanionTableExists, + mainTableHasColumns, +} from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; import { @@ -875,20 +879,21 @@ export class CollectionMutationService extends BaseService { ); } - /** Whether the companion `_locales` table physically exists (migration has run). */ + /** + * Whether the companion `_locales` table physically exists (migration has run). + * + * Delegates to the shared probe, which returns false only for a dialect-verified + * missing-table error and rethrows anything else. Catching every exception here + * turned a connection timeout or a permission error into "the table is absent", + * which the callers below read as a schema state: one refuses the write with a + * 409 telling the operator to re-run sync, the other decides the collection has + * no publish lifecycle. Both are the wrong answer to "the database is + * unreachable", and both hide a failure the caller would otherwise retry. + */ private async companionTableExists( companionTableName: string ): Promise { - const q = - this.adapter.dialect === "mysql" - ? `\`${companionTableName}\`` - : `"${companionTableName}"`; - try { - await this.adapter.executeQuery(`SELECT 1 FROM ${q} LIMIT 0`); - return true; - } catch { - return false; - } + return sharedCompanionTableExists(this.adapter, companionTableName); } /** @@ -942,6 +947,65 @@ export class CollectionMutationService extends BaseService { // `migrate`, the dev auto-sync leaves localized columns on the MAIN table (Option B), so // writes must go there — return null and let the localized values flow to main as today. if (!(await this.companionTableExists(companion.companionTableName))) { + // The main table carries no language of its own, so anything written there + // while the companion is missing is later read as the DEFAULT language — + // that is the assumption the companion seed makes when it copies those + // columns across. A write in another language therefore has nowhere honest + // to go, and both ways of letting it through lose content: + // + // UPDATE overwrites. The row already holds the default language on main, + // so a non-default write replaces it and regenerates the slug from the + // translation, silently and with a success response. + // + // CREATE mis-files. The values land on main, and the seed then copies + // them into the default language's row — so Spanish text is served as + // English, and Spanish itself has no translation at all. + // + // The window is real: `db:sync` flips the registry's `localized` flag in + // its own process while the running server has yet to create the companion. + // Refuse either way; the default language still writes to main, which is + // the documented pre-migration fallback. + const requested = resolveRequestedLocale(this.localization, locale); + if (requested !== this.localization.defaultLocale) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this collection yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + collection: collectionName, + locale: requested, + defaultLocale: this.localization.defaultLocale, + companionTable: companion.companionTableName, + }, + }); + } + // The default language keeps the fallback, but only where it can actually + // work. A collection localized from creation keeps its translatable columns + // solely on the companion, and the generated main-table schema omits them, so + // returning null here would leave those values in the main payload for a table + // that has no columns for them. That write cannot land: it reaches the driver + // and fails as a 500. Refusing here says the same thing in terms the caller can + // act on, and says it before anything is attempted. + const fallbackPossible = await mainTableHasColumns( + this.adapter, + // The companion is always `
_locales`, so the main table is its stem. + companion.companionTableName.replace(/_locales$/, ""), + companion.localizedFields.map(f => f.column) + ); + if (!fallbackPossible) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this collection yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + collection: collectionName, + locale: requested, + companionTable: companion.companionTableName, + }, + }); + } return null; } @@ -2196,6 +2260,17 @@ export class CollectionMutationService extends BaseService { // collection opted out of recording), so the post-commit fast drain is // scheduled only for a write that recorded something. let recorded = false; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection and deadlock a single-connection pool, and a NextlyError raised in + // the callback is reclassified by the adapter into an opaque database error — + // so the actionable 409 would never reach the caller. + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + })) ?? new Map(); await this.adapter.transaction(async tx => { const rawEntry = await tx.insert(tableName, entryData, { returning: "*", @@ -2238,15 +2313,19 @@ export class CollectionMutationService extends BaseService { this.fieldGroupDataService && Object.keys(componentFieldData).length > 0 ) { - await this.fieldGroupDataService.saveComponentDataInTransaction(tx, { - parentId: entry.id as string, - parentTable: tableName, - fields: fields as unknown as FieldConfig[], - data: componentFieldData, - // i18n: thread the write locale so an embedded localized component writes - // translatable fields to its companion within the same transaction. - locale: params.locale, - }); + await this.fieldGroupDataService.saveComponentDataInTransaction( + tx, + { + parentId: entry.id as string, + parentTable: tableName, + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + // i18n: thread the write locale so an embedded localized component writes + // translatable fields to its companion within the same transaction. + locale: params.locale, + }, + fieldGroupPresence + ); } // Write many-to-many junction rows inside the transaction so a junction @@ -4198,6 +4277,17 @@ export class CollectionMutationService extends BaseService { // resolves, so a rolled-back attempt (a version conflict) or a commit // failure never flags a durable event that isn't there. let recorded = false; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection and deadlock a single-connection pool, and a NextlyError raised in + // the callback is reclassified by the adapter into an opaque database error — + // so the actionable 409 would never reach the caller. + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + })) ?? new Map(); await withVersionConflictRetry(() => this.adapter.transaction(async tx => { recorded = false; @@ -4521,7 +4611,8 @@ export class CollectionMutationService extends BaseService { // i18n: thread the write locale so an embedded localized component writes // translatable fields to its companion within the same transaction. locale: params.locale, - } + }, + fieldGroupPresence ); } diff --git a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts index b4781f936..ec81aa4b8 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts @@ -11,6 +11,7 @@ import { FieldGroupMutationService, type SaveComponentDataParams, type DeleteComponentDataParams, + type FieldGroupCompanionPresence, } from "./field-group-mutation-service"; import { FieldGroupQueryService, @@ -126,9 +127,32 @@ export class FieldGroupDataService { saveComponentDataInTransaction( tx: TransactionContext, - params: SaveComponentDataParams + params: SaveComponentDataParams, + presence: FieldGroupCompanionPresence ): Promise { - return this.mutationService.saveComponentDataInTransaction(tx, params); + return this.mutationService.saveComponentDataInTransaction( + tx, + params, + presence + ); + } + + /** + * Verify every localized field group in a payload can be written, BEFORE the caller opens its + * transaction, and return what it found out. See + * {@link FieldGroupMutationService.assertLocalizedFieldGroupsWritable} — this cannot run inside + * the transaction without risking pool starvation, and answering it first keeps a refusal + * exactly as raised. + * + * The returned map is not optional bookkeeping: it must be handed to + * `saveComponentDataInTransaction`, because that is the only way the in-transaction write can + * learn whether a companion exists without probing for it, which would abort the transaction + * on PostgreSQL when it does not. + */ + assertLocalizedFieldGroupsWritable( + params: Pick + ): Promise { + return this.mutationService.assertLocalizedFieldGroupsWritable(params); } deleteComponentData(params: DeleteComponentDataParams): Promise { diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index be12663e2..8bcc9ce6a 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -28,6 +28,8 @@ import type { SanitizedLocalizationConfig } from "../../i18n/config/types"; import { resolveRequestedLocale } from "../../i18n/resolve-locale"; import { buildCompanionSchema, + companionTableExists, + mainTableHasColumns, splitLocalizedWrite, upsertCompanionRow, } from "../../i18n/runtime/companion-io"; @@ -77,6 +79,38 @@ function isFieldGroupField(field: FieldConfig): field is FieldGroupFieldConfig { return field.type === STORAGE_FORMAT.fieldType; } +/** + * The component instances a dynamic-zone payload actually contains. + * + * A repeatable zone sends an array; a non-repeatable one — the supported default — sends a + * single object. The pre-transaction check and the write itself both need this answer, and + * mirroring the normalisation by hand in each is what let them drift: the check read a + * non-repeatable object as "no instances", so that slug never reached the presence map, and the + * write then went looking for a companion table from inside the transaction. That is precisely + * the failure the check exists to prevent. + * + * Deriving it once means the two cannot disagree about what a payload holds. + */ +function resolveZoneInstances( + field: FieldGroupFieldConfig, + value: unknown +): ComponentInstanceData[] { + const raw = field.repeatable ? value : [value]; + return Array.isArray(raw) ? (raw as ComponentInstanceData[]) : []; +} + +/** + * Whether each localized field group's companion table physically exists, keyed by slug and + * resolved BEFORE the write transaction opens. + * + * This answers a question that cannot be asked again once a transaction is open. Probing for + * a relation that does not exist raises an error, and PostgreSQL marks the entire transaction + * aborted the moment one does — so although the probe catches it and reports "absent", every + * following statement fails with `current transaction is aborted`. Carrying the pre-resolved + * answer in is what lets the in-transaction path know without asking. + */ +export type FieldGroupCompanionPresence = ReadonlyMap; + export class FieldGroupMutationService extends BaseService { private readonly registryService: FieldGroupRegistryService; @@ -99,16 +133,38 @@ export class FieldGroupMutationService extends BaseService { * The caller writes `main` to the instance row and, after it has the instance id, upserts * the companion via {@link upsertLocalizedComponent}. */ - private splitLocalizedComponent( + private async splitLocalizedComponent( meta: DynamicFieldGroupRecord, - data: Record - ): { + data: Record, + locale: string | undefined, + // Present only on the in-transaction path. Both members travel together by design: + // inside a transaction the companion's existence must ALREADY be known, because asking + // again means probing a possibly-absent relation, and on PostgreSQL that aborts the + // whole transaction. Omitted on the pooled path, which is free to probe. + tx?: { + adapter: { + dialect: SupportedDialect; + executeQuery( + sql: string, + params?: unknown[] + ): Promise; + }; + presence: FieldGroupCompanionPresence; + } + ): Promise<{ schema: ReturnType; main: Record; companion: Record; - } { + /** Whether the companion physically exists, so callers can reuse it without re-probing. */ + companionExists: boolean; + }> { if (!this.localization || meta.localized !== true) { - return { schema: null, main: data, companion: {} }; + return { + schema: null, + main: data, + companion: {}, + companionExists: false, + }; } const schema = buildCompanionSchema({ slug: meta.slug, @@ -117,7 +173,80 @@ export class FieldGroupMutationService extends BaseService { dialect: this.adapter.dialect, status: false, }); - if (!schema) return { schema: null, main: data, companion: {} }; + if (!schema) + return { + schema: null, + main: data, + companion: {}, + companionExists: false, + }; + // Resolve existence BEFORE splitting. Splitting first and then discovering the + // companion is absent strands the translatable values: they leave the main payload + // and the upsert that would have taken them is skipped, so the write reports success + // having saved nothing. Resolving here also means any refusal is raised before the + // caller opens its transaction, so it leaves exactly as raised rather than through + // the adapter's error classification, which rewraps anything that is not already a + // `DatabaseError`. + // + // Inside a transaction the answer is READ, never probed. A probe against a missing + // relation aborts the entire transaction on PostgreSQL — the error is caught here but + // the connection is already poisoned, so the fallback write that follows would die with + // `current transaction is aborted`. + // + // `resolveZoneInstances` makes the map complete by construction, so a miss means the two + // paths have drifted again. It defaults to "absent" rather than "provisioned" because the + // two are not equally safe to guess wrong: "absent" takes the fallback, which writes to the + // main table and fails loudly there if the columns are gone, whereas "provisioned" splits + // the values out and upserts into a table that may not exist — reintroducing the very abort + // this parameter was added to prevent. + const companionExists = tx + ? (tx.presence.get(meta.slug) ?? false) + : await companionTableExists(this.adapter, schema.companionTableName); + if (!companionExists) { + const writeLocale = resolveRequestedLocale(this.localization, locale); + if (writeLocale !== this.localization.defaultLocale) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + fieldGroupTable: schema.companionTableName, + locale: writeLocale, + }, + }); + } + // Default language keeps the pre-companion fallback — but only where it can + // actually work. A field group whose main `comp_*` table never had these + // columns (localized from creation, or already migrated) would otherwise take + // this branch and hand the values to a table with nowhere to put them, which + // fails at the driver as a 500. Refusing turns that into an answer the caller + // can act on. + // Only introspect on the pooled path. Inside a parent transaction this would + // borrow a second connection and deadlock a single-connection pool — and it does + // not need to, because `assertLocalizedFieldGroupsWritable` has already answered + // the same question before that transaction opened. + const fallbackPossible = + tx !== undefined || + (await mainTableHasColumns( + this.adapter, + meta.tableName, + schema.localizedFields.map(f => f.column) + )); + if (!fallbackPossible) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + fieldGroupTable: schema.companionTableName, + locale: writeLocale, + }, + }); + } + return { schema: null, main: data, companion: {}, companionExists }; + } const { main, companion } = splitLocalizedWrite( data, schema.localizedFields @@ -129,6 +258,7 @@ export class FieldGroupMutationService extends BaseService { schema, main, companion: this.serializeCompanionValues(companion, schema, meta.fields), + companionExists, }; } @@ -262,9 +392,76 @@ export class FieldGroupMutationService extends BaseService { } } + /** + * Answer, BEFORE the caller opens its transaction, whether every localized field group in + * this payload can actually be written. + * + * Two reasons it cannot wait until the write itself. The probes borrow a connection from the + * pool, so running them inside the parent transaction waits for a connection that cannot be + * released until that transaction finishes; and answering here keeps a refusal exactly as + * raised, rather than passing it through the adapter's error classification on the way out of + * a transaction callback, which rewraps anything that is not already a `DatabaseError`. + * + * Idempotent and read-only — it introspects and probes, and writes nothing. + */ + async assertLocalizedFieldGroupsWritable( + params: Pick + ): Promise { + const presence = new Map(); + if (!this.localization) return presence; + for (const field of params.fields) { + if (!isFieldGroupField(field)) continue; + const value = params.data[field.name]; + if (value === undefined || value === null) continue; + // Mirror `saveComponentData`'s dispatch exactly, including its precedence: a field + // carrying both `components` and `component` is written as a dynamic zone, so + // deciding `component` first here would check a type the write never touches. + // + // For a dynamic zone the type travels per instance, so the PAYLOAD decides what is + // written, not the field's list of permitted types. Walking the permitted list + // instead would probe types absent from this write — and would refuse a perfectly + // good save whenever some other permitted type happened to be missing its + // companion. Deduplicated, because a zone commonly repeats one type. + const slugs = new Set(); + if (field.components && field.components.length > 0) { + for (const instance of resolveZoneInstances(field, value)) { + const type = (instance as Record | null)?.[ + STORAGE_FORMAT.wireTypeKey + ]; + if (typeof type === "string" && field.components.includes(type)) { + slugs.add(type); + } + } + } else if (field.component) { + slugs.add(field.component); + } + for (const slug of slugs) { + if (presence.has(slug)) continue; + const meta = await this.registryService.getComponentBySlug(slug); + if (!meta || meta.localized !== true) continue; + // Reuse the same split the write performs: it raises the 409 when the + // companion is missing and the fallback is unavailable, which is exactly the + // decision needed here — and on the pooled adapter, outside any transaction. + // Its verdict on existence is then carried into the transaction, so the write + // never has to ask a question that would abort the transaction to answer. + const { companionExists } = await this.splitLocalizedComponent( + meta, + {}, + params.locale + ); + presence.set(slug, companionExists); + } + } + return presence; + } + async saveComponentDataInTransaction( tx: TransactionContext, - params: SaveComponentDataParams + params: SaveComponentDataParams, + // Resolved by `assertLocalizedFieldGroupsWritable` before this transaction opened. + // Required rather than optional: inside a transaction there is no safe way to work it + // out, since probing a missing companion aborts the transaction on PostgreSQL. + presence: FieldGroupCompanionPresence ): Promise { const { parentId, parentTable, fields, data, locale } = params; @@ -295,6 +492,7 @@ export class FieldGroupMutationService extends BaseService { field, data: fieldData, locale, + presence, }); } else if (field.component) { if (field.repeatable) { @@ -305,6 +503,7 @@ export class FieldGroupMutationService extends BaseService { componentSlug: field.component, data: fieldData, locale, + presence, }); } else { await this.saveSingleComponentInTx(tx, { @@ -314,6 +513,7 @@ export class FieldGroupMutationService extends BaseService { componentSlug: field.component, data: fieldData as ComponentInstanceData, locale, + presence, }); } } @@ -392,9 +592,10 @@ export class FieldGroupMutationService extends BaseService { // i18n: split translatable values out of the main comp_ write — they live on the // companion. `main === data` when the component isn't localized (unchanged path). - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - data + data, + locale ); let instanceId: string; @@ -465,10 +666,18 @@ export class FieldGroupMutationService extends BaseService { componentSlug: string; data: ComponentInstanceData; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, componentSlug, data, locale } = - params; + const { + parentId, + parentTable, + fieldName, + componentSlug, + data, + locale, + presence, + } = params; try { const componentMeta = @@ -494,9 +703,14 @@ export class FieldGroupMutationService extends BaseService { ); // i18n: split translatable values out of the main comp_ write (companion-owned). - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - data + data, + locale, + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); let instanceId: string; @@ -585,9 +799,10 @@ export class FieldGroupMutationService extends BaseService { // i18n: split translatable values out per instance (companion-owned). The // diff-by-id update keeps the instance id stable, so companion rows for OTHER // locales survive a re-save in one locale. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - instance + instance, + locale ); await this.prepareInstanceForWrite( @@ -665,10 +880,18 @@ export class FieldGroupMutationService extends BaseService { componentSlug: string; data: unknown; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, componentSlug, data, locale } = - params; + const { + parentId, + parentTable, + fieldName, + componentSlug, + data, + locale, + presence, + } = params; if (!Array.isArray(data)) { this.logger.warn("Repeatable component data is not an array", { @@ -699,9 +922,14 @@ export class FieldGroupMutationService extends BaseService { const instance = instances[i]; const instanceId = instance.id; // i18n: split translatable values out (companion-owned) per instance. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - instance + instance, + locale, + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); await this.prepareInstanceForWrite( @@ -784,11 +1012,11 @@ export class FieldGroupMutationService extends BaseService { const { parentId, parentTable, fieldName, field, data, locale } = params; const allowedSlugs = field.components ?? []; - const instances = field.repeatable - ? (data as ComponentInstanceData[]) - : [data as ComponentInstanceData]; + // Shared with the pre-transaction check, so the two cannot disagree about which + // instances this payload holds. + const instances = resolveZoneInstances(field, data); - if (!Array.isArray(instances)) { + if (instances.length === 0 && data !== null && data !== undefined) { this.logger.warn("Multi-component data is not an array", { fieldName }); return; } @@ -866,9 +1094,10 @@ export class FieldGroupMutationService extends BaseService { const componentFields = meta.fields; const instanceId = instance.id; // i18n: split translatable values out per instance using its own component meta. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( meta, - instance + instance, + locale ); await this.prepareInstanceForWrite( @@ -951,16 +1180,18 @@ export class FieldGroupMutationService extends BaseService { field: FieldGroupFieldConfig; data: unknown; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, field, data, locale } = params; + const { parentId, parentTable, fieldName, field, data, locale, presence } = + params; const allowedSlugs = field.components ?? []; - const instances = field.repeatable - ? (data as ComponentInstanceData[]) - : [data as ComponentInstanceData]; + // Shared with the pre-transaction check, so the two cannot disagree about which + // instances this payload holds. + const instances = resolveZoneInstances(field, data); - if (!Array.isArray(instances)) { + if (instances.length === 0 && data !== null && data !== undefined) { this.logger.warn("Multi-component data is not an array", { fieldName }); return; } @@ -1020,9 +1251,14 @@ export class FieldGroupMutationService extends BaseService { const componentFields = meta.fields; const instanceId = instance.id; // i18n: split translatable values out per instance using its own component meta. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( meta, - instance + instance, + locale, + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); await this.prepareInstanceForWrite( diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts new file mode 100644 index 000000000..02764f179 --- /dev/null +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -0,0 +1,488 @@ +/** + * A localized write with no companion table must not destroy content. + * + * The dangerous state is a TRANSITION: a collection created WITHOUT localization + * keeps its translatable columns on the main table, and enabling localization + * later moves them to `dc__locales`. `nextly db:sync` runs in its own + * process — it flips the registry's `localized` flag but historically left + * companion creation to the next boot — so a running server can believe the + * collection is localized, render the full locale switcher, and still have no + * companion table. + * + * Before this guard, a write in a NON-default locale fell through to the main + * table in that window, overwriting the default language's values and + * regenerating the slug from the translation, while reporting success. + * Reproduced on published 0.0.2-alpha.43. + * + * The default locale legitimately writes to the main table until the companion + * exists ("Option B"), so that path must keep working unchanged. + * + * File-backed SQLite: `createTestNextly` disconnects the adapter as it boots, so + * an in-memory database would not survive the second boot that performs the + * transition. + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + defineCollection, + defineFieldGroup, + fieldGroup, + text, +} from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { + createTestNextly, + getConfiguredTestDialects, + type TestNextly, +} from "../../../plugins/test-nextly"; + +import type { CollectionsHandler } from "../../../services/collections-handler"; + +let dir: string; +let dbPath: string; +let current: TestNextly | undefined; +// The harness snapshots DB_DIALECT when IT creates an adapter. These tests build +// their own first, so the snapshot captures the already-overwritten "sqlite" and +// never puts the real dialect back — which, in a single-fork run, would make every +// later file resolve environment-backed schema behaviour as SQLite. +let previousDialect: string | undefined; + +beforeEach(() => { + previousDialect = process.env.DB_DIALECT; + dir = mkdtempSync(join(tmpdir(), "nextly-i18n-window-")); + dbPath = join(dir, "test.db"); +}); + +afterEach(async () => { + await current?.destroy(); + current = undefined; + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; + rmSync(dir, { recursive: true, force: true }); +}); + +const localization = { locales: ["en", "es"], defaultLocale: "en" }; + +/** Same collection, with localization off (columns on main) or on (companion). */ +const posts = (localized: boolean) => + defineCollection({ + slug: "i18nwin_posts", + localized, + fields: [text({ name: "title", localized: true })], + }); + +/** + * Same shape plus a genuinely SHARED field. `localized: false` is load-bearing: in a + * localized collection a text field localizes by default (`defaultLocalizedForType`), so + * without the explicit flag `author` would live on the companion too and the main table + * would have no column at all. + * + * That distinction is the whole point of this fixture. With a real shared column the + * UPDATE still has something valid to write, which is the shape that can commit a partial + * row rather than failing outright. + */ +const mixedPosts = (localized: boolean) => + defineCollection({ + slug: "i18nwin_mixed", + localized, + fields: [ + text({ name: "title", localized: true }), + text({ name: "author", localized: false }), + ], + }); + +async function boot(localized: boolean): Promise { + process.env.DB_DIALECT = "sqlite"; + const adapter = await createAdapter({ + type: "sqlite", + url: `file:${dbPath}`, + } as Parameters[0]); + return createTestNextly({ + adapter, + collections: [posts(localized)], + localization, + }); +} + +/** + * Read the PHYSICAL main-table title. Once the collection is localized the + * registered runtime schema omits translatable columns, so `adapter.select` + * would not return the column that the bug overwrites — the raw read is the + * only thing that shows what actually landed on disk. + */ +async function physicalTitle(handle: TestNextly): Promise { + const rows = await handle.adapter.executeQuery<{ title: string }>( + "SELECT title FROM dc_i18nwin_posts LIMIT 1" + ); + return rows[0]?.title; +} + +/** Re-open in the localized state, then remove the companion to recreate the window. */ +async function enterWindow(): Promise { + await current?.destroy(); + current = undefined; + const handle = await boot(true); + await handle.adapter.executeQuery( + "DROP TABLE IF EXISTS dc_i18nwin_posts_locales" + ); + return handle; +} + +describe("localized write without a companion table (integration)", () => { + it("refuses a non-default-locale write instead of overwriting the default", async () => { + // Boot 1: not localized, so `title` lives on the main table. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "i18nwin_posts", overrideAccess: true }, + { title: "How to Build a Blog" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + current = await enterWindow(); + + const translated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + locale: "es", + }, + { title: "Cómo crear un blog" } + ); + + // The write must not report success, and it must be OUR refusal rather than + // a driver error escaping — a raw "no such table" would also be a failure + // here while meaning the guard never ran. + expect(translated.success).toBe(false); + expect(translated.statusCode).toBe(409); + expect(translated.message).toMatch(/Translations are not ready/); + + // ...and the default-language content must be untouched. This is the + // assertion that fails without the guard: `es` landed on the main row. + expect(await physicalTitle(current)).toBe("How to Build a Blog"); + }); + + it("still writes the default locale to the main table when no companion exists", async () => { + // "Option B" preserved: before the companion is created the default language + // legitimately lives on the main table, so this path must be unchanged. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "i18nwin_posts", overrideAccess: true }, + { title: "First" } + ); + const id = (created.data as { id: string }).id; + + current = await enterWindow(); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "First (edited)" } + ); + + expect(updated.success).toBe(true); + expect(await physicalTitle(current)).toBe("First (edited)"); + }); +}); + +/** + * The guard has to hold on every dialect, and the SQLite cases above cannot show + * that. What decides whether it fires is `companionTableExists`, which probes + * with a dialect-quoted `SELECT 1` and swallows the failure — and on Postgres a + * statement that errors inside an open transaction poisons the rest of it, so + * "does a refusal come back at all, and is it ours" is a question only a real + * server answers. + * + * One boot is enough here, so the harness can provision a throwaway database per + * dialect: dropping the companion after the entry exists reproduces the same + * missing-table state the db:sync transition creates. The collection is localized + * from the start, so the translatable value never sat on the main table and there + * is nothing to overwrite — without the guard these return an opaque 500 from the + * driver rather than losing content. Proving the DEFAULT language's content + * survives still needs the two-boot SQLite case above, which is the only one that + * starts with the value on the main table. + */ +describe.each(getConfiguredTestDialects())( + "localized write without a companion table on %s (integration)", + dialect => { + it("refuses a non-default-locale update once the companion is gone", async () => { + current = await createTestNextly({ + dialect, + collections: [posts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_posts", + overrideAccess: true, + locale: "en", + }, + { title: "Original" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_posts_locales`" : '"dc_i18nwin_posts_locales"'}` + ); + + const translated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + locale: "es", + }, + { title: "Traducción" } + ); + + expect(translated.success).toBe(false); + expect(translated.statusCode).toBe(409); + expect(translated.message).toMatch(/Translations are not ready/); + }); + + it("refuses a default-locale update when the main table never had the column", async () => { + // The other half of the same window. The default language keeps the pre-companion + // fallback, so the split hands its values back to the main payload — but a + // collection localized from creation never had those columns on the main table. + // Without the guard the write reaches the driver and dies there: measured at 409 + // with it, 500 without, on every dialect. The content is not lost, but the caller + // is told nothing it can act on. + current = await createTestNextly({ + dialect, + collections: [posts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_posts", + overrideAccess: true, + locale: "en", + }, + { title: "Original" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_posts_locales`" : '"dc_i18nwin_posts_locales"'}` + ); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "Edited" } + ); + + expect(updated.success).toBe(false); + expect(updated.statusCode).toBe(409); + expect(updated.message).toMatch(/Translations are not ready/); + }); + + it("does not half-apply the write when a shared field accompanies the translation", async () => { + // A real shared column alongside the translatable one is the shape most likely to + // commit a partial write, because `author` on its own would make a perfectly valid + // UPDATE. Measured: it still does not — the statement carries `title` too, the main + // table has no such column, and the whole statement fails as a 500. So the guard's + // contribution here is an actionable 409 in place of a driver error, not the + // prevention of a silent half-write. + // + // The `author` assertion holds that line: were the write ever to become partial, + // keeping the shared half and dropping the translation, this is what would catch it. + current = await createTestNextly({ + dialect, + collections: [mixedPosts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_mixed", + overrideAccess: true, + locale: "en", + }, + { title: "Original", author: "Ada" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_mixed_locales`" : '"dc_i18nwin_mixed_locales"'}` + ); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_mixed", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "Edited", author: "Grace" } + ); + + expect(updated.success).toBe(false); + expect(updated.statusCode).toBe(409); + + // The refusal has to be total. A half-applied write that keeps the shared field and + // drops the translation is exactly the loss this guard exists to prevent. + const rows = await current.adapter.executeQuery<{ author: string }>( + `SELECT author FROM ${dialect === "mysql" ? "`dc_i18nwin_mixed`" : '"dc_i18nwin_mixed"'}` + ); + expect(rows[0]?.author).toBe("Ada"); + }); + } +); + +/** + * A dynamic zone permits several field-group types, but a write only touches the ones its + * payload actually contains. The pre-transaction guard has to scope itself the same way. + * + * Scoping it to the PERMITTED list instead refuses a perfectly good save whenever any other + * allowed type is missing its companion — a type this write was never going to touch. The + * parent collection here is NOT localized, which is also the case that reaches this guard + * without passing through the collection or single ones. + */ +/** + * A dynamic zone defaults to `repeatable: false`, and then its payload is a single OBJECT rather + * than an array. The pre-transaction check and the write must agree about that, or the check + * silently covers nothing: reading the object as "no instances" leaves the slug out of the + * presence map, and the write goes looking for the companion table from inside the transaction — + * which is the failure the check exists to prevent, and on PostgreSQL aborts the transaction. + * + * The refusal must therefore still be the actionable 409, not a driver error. + */ +describe.each(getConfiguredTestDialects())( + "non-repeatable dynamic zone with no companion on %s (integration)", + dialect => { + it("refuses with a 409 rather than a database failure", async () => { + current = await createTestNextly({ + dialect, + fieldGroups: [ + defineFieldGroup({ + slug: "znsingle", + localized: true, + fields: [text({ name: "heading", localized: true })], + }), + ], + collections: [ + defineCollection({ + slug: "i18nwin_single", + fields: [ + text({ name: "title" }), + // No `repeatable`, so the payload below is one object, not an array. + fieldGroup({ name: "hero", components: ["znsingle"] }), + ], + }), + ], + localization, + }); + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`comp_znsingle_locales`" : '"comp_znsingle_locales"'}` + ); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_single", + overrideAccess: true, + locale: "es", + }, + { + title: "Page", + hero: { _componentType: "znsingle", heading: "Hola" }, + } + ); + + expect(created.success).toBe(false); + expect(created.statusCode).toBe(409); + expect(created.message).toMatch(/Translations are not ready/); + }); + } +); + +describe("dynamic zone whose unused field group has no companion (integration)", () => { + it("saves a block type whose companion exists while another permitted type's is missing", async () => { + current = await createTestNextly({ + fieldGroups: [ + defineFieldGroup({ + slug: "znok", + localized: true, + fields: [text({ name: "heading", localized: true })], + }), + defineFieldGroup({ + slug: "znbroken", + localized: true, + fields: [text({ name: "caption", localized: true })], + }), + ], + collections: [ + defineCollection({ + slug: "i18nwin_zone", + fields: [ + text({ name: "title" }), + fieldGroup({ + name: "layout", + components: ["znok", "znbroken"], + repeatable: true, + }), + ], + }), + ], + localization, + }); + + // Only the type this write does NOT use loses its companion. + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS comp_znbroken_locales" + ); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "i18nwin_zone", overrideAccess: true, locale: "en" }, + { + title: "Page", + layout: [{ _componentType: "znok", heading: "Hello" }], + } + ); + + expect(created.success).toBe(true); + }); +}); diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index b2f7e2152..4f24a6614 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -176,6 +176,199 @@ export async function upsertCompanionRow( ); } +/** + * Adapter surface for asking about a table's PHYSICAL shape: the Drizzle handle, which the + * shared introspection helper needs. Separate from {@link CompanionWriteAdapter} so the + * read/write helpers, which never introspect, keep their narrower contract. + */ +export interface CompanionIntrospectAdapter extends CompanionWriteAdapter { + getDrizzle(): T; +} + +/** + * Add localized columns an EXISTING companion is missing. No-op when the companion is absent — + * creating it is {@link ensureCompanionTable}'s job. + * + * Needed because a companion is created once and then never revisited, while the entity's field + * list keeps moving. Marking a further field localized on an already-localized entity leaves the + * companion a column short, and the write splits that value straight into a column that is not + * there. + * + * **Additive only, deliberately.** A field that stops being localized leaves its companion column + * in place rather than dropping it: this runs unattended, and `db:sync` persists registry metadata + * BEFORE its destructive prompt, so a drop here would execute even for an operator who then + * declined the change. An unused column is recoverable; a dropped one is not. + * + * Issues DDL, so it belongs to `db:sync` and never to boot — a running deployment must not alter + * its own schema because a config file changed. + */ +export async function reconcileCompanionColumns( + adapter: CompanionIntrospectAdapter, + args: { + slug: string; + tableName: string; + fields: CompanionFieldLike[]; + dialect: SupportedDialect; + status?: boolean; + }, + onError?: (error: unknown) => void +): Promise { + const companionTableName = `${args.tableName}_locales`; + // Hoisted so the concurrency recheck in the catch can reuse it. + const localizedNames = new Set(resolveLocalizedFieldNames(args.fields, true)); + const desired = args.fields.filter(f => localizedNames.has(f.name)); + try { + if (!(await companionTableExists(adapter, companionTableName))) return; + if (desired.length === 0) return; + + const { introspectLiveSnapshot } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const snapshot = await introspectLiveSnapshot( + adapter.getDrizzle(), + adapter.dialect, + [companionTableName] + ); + const present = new Set( + snapshot.tables + .find(t => t.name === companionTableName) + ?.columns.map(c => c.name) ?? [] + ); + // `_status` is deliberately NOT reconciled here. Adding the column is one statement and + // backfilling the default-locale row from the main row is another, and the pair cannot be + // made retryable from physical shape alone: when the ADD lands and the backfill does not, + // every later run sees the column present, concludes the companion is in step, and leaves + // previously published content reading as draft while reporting success. MySQL commits DDL + // implicitly, so the two cannot be made atomic there either. + // + // Deciding it correctly needs a record of whether the backfill has run — the localization + // transition state this codebase does not keep yet. Until it does, switching Draft/Published + // on for an already-localized entity belongs to the migration path, which fails loudly on a + // missing column rather than silently hiding published rows. + // + // The localized-column reconcile below has no such weakness: a missing column is visible on + // every run, so a partial apply simply finishes on the next one. + const hasStatus = present.has("_status"); + + // Draft/Published was switched on after this companion was created, so `_status` is now + // required and absent. Reconciling it is unsafe for the reasons above — but returning + // quietly is worse than either: the caller persists `status: true`, reports success, and + // every later per-locale status read or write hits a column that is not there. Report it + // so `db:sync` exits non-zero and the operator learns now rather than at the next publish. + // + // Only this direction is a problem. Status switched OFF while the column remains is + // harmless: the column is simply unused, and the additive policy keeps it. + if (args.status === true && !hasStatus) { + onError?.( + new Error( + `The translations table ${companionTableName} predates Draft/Published being enabled ` + + `for "${args.slug}", so it has no _status column. Run \`nextly migrate\` to add it: ` + + `an unattended sync cannot, because adding the column and back-filling the ` + + `default locale's status cannot be retried safely if only the first half lands.` + ) + ); + return; + } + + // Nothing missing — the overwhelmingly common case, and worth leaving before the statement + // builder runs. + if (desired.every(f => present.has(toColumn(f.name)))) return; + + // Feed the canonical builder the columns that already exist as `oldLocalized`, so what it + // emits is exactly the difference. `old` is a subset of `new` by construction here, which + // is what keeps the result additive. + const { buildCompanionReconcileStatements } = await import( + "../migration/reconcile-companion" + ); + const statements = buildCompanionReconcileStatements({ + slug: args.slug, + tableName: args.tableName, + oldLocalized: desired.filter(f => present.has(toColumn(f.name))), + newLocalized: desired, + dialect: args.dialect, + // Report the companion's ACTUAL status shape on both sides, so the builder sees no status + // change and emits only the column difference — never an ADD or DROP of `_status`. + status: hasStatus, + companionHasStatus: hasStatus, + companionExists: true, + }); + for (const stmt of statements) { + await adapter.executeQuery(stmt); + } + } catch (error) { + // Same check-then-act window as the create path: a concurrent sync or reload may have added + // the very columns this run was adding, and `ADD COLUMN` is not idempotent. Re-introspect + // and treat "the shape we wanted is now present" as success, whoever produced it. + try { + const { introspectLiveSnapshot: reread } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const after = await reread(adapter.getDrizzle(), adapter.dialect, [ + companionTableName, + ]); + const now = new Set( + after.tables + .find(t => t.name === companionTableName) + ?.columns.map(c => c.name) ?? [] + ); + if (desired.every(f => now.has(toColumn(f.name)))) return; + } catch { + // Fall through to reporting the original error: if we cannot even re-read the table, + // the reconcile genuinely did not succeed. + } + onError?.(error); + } +} + +/** + * Whether the main table still physically carries ALL of `columnNames`. + * + * This answers one question for every entity type: **can the pre-companion fallback actually + * persist anything?** While the companion is missing, a write in the default language is meant + * to stay on the main table — but that is only true for an entity whose columns are still + * there. An entity localized from creation (or one whose migration has run) keeps them only on + * the companion, and its registered runtime table omits them, so the write carries keys the + * table has no columns for. Measured on all three dialects, that surfaces as a driver error + * and a 500 rather than a wrong value — the write does not quietly commit. Answering the + * question up front turns that opaque failure into a refusal the caller can act on. + * + * Goes through the same introspection the schema pipeline uses rather than a `SELECT ... LIMIT 0` + * probe. That matters beyond convention: a probe cannot tell "this column does not exist" from + * "the database is unreachable", so a transient failure would read as a missing column and + * produce a misleading "translations are not ready" refusal instead of the real error. + * Introspection fails loudly, and this deliberately does not catch. + * + * MUST be called before the caller opens its transaction. It borrows a connection from the pool, + * so running it inside one waits for a connection that cannot be released until that transaction + * finishes — starvation on a small pool. Resolving it first also keeps a refusal exactly as + * raised: errors leaving a transaction callback pass through the adapter's error classification, + * which rewraps anything that is not already a `DatabaseError`. + */ +export async function mainTableHasColumns( + adapter: CompanionIntrospectAdapter, + tableName: string, + columnNames: readonly (string | undefined)[] +): Promise { + const wanted = columnNames.filter((c): c is string => Boolean(c)); + if (wanted.length === 0) return false; + const { introspectLiveSnapshot } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const snapshot = await introspectLiveSnapshot( + adapter.getDrizzle(), + adapter.dialect, + [tableName] + ); + const table = snapshot.tables.find(t => t.name === tableName); + if (!table) return false; + const present = new Set(table.columns.map(c => c.name)); + // EVERY column, not just one. A partially migrated main table — an older field keeping its + // legacy column while a newer localized field never had one — would otherwise pass on the + // first column and then fail at the driver on a later one, which is the opaque 500 this + // check exists to replace with an actionable refusal. + return wanted.every(c => present.has(c)); +} + // Whether a probe error is a verified "this TABLE does not exist" for the // current dialect, as opposed to a transient/connection/permission error or a // different missing resource (a missing DATABASE, schema, column, or role). The @@ -243,11 +436,15 @@ export async function companionHasStatusColumn( /** * Boot/db:sync helper: physically create the companion `_locales` table if it does - * not already exist. Idempotent and safe to run on every boot — a no-op once the table exists (or - * when the entity has no translatable fields). This is the db:sync/dev-boot counterpart to the - * migration-owned companion creation (`nextly migrate`), so a code-first localized collection / - * single / component gets a working companion without a manual migrate step. Best-effort: a - * failure (e.g. main table not yet created) is swallowed so it retries on the next boot. + * not already exist. CREATION ONLY — the table is left empty, so an entity that already holds + * content on its main table will read null for every localized field until that content is + * copied across; a successful return is not evidence that it was. Idempotent and safe to run on + * every boot — a no-op once the table exists (or when the entity has no translatable fields). + * This is the + * db:sync/dev-boot counterpart to the migration-owned companion creation (`nextly migrate`), so a + * code-first localized collection / single / component gets a working companion without a manual + * migrate step. Best-effort: a failure (e.g. main table not yet created) is swallowed so it + * retries on the next boot. */ export async function ensureCompanionTable( adapter: CompanionWriteAdapter, @@ -257,7 +454,12 @@ export async function ensureCompanionTable( fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; - } + }, + /** + * Notified when creation fails. Optional so existing callers are unchanged; + * without it the previous swallow-and-retry-next-boot behaviour is preserved. + */ + onError?: (error: unknown) => void ): Promise { const companionTableName = `${args.tableName}_locales`; try { @@ -281,8 +483,25 @@ export async function ensureCompanionTable( for (const stmt of statements) { await adapter.executeQuery(stmt); } - } catch { - // Best-effort: main table may not exist yet on a very first boot — the companion - // will be created on the next boot (or by `nextly migrate`). + } catch (error) { + // Another process may have created it between the probe and the CREATE — `db:sync` and a + // dev boot/HMR reload provision the same companions, and `CREATE TABLE` is not idempotent + // here. Losing that race is a success: the table the caller wanted now exists. Confirmed by + // re-checking rather than by reading the error text, so this cannot swallow a real failure + // that happens to mention the table. + if ( + await companionTableExists(adapter, companionTableName).catch(() => false) + ) { + return; + } + // Best-effort: the main table may not exist yet on a very first boot, where the + // companion is created on the next boot (or by `nextly migrate`). That case is + // expected and self-healing. Anything else is NOT — a persistent failure here + // leaves the entity marked localized with no place to store translations, and + // swallowing it silently is how that state went unnoticed. Report it through + // the optional reporter so a caller (db:sync, boot) can surface it; the + // function still resolves, because refusing to boot over a companion is worse + // than booting with non-default-locale writes refused. + onError?.(error); } } diff --git a/packages/nextly/src/domains/i18n/writes-create.integration.test.ts b/packages/nextly/src/domains/i18n/writes-create.integration.test.ts index 629ede016..7f8a8d69f 100644 --- a/packages/nextly/src/domains/i18n/writes-create.integration.test.ts +++ b/packages/nextly/src/domains/i18n/writes-create.integration.test.ts @@ -132,7 +132,7 @@ describe("createEntry — localized write routing (M5a)", () => { expect(rows).toEqual([{ _locale: "en", heading: "Hello" }]); }); - it("dev (no companion table): localized value stays on the main table, write succeeds", async () => { + it("dev (no companion table): the DEFAULT language stays on the main table, write succeeds", async () => { const t = await boot(); const handler = handlerOf(t); const adapter = t.adapter as unknown as { @@ -146,13 +146,38 @@ describe("createEntry — localized write routing (M5a)", () => { await adapter.executeQuery( 'ALTER TABLE "dc_pages" ADD COLUMN "heading" text' ); + const res = await handler.createEntry( + { collectionName: "pages", locale: "en", overrideAccess: true }, + { title: "T", heading: "Hello" } + ); + expect(res.success).toBe(true); + const rows = await adapter.executeQuery('SELECT "heading" FROM "dc_pages"'); + expect(rows).toEqual([{ heading: "Hello" }]); + }); + + it("dev (no companion table): a NON-default language is refused", async () => { + const t = await boot(); + const handler = handlerOf(t); + const adapter = t.adapter as unknown as { + executeQuery: (sql: string) => Promise[]>; + }; + await adapter.executeQuery('DROP TABLE IF EXISTS "dc_pages_locales"'); + await adapter.executeQuery( + 'ALTER TABLE "dc_pages" ADD COLUMN "heading" text' + ); + // The main table has no language of its own: whatever sits there is read as the + // DEFAULT language, and that is what the companion seed copies across when the + // table appears. Letting `de` land on main would publish German text as English + // and leave German itself with no translation, so the write is refused instead. const res = await handler.createEntry( { collectionName: "pages", locale: "de", overrideAccess: true }, { title: "T", heading: "Hallo" } ); - expect(res.success).toBe(true); + expect(res.success).toBe(false); + expect(res.statusCode).toBe(409); + expect(res.message).toMatch(/Translations are not ready/); const rows = await adapter.executeQuery('SELECT "heading" FROM "dc_pages"'); - expect(rows).toEqual([{ heading: "Hallo" }]); + expect(rows).toEqual([]); }); }); diff --git a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts new file mode 100644 index 000000000..e32ef75c5 --- /dev/null +++ b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts @@ -0,0 +1,167 @@ +/** + * A localized Single whose companion table is missing must still persist a write in the + * DEFAULT language. + * + * While the companion is absent, translatable values belong on the main table — the same + * pre-migration fallback collections use. But a localized Single's registered runtime table is + * generated WITHOUT its translatable columns (`runtime-schema-generator` skips them as + * companion-owned), so keeping those values in the main payload writes them through a schema + * that does not declare them. If the ORM drops such keys, the update reports success while + * saving nothing, and the submitted content is gone with no error anywhere. + * + * The physical read is the point: the runtime schema omits the column, so an API read cannot + * distinguish "saved" from "silently dropped". + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { defineSingle, text } from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { + createTestNextly, + getConfiguredTestDialects, + type TestNextly, +} from "../../../plugins/test-nextly"; + +import type { SingleEntryService } from "../services/single-entry-service"; + +let dir: string; +let dbPath: string; +let current: TestNextly | undefined; +// The harness snapshots DB_DIALECT when IT creates an adapter. These tests build +// their own first, so the snapshot captures the already-overwritten "sqlite" and +// never puts the real dialect back — which, in a single-fork run, would make every +// later file resolve environment-backed schema behaviour as SQLite. +let previousDialect: string | undefined; + +beforeEach(() => { + previousDialect = process.env.DB_DIALECT; + dir = mkdtempSync(join(tmpdir(), "nextly-single-window-")); + dbPath = join(dir, "test.db"); +}); + +afterEach(async () => { + await current?.destroy(); + current = undefined; + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; + rmSync(dir, { recursive: true, force: true }); +}); + +const localization = { locales: ["en", "es"], defaultLocale: "en" }; + +const settings = (localized: boolean) => + defineSingle({ + slug: "swin_settings", + localized, + fields: [text({ name: "headline", localized: true })], + }); + +async function boot(localized: boolean): Promise { + process.env.DB_DIALECT = "sqlite"; + const adapter = await createAdapter({ + type: "sqlite", + url: `file:${dbPath}`, + } as Parameters[0]); + return createTestNextly({ + adapter, + singles: [settings(localized)], + localization, + }); +} + +describe("localized single without a companion table (integration)", () => { + it("persists a default-language write to the main table", async () => { + // Boot 1: not localized, so `headline` is an ordinary main-table column. + current = await boot(false); + await current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Original" }, + } as Parameters[0]); + + // Re-open localized, then remove the companion to recreate the window. + await current.destroy(); + current = await boot(true); + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS single_swin_settings_locales" + ); + + await current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Edited while the companion was missing" }, + locale: "en", + } as Parameters[0]); + + const rows = await current.adapter.executeQuery<{ headline: string }>( + "SELECT headline FROM single_swin_settings" + ); + expect(rows[0]?.headline).toBe("Edited while the companion was missing"); + }); + + it("refuses a default-language write when the main table never had the column", async () => { + // Localized from creation, so `headline` only ever existed on the companion and the + // main table has no column to fall back to. The case above cannot show this: there + // the Single started life unlocalized, so its legacy main column is still present and + // the fallback genuinely works. + current = await boot(true); + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS single_swin_settings_locales" + ); + + await expect( + current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Edited while the companion was missing" }, + locale: "en", + } as Parameters[0]) + ).rejects.toThrow(/Translations are not ready/); + }); +}); + +/** + * The same refusal, on a real server, for a Single localized from creation — the case the + * SQLite pair above cannot cover, because there the Single starts life unlocalized and so + * keeps a usable main column. + * + * This pins the refusal down to an actionable 409 on every dialect. It asserts the status + * code rather than the message because the message is the weaker signal: when a + * `NextlyError` reaches an adapter's `classifyError` it is rewrapped, but the original + * `message` is copied onto the wrapper, so the text alone cannot tell a clean refusal from + * a rewrapped one. + * + * Note what this does NOT cover: the write path also resolves this before opening its + * transaction, so the probe cannot wait on a connection the transaction itself holds. That + * matters only on a single-connection pool, where the symptom is a hang rather than a + * wrong value, so it is left to the pool's own configuration rather than pinned here. + */ +describe.each(getConfiguredTestDialects())( + "localized single without a companion table on %s (integration)", + dialect => { + it("refuses a default-language write with an actionable 409", async () => { + current = await createTestNextly({ + dialect, + singles: [settings(true)], + localization, + }); + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`single_swin_settings_locales`" : '"single_swin_settings_locales"'}` + ); + + const result = await current + .getService("singleEntryService") + .update( + "swin_settings", + { headline: "Edited while the companion was missing" }, + { locale: "en" } + ); + + expect(result.success).toBe(false); + expect(result.statusCode).toBe(409); + expect(result.message).toMatch(/Translations are not ready/); + }); + } +); diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 23af7a3a2..1e47a55bf 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -81,6 +81,7 @@ import { import { buildCompanionSchema, companionTableExists, + mainTableHasColumns, splitLocalizedWrite, upsertCompanionRow, type CompanionSchema, @@ -865,6 +866,53 @@ export class SingleMutationService extends BaseService { companion.companionTableName ) : false; + // A localized single whose companion table does not exist yet has nowhere to put + // a NON-default locale's values: the split below moves them out of the main + // payload and the companion upsert is then skipped, so the translation is + // silently dropped while the write reports success. The window is real — + // `db:sync` flips the registry's `localized` flag in its own process before the + // running server creates the companion — so refuse rather than discard. + // + // The DEFAULT locale keeps the pre-companion fallback, but only where it can + // actually work: an entity localized from creation keeps its translatable + // columns solely on the companion, and its registered runtime table omits them, + // so those keys would be dropped by the ORM while `updated_at` still moved. + // + // Both checks run HERE, before `adapter.transaction` opens. Introspecting from + // inside it would borrow a second connection while the transaction holds one, + // which on a small pool means waiting for a connection that cannot be released + // until this transaction finishes. Resolving it first also keeps the refusal + // exactly as raised: errors leaving a transaction callback pass through the + // adapter's error classification, which rewraps anything that is not already a + // `DatabaseError`. + if (companion && !companionPhysicallyExists && this.localization) { + // Captured so the closure below keeps the narrowed type. + const defaultLocale = this.localization.defaultLocale; + const refuse = (): never => { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + single: singleMeta.slug, + locale: writeLocale, + defaultLocale, + companionTable: companion.companionTableName, + }, + }); + }; + if (writeLocale !== undefined && writeLocale !== defaultLocale) { + refuse(); + } + const fallbackPossible = await mainTableHasColumns( + this.adapter, + singleMeta.tableName, + companion.localizedFields.map(f => f.column) + ); + if (!fallbackPossible) refuse(); + } + // Same pre-transaction, pooled probe for the auto-create default seed: it // is keyed on the DEFAULT locale (not the write locale), so it needs its // own existence check rather than reusing `companionPhysicallyExists`. @@ -875,6 +923,20 @@ export class SingleMutationService extends BaseService { ) : false; let updatedRows: SingleDocument[]; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection while the transaction holds one, which on a small pool means + // waiting for a connection that cannot be released until it finishes. It also + // keeps the refusal exactly as raised: errors leaving a transaction callback + // pass through the adapter's error classification, which rewraps anything that + // is not already a `DatabaseError`. + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fieldConfigs, + data: componentFieldData, + locale: options.locale, + })) ?? new Map(); + try { // Retry the whole update+capture transaction on a version_no allocation // race; the re-run re-reads the max. The single UPDATE is deterministic. @@ -1054,12 +1116,18 @@ export class SingleMutationService extends BaseService { // row, not the main table. `companion` and `writeLocale` were resolved // above (before validation); the split reuses them. Done inside the // closure so a retry re-splits the freshly-timestamped payload. - const { main: mainPayload, companion: companionData } = companion - ? splitLocalizedWrite(updatePayload, companion.localizedFields) - : { - main: updatePayload, - companion: {} as Record, - }; + // Only split when the companion physically exists. Splitting first and + // then skipping the companion upsert (gated on the same flag below) would + // drop the translatable values on the floor. While the table is absent + // they stay on the main table — the pre-companion fallback — which the + // pre-transaction guard above has already proven is actually possible. + const { main: mainPayload, companion: companionData } = + companion && companionPhysicallyExists + ? splitLocalizedWrite(updatePayload, companion.localizedFields) + : { + main: updatePayload, + companion: {} as Record, + }; // per-locale status. The status the companion row carries — // from `updatePayload` (not `mainPayload`, which may have `status` @@ -1502,7 +1570,8 @@ export class SingleMutationService extends BaseService { fields: fieldConfigs, data: attemptComponentData, locale: options.locale, - } + }, + fieldGroupPresence ); } diff --git a/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts b/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts index 2573324cc..de9a6075d 100644 --- a/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts +++ b/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, it, expect } from "vitest"; import { applyStoredRecordingDecisions, + currentRecordingGeneration, isRecordingDisabledByConfig, clearWebhookRecording, isWebhookRecordingEnabled, @@ -194,4 +195,26 @@ describe("webhook recording policy", () => { expect(isRecordingDisabledByConfig("collection", "posts")).toBe(false); expect(isRecordingDisabledByConfig("collection", "never-seen")).toBe(false); }); + + it("rejects a refresh captured before a reset, even when the counter collides", () => { + // ABA: a background read captures generation N, then `clearServices()` + // resets and the next boot performs the SAME number of writes, so a + // zero-based counter lands back on exactly N. The stale snapshot — taken + // against the previous adapter and config — would then pass the guard and + // wipe every `db` decision the new boot had just published. + // + // Constructed so both epochs reach the identical count; that collision is + // the whole point, and without a monotonic counter this test fails. + resetWebhookRecordingPolicy(); + setWebhookRecording("collection", "old-epoch", false, "db"); + const captured = currentRecordingGeneration(); + + resetWebhookRecordingPolicy(); + setWebhookRecording("collection", "enquiries", false, "db"); + + applyStoredRecordingDecisions([], captured); + + // The stale snapshot was discarded, so the new boot's opt-out survives. + expect(isWebhookRecordingEnabled("collection", "enquiries")).toBe(false); + }); }); diff --git a/packages/nextly/src/domains/webhooks/recording-policy.ts b/packages/nextly/src/domains/webhooks/recording-policy.ts index f02ad0b96..11dce8474 100644 --- a/packages/nextly/src/domains/webhooks/recording-policy.ts +++ b/packages/nextly/src/domains/webhooks/recording-policy.ts @@ -294,6 +294,12 @@ export function resetWebhookRecordingPolicy(): void { storedRefresh.refresher = null; storedRefresh.readAtMs = 0; storedRefresh.queued = false; - storedRefresh.generation = 0; + // Advance rather than zero. A refresh that captured generation N before this + // reset may still be in flight; zeroing would let the next boot's ordinary + // increments climb back to N, so that stale read — taken against the previous + // adapter and config — would pass the guard and replace every `db` decision + // the new boot had just published, re-enabling recording for an opted-out + // entity. A counter that only ever increases can never be matched again. + storedRefresh.generation += 1; storedRefresh.now = () => Date.now(); } diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 76a9475de..0868f7fe4 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -98,7 +98,11 @@ type LoggerLike = { // adapter-drizzle would couple this module to the adapter package. interface AdapterLike { readonly dialect: "postgresql" | "mysql" | "sqlite"; - getDrizzle(): unknown; + getDrizzle(): T; + // Needed to provision the localized companion below: creating the table and adding + // columns to it are both DDL, and the status backfill that accompanies a new `_status` + // column is a write. + executeQuery(sql: string, params?: unknown[]): Promise; } type CollectionDef = { @@ -552,6 +556,135 @@ function republishRecordingPolicies( ); } +/** + * Create, and bring into step, the `_locales` companion of every localized collection, single + * and field group in the reloaded config. + * + * It does NOT seed existing content into the companion. `ensureCompanionTable` is + * creation-only and leaves whatever is already on the main table where it is, so a successful + * reload is not evidence that default-locale data has been carried across — enabling + * localization on an entity that already has content still leaves that content unreadable + * until the transition seeds it. Copying it is the gated pipeline's job. + * + * The reload path is the `next dev` counterpart to the CLI's `ensureLocalizedCompanions`: it is + * where a config edit lands when the app is running under plain `next dev` rather than + * `nextly db:sync --watch`. `ensureCompanionTable` is idempotent, so entities that already have + * their companion cost one introspection each. + * + * Never throws: a companion that cannot be provisioned must not take down a config reload. The + * write guard in the mutation services is what protects content in the meantime. + */ +async function ensureLocalizedCompanionsForReload( + adapter: AdapterLike, + config: { + collections?: unknown[]; + singles?: unknown[]; + fieldGroups?: unknown[]; + localization?: { defaultLocale?: string }; + }, + // `:` for every entity whose schema change was classified unsafe (or whose diff + // threw) this cycle, so it was NOT applied. Those must be skipped: creating a companion for + // a transition that has not happened is worse than leaving it absent. The Schema Builder + // later applies the real transition, and `buildCompanionTransitionStatements` decides + // whether to SEED the existing main-table values by looking at whether the companion is + // already there. Finding one — empty, created from a config that was never applied — sends + // it down the plain reconcile branch instead, and the default-locale content is lost. + // + // Deliberately per entity rather than a single "something was deferred" flag: entities whose + // schema IS in step still need provisioning on this pass, and skipping them wholesale + // reintroduces the missing-companion window this function exists to close. + deferred: ReadonlySet = new Set() +): Promise { + // Same policy the CLI applies: production schema changes belong to `nextly migrate`. + if (process.env.NODE_ENV === "production") return; + + const { ensureCompanionTable, reconcileCompanionColumns } = await import( + "../domains/i18n/runtime/companion-io" + ); + const { resolveCollectionTableName, resolveComponentTableName } = + await import("../domains/schema/utils/resolve-table-name"); + const { resolveSingleTableName } = await import( + "../domains/singles/services/resolve-single-table-name" + ); + + type Localizable = { + slug?: string; + dbName?: string; + localized?: boolean; + status?: boolean; + fields?: { name: string; type: string; localized?: boolean }[]; + }; + // The kind prefixes the `deferred` keys, because a collection and a single may share a slug + // and only one of them may have been deferred. + const groups: [string, Localizable[], (e: Localizable) => string][] = [ + [ + "collection", + (config.collections ?? []) as Localizable[], + e => resolveCollectionTableName(e.slug!, e.dbName), + ], + [ + "single", + (config.singles ?? []) as Localizable[], + e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), + ], + [ + "fieldGroup", + (config.fieldGroups ?? []) as Localizable[], + e => resolveComponentTableName(e.slug!), + ], + ]; + + for (const [kind, entities, resolveTableName] of groups) { + for (const entity of entities) { + if (!entity.slug || entity.localized !== true) continue; + if (deferred.has(`${kind}:${entity.slug}`)) continue; + await ensureCompanionTable( + adapter, + { + slug: entity.slug, + tableName: resolveTableName(entity), + fields: entity.fields ?? [], + dialect: adapter.dialect, + status: entity.status === true, + }, + error => { + console.warn( + `[nextly] Could not prepare the translations table for "${entity.slug}". ` + + `Writes in a non-default locale will be refused until it exists: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + } + ); + // Creating the table is not enough on its own: `ensureCompanionTable` returns + // immediately when one already exists, so marking a FURTHER field localized on an + // already-localized entity takes the no-DDL path, syncs its metadata, and leaves the + // companion a column short — the write then splits that value into a column that is not + // there. The CLI sync reconciles for the same reason; the HMR path needs it too. + // + // Safe here despite issuing DDL, because the production guard at the top of this + // function has already returned: this runs only under `next dev`. The reconcile is + // additive, so it never removes a column even when a field stops being localized. + await reconcileCompanionColumns( + adapter, + { + slug: entity.slug, + tableName: resolveTableName(entity), + fields: entity.fields ?? [], + dialect: adapter.dialect, + status: entity.status === true, + }, + error => { + console.warn( + `[nextly] Could not update the translations table for "${entity.slug}". ` + + `Newly translatable fields may fail to save until it is in step: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + } + ); + } + } +} + // Reload entry point. resolver is optional and exists primarily for tests. // dispatcher is also test-only: injects a fake PromptDispatcher (e.g., one // that records prompts and auto-confirms) so tests don't need a real TTY. @@ -636,6 +769,7 @@ async function runReload(opts?: { singles?: SingleDef[]; fieldGroups?: ComponentDef[]; webhookAuditEnabled?: boolean; + localization?: { defaultLocale?: string }; } | undefined; let previousFieldTypes: PluginFieldType[] | undefined; @@ -949,6 +1083,10 @@ async function runReload(opts?: { // metadata-only sync below must be skipped rather than persist unmigrated // schema metadata; it retries on the next clean reload or restart. let deferredSchemaChange = false; + // Which entities were deferred, as `:`. The flag above answers "may the + // metadata-only sync run at all"; this answers "may THIS entity be provisioned", which is a + // per-entity question — see `ensureLocalizedCompanionsForReload`. + const deferredEntities = new Set(); const desiredCollections: Record = {}; for (const target of targets) { @@ -993,6 +1131,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`collection:${target.slug}`); desiredCollections[target.slug] = entry; continue; } @@ -1004,6 +1143,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`collection:${target.slug}`); } } @@ -1045,6 +1185,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`single:${target.slug}`); desiredSingles[target.slug] = entry; continue; } @@ -1056,6 +1197,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping single '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`single:${target.slug}`); } } @@ -1093,6 +1235,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`fieldGroup:${target.slug}`); desiredComponents[target.slug] = entry; continue; } @@ -1104,6 +1247,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping component '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`fieldGroup:${target.slug}`); } } @@ -1112,6 +1256,17 @@ async function runReload(opts?: { // not surface as a schema diff — so run the idempotent metadata sync before // returning, otherwise a metadata-only edit (e.g. toggling `versions`) would // not persist until the dev server restarts. + // Also provision on the no-DDL path. A missing `_locales` table produces no schema + // diff — companion tables are excluded from it — so `hasChanges` stays false and the + // reload would return before ever reaching the call after the apply, which is exactly + // the missing-companion state this repairs. Idempotent, so running it here and after + // a successful apply costs one existence probe per localized entity. + await ensureLocalizedCompanionsForReload( + adapter, + newConfig, + deferredEntities + ); + if (!hasChanges) { // Only sync when the schema is genuinely in step (every entity had a zero-op // diff). If a real schema change was deferred (unsafe/needs review) or a diff @@ -1291,6 +1446,19 @@ async function runReload(opts?: { }); if (applyResult.success) { + // Create the `_locales` companion of every localized entity, now that the apply + // has produced their main tables. `next dev` routes config edits here rather + // than through the CLI watcher, so without this an entity turned localized under + // ordinary HMR had its companion registered in the runtime registry while the + // database had no such table — non-default writes were then refused until a + // restart. Runs after the apply because the companion carries a foreign key to + // its main table, which a brand-new entity does not have before it. + await ensureLocalizedCompanionsForReload( + adapter, + newConfig, + deferredEntities + ); + // Publish each scope's recording policy only AFTER its field-tree metadata // sync succeeds (see the assignment after the syncs below): the DDL applied, // but if a sync then fails, activating the new decision while the mutation