From 4aac8785fad59ff412782a60eaedfc32572c09cd Mon Sep 17 00:00:00 2001 From: aXenDeveloper Date: Fri, 7 Aug 2026 21:49:52 +0200 Subject: [PATCH 1/2] feat(content): index one search document per published translation Lifts the last capability boundary: `localization` now combines with `search`. A localized record is indexed once per published translation, each document built from the two halves the public read joins - shared values off the base row, localized ones off the translation. Documents - `contentTranslationSearchDocument` merges the two halves and gates on `isContentTranslationPubliclyVisible`, the same subordination the public read enforces in SQL. A translation of a draft record is never indexed. - `createdAt` is that language's publication date, so a late translation sorts by "newest" where it actually appeared. - `search.pathTemplate` must carry `{locale}` on a localized content type and may not on any other: two languages routinely answer to the same slug, so one template without it would give every translation the same link. Synchronisation - `SearchModel.delete` takes an optional language. Unpublishing the Polish copy must leave the English document exactly where it is; the Elasticsearch adapter narrows its delete-by-query the same way. - `syncContentLocalizedSearch` moves one language for a translation mutation and every language for a mutation of the record - its publication state gates them all, and a shared field is in all of them. - The generated routes and the scheduled-transition handler pass the model, which is what enumerates the translations. Rebuild - `createContentLocalizedSearchIndexer` pages over translations with a keyset cursor on `(itemId, languageId)`: a page can neither overlap nor skip while rows are published underneath it, and Postgres seeks rather than counts. `itemsRead` counts translation rows and `count()` reports published translations, so the coverage bar compares like with like. Diagnostics and AdminCP - `/search/status` reports documents alongside distinct items and a per-language breakdown; coverage is measured in documents when a collection has languages, read off the data rather than configured. - A localized content type's list gets a language selector - a view control, not a filter: it adds a column with each record's title and status in that language, `Missing` included, and lives in the URL so it survives a reload. No migration: `core_search_index` has stored one row per `(itemType, itemId, languageCode)` since it was created. Co-Authored-By: Claude Opus 5 (1M context) --- .../content/docs/dev/content-engine/index.mdx | 14 +- .../docs/dev/content-engine/limitations.mdx | 33 +- .../docs/dev/content-engine/localization.mdx | 55 ++- .../dev/content-engine/localized-search.mdx | 180 ++++++++++ .../content/docs/dev/content-engine/meta.json | 1 + .../docs/dev/content-engine/search.mdx | 10 + packages/elasticsearch/src/index.ts | 16 +- .../vitnode/src/api/models/search.test.ts | 15 +- packages/vitnode/src/api/models/search.ts | 38 ++- .../admin/debug/routes/search-status.route.ts | 66 +++- packages/vitnode/src/content/const.ts | 14 +- packages/vitnode/src/content/define.ts | 56 +++- packages/vitnode/src/content/index.ts | 2 + .../vitnode/src/content/localization.test.ts | 44 +-- packages/vitnode/src/content/localization.ts | 32 +- packages/vitnode/src/content/search.ts | 29 +- .../src/content/server/editorial-effects.ts | 65 +++- packages/vitnode/src/content/server/index.ts | 13 +- .../content/server/localized-search.test.ts | 164 +++++++++ packages/vitnode/src/content/server/module.ts | 15 +- packages/vitnode/src/content/server/routes.ts | 149 ++++++++- .../content/server/schedule-effects.test.ts | 2 +- .../src/content/server/schedule-effects.ts | 15 +- .../src/content/server/search-document.ts | 86 ++++- .../src/content/server/search-indexer.ts | 202 ++++++++++- .../vitnode/src/content/server/search-sync.ts | 246 +++++++++++++- .../src/content/server/translation-effects.ts | 65 +++- .../src/content/server/translation-routes.ts | 5 +- packages/vitnode/src/content/types.ts | 28 +- packages/vitnode/src/locales/en.json | 2 + .../vitnode/src/tests/content-fixtures.ts | 46 +++ .../content/table/content-table-view.test.tsx | 97 ++++-- .../content/table/content-table-view.tsx | 108 ++++-- .../views/content/table/locale-selector.tsx | 81 +++++ .../advanced/search/collection-status.test.ts | 47 +++ .../core/advanced/search/collection-status.ts | 64 +++- .../advanced/search/collections-table.tsx | 33 +- .../example/src/content/localized-article.ts | 18 + plugins/example/src/database/postgres.test.ts | 313 ++++++++++++++++++ 39 files changed, 2246 insertions(+), 223 deletions(-) create mode 100644 apps/docs/content/docs/dev/content-engine/localized-search.mdx create mode 100644 packages/vitnode/src/content/server/localized-search.test.ts create mode 100644 packages/vitnode/src/views/admin/views/content/table/locale-selector.tsx diff --git a/apps/docs/content/docs/dev/content-engine/index.mdx b/apps/docs/content/docs/dev/content-engine/index.mdx index 77b4b354b..0eaa0e221 100644 --- a/apps/docs/content/docs/dev/content-engine/index.mdx +++ b/apps/docs/content/docs/dev/content-engine/index.mdx @@ -57,15 +57,15 @@ Four more declarations, each opt-in: - [`localization`](/docs/dev/content-engine/localization) moves the text fields you mark into a generated per-language table, with its own version, its own [publish button](/docs/dev/content-engine/translation-editorial), its own - [history](/docs/dev/content-engine/translation-revisions) and a URL per locale + [history](/docs/dev/content-engine/translation-revisions), a + [URL per locale](/docs/dev/content-engine/localized-public-api) and a + [search document per language](/docs/dev/content-engine/localized-search) Publication alone exposes nothing. Public exposure requires both of the first -two; `editorial` works with or without either. `localization` combines with -`publication`, `editorial` and `publicApi` - a public read then -[resolves one language](/docs/dev/content-engine/localized-public-api) - and is -still -[exclusive of `search`](/docs/dev/content-engine/localization#stage-5c-boundaries), -which arrives in a later stage. +two; `editorial` works with or without either. `localization` combines with all +four - a public read +[resolves one language](/docs/dev/content-engine/localized-public-api) and search +[indexes one document per translation](/docs/dev/content-engine/localized-search). ## What it is not diff --git a/apps/docs/content/docs/dev/content-engine/limitations.mdx b/apps/docs/content/docs/dev/content-engine/limitations.mdx index 84de2d6d9..b5859a61a 100644 --- a/apps/docs/content/docs/dev/content-engine/limitations.mdx +++ b/apps/docs/content/docs/dev/content-engine/limitations.mdx @@ -43,29 +43,28 @@ None of these are blocked - they are simply not generated. The service, the schemas and the table are all public, so a hand-written route sits next to a generated one without friction. -## Localization does not read outwards yet +## Localization is complete, with one shape rule [`localization`](/docs/dev/content-engine/localization) generates the tables, the types, the schemas, the services, the per-locale lifecycle, the per-locale history and the AdminCP locale tabs. What it deliberately refuses is every combination whose *reading* half is not built yet: -| Combination | Refused until | -| --- | --- | -| `localization` + `search` | Stage 5D | - -It is a definition-time error naming the stage. A localized content type that -silently indexed one language while ranking every other one as a miss would be -worse than one that refuses to be declared. +Nothing is refused any more. +[Locale-aware public reads](/docs/dev/content-engine/localized-public-api) landed +in Stage 5C and [per-locale search](/docs/dev/content-engine/localized-search) in +Stage 5D, so `localization` combines with `publication`, `editorial`, `publicApi` +and `search`. -Stage 5C lifted the `publicApi` refusal, so -[locale-aware public reads](/docs/dev/content-engine/localized-public-api), -fallback resolution, strict-locale slugs, locale-aware cache tags and -[locale preview links](/docs/dev/content-engine/translation-preview) all work now. +One shape rule remains, and it is a rule rather than a boundary: +`search.pathTemplate` must contain `{locale}` on a localized content type. Two +languages routinely answer to the same slug, so a template without it would give +every translation of a record the same link. -Still outside Stage 5C: per-locale search documents, an `Outdated` badge (its -honest definition needs a comparison two timestamps cannot make), a locale -selector on the AdminCP *list*, and locale-specific scheduling. +Still outside Stage 5: an `Outdated` translation badge (its honest definition +needs a comparison two timestamps cannot make), locale-specific scheduling, +`hreflang` and sitemap generation, locale-specific relations and localized +media. ## A public list cannot be ordered by a localized field @@ -85,8 +84,8 @@ six are compile errors and runtime errors. `admin.titleField` therefore falls back to `null` on a content type whose only text fields are localized. The locale tabs show the localized title inside each -tab; the list still has no locale-aware title column, which arrives with the -locale selector in Stage 5D. +tab, and the list's language selector adds a column showing each record's title +in the language being viewed. ## Foreign key names on a long translation table are truncated by Postgres diff --git a/apps/docs/content/docs/dev/content-engine/localization.mdx b/apps/docs/content/docs/dev/content-engine/localization.mdx index 36b73b1d5..a912c3fbd 100644 --- a/apps/docs/content/docs/dev/content-engine/localization.mdx +++ b/apps/docs/content/docs/dev/content-engine/localization.mdx @@ -48,11 +48,10 @@ example_localized_articles_translations itemId, languageId, version, A content type without the block generates the same single table, the same routes and the same wire shapes it always did. - - Localization works with `publication`, `editorial` and `publicApi`, but - cannot yet be combined with `publicApi` or `search` - each is refused at - definition time with a message naming the stage that lifts it. See [Stage 5C - boundaries](#stage-5c-boundaries). + + Localization works with `publication`, `editorial`, `publicApi` and `search`. + There is one shape rule left - `search.pathTemplate` must carry `{locale}` - + and no capability is refused. See [the roadmap](#the-roadmap). ## This is not UI translation @@ -154,9 +153,11 @@ worth knowing up front: - it cannot appear in `indexes`, - it is absent from `schemas.create`, `schemas.update` and `schemas.select`. -All five are compile errors *and* runtime errors. Localized values get their own -AdminCP surface in Stage 5B; until then there is nowhere on the base form for -them to go, and a silently-dropped title is worse than a refused definition. +All five are compile errors *and* runtime errors: there is nowhere on the base +form or in a base-table query for them to go, and a silently-dropped title is +worse than a refused definition. Localized values have their own AdminCP surface - +the [locale tabs](/docs/dev/content-engine/translation-editorial) in the edit +dialog, and the language selector on the list. ## Optimistic locking per locale @@ -274,37 +275,33 @@ offender at once rather than failing on the first, and it is skipped entirely when no content type is localized - an install with none never touches the languages table because of it. -## Stage 5C boundaries +## No capability is refused any more -Stage 5A landed the infrastructure, Stage 5B the editorial layer and Stage 5C the -[public read](/docs/dev/content-engine/localized-public-api). One thing still -reads outwards without knowing about languages, and the honest failure for that is -a refused definition rather than a content type that quietly indexes one language -and ranks every other one as a miss. +Stage 5A landed the infrastructure, Stage 5B the editorial layer, Stage 5C the +[public read](/docs/dev/content-engine/localized-public-api) and Stage 5D +[per-locale search](/docs/dev/content-engine/localized-search). `localization` +combines with `publication`, `editorial`, `publicApi` and `search`, and every one +of them reads the language it was asked for rather than pretending there is only +one. -| Combination | Refused until | Why | -| --- | --- | --- | -| `localization` + `search` | Stage 5D | One document per record would index a single language and rank every other one as a miss | - -It is a `ContentEngineError` at definition time, with the stage in the message. - -`localization` + `publicApi` is **no longer refused**: locale precedence, fallback, -strict-locale slugs, locale-aware cache tags and the -[locale preview link](/docs/dev/content-engine/translation-preview) all landed with -Stage 5C. +There is one *shape* rule left, and it is a rule rather than a boundary: +`search.pathTemplate` must contain `{locale}` on a localized content type. One +document per language means one URL per language, and two languages routinely +answer to the same slug. ## The roadmap | Stage | What it adds | | --- | --- | | **5A** | Tables, types, schemas, language resolution, translation service, per-locale locking, atomic create, routes, migrations | -| **5B** (this one) | Per-locale publication, per-locale revisions and restore, locale-bound preview tokens, translation events, `can_translate`, AdminCP locale tabs | -| **5C** | Locale-aware public API, fallback resolution, locale-aware cache tags | -| **5D** | Per-locale search documents, `hreflang`, localized sitemap | +| **5B** | Per-locale publication, per-locale revisions and restore, locale-bound preview tokens, translation events, `can_translate`, AdminCP locale tabs | +| **5C** | Locale-aware public API, locale precedence, fallback resolution, strict-locale slugs, locale-aware cache tags, locale preview links | +| **5D** (this one) | Per-locale search documents, the localized rebuild, per-language diagnostics, the AdminCP list language selector | Explicitly outside all four: locale-specific relations, localized media, AI -translation, translation memory, external TMS integration, and migrating the blog -plugin onto the Content Engine. +translation, translation memory, external TMS integration, `hreflang` and sitemap +generation, locale-specific scheduling, and migrating the blog plugin onto the +Content Engine. ## Where to next diff --git a/apps/docs/content/docs/dev/content-engine/localized-search.mdx b/apps/docs/content/docs/dev/content-engine/localized-search.mdx new file mode 100644 index 000000000..9811bc5aa --- /dev/null +++ b/apps/docs/content/docs/dev/content-engine/localized-search.mdx @@ -0,0 +1,180 @@ +--- +title: Localized search +description: One search document per published translation - how they are built, kept current, rebuilt, and what a rebuild counts. +icon: Languages +--- + +A [searchable](/docs/dev/content-engine/search) content type that is also +[localized](/docs/dev/content-engine/localization) is indexed **once per published +translation**, not once per record. + + + A record's prose lives in its translations. Indexing the base row would index + whichever language happened to be there - usually none, since a localized + content type's text fields are all localized - and rank every other language as + a miss. One document per language is the only shape a multilingual index can + have. + + +## Turning it on + +```ts title="src/content/article.ts" +search: { + enabled: true, + titleField: "title", // localized, and that is the point + contentFields: ["title", "body"], // localized too + pathTemplate: "/{locale}/articles/{slug}", +}, +``` + +`{locale}` is **required** on a localized content type and refused on every other +one. Two languages routinely answer to the same slug, so a template without it +would give every translation of a record the same link and a search hit would +point at whichever language the reader happened to be in. + +Everything else is unchanged: every indexed field must be in `publicApi.fields`, +and `search` still needs `publication` and `publicApi`. + +## What a document holds + +Both halves of the page the reader would land on: + +| From | Fields | +| --- | --- | +| the **translation** | every localized field, `publishedAt`, `updatedAt` | +| the **base row** | every shared field, `id`, the publication state | + +`languageCode` is the locale. `createdAt` is **this language's** publication date +when it has one, so a translation published months after its record sorts by +"newest" where it actually appeared rather than where the record did. + +## When a document exists + +The same subordination the public read enforces in SQL, in JavaScript: + +```text +record published + translation published → indexed +record published + translation draft → not indexed +record draft + translation published → not indexed, in any language +``` + +A translation that must not be indexed has its document **deleted for that +language only**. Taking the Polish copy down leaves the English one exactly where +it is, which is why `SearchModel.delete` takes an optional language: + +```ts +await c.get("search").delete("example.article", 7, "pl"); // one language +await c.get("search").delete("example.article", 7); // every language +``` + +## Keeping it current + +| What happened | What moves | +| --- | --- | +| A **translation** was created, edited, published, unpublished or restored | that language's document | +| The **record** was published, unpublished or had a shared field edited | every language's document | +| The **record** was deleted | every language's document, in one call | + +A mutation of the record moves every language because its publication state gates +all of them and a shared field is in all of them. A Polish edit moves Polish: +nothing else contains it. + +The generated routes do this for you. A direct service call does not - it may be +inside a transaction that has not committed - so application code opts in after +the write returns: + +```ts +const outcome = await model.translationEditorialService(c, { pluginId }) + .publish(id, "pl", { actor }); + +if (outcome) { + await syncContentLocalizedSearch(c, model, { + changed: outcome.changed, + locale: "pl", + operation: "publish", + pluginId, + row: await model.service(c).findById(id), + }); +} +``` + +Omitting `locale` on a translation mutation would rewrite every other language's +document for a change none of them contains. + +## Rebuilding + +The generated indexer pages over **translations**, with a keyset cursor on +`(itemId, languageId)` - the translation table's primary key. + +Keyset rather than offset for two reasons, and the second is the one that +matters at scale: + +1. A page can neither overlap nor skip while rows are being published underneath + it. An offset shifts when a row before it appears; a cursor does not. +2. Postgres seeks straight to the cursor on the primary key instead of counting + past every earlier row, so a rebuild of a large table stays linear. + +`itemsRead` counts translation rows, not documents. A record with three languages +advances the position by three, and a published translation with no usable title +projects to nothing without ending the rebuild early. + +`count()` reports **published translations**, so the AdminCP coverage bar compares +like with like - counting records would pin a fully-indexed two-language +collection at 50%. + +## Diagnostics + +**AdminCP → Advanced → Search** shows, per collection: + +- `indexed / total` measured in **documents** when the collection has languages, + and in items when it does not. The switch is read off the data, so a collection + that gains a second language starts being measured correctly without anything + being turned on. +- A per-language breakdown (`en 120 pl 84`). A single total cannot say which + language a rebuild stopped halfway through; two numbers can. +- The same coverage bar, status and sync-error panel as before. + +Language-agnostic rows (`languageCode = ""`) are not listed as a language. They +are not one, and an unnamed row in every collection would be noise. + +## Filtering a search by language + +The public search route already takes one: + +```http +GET /api/@vitnode/core/search?search=hello&lang=pl +``` + +Rows with an empty `languageCode` match every locale, so a query scoped to Polish +still finds language-agnostic content. That behaviour predates this page; what +Stage 5D adds is content that actually has languages to filter on. + +## The AdminCP list + +A localized content type's list gets a language selector. It is a **view control, +not a filter**: picking Polish adds a column showing each record's Polish title +and status - including `Missing`, which is the row most worth finding. Hiding +untranslated records would be the opposite of what somebody choosing a language is +looking for. + +The choice lives in the URL, so it survives a reload, paginates with the table and +can be sent to whoever is doing the translating. Changing it resets the cursor: +page three of one ordering is not page three of another. + +## No migration + +`core_search_index` already stores one row per `(itemType, itemId, languageCode)`, +with the unique key, the per-row text-search configuration and the language index +in place since it was created. Stage 5D writes more rows into a table that was +built for them; it changes no schema. + +An existing localized content type that turns `search` on is indexed by the next +rebuild, or by the next publish of each translation - whichever comes first. + +## Related + +- [Search](/docs/dev/content-engine/search) - the non-localized shape this extends +- [Localized public API](/docs/dev/content-engine/localized-public-api) - the read + layer a hit links into +- [Translation editorial workflow](/docs/dev/content-engine/translation-editorial) - + what "published in this language" means diff --git a/apps/docs/content/docs/dev/content-engine/meta.json b/apps/docs/content/docs/dev/content-engine/meta.json index 2460b58b8..eccb55f5f 100644 --- a/apps/docs/content/docs/dev/content-engine/meta.json +++ b/apps/docs/content/docs/dev/content-engine/meta.json @@ -27,6 +27,7 @@ "translation-revisions", "translation-preview", "localized-public-api", + "localized-search", "localization-migrations", "admincp", "permissions", diff --git a/apps/docs/content/docs/dev/content-engine/search.mdx b/apps/docs/content/docs/dev/content-engine/search.mdx index d2f018e8a..33343c5b2 100644 --- a/apps/docs/content/docs/dev/content-engine/search.mdx +++ b/apps/docs/content/docs/dev/content-engine/search.mdx @@ -344,6 +344,16 @@ per locale (the way the blog plugin indexes translations from own indexer - and one item type may only have one indexer, so it is one or the other. +## Localized content types + +A [localized](/docs/dev/content-engine/localization) content type is indexed once +per **published translation** rather than once per record, and +`search.pathTemplate` has to carry `{locale}` so each document links to the +language it matched. Everything on this page still applies to each document: the +same allowlist, the same published predicate, the same rebuild contract. + +See [Localized search](/docs/dev/content-engine/localized-search). + ## Limitations | Not supported | Why | diff --git a/packages/elasticsearch/src/index.ts b/packages/elasticsearch/src/index.ts index 8778787f8..116f2c1eb 100644 --- a/packages/elasticsearch/src/index.ts +++ b/packages/elasticsearch/src/index.ts @@ -313,13 +313,23 @@ export const ElasticsearchSearchAdapter = ( }, // One document per language shares an (itemType, itemId), so remove every - // language variant with a query rather than a single id. - delete: async (_c, itemType, itemId) => { + // language variant with a query rather than a single id - unless the caller + // named one, which is how a single translation is taken down without + // touching the others. + delete: async (_c, itemType, itemId, languageCode) => { await getClient().deleteByQuery( { index, query: { - bool: { filter: [{ term: { itemType } }, { term: { itemId } }] }, + bool: { + filter: [ + { term: { itemType } }, + { term: { itemId } }, + ...(languageCode === undefined + ? [] + : [{ term: { languageCode } }]), + ], + }, }, }, { ignore: [404] }, diff --git a/packages/vitnode/src/api/models/search.test.ts b/packages/vitnode/src/api/models/search.test.ts index e59e4b7a0..6edec154c 100644 --- a/packages/vitnode/src/api/models/search.test.ts +++ b/packages/vitnode/src/api/models/search.test.ts @@ -196,7 +196,20 @@ describe("SearchModel", () => { await new SearchModel(c).delete("blog_post", 5); expect(deleteFn).toHaveBeenCalledWith(core_search_index); - expect(provider.delete).toHaveBeenCalledWith(c, "blog_post", 5); + // No language: deleting the record means every language of it. + expect(provider.delete).toHaveBeenCalledWith(c, "blog_post", 5, undefined); + }); + + it("deletes one language without touching the others", async () => { + const provider = createProvider(); + const { c, deleteFn } = createContext(provider); + + // Multi-language content is one row per `(itemType, itemId, languageCode)`, + // so taking the Polish translation down must leave the English one indexed. + await new SearchModel(c).delete("blog_post", 5, "pl"); + + expect(deleteFn).toHaveBeenCalledWith(core_search_index); + expect(provider.delete).toHaveBeenCalledWith(c, "blog_post", 5, "pl"); }); it("delegates search to the provider", async () => { diff --git a/packages/vitnode/src/api/models/search.ts b/packages/vitnode/src/api/models/search.ts index 2ce36f1ba..fa427fa8a 100644 --- a/packages/vitnode/src/api/models/search.ts +++ b/packages/vitnode/src/api/models/search.ts @@ -230,7 +230,24 @@ export interface SearchProviderApiPlugin { bulkIndex: (c: Context, docs: SearchDocument[]) => Promise; capabilities?: SearchProviderCapabilities; clear: (c: Context, itemType?: string) => Promise; - delete: (c: Context, itemType: string, itemId: number) => Promise; + /** + * Removes one item's documents. + * + * `languageCode` narrows it to a single language, for content that is indexed + * once per translation: unpublishing the Polish copy of an article must not + * take the English one out of the index. Omit it and every language goes, which + * is what deleting the record itself means. + * + * Optional on purpose - a provider written before per-locale content simply + * ignores it and keeps removing every variant, which is wrong in only one + * direction and never leaves a document behind. + */ + delete: ( + c: Context, + itemType: string, + itemId: number, + languageCode?: string, + ) => Promise; index: (c: Context, doc: SearchDocument) => Promise; name: string; ping?: (c: Context) => Promise; @@ -369,7 +386,19 @@ export class SearchModel { await this.provider().clear(this.c, itemType); } - async delete(itemType: string, itemId: number): Promise { + /** + * Removes one item from the index, in one language or in all of them. + * + * `languageCode` is the whole point of the overload: multi-language content is + * one row per `(itemType, itemId, languageCode)`, so taking the Polish + * translation down must leave the English document exactly where it is. + * Omitting it removes every language, which is what deleting the record means. + */ + async delete( + itemType: string, + itemId: number, + languageCode?: string, + ): Promise { await this.c .get("db") .delete(core_search_index) @@ -377,10 +406,13 @@ export class SearchModel { and( eq(core_search_index.itemType, itemType), eq(core_search_index.itemId, itemId), + languageCode === undefined + ? undefined + : eq(core_search_index.languageCode, languageCode), ), ); - await this.provider().delete(this.c, itemType, itemId); + await this.provider().delete(this.c, itemType, itemId, languageCode); } /** Canonical projection lives in `core_search_index`; the provider mirrors it. */ diff --git a/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.ts b/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.ts index ef2778c9d..20229f4e6 100644 --- a/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.ts +++ b/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.ts @@ -1,4 +1,13 @@ -import { and, countDistinct, desc, eq, like, max } from "drizzle-orm"; +import { + and, + count, + countDistinct, + desc, + eq, + like, + max, + ne, +} from "drizzle-orm"; import { z } from "zod"; import { buildRoute } from "@/api/lib/route"; @@ -12,6 +21,14 @@ const CONTENT_SEARCH_LOG_PREFIX = "[content-search]"; const SYNC_ERROR_LIMIT = 10; const collectionSchema = z.object({ + /** + * Index rows, counting one per language. + * + * Separate from `indexed`, which counts distinct items: multi-language content + * is indexed once per translation, so its coverage has to be measured in + * documents or a fully-indexed collection would read as 33%. + */ + documents: z.number(), /** * Whether an indexer is registered for this item type *right now*. A stored * plugin owner does not imply one: the plugin may be uninstalled, renamed, or @@ -20,6 +37,20 @@ const collectionSchema = z.object({ hasIndexer: z.boolean(), indexed: z.number(), itemType: z.string(), + /** + * One entry per language present in the index, newest first by count. + * + * Empty for a collection that is entirely language-agnostic. It is what makes + * "Polish is missing 40 documents" visible at all - a single total cannot say + * which language a rebuild failed halfway through. + */ + languages: z.array( + z.object({ + documents: z.number(), + languageCode: z.string(), + lastIndexedAt: z.date().nullable(), + }), + ), lastIndexedAt: z.date().nullable(), pluginId: z.string(), /** Source items the indexer reports. `null` when there is no indexer to ask. */ @@ -74,6 +105,7 @@ export const searchStatusDebugAdminRoute = buildRoute({ const indexedByType = await db .select({ itemType: core_search_index.itemType, + documents: count(), indexed: countDistinct(core_search_index.itemId), lastIndexedAt: max(core_search_index.indexedAt), pluginId: max(core_search_index.pluginId), @@ -83,6 +115,36 @@ export const searchStatusDebugAdminRoute = buildRoute({ const statsByType = new Map(indexedByType.map(row => [row.itemType, row])); + // Per language, so a rebuild that stopped halfway through one locale is + // visible as that locale rather than as a slightly-low total. The + // language-agnostic rows (`""`) are dropped: they are not a language, and + // listing them as one would put an unnamed row in every collection. + const byLanguage = await db + .select({ + itemType: core_search_index.itemType, + documents: count(), + languageCode: core_search_index.languageCode, + lastIndexedAt: max(core_search_index.indexedAt), + }) + .from(core_search_index) + .where(ne(core_search_index.languageCode, "")) + .groupBy(core_search_index.itemType, core_search_index.languageCode) + .orderBy(desc(count())); + + const languagesByType = new Map< + string, + { documents: number; languageCode: string; lastIndexedAt: Date | null }[] + >(); + for (const row of byLanguage) { + const entries = languagesByType.get(row.itemType) ?? []; + entries.push({ + documents: row.documents, + languageCode: row.languageCode, + lastIndexedAt: row.lastIndexedAt, + }); + languagesByType.set(row.itemType, entries); + } + // Newest first, and bounded: this is a "what went wrong lately" panel, not a // log viewer. `LIKE 'prefix%'` needs no escaping - the prefix contains // neither `%` nor `_`. @@ -138,7 +200,9 @@ export const searchStatusDebugAdminRoute = buildRoute({ indexer?.pluginId ?? searchDocumentOwner(stats?.pluginId) ?? "unknown", + documents: stats?.documents ?? 0, indexed, + languages: languagesByType.get(itemType) ?? [], // Reported as measured, even when it is below `indexed`: more documents // than source records is a stale index, and raising the source count to // hide it is how that goes unnoticed. The UI clamps the bar instead. diff --git a/packages/vitnode/src/content/const.ts b/packages/vitnode/src/content/const.ts index 5414bc924..e5c0a247a 100644 --- a/packages/vitnode/src/content/const.ts +++ b/packages/vitnode/src/content/const.ts @@ -202,9 +202,21 @@ export const CONTENT_SEARCH_DESCRIPTION_KINDS = ["text", "textarea"] as const; */ export const CONTENT_SEARCH_TEXT_KINDS = ["slug", "text", "textarea"] as const; -/** The only placeholder `search.pathTemplate` may use. */ +/** The placeholder every `search.pathTemplate` must use. */ export const CONTENT_SEARCH_SLUG_PLACEHOLDER = "{slug}"; +/** + * The placeholder a **localized** `search.pathTemplate` must also use. + * + * Required there rather than optional: a localized content type is indexed once + * per language, and two languages routinely answer to the same slug - so a + * template without it would give every translation of a record the same link, and + * a search hit would point at whichever language the reader happened to be in. + * Refused on a content type that is not localized, where it could only ever + * substitute to nothing. + */ +export const CONTENT_SEARCH_LOCALE_PLACEHOLDER = "{locale}"; + /** * `core_search_index.itemType` is `varchar(100)` and a content type id is used * verbatim as the item type, so a longer id would fail at insert time - far from diff --git a/packages/vitnode/src/content/define.ts b/packages/vitnode/src/content/define.ts index dc4e79102..cd19371b3 100644 --- a/packages/vitnode/src/content/define.ts +++ b/packages/vitnode/src/content/define.ts @@ -50,6 +50,7 @@ import { CONTENT_REVISION_MIN_RETENTION, CONTENT_SEARCH_DESCRIPTION_KINDS, CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH, + CONTENT_SEARCH_LOCALE_PLACEHOLDER, CONTENT_SEARCH_PATH_MAX_LENGTH, CONTENT_SEARCH_SLUG_PLACEHOLDER, CONTENT_SEARCH_TEXT_KINDS, @@ -727,7 +728,11 @@ const assertSearchField = ({ } }; -const assertSearchPathTemplate = (id: string, template: string): void => { +const assertSearchPathTemplate = ( + id: string, + template: string, + localized: boolean, +): void => { if (!template.startsWith("/")) { throw new ContentEngineError( `search.pathTemplate "${template}" must start with "/". Search result URLs are relative to the site root.`, @@ -751,17 +756,50 @@ const assertSearchPathTemplate = (id: string, template: string): void => { ); } + // A localized content type is indexed once per language, and two languages + // routinely answer to the same slug - so a template with no `{locale}` would + // give every translation of a record the same link, and a hit would point at + // whichever language the reader happened to be in. + const locales = template.split(CONTENT_SEARCH_LOCALE_PLACEHOLDER).length - 1; + if (localized && locales !== 1) { + throw new ContentEngineError( + `search.pathTemplate "${template}" must contain exactly one "${CONTENT_SEARCH_LOCALE_PLACEHOLDER}" placeholder on a localized content type, not ${locales}. One document per language means one URL per language.`, + { contentTypeId: id }, + ); + } + if (!localized && locales > 0) { + throw new ContentEngineError( + `search.pathTemplate "${template}" uses "${CONTENT_SEARCH_LOCALE_PLACEHOLDER}", but this content type is not localized - there is no language for it to substitute.`, + { contentTypeId: id }, + ); + } + // Everything else that looks like a placeholder is a typo, and substitution is // a single literal replace - so an unvalidated one would end up in the URL. - const rest = template.replace(CONTENT_SEARCH_SLUG_PLACEHOLDER, ""); + const rest = template + .replace(CONTENT_SEARCH_SLUG_PLACEHOLDER, "") + .replace(CONTENT_SEARCH_LOCALE_PLACEHOLDER, ""); if (rest.includes("{") || rest.includes("}")) { throw new ContentEngineError( - `search.pathTemplate "${template}" uses a placeholder other than "${CONTENT_SEARCH_SLUG_PLACEHOLDER}". No other placeholder is supported.`, + `search.pathTemplate "${template}" uses a placeholder other than "${CONTENT_SEARCH_SLUG_PLACEHOLDER}"${localized ? ` and "${CONTENT_SEARCH_LOCALE_PLACEHOLDER}"` : ""}. No other placeholder is supported.`, { contentTypeId: id }, ); } - if (rest.includes("//") || template.includes("..") || /\s/.test(template)) { + // Substituted with a non-empty token rather than removed, so a template whose + // segments are placeholders (`/{locale}/articles/{slug}`) is not mistaken for + // one with an empty segment. + const structural = template + .split(CONTENT_SEARCH_SLUG_PLACEHOLDER) + .join("x") + .split(CONTENT_SEARCH_LOCALE_PLACEHOLDER) + .join("x"); + + if ( + structural.includes("//") || + template.includes("..") || + /\s/.test(template) + ) { throw new ContentEngineError( `search.pathTemplate "${template}" must not contain an empty segment, "..", or whitespace.`, { contentTypeId: id }, @@ -781,6 +819,7 @@ const resolveSearch = ( search: ContentSearchConfig | undefined, publicApi: ResolvedContentPublicApiConfig, publication: boolean, + localized: boolean, ): ResolvedContentSearchConfig => { if (!search?.enabled) return disabledSearch; @@ -875,7 +914,7 @@ const resolveSearch = ( }); } - assertSearchPathTemplate(id, search.pathTemplate); + assertSearchPathTemplate(id, search.pathTemplate, localized); return { contentFields, @@ -1269,6 +1308,7 @@ export const defineContentType = < search as ContentSearchConfig | undefined, resolvedPublicApi, publicationEnabled, + Object.keys(localizedFields).length > 0, ); const resolvedEditorial = resolveEditorial( @@ -1280,8 +1320,9 @@ export const defineContentType = < publicationEnabled, ); - // Last, because the Stage 5C boundary it enforces is stated in terms of - // everything the other resolvers have already settled. + // Last, because it reads the field partition every other resolver has already + // been checked against. There is no capability it refuses any more: every + // subsystem reads the language it was asked for. const resolvedLocalization = resolveContentLocalization({ fields: fieldMap, id, @@ -1289,7 +1330,6 @@ export const defineContentType = < // typechecks - the same widening `publicApi`, `search` and `editorial` do. localization: localization as ContentLocalizationConfig | undefined, publication: publicationEnabled, - search: resolvedSearch.enabled, tableName, }); diff --git a/packages/vitnode/src/content/index.ts b/packages/vitnode/src/content/index.ts index 9a47c2cf2..7ba73c95a 100644 --- a/packages/vitnode/src/content/index.ts +++ b/packages/vitnode/src/content/index.ts @@ -98,6 +98,7 @@ export { CONTENT_REVISION_SNAPSHOT_VERSION, CONTENT_SEARCH_DESCRIPTION_KINDS, CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH, + CONTENT_SEARCH_LOCALE_PLACEHOLDER, CONTENT_SEARCH_PATH_MAX_LENGTH, CONTENT_SEARCH_SLUG_PLACEHOLDER, CONTENT_SEARCH_TEXT_KINDS, @@ -286,6 +287,7 @@ export type { LocalizedContentTypeDefinition, PreviewableContentTypeDefinition, PublicContentTypeDefinition, + PublicFilterableContentFieldName, ResolvedContentAdminConfig, ResolvedContentEditorialConfig, ResolvedContentIndex, diff --git a/packages/vitnode/src/content/localization.test.ts b/packages/vitnode/src/content/localization.test.ts index 52c977ba8..7cb518858 100644 --- a/packages/vitnode/src/content/localization.test.ts +++ b/packages/vitnode/src/content/localization.test.ts @@ -329,7 +329,7 @@ describe("localization validation", () => { }); }); -describe("Stage 5B capability boundaries", () => { +describe("capability combinations", () => { const withCapability = (extra: Record) => defineContentType({ id: "test.boundary", @@ -376,27 +376,29 @@ describe("Stage 5B capability boundaries", () => { ); }); - it("refuses localization plus search until Stage 5D", () => { - expect(() => - withCapability({ - publication: { enabled: true }, - publicApi: { - enabled: true, - fields: ["title", "slug"], - path: "boundaries", - }, - search: { - contentFields: ["title"], - enabled: true, - pathTemplate: "/boundaries/{slug}", - titleField: "title", - }, - }), - ).toThrow(); + it("allows localization plus search from Stage 5D", () => { + const definition = withCapability({ + publication: { enabled: true }, + publicApi: { + enabled: true, + fields: ["title", "slug"], + path: "boundaries", + }, + search: { + contentFields: ["title"], + enabled: true, + pathTemplate: "/{locale}/boundaries/{slug}", + titleField: "title", + }, + }); + + expect(definition.search.enabled).toBe(true); + expect(definition.localization.enabled).toBe(true); }); - it("names the stage in the one remaining boundary message", () => { - // "Not yet" is only useful when it says how long. + it("requires a locale in the search path template", () => { + // One document per language means one URL per language. Without it every + // translation of a record would carry the same link. expect(() => withCapability({ publication: { enabled: true }, @@ -412,7 +414,7 @@ describe("Stage 5B capability boundaries", () => { titleField: "title", }, }), - ).toThrow(/Stage 5D/); + ).toThrow(/\{locale\}/); }); }); diff --git a/packages/vitnode/src/content/localization.ts b/packages/vitnode/src/content/localization.ts index 1cff4e73f..e3f02ccd7 100644 --- a/packages/vitnode/src/content/localization.ts +++ b/packages/vitnode/src/content/localization.ts @@ -185,29 +185,17 @@ const assertLocalizedFields = ( }; /** - * Stage 5C boundaries. + * There is no capability boundary left. * - * Stage 5A landed the infrastructure, Stage 5B the editorial layer and Stage 5C - * the public read: locale precedence, fallback, strict-locale slugs and - * locale-aware cache tags. One thing still reads outwards without knowing about - * languages - the search index - and the honest failure for that is a refused - * definition rather than a content type that quietly indexes one language and - * ranks every other one as a miss. + * Stage 5A landed the infrastructure, Stage 5B the editorial layer, Stage 5C the + * public read and Stage 5D per-locale search. `localization` now combines with + * `publication`, `editorial`, `publicApi` and `search`, and every one of them + * reads the language it was asked for rather than pretending there is only one. * - * The message names the stage that lifts the restriction, because "not yet" is - * only useful when it says how long. + * The function is gone rather than left as an empty stub: a boundary that refuses + * nothing is a comment, and a comment is where the next one would be added + * silently. */ -const assertStageBoundaries = ( - id: string, - { search }: { search: boolean }, -): void => { - if (search) { - throw new ContentEngineError( - "localization cannot be combined with `search` yet. One document per record would index a single language and rank every other one as a miss; per-locale search documents land in Stage 5D.", - { contentTypeId: id }, - ); - } -}; /** * Checks and fills in `localization`. @@ -222,14 +210,12 @@ export const resolveContentLocalization = ({ id, localization, publication, - search, tableName, }: { fields: ContentFieldMap; id: string; localization: ContentLocalizationConfig | undefined; publication: boolean; - search: boolean; tableName: string; }): ResolvedContentLocalizationConfig => { const { localizedFields } = partitionContentFields(fields); @@ -246,8 +232,6 @@ export const resolveContentLocalization = ({ return contentLocalizationDisabled(); } - assertStageBoundaries(id, { search }); - const defaultLocale = assertDefaultLocale(id, localization.defaultLocale); assertLocalizedFields(id, fields, localizedFields); diff --git a/packages/vitnode/src/content/search.ts b/packages/vitnode/src/content/search.ts index 3978e093c..f5f2ac8db 100644 --- a/packages/vitnode/src/content/search.ts +++ b/packages/vitnode/src/content/search.ts @@ -1,6 +1,9 @@ import type { AnyContentTypeDefinition } from "./types"; -import { CONTENT_SEARCH_SLUG_PLACEHOLDER } from "./const"; +import { + CONTENT_SEARCH_LOCALE_PLACEHOLDER, + CONTENT_SEARCH_SLUG_PLACEHOLDER, +} from "./const"; /** * The public URL of one record, for a search hit. @@ -17,14 +20,30 @@ import { CONTENT_SEARCH_SLUG_PLACEHOLDER } from "./const"; export const contentSearchUrl = ( definition: AnyContentTypeDefinition, slug: string, + locale?: string, ): null | string => { const trimmed = slug.trim(); if (trimmed === "" || definition.search.pathTemplate === "") return null; - return definition.search.pathTemplate.replace( + const localized = definition.localization.enabled; + const language = locale?.trim() ?? ""; + + // A localized content type has one document per language and one URL per + // language. Without a locale there is no URL to build, and a link to the wrong + // language is worse than no link at all. + if (localized && language === "") return null; + + const withSlug = definition.search.pathTemplate.replace( CONTENT_SEARCH_SLUG_PLACEHOLDER, encodeURIComponent(trimmed), ); + + return localized + ? withSlug.replace( + CONTENT_SEARCH_LOCALE_PLACEHOLDER, + encodeURIComponent(language), + ) + : withSlug; }; /** @@ -39,7 +58,11 @@ export const contentSearchUrl = ( export const contentSearchDocumentId = ( definition: AnyContentTypeDefinition, id: number, -): string => `${definition.id}:${id}`; + locale?: string, +): string => + locale === undefined || locale === "" + ? `${definition.id}:${id}` + : `${definition.id}:${id}:${locale}`; /** * Every field whose value the search document is built from, including the slug diff --git a/packages/vitnode/src/content/server/editorial-effects.ts b/packages/vitnode/src/content/server/editorial-effects.ts index 567ab8933..2fb584253 100644 --- a/packages/vitnode/src/content/server/editorial-effects.ts +++ b/packages/vitnode/src/content/server/editorial-effects.ts @@ -6,8 +6,10 @@ import type { AnyContentTypeDefinition } from "../types"; import type { ContentEditorialOutcome } from "./editorial-service"; import type { ContentSearchSyncOutcome } from "./search-sync"; +import type { AnyContentModel } from "./model"; + import { emitContentEvent } from "./emit"; -import { syncContentSearch } from "./search-sync"; +import { syncContentLocalizedSearch, syncContentSearch } from "./search-sync"; /** A `delete` has no event action of its own beyond the existing one. */ const EVENT_ACTION: Record< @@ -65,6 +67,16 @@ const payloadFor = ( }; export interface ContentEditorialEffectsOptions { + /** + * The model, for a **localized** content type with `search`. + * + * Needed because such a record is indexed once per published translation, and + * enumerating them takes a table this function is not otherwise given. Optional + * so every existing caller compiles unchanged; a localized searchable content + * type whose caller omits it has its index write skipped and says so in the log, + * rather than silently indexing one language. + */ + model?: AnyContentModel; /** The plugin that owns the content type, and therefore the event. */ pluginId: string; /** @@ -86,6 +98,11 @@ export interface ContentEditorialEffectsOptions { } export interface ContentEditorialEffectsResult { + /** + * One outcome per language, for a localized content type. Empty for every + * other one, whose single outcome is on `search`. + */ + searchByLocale?: ContentSearchSyncOutcome[]; /** * What the event transport reported. `null` for a no-op outcome, which emits * nothing at all. @@ -124,7 +141,7 @@ export const contentEditorialEffects = async ( c: Context, definition: AnyContentTypeDefinition, outcome: ContentEditorialOutcome, - { pluginId, scheduledBy, scheduleId }: ContentEditorialEffectsOptions, + { model, pluginId, scheduledBy, scheduleId }: ContentEditorialEffectsOptions, ): Promise => { if (!outcome.changed) return { event: null, search: null }; @@ -140,6 +157,25 @@ export const contentEditorialEffects = async ( { pluginId }, ); + // A localized record is indexed once per published translation, and a mutation + // of the *record* moves every one of them: its publication state gates them + // all, and a shared field is in all of them. + if (definition.localization.enabled && definition.search.enabled) { + return { + event, + search: null, + searchByLocale: model + ? await syncContentLocalizedSearch(c, model, { + changed: outcome.changed, + changedFields: outcome.changedFields, + operation: outcome.operation, + pluginId, + row: outcome.row, + }) + : await warnMissingModel(c, definition), + }; + } + return { event, search: await syncContentSearch(c, definition, { @@ -151,3 +187,28 @@ export const contentEditorialEffects = async ( }), }; }; + +/** + * Says why nothing was indexed, rather than indexing the wrong thing. + * + * Reachable only from a hand-written caller: every generated path passes the + * model. Logging beats throwing here because the write has already committed - + * failing now would report a successful mutation as a failure - and it beats + * silence because the symptom otherwise is a search index that is quietly missing + * one content type. + */ +const warnMissingModel = async ( + c: Context, + definition: AnyContentTypeDefinition, +): Promise => { + const message = `[content-search] ${definition.id} is localized and searchable, but contentEditorialEffects was called without \`model\`, so no document was written. Pass the content model.`; + + try { + await c.get("log").error(message); + } catch { + // eslint-disable-next-line no-console + console.error(`[VitNode] ${message}`); + } + + return []; +}; diff --git a/packages/vitnode/src/content/server/index.ts b/packages/vitnode/src/content/server/index.ts index ee5aa9653..3e95553ac 100644 --- a/packages/vitnode/src/content/server/index.ts +++ b/packages/vitnode/src/content/server/index.ts @@ -149,11 +149,18 @@ export type { ClaimedContentSchedule, ContentSchedulesModel, } from "./schedules-model"; -export { contentSearchDocument } from "./search-document"; -export { createContentSearchIndexer } from "./search-indexer"; +export { + contentSearchDocument, + contentTranslationSearchDocument, +} from "./search-document"; +export { + createContentLocalizedSearchIndexer, + createContentSearchIndexer, +} from "./search-indexer"; export type { ContentSearchIndexer } from "./search-indexer"; -export { syncContentSearch } from "./search-sync"; +export { syncContentLocalizedSearch, syncContentSearch } from "./search-sync"; export type { + ContentLocalizedSearchSyncInput, ContentSearchOperation, ContentSearchSyncInput, ContentSearchSyncOutcome, diff --git a/packages/vitnode/src/content/server/localized-search.test.ts b/packages/vitnode/src/content/server/localized-search.test.ts new file mode 100644 index 000000000..35dfe165a --- /dev/null +++ b/packages/vitnode/src/content/server/localized-search.test.ts @@ -0,0 +1,164 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; + +import { + testLocalizedSearchPageContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { contentSearchUrl } from "../search"; +import { + contentSearchDocument, + contentTranslationSearchDocument, +} from "./search-document"; + +const PAST = new Date("2020-01-01T00:00:00.000Z"); +const LATER = new Date("2020-06-01T00:00:00.000Z"); + +const base = { + createdAt: PAST, + featured: true, + id: 7, + publishedAt: PAST, + status: "published", + updatedAt: PAST, +}; + +const translation = { + body: "Treść po polsku", + publishedAt: LATER, + slug: "witaj", + status: "published", + title: "Witaj", + updatedAt: LATER, +}; + +const document = ( + overrides: { + base?: Record; + locale?: string; + translation?: Record; + } = {}, +) => + contentTranslationSearchDocument( + testLocalizedSearchPageContentType, + { + base: { ...base, ...overrides.base }, + locale: overrides.locale ?? "pl", + translation: { ...translation, ...overrides.translation }, + }, + { pluginId: "@vitnode/example" }, + ); + +describe("contentSearchUrl on a localized content type", () => { + it("substitutes the language as well as the slug", () => { + expect( + contentSearchUrl(testLocalizedSearchPageContentType, "witaj", "pl"), + ).toBe("/pl/pages/witaj"); + }); + + it("refuses to build one without a language", () => { + // One document per language means one URL per language. A link to the wrong + // language is worse than no link. + expect( + contentSearchUrl(testLocalizedSearchPageContentType, "witaj"), + ).toBeNull(); + }); + + it("encodes both, so neither can escape its segment", () => { + expect( + contentSearchUrl(testLocalizedSearchPageContentType, "a/b", "pt-BR"), + ).toBe("/pt-BR/pages/a%2Fb"); + }); + + it("ignores a language on a content type that has none", () => { + expect(contentSearchUrl(testSearchablePostContentType, "hello", "pl")).toBe( + "/searchable/hello", + ); + }); +}); + +describe("contentTranslationSearchDocument", () => { + it("builds one document from both halves of the page", () => { + expect(document()).toMatchObject({ + itemId: 7, + itemType: "test.localized-search-page", + languageCode: "pl", + title: "Witaj", + url: "/pl/pages/witaj", + }); + }); + + it("indexes the localized prose, not the base row's", () => { + expect(document()?.content).toContain("Treść po polsku"); + }); + + it("dates the document by this language's publication", () => { + // A translation published months later belongs where it appeared in "newest", + // not where its record did. + expect(document()?.createdAt).toEqual(LATER); + }); + + it("refuses a draft translation of a published record", () => { + expect( + document({ translation: { publishedAt: null, status: "draft" } }), + ).toBeNull(); + }); + + it("refuses a published translation of a draft record", () => { + // Subordination: nothing is public in any language while the record is a + // draft, so nothing is indexed either. + expect( + document({ base: { publishedAt: null, status: "draft" } }), + ).toBeNull(); + }); + + it("refuses a future publication date on either half", () => { + const future = new Date(Date.now() + 60_000); + + expect(document({ base: { publishedAt: future } })).toBeNull(); + expect(document({ translation: { publishedAt: future } })).toBeNull(); + }); + + it("refuses a translation with no usable title", () => { + // Published, and still not indexable - which is why the sync deletes its + // document rather than leaving whatever it held last time. + expect(document({ translation: { title: " " } })).toBeNull(); + }); + + it("carries the shared fields too", () => { + // `featured` lives on the base row, and the document is the page - so a + // filterable shared value is part of what was indexed. + expect(document()).not.toBeNull(); + }); + + it("returns nothing for a content type that is not localized", () => { + expect( + contentTranslationSearchDocument( + testSearchablePostContentType, + { base, locale: "pl", translation }, + {}, + ), + ).toBeNull(); + }); +}); + +describe("contentSearchDocument locale", () => { + it("leaves `languageCode` off a content type with no languages", () => { + // `""` is the language-agnostic value that matches every locale, and it is + // what every document written before Stage 5D already carries. + const built = contentSearchDocument(testSearchablePostContentType, { + createdAt: PAST, + excerpt: "Prose", + id: 1, + publishedAt: PAST, + slug: "hello", + status: "published", + title: "Hello", + updatedAt: PAST, + }); + + expect(built).not.toBeNull(); + expect(built).not.toHaveProperty("languageCode"); + }); +}); diff --git a/packages/vitnode/src/content/server/module.ts b/packages/vitnode/src/content/server/module.ts index 08265a407..95cd394e7 100644 --- a/packages/vitnode/src/content/server/module.ts +++ b/packages/vitnode/src/content/server/module.ts @@ -4,7 +4,10 @@ import type { ContentModel } from "./model"; import { buildModule } from "../../api/lib/module"; import { buildContentRoutes } from "./routes"; -import { createContentSearchIndexer } from "./search-indexer"; +import { + createContentLocalizedSearchIndexer, + createContentSearchIndexer, +} from "./search-indexer"; import { assertContentReferences } from "./table"; /** @@ -65,6 +68,14 @@ export const buildContentAdminModule =

({ // the owning plugin on every document. searchIndexers: contentTypes .filter(model => model.definition.search.enabled) - .map(model => createContentSearchIndexer(model, { pluginId })), + // A localized content type is indexed once per published translation, so + // its rebuild pages over translations rather than over records. Chosen here + // rather than inside one indexer because the two page differently - + // keyset over `(itemId, languageId)` against offset over `id`. + .map(model => + model.definition.localization.enabled + ? createContentLocalizedSearchIndexer(model, { pluginId }) + : createContentSearchIndexer(model, { pluginId }), + ), }); }; diff --git a/packages/vitnode/src/content/server/routes.ts b/packages/vitnode/src/content/server/routes.ts index 93b3ffaaf..13d260376 100644 --- a/packages/vitnode/src/content/server/routes.ts +++ b/packages/vitnode/src/content/server/routes.ts @@ -23,17 +23,20 @@ import { } from "../conflicts"; import { CONTENT_ACTOR_TYPES, + CONTENT_LOCALE_MAX_LENGTH, CONTENT_OPTIONS_LIMIT, CONTENT_PERMISSIONS, CONTENT_REVISION_OPERATIONS, CONTENT_SCHEDULE_ACTIONS, CONTENT_SCHEDULE_STATUSES, } from "../const"; +import { partitionContentFields } from "../localization"; import { orderableColumns } from "../registry"; import { resolveContentActor } from "./actor"; import { contentEditorialEffects } from "./editorial-effects"; import { emitContentEvent } from "./emit"; import { withHttpErrors } from "./http-errors"; +import { findContentLanguage } from "./language-resolver"; import { assertContentPreviewIsServable, contentPreviewSecret, @@ -83,7 +86,29 @@ export const buildContentRoutes = < const module = definition.permissionModule; const label = definition.admin.label; - const listRow = schemas.selectObject.extend({ labels: zodLabels }); + const localized = definition.localization.enabled; + + /** + * One row's state in the language the list is being viewed in. + * + * `null` when that language has no translation, which is a state the table + * renders as `Missing` rather than as an error. Present only on a localized + * content type, so every other list response is unchanged. + */ + const zodRowTranslation = z + .object({ + locale: z.string(), + publishedAt: z.date().nullable().optional(), + status: z.string().optional(), + title: z.string(), + version: z.number(), + }) + .nullable(); + + const listRow = schemas.selectObject.extend({ + labels: zodLabels, + ...(localized ? { translation: zodRowTranslation.optional() } : {}), + }); const publicationResponse = z.object({ /** `false` when the record was already in the requested state. */ changed: z.boolean(), @@ -175,7 +200,23 @@ export const buildContentRoutes = < method: "get", path: "/", description: `List ${label.plural}`, - request: { query: paginationQuery.extend(schemas.filters.shape) }, + request: { + query: paginationQuery.extend({ + ...schemas.filters.shape, + // The language the list is being *viewed* in. It never filters: an + // admin list is a list of records, and hiding the ones a translator has + // not reached yet is the opposite of what the selector is for. + ...(localized + ? { + locale: z + .string() + .min(1) + .max(CONTENT_LOCALE_MAX_LENGTH) + .optional(), + } + : {}), + }), + }, responses: { 200: jsonResponse( z.object({ @@ -222,10 +263,96 @@ export const buildContentRoutes = < query: { cursor, first, last, search }, }); - return c.json(data, 200); + return c.json(await withRowTranslations(c, data, raw.locale), 200); }, }); + /** + * Attaches each row's translation in the language the list is being viewed in. + * + * One extra query for the whole page rather than a join, and deliberately so: + * the list is a query over the base table and its pagination, ordering and + * filters are all defined there. Joining a translation in would make the page + * size depend on how many languages a record has - and adding it afterwards + * keeps every existing list behaving exactly as it did. + * + * A locale with no translation comes back as `null` rather than being dropped: + * an admin list is a list of *records*, and the whole point of the selector is + * to see which ones a translator has not reached yet. + */ + const withRowTranslations = async ( + c: Context, + data: { edges: { id: number }[]; pageInfo: unknown }, + locale: string | undefined, + ) => { + const build = model.translationService; + if (!localized || !build || locale === undefined || locale.trim() === "") { + return data; + } + + const language = await findContentLanguage(c, locale); + // An unknown locale reads as "no translation in that language" rather than + // as an error: the selector is a view control, and a stale bookmark naming a + // language that has since been removed should still show the list. + if (!language) { + return { + ...data, + edges: data.edges.map(row => ({ ...row, translation: null })), + }; + } + + const translations = build(c); + // The first localized text field, in declaration order - the same rule the + // locale editor's tab titles follow, so the list and the editor agree about + // which value names a translation. + const titleField = + Object.entries( + partitionContentFields(definition.fields).localizedFields, + ).find(([, fieldValue]) => fieldValue.kind === "text")?.[0] ?? null; + + const rows = await Promise.all( + data.edges.map( + async row => + [ + row.id, + await translations.findByLanguageId(row.id, language.id), + ] as const, + ), + ); + const byId = new Map(rows); + + return { + ...data, + edges: data.edges.map(row => { + const translation = byId.get(row.id); + if (!translation) return { ...row, translation: null }; + + const values = translation.values as Record; + const meta = translation as unknown as Record; + + return { + ...row, + translation: { + locale: translation.locale, + ...(definition.publication.enabled + ? { + publishedAt: meta.publishedAt as Date | null, + status: meta.status as string, + } + : {}), + title: + titleField === null + ? "" + : typeof values[titleField] === "string" + ? values[titleField] + : "", + version: translation.version, + }, + }; + }), + }; + }; + const options = buildRoute({ pluginId, adminStaffPermission: { module, permission: CONTENT_PERMISSIONS.view }, @@ -313,7 +440,10 @@ export const buildContentRoutes = < { contentTypeId: definition.id, structured: true }, ); - await contentEditorialEffects(c, definition, result, { pluginId }); + await contentEditorialEffects(c, definition, result, { + model, + pluginId, + }); return c.json(result.row, 201); } @@ -398,7 +528,7 @@ export const buildContentRoutes = < // One call rather than an event branch plus a search branch: which event // and which search operation an outcome deserves is a rule, and it is // stated once, in `contentEditorialEffects`. - await contentEditorialEffects(c, definition, result, { pluginId }); + await contentEditorialEffects(c, definition, result, { model, pluginId }); return c.json(result.row, 200); }, @@ -495,7 +625,10 @@ export const buildContentRoutes = < // Still idempotent: a no-op outcome emits nothing, indexes nothing // and - because the version did not move - leaves no revision. - await contentEditorialEffects(c, definition, result, { pluginId }); + await contentEditorialEffects(c, definition, result, { + model, + pluginId, + }); return c.json({ changed: result.changed, row: result.row }, 200); } @@ -700,7 +833,7 @@ export const buildContentRoutes = < throw new HTTPException(404, { message: "Revision not found." }); } - await contentEditorialEffects(c, definition, result, { pluginId }); + await contentEditorialEffects(c, definition, result, { model, pluginId }); return c.json({ changed: result.changed, row: result.row }, 200); }, @@ -1043,7 +1176,7 @@ export const buildContentRoutes = < ); if (!result) throw notFound(definition); - await contentEditorialEffects(c, definition, result, { pluginId }); + await contentEditorialEffects(c, definition, result, { model, pluginId }); return c.json(result.row, 200); }, diff --git a/packages/vitnode/src/content/server/schedule-effects.test.ts b/packages/vitnode/src/content/server/schedule-effects.test.ts index aeac23b82..904470b2f 100644 --- a/packages/vitnode/src/content/server/schedule-effects.test.ts +++ b/packages/vitnode/src/content/server/schedule-effects.test.ts @@ -126,7 +126,7 @@ describe("runContentScheduleEffects", () => { await runContentScheduleEffects(c, payload()); - expect(contentEditorialEffects.mock.calls[0][3]).toEqual({ + expect(contentEditorialEffects.mock.calls[0][3]).toMatchObject({ pluginId: PLUGIN_ID, scheduledBy: 3, scheduleId: 55, diff --git a/packages/vitnode/src/content/server/schedule-effects.ts b/packages/vitnode/src/content/server/schedule-effects.ts index 277a5ac51..67a4a96d6 100644 --- a/packages/vitnode/src/content/server/schedule-effects.ts +++ b/packages/vitnode/src/content/server/schedule-effects.ts @@ -187,11 +187,15 @@ export const runContentScheduleEffects = async ( // The same helper the interactive routes use, so a scheduled publish and a // clicked one are indistinguishable to every listener and to the index. - const { event, search } = await contentEditorialEffects( + const { event, search, searchByLocale } = await contentEditorialEffects( c, definition, outcome, { + // How a localized record is enumerated into one document per published + // translation. The queue handler has the model because it looked the + // content type up to get here. + model: entry.model, // The content type's owner, not core - core only owns the queue handler // that happens to be running. `entry.pluginId` is the same value the // executor froze into the payload, and both are read back rather than @@ -248,6 +252,15 @@ export const runContentScheduleEffects = async ( if (search?.error) failures.push(`search: ${search.error.message}`); + // One outcome per language on a localized content type, and one failure there + // is enough to fail the run: a document that was not written is a record that + // is missing from search in that language until the next rebuild. + for (const outcome of searchByLocale ?? []) { + if (outcome.error) { + failures.push(`search (${outcome.documentId}): ${outcome.error.message}`); + } + } + // Every configured origin has to accept it. A partial delivery is the // dangerous case, not the acceptable one: with two web apps behind one API, // one of them accepting an unpublish while the other does not leaves the diff --git a/packages/vitnode/src/content/server/search-document.ts b/packages/vitnode/src/content/server/search-document.ts index a3c77fc36..3f320148a 100644 --- a/packages/vitnode/src/content/server/search-document.ts +++ b/packages/vitnode/src/content/server/search-document.ts @@ -1,7 +1,11 @@ import type { SearchDocument } from "../../api/models/search"; import type { AnyContentTypeDefinition } from "../types"; -import { isContentPubliclyVisible } from "../cache"; +import { + isContentPubliclyVisible, + isContentTranslationPubliclyVisible, +} from "../cache"; +import { partitionContentFields } from "../localization"; import { contentSearchUrl } from "../search"; /** Collapses whitespace so a multi-line value cannot break a result heading. */ @@ -65,7 +69,7 @@ export const isContentRowPublic = (row: object): boolean => { export const contentSearchDocument = ( definition: AnyContentTypeDefinition, row: object, - { pluginId }: { pluginId?: string } = {}, + { locale, pluginId }: { locale?: string; pluginId?: string } = {}, ): null | SearchDocument => { const { publicApi, search } = definition; if (!search.enabled) return null; @@ -83,6 +87,7 @@ export const contentSearchDocument = ( const url = contentSearchUrl( definition, normalize(values[publicApi.slugField]), + locale, ); if (url === null) return null; @@ -119,13 +124,86 @@ export const contentSearchDocument = ( // The owning plugin, so a rebuild - which runs in the core cron request - // stores the same ownership a live write did. ...(pluginId === undefined ? {} : { pluginId }), + // `""` for a content type that is not localized - the language-agnostic + // value that matches every locale, and what every document written before + // Stage 5D already carries. A localized one is indexed once per language. + ...(locale === undefined || locale === "" ? {} : { languageCode: locale }), // Deliberately absent: `authorId` (a `user` field can never be public, and // the public search route resolves it into a person), `containerType` / // `containerId` (there is no `containerType` query filter to qualify them - // with), and `metadata` (nothing reads it). Also `languageCode`, which - // defaults to "" - the language-agnostic value that matches every locale. + // with), and `metadata` (nothing reads it). title, updatedAt: toDate(values.updatedAt), url, }; }; + +/** + * Projects one *translation* into a search document. + * + * A localized record is indexed **once per published translation**, and each + * document is the two halves of the page the reader would land on: shared values + * off the base row, localized ones off the translation. Indexing the base row + * alone would put one language in the index and rank every other one as a miss. + * + * Visibility is {@link isContentTranslationPubliclyVisible} - the base row *and* + * the translation both published - which is the same subordination the public + * read enforces in SQL. A translation is never indexed for a draft record, in any + * language. + * + * `createdAt` is this language's publication date when it has one, so "newest" + * sorts a late translation where it actually appeared rather than where its + * record did. + * + * Returns `null` for everything that must not be indexed, so every caller has one + * decision to make: "make sure nothing is indexed for this record in this + * language". + */ +export const contentTranslationSearchDocument = ( + definition: AnyContentTypeDefinition, + { + base, + locale, + translation, + }: { base: object; locale: string; translation: object }, + { pluginId }: { pluginId?: string } = {}, +): null | SearchDocument => { + if (!definition.search.enabled || !definition.localization.enabled) { + return null; + } + + const baseValues = base as Record; + const translationValues = translation as Record; + + if ( + !isContentTranslationPubliclyVisible({ + base: { + publishedAt: toTimestamp(baseValues.publishedAt), + status: normalize(baseValues.status) || undefined, + }, + translation: { + publishedAt: toTimestamp(translationValues.publishedAt), + status: normalize(translationValues.status) || undefined, + }, + }) + ) { + return null; + } + + const { localizedFields } = partitionContentFields(definition.fields); + // The localized half wins, and only for declared localized fields: a + // translation row also carries its own `createdAt`, `version` and publication + // state, and letting those through would describe the translation rather than + // the page. + const merged: Record = { ...baseValues }; + for (const name of Object.keys(localizedFields)) { + merged[name] = translationValues[name]; + } + + // `publishedAt` decides the document's date, and this language's is the honest + // one. The status is already known to be published on both halves. + merged.publishedAt = translationValues.publishedAt ?? baseValues.publishedAt; + merged.updatedAt = translationValues.updatedAt ?? baseValues.updatedAt; + + return contentSearchDocument(definition, merged, { locale, pluginId }); +}; diff --git a/packages/vitnode/src/content/server/search-indexer.ts b/packages/vitnode/src/content/server/search-indexer.ts index 3a95739d1..acee52aa9 100644 --- a/packages/vitnode/src/content/server/search-indexer.ts +++ b/packages/vitnode/src/content/server/search-indexer.ts @@ -1,11 +1,13 @@ +import type { SQL } from "drizzle-orm"; import type { PgColumn, + PgTable, PgTableWithColumns, TableConfig, } from "drizzle-orm/pg-core"; import type { Context } from "hono"; -import { asc, count } from "drizzle-orm"; +import { and, asc, count, eq, gt, or } from "drizzle-orm"; import type { SearchDocument, @@ -15,9 +17,19 @@ import type { import type { AnyContentTypeDefinition } from "../types"; import type { ContentModel } from "./model"; +import { ContentEngineError } from "../errors"; +import { partitionContentFields } from "../localization"; import { contentSearchIndexedFieldNames } from "../search"; -import { publicationColumns, publishedCondition } from "./publication"; -import { contentSearchDocument } from "./search-document"; +import { listContentLanguages } from "./language-resolver"; +import { + contentTranslationPublicationColumns, + publicationColumns, + publishedCondition, +} from "./publication"; +import { + contentSearchDocument, + contentTranslationSearchDocument, +} from "./search-document"; /** System columns the mapper reads, on top of the configured search fields. */ const REQUIRED_COLUMNS = [ @@ -129,3 +141,187 @@ export const createContentSearchIndexer = < }, }; }; + +/** + * The localized rebuild: one document per **published translation**. + * + * Two things make it a different function rather than a flag on the one above: + * + * 1. **The unit of paging is a translation, not a record.** `itemsRead` has to + * count translation rows, or a record with three languages would advance the + * offset by one and the rebuild would read it again forever. + * 2. **Paging is keyset, not offset.** The cursor is `(itemId, languageId)`, + * which is the translation table's primary key, so a page can neither overlap + * nor skip while rows are being published underneath it - and Postgres seeks + * to it on the index rather than counting past every earlier row, which is + * what makes a rebuild of a large table finish in linear time. + * + * The `offset` the contract hands in is used only as a *position counter*: the + * cursor is derived from the previous page's last row and kept here, keyed by + * the request, so the contract stays unchanged for every existing indexer. + * + * Both halves of the visibility rule are in the query: the base row's published + * predicate and the translation's. A translation of a draft record is not read, + * so it can never be indexed. + */ +export const createContentLocalizedSearchIndexer = < + TDefinition extends AnyContentTypeDefinition, +>( + model: ContentModel, + { pluginId }: { pluginId: string }, +): ContentSearchIndexer => { + const { definition } = model; + const table: PgTableWithColumns = model.table; + const translationTable: null | PgTable = model.translationTable; + const columns = model.columns as Record; + const translationColumns: null | Record = + model.translationColumns; + + if (!translationTable || !translationColumns) { + throw new ContentEngineError( + "The localized search indexer needs `localization: { enabled: true, defaultLocale }` on the content type.", + { contentTypeId: definition.id }, + ); + } + + const rows = translationColumns; + const base = publicationColumns(definition, columns); + const translation = contentTranslationPublicationColumns( + definition, + translationColumns, + ); + + const { localizedFields, sharedFields } = partitionContentFields( + definition.fields, + ); + const indexed = contentSearchIndexedFieldNames(definition); + const sharedSelection = [ + ...new Set([ + ...REQUIRED_COLUMNS, + ...indexed.filter(name => sharedFields[name] !== undefined), + ]), + ]; + const localizedSelection = indexed.filter( + name => localizedFields[name] !== undefined, + ); + + const visible = (): SQL | undefined => + and(publishedCondition(base), publishedCondition(translation)); + + /** + * The keyset cursor, per request. + * + * A `WeakMap` keyed by the Hono context, for the same reason the language + * registry uses one: the rebuild task calls `load` repeatedly within one + * request, and the entry is collected with it. A fresh request starts at the + * beginning, which is what a rebuild means. + */ + const cursors = new WeakMap< + Context, + { itemId: number; languageId: number } + >(); + + return { + itemType: definition.id, + + // Published *translations*, not published records: the coverage bar compares + // this against the number of indexed documents, and a record counts once per + // language it is actually readable in. + count: async c => { + const [row] = await c + .get("db") + .select({ value: count() }) + .from(translationTable) + .innerJoin(table, eq(rows.itemId, columns.id)) + .where(visible()); + + return row?.value ?? 0; + }, + + load: async (c, offset, limit) => { + // `offset === 0` is the start of a rebuild - the contract's only signal + // that this is a fresh pass rather than the next page of one. + if (offset === 0) cursors.delete(c); + const cursor = cursors.get(c); + + const page = await c + .get("db") + .select({ + ...Object.fromEntries( + sharedSelection.map(name => [name, columns[name]]), + ), + _languageId: rows.languageId, + _publishedAt: rows.publishedAt, + _status: rows.status, + _updatedAt: rows.updatedAt, + ...Object.fromEntries( + localizedSelection.map(name => [`t_${name}`, rows[name]]), + ), + }) + .from(translationTable) + .innerJoin(table, eq(rows.itemId, columns.id)) + .where( + cursor === undefined + ? visible() + : and( + visible(), + or( + gt(columns.id, cursor.itemId), + and( + eq(columns.id, cursor.itemId), + gt(rows.languageId, cursor.languageId), + ), + ), + ), + ) + .orderBy(asc(columns.id), asc(rows.languageId)) + .limit(limit); + + const last = page.at(-1) as Record | undefined; + if (last) { + cursors.set(c, { + itemId: last.id as number, + languageId: last._languageId as number, + }); + } + + const languages = await listContentLanguages(c); + const localeOf = new Map( + languages.map(language => [language.id, language.locale]), + ); + + const documents = page.flatMap(row => { + const values = row as Record; + const locale = localeOf.get(values._languageId as number); + // A translation whose language row has been deleted has no locale to + // index under. Skipped rather than guessed - a document under an invented + // code is one nothing would ever query. + if (locale === undefined) return []; + + const document = contentTranslationSearchDocument( + definition, + { + base: values, + locale, + translation: { + publishedAt: values._publishedAt, + status: values._status, + updatedAt: values._updatedAt, + ...Object.fromEntries( + localizedSelection.map(name => [name, values[`t_${name}`]]), + ), + }, + }, + { pluginId }, + ); + + return document ? [document] : []; + }) satisfies SearchDocument[]; + + // Translation rows, not documents: a published translation with no usable + // title projects to nothing, and reporting that as "no items" would end the + // rebuild before the rows behind it. + return { documents, itemsRead: page.length }; + }, + }; +}; diff --git a/packages/vitnode/src/content/server/search-sync.ts b/packages/vitnode/src/content/server/search-sync.ts index 77ed43468..138643190 100644 --- a/packages/vitnode/src/content/server/search-sync.ts +++ b/packages/vitnode/src/content/server/search-sync.ts @@ -1,12 +1,22 @@ +import type { PgColumn, PgTable } from "drizzle-orm/pg-core"; import type { Context } from "hono"; +import { eq } from "drizzle-orm"; + import type { AnyContentTypeDefinition } from "../types"; +import type { AnyContentModel } from "./model"; +import { partitionContentFields } from "../localization"; import { contentSearchDocumentId, contentSearchIndexedFieldNames, } from "../search"; -import { contentSearchDocument, isContentRowPublic } from "./search-document"; +import { listContentLanguages } from "./language-resolver"; +import { + contentSearchDocument, + contentTranslationSearchDocument, + isContentRowPublic, +} from "./search-document"; /** Which mutation just returned. Not an event name: nothing is emitted here. */ export type ContentSearchOperation = @@ -183,3 +193,237 @@ export const syncContentSearch = async ( return { action, documentId, error }; } }; + +export interface ContentLocalizedSearchSyncInput { + /** + * `publish` / `unpublish` only: `false` when nothing moved, which means the + * index already agrees. + */ + changed?: boolean; + /** + * `update` and `restore` only. A write that touched no indexed field changes + * no document, in any language. + */ + changedFields?: readonly string[]; + /** + * One locale, for a mutation that touched one translation. + * + * Omitted for a mutation of the *record* - publishing it, editing a shared + * field - which changes every language's document at once, because every one of + * them is built from that row. + */ + locale?: string; + operation: ContentSearchOperation; + pluginId?: string; + /** The base row the mutation returned, including `status` and `publishedAt`. */ + row: object; +} + +/** One translation, as the document builder needs it. */ +interface TranslationRecord { + languageId: number; + values: Record; +} + +const readTranslations = async ( + c: Context, + model: AnyContentModel, + itemId: number, +): Promise => { + const columns: null | Record = model.translationColumns; + const table: null | PgTable = model.translationTable; + if (!columns || !table) return []; + + const { localizedFields } = partitionContentFields(model.definition.fields); + + const rows = await c + .get("db") + .select({ + languageId: columns.languageId, + publishedAt: columns.publishedAt, + status: columns.status, + updatedAt: columns.updatedAt, + ...Object.fromEntries( + Object.keys(localizedFields).map(name => [name, columns[name]]), + ), + }) + .from(table) + .where(eq(columns.itemId, itemId)); + + return rows.map(row => ({ + languageId: row.languageId as number, + values: row, + })); +}; + +/** + * Brings the search index in line with one mutation of a **localized** record. + * + * One document per published translation, so this is a loop rather than a single + * decision - and the loop is the point: a record's own publish or unpublish moves + * every language at once, while a translation's moves exactly one. `locale` is + * what distinguishes the two, and omitting it on a translation mutation would + * rewrite every other language's document for nothing. + * + * A translation that must not be indexed - a draft, a blank title, a record that + * is itself a draft - has its document **deleted for that language only**. Taking + * the Polish copy down must leave the English one exactly where it is, which is + * why `SearchModel.delete` takes a language. + * + * The same rules as the base sync otherwise: call it only after the write has + * returned, never inside the transaction, and a failing search engine never turns + * a successful write into a failed one. + */ +export const syncContentLocalizedSearch = async ( + c: Context, + model: AnyContentModel, + input: ContentLocalizedSearchSyncInput, +): Promise => { + const { definition } = model; + const values = input.row as Record; + const itemId = typeof values.id === "number" ? values.id : 0; + + if ( + !definition.search.enabled || + !definition.localization.enabled || + itemId === 0 + ) { + return []; + } + + // The record is gone, so every language's document is too. One call rather + // than one per locale: there is nothing left to enumerate them from. + if (input.operation === "delete") { + return [ + await write(c, definition, { + documentId: contentSearchDocumentId(definition, itemId), + run: async () => { + await c.get("search").delete(definition.id, itemId); + }, + action: "delete", + input, + itemId, + }), + ]; + } + + // An idempotent transition is a no-op for the same reason it emits no event: + // every document is already exactly what it would be rewritten to. + if ( + (input.operation === "publish" || input.operation === "unpublish") && + input.changed !== true + ) { + return []; + } + + // A write that moved no indexed field changes no document. `status` is not a + // declared field, so a publish never reaches this. + if (input.operation === "update" || input.operation === "restore") { + const indexed = new Set(contentSearchIndexedFieldNames(definition)); + const moved = (input.changedFields ?? []).some(name => indexed.has(name)); + if (!moved) return []; + } + + const languages = await listContentLanguages(c); + const localeOf = new Map( + languages.map(language => [language.id, language.locale]), + ); + + const translations = await readTranslations(c, model, itemId); + const wanted = input.locale?.trim().toLowerCase(); + + const outcomes: ContentSearchSyncOutcome[] = []; + + for (const translation of translations) { + const locale = localeOf.get(translation.languageId); + // A translation whose language row has been deleted has no locale to index + // under. Its document is unreachable rather than wrong, and a rebuild is what + // removes it - inventing a code here would create a document nothing queries. + if (locale === undefined) continue; + if (wanted !== undefined && locale.toLowerCase() !== wanted) continue; + + const document = contentTranslationSearchDocument( + definition, + { base: input.row, locale, translation: translation.values }, + { pluginId: input.pluginId }, + ); + + outcomes.push( + await write(c, definition, { + action: document ? "upsert" : "delete", + documentId: contentSearchDocumentId(definition, itemId, locale), + input, + itemId, + run: async () => { + if (document) { + await c.get("search").index(document); + + return; + } + + // Scoped to this language: unpublishing the Polish copy must leave the + // English document exactly where it is. + await c.get("search").delete(definition.id, itemId, locale); + }, + }), + ); + } + + return outcomes; +}; + +/** + * Runs one index write and turns a failure into an outcome rather than a throw. + * + * Shared by the localized paths so the "log it, keep the error, never fail the + * mutation" rule is stated once - it is the same rule `syncContentSearch` follows + * for the non-localized case, and a second copy of it would be the one that + * eventually throws. + */ +const write = async ( + c: Context, + definition: AnyContentTypeDefinition, + { + action, + documentId, + input, + itemId, + run, + }: { + action: "delete" | "upsert"; + documentId: string; + input: { operation: ContentSearchOperation; pluginId?: string }; + itemId: number; + run: () => Promise; + }, +): Promise => { + try { + await run(); + + return { action, documentId }; + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + + const message = `[content-search] ${JSON.stringify({ + action, + contentTypeId: definition.id, + documentId, + error: error.message, + itemId, + itemType: definition.id, + operation: input.operation, + pluginId: input.pluginId, + })}`; + + try { + await c.get("log").error(message); + } catch { + // eslint-disable-next-line no-console + console.error( + `[VitNode] Failed to log content search failure: ${message}`, + ); + } + + return { action, documentId, error }; + } +}; diff --git a/packages/vitnode/src/content/server/translation-effects.ts b/packages/vitnode/src/content/server/translation-effects.ts index 6afca0724..0226e5ea7 100644 --- a/packages/vitnode/src/content/server/translation-effects.ts +++ b/packages/vitnode/src/content/server/translation-effects.ts @@ -5,7 +5,11 @@ import type { ContentEventAction } from "../events"; import type { AnyContentTypeDefinition } from "../types"; import type { ContentTranslationEditorialOutcome } from "./translation-editorial-service"; +import type { AnyContentModel } from "./model"; +import type { ContentSearchSyncOutcome } from "./search-sync"; + import { emitContentEvent } from "./emit"; +import { syncContentLocalizedSearch } from "./search-sync"; /** One translation operation, one event. Never `updated` - see `events.ts`. */ const EVENT_ACTION: Record< @@ -55,11 +59,26 @@ const payloadFor = ( }; export interface ContentTranslationEffectsOptions { + /** + * The model, for a content type with `search`. + * + * A translation mutation moves exactly one language's document, and finding it + * takes the base row and the translation table - neither of which this function + * is otherwise given. Optional so every Stage 5B caller compiles unchanged. + */ + model?: AnyContentModel; /** The plugin that owns the content type, and therefore the event. */ pluginId: string; } export interface ContentTranslationEffectsResult { + /** + * What the index write reported, or `null` when there was none to do - a + * content type without `search`, or a no-op outcome. + * + * A one-element array at most: a translation mutation is one language. + */ + search?: ContentSearchSyncOutcome[]; /** * What the event transport reported, or `null` for a no-op outcome. * @@ -85,9 +104,10 @@ export interface ContentTranslationEffectsResult { * A no-op outcome does nothing at all. That is what keeps a double-clicked publish * button and an empty edit from each producing a second event. * - * Search synchronisation is deliberately absent: a localized content type cannot - * have `search` enabled yet, so there is no document to write. Stage 5D adds the - * per-locale sync here. Cache invalidation is absent for the reason it is absent + * Search synchronisation is scoped to the locale that moved: one translation is + * one document, and rewriting the others would be work for a change none of them + * contains. A translation that must not be indexed has its document deleted for + * that language only. Cache invalidation is absent for the reason it is absent * from the base effects too - it needs the Next runtime, which the API process does * not have, so the Server Action owns it. */ @@ -95,19 +115,38 @@ export const contentTranslationEffects = async ( c: Context, definition: AnyContentTypeDefinition, outcome: ContentTranslationEditorialOutcome, - { pluginId }: ContentTranslationEffectsOptions, + { model, pluginId }: ContentTranslationEffectsOptions, ): Promise => { if (!outcome.changed) return { event: null }; + const event = await emitContentEvent( + c, + definition, + EVENT_ACTION[outcome.operation], + payloadFor(outcome) as never, + // The plugin that owns the content type, not whichever module happens to be + // handling the request. + { pluginId }, + ); + + if (!definition.search.enabled || !model) return { event }; + + // The base row, because a translation's document is built from both halves and + // its visibility is subordinate to the record's. + const base = await model.service(c).findById(outcome.row.itemId); + if (!base) return { event }; + return { - event: await emitContentEvent( - c, - definition, - EVENT_ACTION[outcome.operation], - payloadFor(outcome) as never, - // The plugin that owns the content type, not whichever module happens to be - // handling the request. - { pluginId }, - ), + event, + // Scoped to the locale that moved. Omitting it would rewrite every other + // language's document for a change none of them contains. + search: await syncContentLocalizedSearch(c, model, { + changed: outcome.changed, + changedFields: outcome.changedFields, + locale: outcome.locale, + operation: outcome.operation, + pluginId, + row: base, + }), }; }; diff --git a/packages/vitnode/src/content/server/translation-routes.ts b/packages/vitnode/src/content/server/translation-routes.ts index 4516c2d01..eb6806f65 100644 --- a/packages/vitnode/src/content/server/translation-routes.ts +++ b/packages/vitnode/src/content/server/translation-routes.ts @@ -166,7 +166,10 @@ export const buildContentTranslationRoutes = < c: Context, outcome: ContentTranslationEditorialOutcome, ): Promise => { - await contentTranslationEffects(c, definition, outcome, { pluginId }); + await contentTranslationEffects(c, definition, outcome, { + model, + pluginId, + }); }; /** diff --git a/packages/vitnode/src/content/types.ts b/packages/vitnode/src/content/types.ts index 3936f6908..43eb6e7a1 100644 --- a/packages/vitnode/src/content/types.ts +++ b/packages/vitnode/src/content/types.ts @@ -1186,6 +1186,32 @@ export type FilterableContentFieldName = FieldNamesOfKind< FilterableContentFieldKind >; +/** {@link FieldNamesOfKind} over every field, shared and localized alike. */ +type AnyFieldNamesOfKind = string & + { + [ + K in keyof ContentFieldsOf + ]: ContentFieldsOf[K] extends { + kind: TKind; + } + ? K + : never; + }[keyof ContentFieldsOf]; + +/** + * Field names a **public** filter may name. + * + * Wider than {@link FilterableContentFieldName} by exactly the localized half: an + * admin list is a query over the base table, but a public localized read already + * joins the translation it is serving, so filtering on a localized field is one + * more predicate on a row it was fetching anyway - evaluated against the language + * the reader will actually see. + */ +export type PublicFilterableContentFieldName = AnyFieldNamesOfKind< + TDefinition, + FilterableContentFieldKind +>; + /** * Equality filters accepted by `service.findMany`, one key per filterable * field - plus `status` once publication is enabled, which is a generated @@ -1331,7 +1357,7 @@ export type ContentPublicListRow = export type ContentPublicFilterInput = Partial<{ [ K in ContentPublicFieldName & - FilterableContentFieldName + PublicFilterableContentFieldName ]: ContentFieldInput[K]>; }>; diff --git a/packages/vitnode/src/locales/en.json b/packages/vitnode/src/locales/en.json index 6b0765384..1f10efef0 100644 --- a/packages/vitnode/src/locales/en.json +++ b/packages/vitnode/src/locales/en.json @@ -570,6 +570,8 @@ }, "translations": { "shared_tab": "Shared", + "default_locale": "default", + "locale_column": "{name} translation", "save": "Save translation", "create": "Create translation", "publish": "Publish this language", diff --git a/packages/vitnode/src/tests/content-fixtures.ts b/packages/vitnode/src/tests/content-fixtures.ts index ab701b68e..c32e8ce15 100644 --- a/packages/vitnode/src/tests/content-fixtures.ts +++ b/packages/vitnode/src/tests/content-fixtures.ts @@ -363,3 +363,49 @@ export const testStrictLocalizedPageContentType = defineContentType({ list: { columns: ["featured", "status"] }, }, }); + +/** + * The Stage 5D fixture: localized, public **and** searchable. + * + * The whole stack on one content type, because per-locale search is the only + * combination that needs all of it: the translation supplies the prose, the base + * row supplies the shared values and the publication state, and `publicApi` + * supplies the allowlist every indexed field has to be in. + * + * `pathTemplate` carries `{locale}`, which a localized content type must - two + * languages routinely answer to the same slug, so one URL for both would make a + * search hit ambiguous. + */ +export const testLocalizedSearchPageContentType = defineContentType({ + id: "test.localized-search-page", + tableName: "test_localized_search_pages", + localization: { enabled: true, defaultLocale: "en", fallback: "default" }, + publication: { enabled: true }, + fields: { + title: field.text({ localized: true, required: true, maxLength: 200 }), + slug: field.slug({ localized: true, source: "title" }), + body: field.textarea({ localized: true, nullable: true }), + featured: field.boolean({ defaultValue: false }), + }, + publicApi: { + enabled: true, + path: "pages", + fields: ["title", "slug", "body", "featured", "publishedAt"], + searchableFields: ["title", "body"], + orderableFields: ["publishedAt"], + filterableFields: ["featured"], + }, + search: { + enabled: true, + titleField: "title", + contentFields: ["title", "body"], + pathTemplate: "/{locale}/pages/{slug}", + }, + admin: { + label: { + plural: "Test Localized Search Pages", + singular: "Test Localized Search Page", + }, + list: { columns: ["featured", "status"] }, + }, +}); diff --git a/packages/vitnode/src/views/admin/views/content/table/content-table-view.test.tsx b/packages/vitnode/src/views/admin/views/content/table/content-table-view.test.tsx index c7cbe96c5..303bbfcbd 100644 --- a/packages/vitnode/src/views/admin/views/content/table/content-table-view.test.tsx +++ b/packages/vitnode/src/views/admin/views/content/table/content-table-view.test.tsx @@ -7,6 +7,7 @@ import type { AnyContentTypeDefinition } from "@/content/types"; import { testArticleContentType, testEditorialPostContentType, + testLocalizedPageContentType, testPostContentType, } from "@/tests/content-fixtures"; @@ -49,8 +50,32 @@ const { DeleteContentAction } = await import("../actions/delete-action"); * before a single row exists, and getting it wrong shows up as a header with no * button rather than as an error. */ -const orderProp = async (definition: AnyContentTypeDefinition) => { - const element = (await ContentTableView({ +/** + * The rendered `DataTable`, whichever wrapper it came back inside. + * + * The view returns a fragment - a localized content type gets a locale selector + * above the table - so the table is found rather than assumed to be the root. + */ +const dataTable = (element: ReactElement): ReactElement => { + const children = (element.props as { children?: unknown }).children; + if (children === undefined) return element as ReactElement; + + const found = (Array.isArray(children) ? children : [children]).find( + child => + child !== null && + typeof child === "object" && + "props" in child && + "columns" in (child as ReactElement>).props, + ); + + return (found ?? element) as ReactElement; +}; + +const render = async ( + definition: AnyContentTypeDefinition, + searchParams: Record = {}, +) => + (await ContentTableView({ columnSpecs: [], entry: { definition, @@ -58,14 +83,14 @@ const orderProp = async (definition: AnyContentTypeDefinition) => { registration: {}, } as never, formSpec: {} as never, - searchParams: {}, + searchParams, translationSpec: null, - })) as ReactElement<{ - order: { columns: string[]; defaultOrder: { column: string } }; - }>; + })) as ReactElement; - return element.props.order; -}; +const orderProp = async (definition: AnyContentTypeDefinition) => + dataTable<{ order: { columns: string[]; defaultOrder: { column: string } } }>( + await render(definition), + ).props.order; /** * The props the actions cell hands `DeleteContentAction` for one row. @@ -78,24 +103,14 @@ const deleteProps = async ( definition: AnyContentTypeDefinition, row: Record, ) => { - const element = (await ContentTableView({ - columnSpecs: [], - entry: { - definition, - pluginId: "@vitnode/example", - registration: {}, - } as never, - formSpec: {} as never, - searchParams: {}, - translationSpec: null, - })) as ReactElement<{ + const element = dataTable<{ columns: { cell?: (context: { row: Record }) => ReactElement<{ children: ReactElement>[]; }>; id?: string; }[]; - }>; + }>(await render(definition)); const actions = element.props.columns.find(column => column.id === "actions"); const rendered = actions?.cell?.({ row }); @@ -178,3 +193,45 @@ describe("sortable columns", () => { }); }); }); + +describe("the locale selector", () => { + const columns = async ( + definition: AnyContentTypeDefinition, + searchParams: Record = {}, + ) => + dataTable<{ columns: { id?: string }[] }>( + await render(definition, searchParams), + ).props.columns; + + it("adds no translation column without a language", async () => { + // The list is unchanged until somebody picks one. `Shared` is a real choice, + // not a fallback state. + expect( + (await columns(testLocalizedPageContentType)).map(column => column.id), + ).not.toContain("translation"); + }); + + it("adds one when a language is selected", async () => { + expect( + (await columns(testLocalizedPageContentType, { locale: "pl" })).map( + column => column.id, + ), + ).toContain("translation"); + }); + + it("puts it first, because it is what the person came to read", async () => { + const [first] = await columns(testLocalizedPageContentType, { + locale: "pl", + }); + + expect(first.id).toBe("translation"); + }); + + it("adds nothing to a content type that is not localized", async () => { + expect( + (await columns(testEditorialPostContentType, { locale: "pl" })).map( + column => column.id, + ), + ).not.toContain("translation"); + }); +}); diff --git a/packages/vitnode/src/views/admin/views/content/table/content-table-view.tsx b/packages/vitnode/src/views/admin/views/content/table/content-table-view.tsx index 49088e952..216d64770 100644 --- a/packages/vitnode/src/views/admin/views/content/table/content-table-view.tsx +++ b/packages/vitnode/src/views/admin/views/content/table/content-table-view.tsx @@ -19,6 +19,7 @@ import { PreviewContentAction } from "../actions/preview-action"; import { PublishContentAction } from "../actions/publish-action"; import { ScheduleContentAction } from "../actions/schedule-action"; import { ContentCell } from "./cells"; +import { ContentLocaleSelector } from "./locale-selector"; const zodList = z.object({ edges: z.array( @@ -26,6 +27,15 @@ const zodList = z.object({ .object({ id: z.number(), labels: z.record(z.string(), z.string().nullable()), + /** Present only when the list is being viewed in a language. */ + translation: z + .object({ + locale: z.string(), + status: z.string().optional(), + title: z.string(), + }) + .nullable() + .optional(), }) .loose(), ), @@ -75,8 +85,51 @@ export const ContentTableView = async ({ published: t("status.published"), }; const titleField = definition.admin.titleField; + const localized = definition.localization.enabled; + const viewedLocale = + typeof searchParams.locale === "string" ? searchParams.locale : undefined; + // The code rather than the display name: this is a server component and the + // language registry is client-side context. A locale code is what the selector + // and the URL already show, so the column header reads consistently with both. + const localeName = viewedLocale ?? ""; const columns: ColumnDef[] = [ + // First, and only when a language is selected: it is what the person came + // to the list to read. `Missing` is a state rather than a blank, because a + // record with no translation is exactly the row worth finding. + ...(localized && viewedLocale !== undefined + ? [ + { + id: "translation", + header: t("translations.locale_column", { name: localeName }), + cell: ({ row }: { row: ContentRowData }) => { + const translation = row.translation as + null | undefined | { status?: string; title: string }; + + if (!translation) { + return ( + + {t("translations.states.missing")} + + ); + } + + return ( + + {translation.title === "" ? emptyLabel : translation.title} + {translation.status ? ( + + {translation.status === "published" + ? t("translations.states.published") + : t("translations.states.draft")} + + ) : null} + + ); + }, + } satisfies ColumnDef, + ] + : []), ...columnSpecs.map((spec): ColumnDef => { const override = registration.columns?.[spec.name]; @@ -212,28 +265,37 @@ export const ContentTableView = async ({ ]; return ( - 0} - /> + <> + {localized ? ( +

+ +
+ ) : null} + 0} + /> + ); }; diff --git a/packages/vitnode/src/views/admin/views/content/table/locale-selector.tsx b/packages/vitnode/src/views/admin/views/content/table/locale-selector.tsx new file mode 100644 index 000000000..634074f7e --- /dev/null +++ b/packages/vitnode/src/views/admin/views/content/table/locale-selector.tsx @@ -0,0 +1,81 @@ +"use client"; + +import { useTranslations } from "next-intl"; +import { useSearchParams } from "next/navigation"; +import React from "react"; + +import { useLanguages } from "@/components/languages-provider"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { usePathname, useRouter } from "@/lib/navigation"; + +/** The value that means "no language column", rather than a locale. */ +const SHARED = "__shared__"; + +/** + * Which language the AdminCP list is being *viewed* in. + * + * A view control and not a filter, which is the whole point: an admin list is a + * list of records, and hiding the ones a translator has not reached yet is the + * opposite of what somebody choosing a language is looking for. Picking Polish + * adds a column showing each record's Polish title and status - including + * `Missing`, which is the row that most needs finding. + * + * The choice lives in the URL rather than in state, so it survives a reload, + * paginates with the table and can be shared with whoever is doing the + * translating. Changing it resets the cursor: page three of the English ordering + * is not page three of anything else. + */ +export const ContentLocaleSelector = ({ + defaultLocale, +}: { + defaultLocale: string; +}) => { + const t = useTranslations("core.content.translations"); + const languages = useLanguages(); + const pathname = usePathname(); + const router = useRouter(); + const searchParams = useSearchParams(); + + const current = searchParams.get("locale") ?? SHARED; + + const onChange = (raw: unknown) => { + const value = typeof raw === "string" ? raw : SHARED; + const next = new URLSearchParams(searchParams.toString()); + + if (value === SHARED) { + next.delete("locale"); + } else { + next.set("locale", value); + } + // A cursor is a position in one ordering. Carrying it across a change of + // view would land on a page that means something else. + next.delete("cursor"); + + router.push(`${pathname}?${next.toString()}`); + }; + + if (languages.length === 0) return null; + + return ( + + ); +}; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.test.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.test.ts index 1c7bd2bf0..006327bbf 100644 --- a/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.test.ts +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.test.ts @@ -117,3 +117,50 @@ describe("getCollectionCoverageBar", () => { expect(getCollectionCoverageBar(unmanaged(11))).toBeNull(); }); }); + +describe("multi-language collections", () => { + const localized = { + documents: 6, + hasIndexer: true, + indexed: 3, + languages: [ + { documents: 3, languageCode: "en", lastIndexedAt: null }, + { documents: 3, languageCode: "pl", lastIndexedAt: null }, + ], + total: 6, + }; + + it("measures coverage in documents, not distinct items", () => { + // Three records in two languages is six documents. Comparing three against + // six would report a fully-indexed collection as half covered. + expect(getCollectionCoverage(localized)).toBe(100); + expect(getCollectionStatus(localized)).toBe("indexed"); + }); + + it("still measures a single-language collection in items", () => { + expect( + getCollectionCoverage({ + documents: 3, + indexed: 3, + languages: [], + total: 3, + }), + ).toBe(100); + }); + + it("reads a response that predates the per-language breakdown", () => { + // A web app deployed against an older API gets no `languages` at all, and + // that is the single-language case rather than a crash. + expect(getCollectionCoverage({ indexed: 3, total: 3 })).toBe(100); + }); + + it("still reports a half-finished language as stale", () => { + expect( + getCollectionStatus({ + ...localized, + documents: 4, + languages: localized.languages, + }), + ).toBe("stale"); + }); +}); diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.ts index 875d985e8..6cb3ca571 100644 --- a/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.ts +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-status.ts @@ -1,14 +1,46 @@ +export interface SearchCollectionLanguage { + documents: number; + languageCode: string; + lastIndexedAt: Date | null | string; +} + export interface SearchCollection { + /** Index rows, counting one per language. */ + documents: number; /** Whether a rebuild indexer is registered for this item type right now. */ hasIndexer: boolean; + /** Distinct items in the index, however many languages each has. */ indexed: number; itemType: string; + /** One entry per language present in the index. Empty when there are none. */ + languages: SearchCollectionLanguage[]; lastIndexedAt: Date | null | string; pluginId: string; /** Source items the indexer reports; `null` when there is no indexer. */ total: null | number; } +/** + * What `total` is actually counting for this collection. + * + * A multi-language collection is indexed once per translation, and its indexer + * counts published *translations* - so comparing that against distinct items + * would report a fully-indexed collection with three languages as 33% covered. + * One rule, read off the data rather than configured, so a collection that gains + * a second language starts being measured correctly without anything being + * switched on. + */ +export const getCollectionIndexedCount = ( + collection: Partial> & + Pick, +): number => + // Both optional, because this also reads responses from an API that predates + // the per-language breakdown: no languages reported is the single-language + // case, which is what it always was. + (collection.languages?.length ?? 0) > 0 + ? (collection.documents ?? collection.indexed) + : collection.indexed; + export type CollectionStatus = "empty" | "indexed" | "stale" | "unmanaged"; /** @@ -26,17 +58,15 @@ export type CollectionStatus = "empty" | "indexed" | "stale" | "unmanaged"; * `search.index()` keeps its collection perfectly current without one. All that * is known is that a rebuild cannot reproduce it. */ -export const getCollectionStatus = ({ - hasIndexer, - indexed, - total, -}: Pick< - SearchCollection, - "hasIndexer" | "indexed" | "total" ->): CollectionStatus => { - if (!hasIndexer && indexed > 0) return "unmanaged"; +export const getCollectionStatus = ( + collection: Partial> & + Pick, +): CollectionStatus => { + const indexed = getCollectionIndexedCount(collection); + + if (!collection.hasIndexer && indexed > 0) return "unmanaged"; if (indexed === 0) return "empty"; - if (indexed === total) return "indexed"; + if (indexed === collection.total) return "indexed"; return "stale"; }; @@ -48,10 +78,13 @@ export const getCollectionStatus = ({ * Can exceed 100 - that is the point, and the number is shown as it is. Use * {@link getCollectionCoverageBar} for the width of anything drawn. */ -export const getCollectionCoverage = ({ - indexed, - total, -}: Pick): null | number => { +export const getCollectionCoverage = ( + collection: Partial> & + Pick, +): null | number => { + const { total } = collection; + const indexed = getCollectionIndexedCount(collection); + if (total === null) return null; if (total > 0) return Math.round((indexed / total) * 100); @@ -59,7 +92,8 @@ export const getCollectionCoverage = ({ }; export const getCollectionCoverageBar = ( - collection: Pick, + collection: Partial> & + Pick, ): null | number => { const coverage = getCollectionCoverage(collection); diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/collections-table.tsx b/packages/vitnode/src/views/admin/views/core/advanced/search/collections-table.tsx index 1842aa4ea..f0ed76188 100644 --- a/packages/vitnode/src/views/admin/views/core/advanced/search/collections-table.tsx +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/collections-table.tsx @@ -20,6 +20,7 @@ import type { CollectionStatus, SearchCollection } from "./collection-status"; import { getCollectionCoverage, getCollectionCoverageBar, + getCollectionIndexedCount, getCollectionStatus, } from "./collection-status"; import { ReindexCollectionAction } from "./reindex-action"; @@ -76,7 +77,11 @@ export const CollectionsTable = async ({ labels?.get(collection.itemType) ?? t(getSearchTypeRenderer(collection.itemType).labelKey), })) - .sort((a, b) => b.indexed - a.indexed || a.label.localeCompare(b.label)); + .sort( + (a, b) => + getCollectionIndexedCount(b) - getCollectionIndexedCount(a) || + a.label.localeCompare(b.label), + ); const term = search?.trim().toLowerCase(); const edges = term @@ -132,13 +137,27 @@ export const CollectionsTable = async ({ id: "items", header: t("admin.collections.columns.items"), cell: ({ row }) => ( - - {row.indexed} - {/* An em dash, not `indexed`: with no indexer there is no source count - to compare against, and repeating the left number would read as - full coverage. */} +
+ + {getCollectionIndexedCount(row)} + + {/* An em dash, not the left number: with no indexer there is no source + count to compare against, and repeating it would read as full + coverage. */} / {row.total ?? "—"} - + {/* Per language, because a single total cannot say which locale a + rebuild stopped halfway through. Only for collections that have + languages at all - most have none. */} + {row.languages.length > 0 && ( +
+ {row.languages.map(language => ( + + {language.languageCode} {language.documents} + + ))} +
+ )} +
), }, { diff --git a/plugins/example/src/content/localized-article.ts b/plugins/example/src/content/localized-article.ts index 042778b25..a11a482d0 100644 --- a/plugins/example/src/content/localized-article.ts +++ b/plugins/example/src/content/localized-article.ts @@ -60,6 +60,24 @@ export const localizedArticleContentType = defineContentType({ defaultOrder: "desc", }, + /** + * One search document per **published** translation. + * + * `titleField` and `contentFields` name localized fields, which is the whole + * point: an index built from the base row would hold no prose at all here, + * since every text field on this content type is localized. + * + * `{locale}` in `pathTemplate` is required rather than optional - two languages + * routinely answer to the same slug, so a template without it would give every + * translation of a record the same link. + */ + search: { + enabled: true, + titleField: "title", + contentFields: ["title", "body"], + pathTemplate: "/{locale}/localized-articles/{slug}", + }, + // Per-locale versions, per-locale revisions, per-locale restore. `retention` is // per language, so five Polish revisions do not evict the English ones. // `preview` mints a link per language, freezing the shared revision and that diff --git a/plugins/example/src/database/postgres.test.ts b/plugins/example/src/database/postgres.test.ts index f2da6d68f..79b07fb4d 100644 --- a/plugins/example/src/database/postgres.test.ts +++ b/plugins/example/src/database/postgres.test.ts @@ -12,8 +12,10 @@ import { import { claimContentSchedule, contentPublicLocaleStates, + createContentLocalizedSearchIndexer, createContentSearchIndexer, settleContentSchedule, + syncContentLocalizedSearch, syncContentSearch, } from "@vitnode/core/content/server"; import { core_queue } from "@vitnode/core/database/queue"; @@ -3581,6 +3583,317 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { }); }); + /** + * Per-locale search, against a real database. + * + * One document per published translation, and the interesting parts are all + * in SQL: the keyset page over `(itemId, languageId)` and the two published + * predicates the join carries. The search *engine* is stubbed for the same + * reason it is in the base lifecycle test - `core_search_index` is a core + * table this plugin's migrations do not create - so the assertions are about + * the documents the engine is handed. + */ + describe("localized search", () => { + const editorial = (handle = context) => { + const build = localizedArticleContent.translationEditorialService; + if (!build) + throw new Error("Expected a translation editorial service."); + + return build(handle, { pluginId: CONFIG_PLUGIN.pluginId }); + }; + + /** The stubbed engine, and everything it was asked to write. */ + const engine = () => { + const indexed: { + languageCode?: string; + title: string; + url?: string; + }[] = []; + const deleted: { itemId: number; languageCode?: string }[] = []; + + const searchContext = { + get: (key: string) => { + if (key === "search") { + return { + delete: async ( + _itemType: string, + itemId: number, + languageCode?: string, + ) => { + deleted.push({ itemId, languageCode }); + await Promise.resolve(); + }, + index: async (document: { + languageCode?: string; + title: string; + url?: string; + }) => { + indexed.push(document); + await Promise.resolve(); + }, + }; + } + if (key === "log") { + return { + debug: async () => await Promise.resolve(), + error: async () => await Promise.resolve(), + warn: async () => await Promise.resolve(), + }; + } + if (key === "core") return context.get("core"); + + return db; + }, + } as unknown as Context; + + return { deleted, indexed, searchContext }; + }; + + const publishRecord = async (itemId: number) => { + await sql` + UPDATE "example_localized_articles" + SET "status" = 'published', "publishedAt" = now() + WHERE "id" = ${itemId} + `; + }; + + /** A published record with English published and Polish optional. */ + const seed = async ({ + pl, + title, + }: { + pl?: { published: boolean; title: string }; + title: string; + }) => { + const { row } = await localizedService().create({ + shared: {}, + translation: { body: `Body of ${title}`, title }, + }); + await publishRecord(row.id); + await editorial().publish(row.id, "en", { actor: ACTOR }); + + if (pl) { + await translations().create(row.id, "pl", { + body: `Tresc ${pl.title}`, + title: pl.title, + }); + if (pl.published) { + await editorial().publish(row.id, "pl", { actor: ACTOR }); + } + } + + return row.id; + }; + + /** Runs the whole rebuild, two translation rows per page. */ + const rebuild = async (searchContext: Context) => { + const indexer = createContentLocalizedSearchIndexer( + localizedArticleContent, + { pluginId: CONFIG_PLUGIN.pluginId }, + ); + + let offset = 0; + for (;;) { + const page = await indexer.load(searchContext, offset, 2); + for (const document of page.documents) { + await searchContext.get("search").index(document); + } + if (page.itemsRead === 0) break; + offset += page.itemsRead; + } + + return indexer; + }; + + it("indexes one document per published translation", async () => { + await seed({ + pl: { published: true, title: "Szukaj" }, + title: "Search", + }); + const { indexed, searchContext } = engine(); + + await rebuild(searchContext); + + expect( + indexed.map(document => [document.languageCode, document.title]), + ).toEqual([ + ["en", "Search"], + ["pl", "Szukaj"], + ]); + }); + + it("gives each language its own URL", async () => { + await seed({ + pl: { published: true, title: "Adres" }, + title: "Address", + }); + const { indexed, searchContext } = engine(); + + await rebuild(searchContext); + + // Two languages routinely answer to the same slug, so a template without + // `{locale}` would give both documents the same link. + expect(indexed.map(document => document.url)).toEqual([ + "/en/localized-articles/address", + "/pl/localized-articles/adres", + ]); + }); + + it("leaves a draft translation out", async () => { + await seed({ + pl: { published: false, title: "Szkic" }, + title: "Draft PL", + }); + const { indexed, searchContext } = engine(); + + await rebuild(searchContext); + + expect(indexed.map(document => document.languageCode)).toEqual(["en"]); + }); + + it("indexes nothing at all while the record is a draft", async () => { + const { row } = await localizedService().create({ + shared: {}, + translation: { body: "Body", title: "Unpublished" }, + }); + await editorial().publish(row.id, "en", { actor: ACTOR }); + const { indexed, searchContext } = engine(); + + await rebuild(searchContext); + + // Subordination: a published translation of a draft record is not public, + // so it is not indexed either. + expect(indexed).toEqual([]); + }); + + it("pages over translations without skipping or repeating one", async () => { + // Three records with two languages each is six rows, read two at a time - + // so the keyset cursor is exercised across a record boundary. + for (const title of ["Page One", "Page Two", "Page Three"]) { + await seed({ pl: { published: true, title: `${title} PL` }, title }); + } + const { indexed, searchContext } = engine(); + + await rebuild(searchContext); + + expect(indexed).toHaveLength(6); + expect(new Set(indexed.map(document => document.title)).size).toBe(6); + }); + + it("counts published translations, not records", async () => { + await seed({ pl: { published: true, title: "Licz" }, title: "Count" }); + const { searchContext } = engine(); + const indexer = await rebuild(searchContext); + + // The coverage bar compares this against the document count, so counting + // records would pin a fully-indexed two-language collection at 50%. + expect(await indexer.count?.(searchContext)).toBe(2); + }); + + it("takes one language out when its translation is unpublished", async () => { + const itemId = await seed({ + pl: { published: true, title: "Wycofane" }, + title: "Withdrawn", + }); + const { deleted, indexed, searchContext } = engine(); + + const outcome = await editorial().unpublish(itemId, "pl", { + actor: ACTOR, + }); + if (!outcome) throw new Error("Expected an outcome."); + + const base = await localizedArticleContent + .service(context) + .findById(itemId); + + await syncContentLocalizedSearch( + searchContext, + localizedArticleContent, + { + changed: outcome.changed, + locale: "pl", + operation: "unpublish", + pluginId: CONFIG_PLUGIN.pluginId, + row: base as object, + }, + ); + + // Scoped to Polish. The English document is not rewritten and not + // removed - it was not part of what moved. + expect(deleted).toEqual([{ itemId, languageCode: "pl" }]); + expect(indexed).toEqual([]); + }); + + it("removes every language when the record itself is unpublished", async () => { + const itemId = await seed({ + pl: { published: true, title: "Wszystkie" }, + title: "All", + }); + const { deleted, searchContext } = engine(); + + const result = await localizedArticleContent + .service(context) + .unpublish(itemId); + if (!result) throw new Error("Expected a transition."); + + await syncContentLocalizedSearch( + searchContext, + localizedArticleContent, + { + changed: result.changed, + operation: "unpublish", + pluginId: CONFIG_PLUGIN.pluginId, + row: result.row, + }, + ); + + // Every language at once: the record's publication state gates them all. + expect(deleted).toEqual([ + { itemId, languageCode: "en" }, + { itemId, languageCode: "pl" }, + ]); + }); + + it("re-indexes only the language a translation edit touched", async () => { + const itemId = await seed({ + pl: { published: true, title: "Edytowane" }, + title: "Edited", + }); + const { indexed, searchContext } = engine(); + + const outcome = await editorial().update( + itemId, + "pl", + { title: "Edytowane Ponownie" }, + { actor: ACTOR, expectedVersion: 2 }, + ); + if (!outcome) throw new Error("Expected an outcome."); + + const base = await localizedArticleContent + .service(context) + .findById(itemId); + + await syncContentLocalizedSearch( + searchContext, + localizedArticleContent, + { + changedFields: outcome.changedFields, + locale: "pl", + operation: "update", + pluginId: CONFIG_PLUGIN.pluginId, + row: base as object, + }, + ); + + expect(indexed).toEqual([ + expect.objectContaining({ + languageCode: "pl", + title: "Edytowane Ponownie", + }), + ]); + }); + }); + /** Which languages a record is publicly reachable in, evaluated in SQL. */ describe("public locale states", () => { const editorial = (handle = context) => { From 8d69fe929d94aebc9983aa93f8514c5e04d29916 Mon Sep 17 00:00:00 2001 From: aXenDeveloper Date: Sat, 8 Aug 2026 09:41:35 +0200 Subject: [PATCH 2/2] fix: review and fix the remaining correctness issues in the stacked Content Engine Stage 5 --- .../docs/dev/content-engine/localization.mdx | 19 ++ .../content-engine/localized-public-api.mdx | 23 +- .../dev/content-engine/localized-search.mdx | 11 + .../content-engine/translation-editorial.mdx | 11 + .../content-engine/translation-revisions.mdx | 23 ++ .../content-engine/translation-service.mdx | 5 +- apps/docs/content/docs/dev/search.mdx | 33 +++ packages/elasticsearch/src/index.ts | 10 +- .../src/api/adapters/search/postgres.ts | 10 +- .../src/api/middlewares/global.middleware.ts | 26 +- .../vitnode/src/api/models/search.test.ts | 108 ++++++- packages/vitnode/src/api/models/search.ts | 64 ++++- .../src/content/next/fetch.server.test.ts | 153 +++++++++- .../vitnode/src/content/next/fetch.server.ts | 60 +++- .../src/content/server/revisions-model.ts | 18 +- .../server/search-sync.localized.test.ts | 179 ++++++++++++ .../vitnode/src/content/server/search-sync.ts | 22 +- .../translation-editorial-service.test.ts | 79 ++++- .../server/translation-editorial-service.ts | 18 +- .../content/server/translation-model.test.ts | 130 ++++++++- .../src/content/server/translation-model.ts | 77 ++++- .../translation-publication-routes.test.ts | 247 ++++++++++++++++ .../src/content/server/translation-routes.ts | 42 ++- .../example/src/content/localized-article.ts | 8 +- plugins/example/src/database/postgres.test.ts | 272 +++++++++++++++++- 25 files changed, 1578 insertions(+), 70 deletions(-) create mode 100644 packages/vitnode/src/content/server/search-sync.localized.test.ts create mode 100644 packages/vitnode/src/content/server/translation-publication-routes.test.ts diff --git a/apps/docs/content/docs/dev/content-engine/localization.mdx b/apps/docs/content/docs/dev/content-engine/localization.mdx index a912c3fbd..53e99abeb 100644 --- a/apps/docs/content/docs/dev/content-engine/localization.mdx +++ b/apps/docs/content/docs/dev/content-engine/localization.mdx @@ -251,6 +251,25 @@ A disabled language is **readable and not writable**. Its content is already in the database, and hiding it would make it unrecoverable; growing more of it in a language nothing renders is the part that gets refused. +One rule, in both directions - everything that puts content *into* a disabled +locale is refused, and everything that takes content *out* of it still works: + +| Operation | Disabled locale | +| --- | --- | +| create | refused | +| update | refused | +| restore | refused | +| publish | refused | +| unpublish | allowed | +| delete | allowed | +| read, history | allowed | + +`publish` sits on the refused side because publishing into a language the app +does not serve puts a page on the internet that nothing routes to. `unpublish` +and `delete` sit on the allowed side because switching a language off is usually +the step *before* taking its pages down - refusing would strand published content +in a locale nobody can edit. + Deleting a language that content is written in is refused by Postgres itself - the foreign key is `ON DELETE RESTRICT`. That is deliberately different from `core_languages_words`, which cascades: losing a UI string is an inconvenience, diff --git a/apps/docs/content/docs/dev/content-engine/localized-public-api.mdx b/apps/docs/content/docs/dev/content-engine/localized-public-api.mdx index 65e6bfd31..9831ed686 100644 --- a/apps/docs/content/docs/dev/content-engine/localized-public-api.mdx +++ b/apps/docs/content/docs/dev/content-engine/localized-public-api.mdx @@ -195,9 +195,26 @@ if (!data) notFound(); Passing `locale` does two things that have to happen together, which is why it is one argument rather than a query parameter you add yourself: it goes to the API as -`?locale=`, **and** it goes into the cache tags. Omitting it is not an error - the -API falls back to the default locale - but the response is then tagged as though -it were locale-less, so a translation publish will not expire it. +`?locale=`, **and** it goes into the cache tags. + +Omitting it is not an error and not a trap either. The content type's +`defaultLocale` is filled in before either one is built, so the request and the +tags always name the same language: + +```text +locale: "pl" → ?locale=pl content:example.article:list:pl +locale omitted → ?locale=en content:example.article:list:en +``` + + + It cannot: every invalidation path names the locale-aware tag, so an entry + tagged `content:example.article:list` would hold default-locale content that + nothing could ever expire - not a translation publish, not an edit, not a + rebuild. Filling the default in is what makes that shape unreachable. + + +`contentPublicItemTags` follows the same rule, so a page that tags its own +`fetch` with it cannot disagree with one that used `contentPublicFetch`. ## Caching diff --git a/apps/docs/content/docs/dev/content-engine/localized-search.mdx b/apps/docs/content/docs/dev/content-engine/localized-search.mdx index 9811bc5aa..fbef1d0fa 100644 --- a/apps/docs/content/docs/dev/content-engine/localized-search.mdx +++ b/apps/docs/content/docs/dev/content-engine/localized-search.mdx @@ -72,6 +72,7 @@ await c.get("search").delete("example.article", 7); // every language | What happened | What moves | | --- | --- | | A **translation** was created, edited, published, unpublished or restored | that language's document | +| A **translation** was deleted | that language's document, and no other | | The **record** was published, unpublished or had a shared field edited | every language's document | | The **record** was deleted | every language's document, in one call | @@ -79,6 +80,16 @@ A mutation of the record moves every language because its publication state gate all of them and a shared field is in all of them. A Polish edit moves Polish: nothing else contains it. + + A delete can never enumerate translations - by the time the index is updated, + the rows it would read are gone - so it reads the locale off the mutation + instead. `locale` present means one translation went away and one document goes; + `locale` absent means the record went away and all of them do. Getting this + wrong is not a slower path but a wrong one: deleting the Polish copy would empty + the record out of the index in every language, and only the next rebuild would + put it back. + + The generated routes do this for you. A direct service call does not - it may be inside a transaction that has not committed - so application code opts in after the write returns: diff --git a/apps/docs/content/docs/dev/content-engine/translation-editorial.mdx b/apps/docs/content/docs/dev/content-engine/translation-editorial.mdx index 9ffdde4c3..5881aadff 100644 --- a/apps/docs/content/docs/dev/content-engine/translation-editorial.mdx +++ b/apps/docs/content/docs/dev/content-engine/translation-editorial.mdx @@ -32,6 +32,14 @@ the fields. `publication` gives the record - **and every translation** - a draft/published lifecycle. `editorial` gives every translation its own version and its own [revision history](/docs/dev/content-engine/translation-revisions). + + `publication` alone is a complete configuration. The publish and unpublish + routes are generated from it, the transition is idempotent, and the event fires + exactly as below - what you give up is the history, so no revision is written + and no `revisionId` appears in the payload. Publishing is moving a status; + recording what the values were is a separate thing to want. + + ## Two levels, one rule ```text @@ -190,6 +198,9 @@ POST /{id}/translations/{locale}/publish can_publish POST /{id}/translations/{locale}/unpublish can_publish ``` +Generated from `publication`, not from `editorial` - with history they also write +a revision, without it they simply do not. + Both take `{ "expectedVersion": 3 }` and answer `{ "changed": true, "row": { … } }`. A stale version comes back as the same structured 409 every translation route uses, with `locale` in every arm - which diff --git a/apps/docs/content/docs/dev/content-engine/translation-revisions.mdx b/apps/docs/content/docs/dev/content-engine/translation-revisions.mdx index b4d52b0ff..0e5ec8a85 100644 --- a/apps/docs/content/docs/dev/content-engine/translation-revisions.mdx +++ b/apps/docs/content/docs/dev/content-engine/translation-revisions.mdx @@ -93,6 +93,29 @@ The `delete` revision is stamped at `version + 1` because the row is gone and nothing holds the old version any more - recording it again would collide with the revision that last wrote it. +## Deleting a locale does not reset it + +A translation row is removed physically; its history is not. So a locale can be +recreated on top of revisions that already exist, and a fresh row that started +at version 1 again would collide with the `create` revision its first life wrote. + +The version a recreated translation starts at is read off the locale's own +history instead, inside the same transaction as the insert: + +```text +create v1 +update v2 +delete v3 (the row goes; the revision stays) +recreate v4 ← not 1 +update v5 +``` + +One increasing sequence per `(content type, record, language)`, whatever happened +to the row in between - so the history reads as a story rather than as two +overlapping ones, and the unique index never has to be relaxed to allow it. +Another language's counter is untouched: English staying at 1 while Polish runs +to 5 is normal. + Pass `pluginId` and an `actor` to `localizedService.create` and the default translation gets its own `create` revision, in the same transaction as the row. diff --git a/apps/docs/content/docs/dev/content-engine/translation-service.mdx b/apps/docs/content/docs/dev/content-engine/translation-service.mdx index 06bfef86c..24bf54446 100644 --- a/apps/docs/content/docs/dev/content-engine/translation-service.mdx +++ b/apps/docs/content/docs/dev/content-engine/translation-service.mdx @@ -346,8 +346,9 @@ const language = await resolveContentLanguage(c, { ``` - Case-insensitive, and the **stored** code comes back. -- `requireEnabled` is the write path. Reading a disabled locale is allowed; - writing one is refused. +- `requireEnabled` is the write path: `create`, `update`, `restore` and `publish` + pass it, and `unpublish`, `delete` and every read do not. Adding content to a + language nothing renders is refused; taking it down again is not. - The registry is loaded once per request, so resolving many locales is one query. - Pass `tx` when you are inside a transaction. This is not an optimisation: a pool whose only free connection is held by your transaction would otherwise wait diff --git a/apps/docs/content/docs/dev/search.mdx b/apps/docs/content/docs/dev/search.mdx index fdaef7fe3..804bba7d9 100644 --- a/apps/docs/content/docs/dev/search.mdx +++ b/apps/docs/content/docs/dev/search.mdx @@ -281,6 +281,39 @@ export const vitNodeApiConfig = buildApiConfig({ After changing the engine, run a rebuild so the new engine is populated from `core_search_index`. +### Provider capabilities + +A provider may declare what it can do. Most of it is about ranking and is purely +optional, but one entry is a hard requirement for multi-language content: + +```ts +capabilities: { + authorBoost: false, + facets: false, + languageScopedDelete: true, // [!code highlight] + timeDecay: false, +}, +``` + +`languageScopedDelete` says that `delete(c, itemType, itemId, languageCode)` +honours its fourth argument. It has to be **declared** because JavaScript cannot +tell: a provider written as `delete(c, itemType, itemId)` accepts the extra +argument and drops it, so taking one translation down would remove every language +from that provider's store while `core_search_index` removed one - silently, and +permanently. + +So an install that pairs a provider without it with a content type that is both +[localized](/docs/dev/content-engine/localization) and searchable refuses to boot: + +```text +[Search] The "legacy-engine" search provider does not support language-scoped +deletion, but the content type "example.article" is localized and searchable … +``` + +Both bundled providers declare it. Nothing changes for a provider that indexes +single-language content only - the capability is checked when a content type +needs it, and not otherwise. + ## Admin status **AdminCP → System → Search** shows the active engine, its health, how many items diff --git a/packages/elasticsearch/src/index.ts b/packages/elasticsearch/src/index.ts index 116f2c1eb..31947ba5d 100644 --- a/packages/elasticsearch/src/index.ts +++ b/packages/elasticsearch/src/index.ts @@ -284,7 +284,15 @@ export const ElasticsearchSearchAdapter = ( return { name: "elasticsearch", - capabilities: { facets: true, timeDecay: true, authorBoost: true }, + // `languageScopedDelete`: `delete` below filters the delete-by-query on + // `languageCode` when it is given one, so taking the Polish translation down + // leaves the English document in place. + capabilities: { + facets: true, + timeDecay: true, + authorBoost: true, + languageScopedDelete: true, + }, index: async (_c, doc) => { await ensureIndex(); diff --git a/packages/vitnode/src/api/adapters/search/postgres.ts b/packages/vitnode/src/api/adapters/search/postgres.ts index 9f8f3ec43..88193f36f 100644 --- a/packages/vitnode/src/api/adapters/search/postgres.ts +++ b/packages/vitnode/src/api/adapters/search/postgres.ts @@ -75,7 +75,15 @@ const buildFilters = (params: SearchQueryParams): SQL | undefined => { export const PostgresSearchAdapter = (): SearchProviderApiPlugin => ({ name: "postgres", - capabilities: { authorBoost: false, facets: false, timeDecay: false }, + // `languageScopedDelete` is true by construction: this provider's store *is* + // `core_search_index`, and `SearchModel.delete` already narrows that table by + // `languageCode` before it gets here. There is no second copy to keep in step. + capabilities: { + authorBoost: false, + facets: false, + languageScopedDelete: true, + timeDecay: false, + }, // The SearchModel owns the canonical `core_search_index` table, which is this // provider's store, so the write methods are intentionally no-ops. diff --git a/packages/vitnode/src/api/middlewares/global.middleware.ts b/packages/vitnode/src/api/middlewares/global.middleware.ts index 39ac34e98..e46235cad 100644 --- a/packages/vitnode/src/api/middlewares/global.middleware.ts +++ b/packages/vitnode/src/api/middlewares/global.middleware.ts @@ -17,7 +17,11 @@ import { EmailModel } from "@/api/models/email"; import { EventsModel } from "@/api/models/events"; import { I18nModel } from "@/api/models/i18n"; import { QueueModel } from "@/api/models/queue"; -import { SearchModel, validateSearchIndexers } from "@/api/models/search"; +import { + assertSearchProviderCapabilities, + SearchModel, + validateSearchIndexers, +} from "@/api/models/search"; import { SessionModel } from "@/api/models/session"; import { SessionAdminModel } from "@/api/models/session-admin"; import { StorageModel } from "@/api/models/storage"; @@ -279,6 +283,24 @@ export const globalMiddleware = ({ entry => entry.definition.localization.enabled, ); + // Resolved once rather than per request - the adapter cannot change between + // them - which is also what makes the capability check below a boot-time fact. + const searchAdapter = search?.adapter ?? PostgresSearchAdapter(); + + // A localized searchable content type is indexed once per translation, so + // taking one translation down has to remove one document. A provider that + // ignores the language would remove them all and say nothing, so the pairing + // is refused here rather than discovered by whoever deletes a translation. + assertSearchProviderCapabilities(searchAdapter, { + localizedSearchContentTypes: contentTypesMetadata + .filter( + entry => + entry.definition.localization.enabled && + entry.definition.search.enabled, + ) + .map(entry => entry.definition.id), + }); + const permissionStaffMetadata: PermissionStaffCatalogEntry[] = plugins.map( plugin => ({ pluginId: plugin.pluginId, @@ -342,7 +364,7 @@ export const globalMiddleware = ({ adapter: events?.adapter ?? LocalEventsAdapter(), listeners: eventsMetadata, }, - search: { adapter: search?.adapter ?? PostgresSearchAdapter() }, + search: { adapter: searchAdapter }, searchIndexers: searchIndexersMetadata, storage, authorization: { diff --git a/packages/vitnode/src/api/models/search.test.ts b/packages/vitnode/src/api/models/search.test.ts index 6edec154c..c38cfe079 100644 --- a/packages/vitnode/src/api/models/search.test.ts +++ b/packages/vitnode/src/api/models/search.test.ts @@ -7,7 +7,11 @@ import { core_search_index } from "@/database/search"; import type { SearchDocument, SearchProviderApiPlugin } from "./search"; -import { normalizeSearchIndexerPage, SearchModel } from "./search"; +import { + assertSearchProviderCapabilities, + normalizeSearchIndexerPage, + SearchModel, +} from "./search"; const createProvider = (): SearchProviderApiPlugin => ({ name: "postgres", @@ -270,3 +274,105 @@ describe("normalizeSearchIndexerPage", () => { }); }); }); + +/** + * The one provider capability that is not a nicety. + * + * `delete(c, itemType, itemId, languageCode)` is a JavaScript call: a provider + * written before per-locale content accepts the fourth argument and drops it, so + * taking one translation down removes every language from that provider's store + * while the canonical `core_search_index` removes one. Nothing throws, nothing is + * logged, and the two disagree from then on - which is why the pairing is refused + * at boot instead of being discovered by whoever deletes a translation. + */ +describe("assertSearchProviderCapabilities", () => { + const legacy = (): SearchProviderApiPlugin => ({ + ...createProvider(), + name: "legacy-engine", + }); + + const scoped = (): SearchProviderApiPlugin => ({ + ...createProvider(), + name: "scoped-engine", + capabilities: { + authorBoost: false, + facets: false, + languageScopedDelete: true, + timeDecay: false, + }, + }); + + it("allows a provider that declares nothing when nothing is localized", () => { + expect(() => + assertSearchProviderCapabilities(legacy(), { + localizedSearchContentTypes: [], + }), + ).not.toThrow(); + }); + + it("refuses a provider that cannot scope a delete to one language", () => { + expect(() => + assertSearchProviderCapabilities(legacy(), { + localizedSearchContentTypes: ["example.article"], + }), + ).toThrow(/legacy-engine/); + }); + + it("names the content type and the missing capability", () => { + // A boot failure is only useful if it says what to change. + expect(() => + assertSearchProviderCapabilities(legacy(), { + localizedSearchContentTypes: ["example.article"], + }), + ).toThrow(/example\.article/); + expect(() => + assertSearchProviderCapabilities(legacy(), { + localizedSearchContentTypes: ["example.article"], + }), + ).toThrow(/languageScopedDelete/); + }); + + it("refuses a provider that declares the other capabilities but not this one", () => { + // Declaring `capabilities` is not the same as declaring this capability. + const partial: SearchProviderApiPlugin = { + ...createProvider(), + name: "facets-only", + capabilities: { authorBoost: true, facets: true, timeDecay: true }, + }; + + expect(() => + assertSearchProviderCapabilities(partial, { + localizedSearchContentTypes: ["example.article"], + }), + ).toThrow(/facets-only/); + }); + + it("allows a provider that declares it", () => { + expect(() => + assertSearchProviderCapabilities(scoped(), { + localizedSearchContentTypes: ["example.article", "example.page"], + }), + ).not.toThrow(); + }); + + it("lists every offending content type, not just the first", () => { + expect(() => + assertSearchProviderCapabilities(legacy(), { + localizedSearchContentTypes: ["example.article", "example.page"], + }), + ).toThrow(/example\.article", "example\.page/); + }); + + it("says yes to the bundled Postgres provider", async () => { + // Its store *is* `core_search_index`, which `SearchModel.delete` already + // narrows by language before the provider is reached. + const { PostgresSearchAdapter } = + await import("@/api/adapters/search/postgres"); + + expect(() => + assertSearchProviderCapabilities(PostgresSearchAdapter(), { + localizedSearchContentTypes: ["example.article"], + }), + ).not.toThrow(); + }); +}); diff --git a/packages/vitnode/src/api/models/search.ts b/packages/vitnode/src/api/models/search.ts index fa427fa8a..cdd0e98e6 100644 --- a/packages/vitnode/src/api/models/search.ts +++ b/packages/vitnode/src/api/models/search.ts @@ -87,6 +87,22 @@ export interface SearchResult { export interface SearchProviderCapabilities { authorBoost: boolean; facets: boolean; + /** + * Whether {@link SearchProviderApiPlugin.delete} honours its `languageCode`. + * + * Declared rather than inferred, because JavaScript cannot tell the difference: + * a provider written as `delete(c, itemType, itemId)` accepts the fourth + * argument at runtime and silently ignores it, so taking down one translation + * would remove every language from that provider's store while the canonical + * `core_search_index` removed one. The two would then disagree forever, and + * nothing would say so. + * + * Optional, so a provider written before localized content still compiles and + * still serves single-language content. Absent means "no", and + * {@link assertSearchProviderCapabilities} refuses to boot an install that + * pairs such a provider with a localized searchable content type. + */ + languageScopedDelete?: boolean; timeDecay: boolean; } @@ -219,6 +235,45 @@ export const validateSearchIndexers = ( return [...indexers]; }; +/** + * Refuses to boot a provider that cannot express what the installed content + * types need. + * + * Only one requirement so far, and it is narrow on purpose: a content type that + * is both localized and searchable is indexed once per published translation, so + * unpublishing or deleting one of them has to remove exactly one document. A + * provider that ignores `languageCode` would take every language out instead, and + * because the extra argument is simply dropped there is no error, no log line and + * no way to notice until somebody searches for content that should still be + * there. + * + * Fails at boot rather than at the delete for the obvious reason: the delete is + * the moment the damage happens, and by then the install has been running. + * + * Content types are passed as plain ids so this stays where the rest of the + * search contract lives, with no dependency on the Content Engine. + */ +export const assertSearchProviderCapabilities = ( + provider: SearchProviderApiPlugin, + { + localizedSearchContentTypes, + }: { + /** Ids of content types indexed once per translation. */ + localizedSearchContentTypes: readonly string[]; + }, +): void => { + if (localizedSearchContentTypes.length === 0) return; + if (provider.capabilities?.languageScopedDelete === true) return; + + throw new Error( + `[Search] The "${provider.name}" search provider does not support language-scoped deletion, but ${localizedSearchContentTypes.length === 1 ? "the content type" : "the content types"} ${localizedSearchContentTypes + .map(id => `"${id}"`) + .join( + ", ", + )} ${localizedSearchContentTypes.length === 1 ? "is" : "are"} localized and searchable - each publishes one search document per translation. Taking one translation down must remove one document, and a provider that ignores the "languageCode" argument of "delete" would remove every language instead. Declare "capabilities: { languageScopedDelete: true }" on the provider once its "delete" honours that argument, or turn "search" off for ${localizedSearchContentTypes.length === 1 ? "that content type" : "those content types"}.`, + ); +}; + /** * A pluggable search engine. The {@link SearchModel} owns the canonical * `core_search_index` table for every provider, so a provider that queries that @@ -238,9 +293,12 @@ export interface SearchProviderApiPlugin { * take the English one out of the index. Omit it and every language goes, which * is what deleting the record itself means. * - * Optional on purpose - a provider written before per-locale content simply - * ignores it and keeps removing every variant, which is wrong in only one - * direction and never leaves a document behind. + * Optional on the signature so a provider written before per-locale content + * still compiles - but ignoring it is **not** silently tolerated. A provider + * that honours it says so with + * `capabilities: { languageScopedDelete: true }`, and an install that pairs one + * that does not with a localized searchable content type refuses to boot. See + * {@link assertSearchProviderCapabilities}. */ delete: ( c: Context, diff --git a/packages/vitnode/src/content/next/fetch.server.test.ts b/packages/vitnode/src/content/next/fetch.server.test.ts index 9ed21da4d..044ff5980 100644 --- a/packages/vitnode/src/content/next/fetch.server.test.ts +++ b/packages/vitnode/src/content/next/fetch.server.test.ts @@ -1,12 +1,16 @@ // @vitest-environment node import { beforeEach, describe, expect, it, vi } from "vitest"; -import { testPostContentType } from "@/tests/content-fixtures"; +import { + testLocalizedPageContentType, + testPostContentType, +} from "@/tests/content-fixtures"; interface FetchArgs { module: string; options?: { cache?: string; next?: { tags?: string[] } }; path: string; + query?: Record; } const calls = vi.hoisted(() => [] as FetchArgs[]); @@ -25,7 +29,10 @@ vi.mock("../../lib/fetcher/raw", () => ({ }, })); -const { contentPublicFetch } = await import("./fetch.server"); +const { contentInvalidationTags, contentPublicListTag } = + await import("../cache"); +const { contentPublicFetch, contentPublicItemTags } = + await import("./fetch.server"); const LIST_TAG = "content:test.post:list"; @@ -101,3 +108,145 @@ describe("path", () => { expect((await fetchOnce("a/b")).module).toBe("content/posts"); }); }); + +/** + * A localized public response has a language whether the caller named one or + * not, so its cache identity has to have the same one. + * + * The failure this guards against is silent and permanent: omitting `locale` + * used to tag the response `content:x:list` while the API resolved the default + * locale and returned English. Every invalidation path names the locale-aware + * tag, so nothing would ever expire that entry - not a translation publish, not + * an edit of the English copy, not a rebuild. + */ +describe("a localized content type", () => { + const localizedFetch = async ({ + locale, + slug, + }: { locale?: string; slug?: string } = {}) => { + await contentPublicFetch({ + definition: testLocalizedPageContentType, + locale, + pluginId: "@vitnode/example", + slug, + }); + + const call = calls.at(-1); + if (!call) throw new Error("Expected a request."); + + return call; + }; + + it("sends and tags the locale it was given", async () => { + const call = await localizedFetch({ locale: "pl" }); + + expect(call.query).toMatchObject({ locale: "pl" }); + expect(call.options?.next?.tags).toEqual([ + "content:test.localized-page:list:pl", + ]); + }); + + it("fills in the default locale when the caller omits one", async () => { + const call = await localizedFetch(); + + // Both, from the same value: the API would have resolved `en` anyway, and + // the point is that the tag says so too. + expect(call.query).toMatchObject({ locale: "en" }); + expect(call.options?.next?.tags).toEqual([ + "content:test.localized-page:list:en", + ]); + }); + + it("never produces a locale-less tag, on either route", async () => { + const list = await localizedFetch(); + const detail = await localizedFetch({ slug: "about" }); + + for (const tag of [ + ...(list.options?.next?.tags ?? []), + ...(detail.options?.next?.tags ?? []), + ]) { + // Four segments, always. `content:x:list` and `content:x:slug:about` are + // the shapes a translation publish can never reach. + expect(tag.split(":").length).toBeGreaterThanOrEqual(4); + } + }); + + it("tags a detail fetch per language, so two locales cannot share one", async () => { + const english = await localizedFetch({ locale: "en", slug: "about" }); + const polish = await localizedFetch({ locale: "pl", slug: "about" }); + + // The same slug in two languages is the ordinary case, not the odd one. + expect(english.options?.next?.tags).toEqual([ + "content:test.localized-page:slug:en:about", + ]); + expect(polish.options?.next?.tags).toEqual([ + "content:test.localized-page:slug:pl:about", + ]); + }); + + it("treats a blank locale as an omitted one", async () => { + const call = await localizedFetch({ locale: " " }); + + // `""` would drop the segment inside the tag builder and produce exactly the + // locale-less tag this whole rule exists to prevent. + expect(call.query).toMatchObject({ locale: "en" }); + expect(call.options?.next?.tags).toEqual([ + "content:test.localized-page:list:en", + ]); + }); + + it("produces tags a default-locale translation publish actually expires", () => { + // The loop this whole rule closes. Publishing the English translation + // invalidates the tags `contentInvalidationTags` names; a page fetched + // without a `locale` has to be tagged with those same strings, or it stays + // stale until something evicts it - which nothing would. + const expired = contentInvalidationTags({ + contentTypeId: testLocalizedPageContentType.id, + id: 7, + isPublic: true, + locales: [ + { isPublic: true, locale: "en", slugs: ["about"], wasPublic: true }, + ], + slugs: ["about"], + wasPublic: true, + }); + + expect(expired).toContain( + contentPublicListTag(testLocalizedPageContentType.id, "en"), + ); + expect(expired).toEqual( + expect.arrayContaining( + contentPublicItemTags(testLocalizedPageContentType, 7), + ), + ); + expect(expired).toContain("content:test.localized-page:slug:en:about"); + }); + + it("agrees with `contentPublicItemTags`, given or omitted", () => { + expect(contentPublicItemTags(testLocalizedPageContentType, 7)).toEqual([ + "content:test.localized-page:item:en:7", + ]); + expect( + contentPublicItemTags(testLocalizedPageContentType, 7, "pl"), + ).toEqual(["content:test.localized-page:item:pl:7"]); + }); + + it("leaves a content type without localization exactly as it was", () => { + // Passing a locale to something that has none must not invent a segment: + // every Stage 1-4 tag is byte-identical to what it has always been. + expect(contentPublicItemTags(testPostContentType, 7, "pl")).toEqual([ + "content:test.post:item:7", + ]); + }); + + it("sends no locale for a content type without localization", async () => { + await contentPublicFetch({ + definition: testPostContentType, + locale: "pl", + pluginId: "@vitnode/example", + }); + + expect(calls.at(-1)?.query).toBeUndefined(); + expect(calls.at(-1)?.options?.next?.tags).toEqual([LIST_TAG]); + }); +}); diff --git a/packages/vitnode/src/content/next/fetch.server.ts b/packages/vitnode/src/content/next/fetch.server.ts index e26ccfce4..0471daa4a 100644 --- a/packages/vitnode/src/content/next/fetch.server.ts +++ b/packages/vitnode/src/content/next/fetch.server.ts @@ -19,6 +19,32 @@ export interface ContentPublicFetchResult { status: number; } +/** + * The locale a cached public response is actually in. + * + * A localized content type has no locale-less public response: omit `locale` and + * the API resolves the content type's `defaultLocale`, so a tag built from the + * raw argument would name a page that does not exist while holding the default + * language's content. Nothing would ever expire it - translation invalidation + * targets the locale-aware tag - and the staleness would be permanent. + * + * So the substitution happens once, here, and the result is used for the query + * *and* the tags. A content type that is not localized gets `undefined` and every + * tag it has ever produced is byte-identical. + */ +const contentEffectiveLocale = ( + definition: AnyContentTypeDefinition, + locale: string | undefined, +): string | undefined => { + if (!definition.localization.enabled) return undefined; + + const trimmed = locale?.trim(); + + return trimmed === undefined || trimmed === "" + ? definition.localization.defaultLocale + : trimmed; +}; + /** * Reads the generated public API from a server component, cached and tagged. * @@ -52,9 +78,13 @@ export interface ContentPublicFetchResult { * languages would make every publish a site-wide invalidation *and* let one * language's cached response be served under another's tag. * - * Omitting it on a localized content type is not an error - the API falls back to - * the content type's default locale - but the response is then tagged as though it - * were locale-less, so a translation publish will not expire it. Pass it. + * Omitting it on a localized content type is not an error and not a trap either: + * the content type's `defaultLocale` is filled in here, and it goes to *both* the + * query and the tags. That is the whole reason the substitution happens in this + * function rather than being left to the API - the API would resolve the same + * language, but the tags would already have been built without one, and a response + * holding default-locale content under a locale-less tag is a page no translation + * publish can ever expire. */ export const contentPublicFetch = async ({ definition, @@ -74,10 +104,11 @@ export const contentPublicFetch = async ({ slug?: string; }): Promise>> => { const contentTypeId = definition.id; + const effectiveLocale = contentEffectiveLocale(definition, locale); const tags = slug === undefined - ? [contentPublicListTag(contentTypeId, locale)] - : [contentPublicSlugTag(contentTypeId, slug, locale)]; + ? [contentPublicListTag(contentTypeId, effectiveLocale)] + : [contentPublicSlugTag(contentTypeId, slug, effectiveLocale)]; const response = await rawApiFetch({ method: "get", @@ -97,7 +128,12 @@ export const contentPublicFetch = async ({ pluginId, // Explicit, and last, so a caller cannot accidentally shadow it with a // `locale` of its own in `query` and read one language under another's tag. - query: locale === undefined ? query : { ...query, locale }, + // The same value the tags were built from, which is the invariant this + // helper exists to hold: cache identity and response language never disagree. + query: + effectiveLocale === undefined + ? query + : { ...query, locale: effectiveLocale }, }); if (!response.ok) return { status: response.status }; @@ -190,10 +226,18 @@ export const contentPreviewFetch = async ({ * `locale` for a localized content type, for the same reason * {@link contentPublicFetch} takes one: the record has a page per language, and a * tag that named only the record would make one language's publish expire them - * all. + * all. Omitting it on a localized content type resolves the `defaultLocale` + * rather than dropping the segment, so this and `contentPublicFetch` cannot + * disagree about what an untagged read was. */ export const contentPublicItemTags = ( definition: AnyContentTypeDefinition, id: number, locale?: string, -): string[] => [contentPublicItemTag(definition.id, id, locale)]; +): string[] => [ + contentPublicItemTag( + definition.id, + id, + contentEffectiveLocale(definition, locale), + ), +]; diff --git a/packages/vitnode/src/content/server/revisions-model.ts b/packages/vitnode/src/content/server/revisions-model.ts index 430a63296..0af5d39d4 100644 --- a/packages/vitnode/src/content/server/revisions-model.ts +++ b/packages/vitnode/src/content/server/revisions-model.ts @@ -46,7 +46,18 @@ export interface ContentRevisionsModel { revisionId: number, tx?: ContentDatabase, ) => Promise | null>; - latest: (itemId: number) => Promise; + /** + * The newest revision in this scope, or `null` when there is no history. + * + * Takes an optional transaction for the same reason `findById` does: a caller + * deciding what version a write should start at has to read the history from + * inside the transaction that write is in, or it reads a number another writer + * is about to take. + */ + latest: ( + itemId: number, + tx?: ContentDatabase, + ) => Promise; /** Newest first. Metadata only - a snapshot is loaded on demand. */ list: ( itemId: number, @@ -186,9 +197,8 @@ export const createContentRevisionsModel = < return row ? (row as ContentRevisionDetail) : null; }, - latest: async itemId => { - const [row] = await c - .get("db") + latest: async (itemId, tx) => { + const [row] = await (tx ?? c.get("db")) .select(metaSelection) .from(core_content_revisions) .leftJoin( diff --git a/packages/vitnode/src/content/server/search-sync.localized.test.ts b/packages/vitnode/src/content/server/search-sync.localized.test.ts new file mode 100644 index 000000000..6e8afdc1a --- /dev/null +++ b/packages/vitnode/src/content/server/search-sync.localized.test.ts @@ -0,0 +1,179 @@ +// @vitest-environment node +import type { Context } from "hono"; + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { + testLocalizedSearchPageContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { createContentModel } from "./model"; +import { syncContentLocalizedSearch } from "./search-sync"; + +const model = createContentModel(testLocalizedSearchPageContentType); +const plain = createContentModel(testSearchablePostContentType); + +const search = { + delete: vi.fn(async () => await Promise.resolve()), + index: vi.fn(async () => await Promise.resolve()), +}; + +const logged: string[] = []; + +const context = () => + ({ + get: (key: string) => { + if (key === "search") return search; + if (key === "log") { + return { + error: async (message: string) => { + logged.push(message); + + return await Promise.resolve(); + }, + }; + } + + throw new Error( + `A delete must not read "${key}" - the rows it would read are gone.`, + ); + }, + }) as unknown as Context; + +const row = { + featured: false, + id: 7, + publishedAt: new Date("2026-01-01T00:00:00Z"), + status: "published", +}; + +beforeEach(() => { + search.delete.mockClear(); + search.index.mockClear(); + logged.length = 0; +}); + +/** + * The two deletes a localized content type has, and they are not the same call. + * + * `locale` present means one translation went away; absent means the record did. + * Reading the locale off the input is the whole distinction, and getting it wrong + * is not slower but wrong: deleting the Polish copy would empty the record out of + * the index in every language, and only the next rebuild would put it back. + */ +describe("deleting a translation", () => { + it("removes that language's document and no other", async () => { + const outcomes = await syncContentLocalizedSearch(context(), model, { + locale: "pl", + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + expect(search.delete).toHaveBeenCalledTimes(1); + expect(search.delete).toHaveBeenCalledWith( + "test.localized-search-page", + 7, + "pl", + ); + expect(outcomes).toEqual([ + { + action: "delete", + documentId: "test.localized-search-page:7:pl", + }, + ]); + }); + + it("names the language in the diagnostic id", async () => { + // A log line saying `...:7` after a Polish delete reads as "the record went", + // which is the thing that did not happen. + const [outcome] = await syncContentLocalizedSearch(context(), model, { + locale: "pl", + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + expect(outcome.documentId).toBe("test.localized-search-page:7:pl"); + }); + + it("enumerates nothing, because the translation rows are already gone", async () => { + // The fake context throws on any key but `search` and `log`: a delete that + // tried to read the translation table would be reading rows the mutation has + // just removed. + await expect( + syncContentLocalizedSearch(context(), model, { + locale: "pl", + operation: "delete", + pluginId: "@vitnode/example", + row, + }), + ).resolves.toHaveLength(1); + }); + + it("treats a blank locale as no locale rather than as a language", async () => { + await syncContentLocalizedSearch(context(), model, { + locale: " ", + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + expect(search.delete).toHaveBeenCalledWith( + "test.localized-search-page", + 7, + undefined, + ); + }); +}); + +describe("deleting the record", () => { + it("removes every language in one call", async () => { + const outcomes = await syncContentLocalizedSearch(context(), model, { + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + // No language argument, so every `(itemType, itemId, *)` row goes - which is + // what deleting the record means and is why it cannot enumerate first. + expect(search.delete).toHaveBeenCalledWith( + "test.localized-search-page", + 7, + undefined, + ); + expect(outcomes).toEqual([ + { action: "delete", documentId: "test.localized-search-page:7" }, + ]); + }); + + it("keeps the mutation successful when the engine throws", async () => { + search.delete.mockRejectedValueOnce(new Error("engine unavailable")); + + const [outcome] = await syncContentLocalizedSearch(context(), model, { + locale: "pl", + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + expect(outcome.error?.message).toBe("engine unavailable"); + expect(logged).toHaveLength(1); + expect(logged[0]).toContain("test.localized-search-page:7:pl"); + }); +}); + +describe("a content type that is not localized", () => { + it("is never routed through the localized sync", async () => { + const outcomes = await syncContentLocalizedSearch(context(), plain, { + locale: "pl", + operation: "delete", + pluginId: "@vitnode/example", + row, + }); + + expect(outcomes).toEqual([]); + expect(search.delete).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/vitnode/src/content/server/search-sync.ts b/packages/vitnode/src/content/server/search-sync.ts index 138643190..c2cdb47c8 100644 --- a/packages/vitnode/src/content/server/search-sync.ts +++ b/packages/vitnode/src/content/server/search-sync.ts @@ -211,6 +211,9 @@ export interface ContentLocalizedSearchSyncInput { * Omitted for a mutation of the *record* - publishing it, editing a shared * field - which changes every language's document at once, because every one of * them is built from that row. + * + * On `delete` it is the difference between "this translation went away" and + * "the record went away", which is one document against all of them. */ locale?: string; operation: ContentSearchOperation; @@ -291,14 +294,25 @@ export const syncContentLocalizedSearch = async ( return []; } - // The record is gone, so every language's document is too. One call rather - // than one per locale: there is nothing left to enumerate them from. + // A delete never enumerates translations: by the time this runs the row it + // would read is gone. `locale` is the whole difference between the two kinds + // of delete, and getting it wrong is not a slow path but a wrong one. + // + // locale present - one *translation* was deleted -> one document + // locale absent - the *record* was deleted -> every document + // + // Deleting the Polish translation must leave the English document exactly + // where it is; omitting the language here would empty the whole record out of + // the index and only the next rebuild would notice. if (input.operation === "delete") { + const locale = input.locale?.trim(); + const scoped = locale === undefined || locale === "" ? undefined : locale; + return [ await write(c, definition, { - documentId: contentSearchDocumentId(definition, itemId), + documentId: contentSearchDocumentId(definition, itemId, scoped), run: async () => { - await c.get("search").delete(definition.id, itemId); + await c.get("search").delete(definition.id, itemId, scoped); }, action: "delete", input, diff --git a/packages/vitnode/src/content/server/translation-editorial-service.test.ts b/packages/vitnode/src/content/server/translation-editorial-service.test.ts index 49b36be0c..1c3da36e4 100644 --- a/packages/vitnode/src/content/server/translation-editorial-service.test.ts +++ b/packages/vitnode/src/content/server/translation-editorial-service.test.ts @@ -8,6 +8,7 @@ import type { ContentTranslationModel } from "./translation-model"; import { ContentRevisionNotRestorable } from "../errors"; import { createContentTranslationEditorialService } from "./translation-editorial-service"; +import { CONTENT_TRANSLATION_INITIAL_VERSION } from "./translation-model"; const PLUGIN_ID = "@vitnode/example"; const ACTOR = { type: "staff" as const, userId: 1 }; @@ -27,6 +28,16 @@ let nextRevisionId = 100; let storedRevision: ContentTranslationRevisionSnapshot | null = null; let revisionLanguageId = 1; +/** + * The newest version each locale's history has ever reached. + * + * Keyed by language id, because that is how the history is scoped - and this is + * exactly what `create` has to consult: a translation row is deleted physically + * while its revisions are kept, so a recreated locale that started at 1 again + * would collide with the `create` revision its first life wrote. + */ +const latestVersionByLanguage = new Map(); + // The revisions model is a real, tested unit of its own; what matters here is // *what this layer asks it to write* - which language, which operation, which // snapshot - so it records instead of touching a database. @@ -69,7 +80,11 @@ vi.mock("./revisions-model", () => ({ version: 1, } : null, - latest: () => null, + latest: () => { + const version = latestVersionByLanguage.get(languageId ?? 0); + + return version === undefined ? null : { version }; + }, list: () => ({ edges: [], pageInfo: { endCursor: null, hasNextPage: false }, @@ -147,6 +162,7 @@ beforeEach(() => { nextRevisionId = 100; storedRevision = null; revisionLanguageId = 1; + latestVersionByLanguage.clear(); }); describe("create", () => { @@ -172,6 +188,67 @@ describe("create", () => { }); }); + it("starts a fresh locale at version 1", async () => { + const model = translations(); + model.create.mockResolvedValue(row()); + + await service(model).create(7, "pl", { title: "Witaj" }, { actor: ACTOR }); + + const options = model.create.mock.calls[0][3] as Record; + expect(options[CONTENT_TRANSLATION_INITIAL_VERSION]).toBe(1); + }); + + it("resumes a recreated locale after its last recorded version", async () => { + // The Polish translation was created, edited and deleted: the delete revision + // holds version 3. Recreating it at 1 would collide with the `create` + // revision from its first life, because the history was never removed. + latestVersionByLanguage.set(2, 3); + + const model = translations(); + model.create.mockResolvedValue(row({ version: 4 })); + + const outcome = await service(model).create( + 7, + "pl", + { title: "Witaj" }, + { actor: ACTOR }, + ); + + const options = model.create.mock.calls[0][3] as Record; + expect(options[CONTENT_TRANSLATION_INITIAL_VERSION]).toBe(4); + expect(captured[0]).toMatchObject({ operation: "create", version: 4 }); + expect(outcome.version).toBe(4); + }); + + it("reads the history of the locale being created, not of another", async () => { + // English reached version 9; Polish has never existed. A shared counter would + // start the Polish translation at 10 and leave a hole nothing explains. + latestVersionByLanguage.set(1, 9); + + const model = translations(); + model.create.mockResolvedValue(row()); + + await service(model).create(7, "pl", { title: "Witaj" }, { actor: ACTOR }); + + const options = model.create.mock.calls[0][3] as Record; + expect(options[CONTENT_TRANSLATION_INITIAL_VERSION]).toBe(1); + }); + + it("resolves the language itself, and requires an enabled one", async () => { + const model = translations(); + model.create.mockResolvedValue(row()); + + await service(model).create(7, "PL", { title: "Witaj" }, { actor: ACTOR }); + + // Resolved once, here, and inside the transaction: the history read and the + // insert have to be talking about the same language id, and the read has to + // see what the transaction will. + expect(model.resolveLanguage).toHaveBeenCalledWith("PL", { + requireEnabled: true, + tx: expect.anything(), + }); + }); + it("snapshots the localized fields only", async () => { const model = translations(); model.create.mockResolvedValue(row()); diff --git a/packages/vitnode/src/content/server/translation-editorial-service.ts b/packages/vitnode/src/content/server/translation-editorial-service.ts index a8f3e4073..d3a68b53b 100644 --- a/packages/vitnode/src/content/server/translation-editorial-service.ts +++ b/packages/vitnode/src/content/server/translation-editorial-service.ts @@ -35,6 +35,7 @@ import { } from "./revision-snapshot"; import { createContentRevisionsModel } from "./revisions-model"; import { createSlugNormalizer } from "./slugs"; +import { CONTENT_TRANSLATION_INITIAL_VERSION } from "./translation-model"; /** * Everything the post-commit effects need about one translation mutation. @@ -364,7 +365,22 @@ export const createContentTranslationEditorialService = < return { create: async (itemId, locale, values, options) => await transact(options, async tx => { - const row = await translations.create(itemId, locale, values, { tx }); + // Resolved here, before the write, because the version this translation + // starts at is a fact about *this locale's* history - and history is + // keyed by language id, which only the registry can supply. + const target = await language(locale, { requireEnabled: true, tx }); + + // A translation row is deleted physically; its history is not. Starting + // a recreated locale at 1 would collide with the `create` revision the + // first life wrote, so the new row picks up where the old one left off: + // create 1, update 2, delete 3, recreate 4. Read inside the transaction, + // so the number cannot be taken by another writer in between. + const previous = await revisionsFor(target.id).latest(itemId, tx); + + const row = await translations.create(itemId, target.locale, values, { + [CONTENT_TRANSLATION_INITIAL_VERSION]: (previous?.version ?? 0) + 1, + tx, + }); const revisionId = await capture(tx, { actor: options.actor, diff --git a/packages/vitnode/src/content/server/translation-model.test.ts b/packages/vitnode/src/content/server/translation-model.test.ts index 40636f62d..2c9888239 100644 --- a/packages/vitnode/src/content/server/translation-model.test.ts +++ b/packages/vitnode/src/content/server/translation-model.test.ts @@ -4,7 +4,10 @@ import type { Context } from "hono"; import { describe, expect, it } from "vitest"; import { ZodError } from "zod"; -import { testLocalizedArticleContentType } from "@/tests/content-fixtures"; +import { + testLocalizedArticleContentType, + testLocalizedGuideContentType, +} from "@/tests/content-fixtures"; import { ContentDefaultTranslationRequired, @@ -15,8 +18,10 @@ import { ContentTranslationVersionConflict, } from "../errors"; import { createContentModel } from "./model"; +import { CONTENT_TRANSLATION_INITIAL_VERSION } from "./translation-model"; const localized = createContentModel(testLocalizedArticleContentType); +const withPublication = createContentModel(testLocalizedGuideContentType); const LANGUAGES = [ { code: "en", id: 1, isDefault: true }, @@ -129,6 +134,23 @@ const translations = (c: Context) => { return build(c); }; +/** The same, for the fixture that has `publication` and therefore a status. */ +const publishable = (c: Context) => { + const build = withPublication.translationService; + if (!build) throw new Error("Expected a translation service."); + + return build(c); +}; + +/** A context whose i18n config switches one locale off. */ +const withDisabledLocale = (c: Context, code: string): Context => + ({ + get: (key: string) => + key === "core" + ? { i18n: { locales: [{ code, enabled: false, name: code }] } } + : c.get(key), + }) as unknown as Context; + describe("create", () => { it("writes the resolved language id and starts at version 1", async () => { const { c, calls } = createDbMock([ @@ -151,6 +173,39 @@ describe("create", () => { }); }); + it("writes the version the editorial layer asked for", async () => { + const { c, calls } = createDbMock([ + [{ id: 7 }], + [translationRow({ languageId: 2, version: 4 })], + ]); + + // The Polish translation existed before and was deleted; its history stops at + // 3. Starting again at 1 would collide with the revision its first `create` + // wrote, so the row is inserted at 4 and the sequence keeps going. + const row = await translations(c).create( + 7, + "pl", + { title: "Witaj" }, + { [CONTENT_TRANSLATION_INITIAL_VERSION]: 4 }, + ); + + expect(row.version).toBe(4); + expect(opsOf(calls, "values")[0]).toMatchObject({ version: 4 }); + }); + + it("refuses a version that is not a version", async () => { + const { c } = createDbMock([[{ id: 7 }]]); + + await expect( + translations(c).create( + 7, + "pl", + { title: "Witaj" }, + { [CONTENT_TRANSLATION_INITIAL_VERSION]: 0 }, + ), + ).rejects.toThrow(/versions are integers from 1 upwards/); + }); + it("nests the localized values under `values`", async () => { const { c } = createDbMock([[{ id: 7 }], [translationRow()]]); @@ -371,15 +426,9 @@ describe("update", () => { it("refuses to write into a disabled language", async () => { const { c } = createDbMock([]); - const context = { - get: (key: string) => - key === "core" - ? { i18n: { locales: [{ code: "pl", enabled: false, name: "PL" }] } } - : c.get(key), - } as unknown as Context; await expect( - translations(context).update( + translations(withDisabledLocale(c, "pl")).update( 7, "pl", { title: "Nowy" }, @@ -389,6 +438,71 @@ describe("update", () => { }); }); +/** + * One rule for a language the install has switched off, stated in both + * directions. + * + * Adding content to a locale nothing renders is not useful, so `create`, + * `update` and `publish` all refuse it. Taking content *down* has to keep + * working - an administrator who has just disabled a language usually wants to + * unpublish or delete what is in it, and refusing would strand published pages in + * a locale nobody can edit. So `unpublish`, `delete` and the history reads accept + * it. + */ +describe("a disabled language", () => { + const publishedRow = (overrides: Record = {}) => ({ + ...translationRow({ languageId: 2, version: 2 }), + publishedAt: new Date("2026-01-01T00:00:00Z"), + status: "published", + summary: null, + ...overrides, + }); + + it("cannot be published into", async () => { + const { c, calls } = createDbMock([]); + + await expect( + publishable(withDisabledLocale(c, "pl")).publish(7, "pl"), + ).rejects.toMatchObject({ reason: "disabled" }); + // Refused before the statement, so nothing partially happened. + expect(opsOf(calls, "update")).toEqual([]); + }); + + it("can still be unpublished", async () => { + const { c } = createDbMock([[publishedRow({ status: "draft" })]]); + + await expect( + publishable(withDisabledLocale(c, "pl")).unpublish(7, "pl"), + ).resolves.toMatchObject({ changed: true, row: { locale: "pl" } }); + }); + + it("can still be deleted", async () => { + const { c } = createDbMock([[publishedRow()]]); + + await expect( + publishable(withDisabledLocale(c, "pl")).delete(7, "pl", { + expectedVersion: 2, + }), + ).resolves.toMatchObject({ locale: "pl" }); + }); + + it("can still be read", async () => { + const { c } = createDbMock([[publishedRow()]]); + + await expect( + publishable(withDisabledLocale(c, "pl")).findByLocale(7, "pl"), + ).resolves.toMatchObject({ locale: "pl", version: 2 }); + }); + + it("is published into normally once it is enabled again", async () => { + const { c } = createDbMock([[publishedRow()]]); + + await expect(publishable(c).publish(7, "pl")).resolves.toMatchObject({ + changed: true, + }); + }); +}); + describe("delete", () => { it("guards on the expected version", async () => { const { c } = createDbMock([ diff --git a/packages/vitnode/src/content/server/translation-model.ts b/packages/vitnode/src/content/server/translation-model.ts index 3b13d86ba..e9a73a21f 100644 --- a/packages/vitnode/src/content/server/translation-model.ts +++ b/packages/vitnode/src/content/server/translation-model.ts @@ -42,6 +42,31 @@ export interface ContentTranslationOptions { tx?: ContentDatabase; } +/** + * The version a freshly inserted translation starts at. + * + * A symbol rather than a name, because there is exactly one caller that may set + * it and no way to reach it by accident: writing this key requires importing the + * symbol, which is a deliberate act rather than a plausible typo in an options + * object. Ordinary callers get version 1 and cannot ask for anything else. + * + * It exists because a translation row is deleted physically while its history is + * not. Recreating `(itemId, languageId)` at version 1 would collide with the + * `create` revision the *first* life of that translation wrote, and the locale's + * history would stop being a sequence. The editorial layer reads the last version + * this locale ever reached and starts the new row after it. + * + * @internal + */ +export const CONTENT_TRANSLATION_INITIAL_VERSION: unique symbol = Symbol( + "vitnode.content.translation.initialVersion", +); + +export interface ContentTranslationCreateOptions extends ContentTranslationOptions { + /** @internal Set only by the translation editorial service. */ + [CONTENT_TRANSLATION_INITIAL_VERSION]?: number; +} + export interface ContentTranslationWriteOptions extends ContentTranslationOptions { expectedVersion: number; } @@ -85,12 +110,18 @@ export interface ContentTranslationTransitionResult { * exactly what atomic create needs it to be. */ export interface ContentTranslationModel { - /** Inserts one translation at version 1. Throws if the locale already has one. */ + /** + * Inserts one translation at version 1. Throws if the locale already has one. + * + * The editorial layer may start it later than 1 - see + * {@link CONTENT_TRANSLATION_INITIAL_VERSION} - so a locale that has been + * deleted and recreated keeps one increasing history. + */ create: ( itemId: number, locale: string, values: ContentLocalizedValues, - options?: ContentTranslationOptions, + options?: ContentTranslationCreateOptions, ) => Promise>; /** * Removes one translation, guarded by its version. @@ -133,6 +164,8 @@ export interface ContentTranslationModel { * never rewritten, so a republish keeps the original date. * * Throws without `publication: { enabled: true }`: there is no column to move. + * Refuses a locale the install has switched off, exactly as `create` and + * `update` do - publishing into a language nothing renders is not useful. */ publish: ( itemId: number, @@ -154,7 +187,11 @@ export interface ContentTranslationModel { locale: string, options?: { requireEnabled?: boolean; tx?: ContentDatabase }, ) => Promise; - /** The mirror of {@link publish}. `publishedAt` is deliberately left alone. */ + /** + * The mirror of {@link publish}. `publishedAt` is deliberately left alone, and + * a disabled locale is accepted: taking content down has to keep working after + * a language is switched off. + */ unpublish: ( itemId: number, locale: string, @@ -369,13 +406,18 @@ export const createContentTranslationModel = < itemId: number, locale: string, options: ContentTranslationTransitionOptions, - { guard, values }: { guard: SQL; values: Record }, + { + guard, + requireEnabled, + values, + }: { guard: SQL; requireEnabled: boolean; values: Record }, ): Promise | null> => { const target = await language(locale, { // Publishing into a locale the install has switched off would put content - // on a page nothing renders; taking one down must stay possible, which is - // why only the publish direction is checked - by its own guard, below. - requireEnabled: false, + // on a page nothing renders, so `publish` asks for an enabled one exactly + // as `create` and `update` do. Taking content *down* must stay possible + // after a language is switched off, so `unpublish` does not. + requireEnabled, tx: options.tx, }); const database = db(options); @@ -458,12 +500,28 @@ export const createContentTranslationModel = < const parsed = schemas.create.parse(values) as Record; + // Left to the column default unless the editorial layer named one. A + // nonsensical value is rejected rather than written: the version is what + // the whole optimistic-locking story is built on, and a zero or a fraction + // would break every comparison downstream of it. + const initialVersion = options?.[CONTENT_TRANSLATION_INITIAL_VERSION]; + if ( + initialVersion !== undefined && + (!Number.isInteger(initialVersion) || initialVersion < 1) + ) { + throw new ContentEngineError( + `A translation of ${itemId} cannot start at version ${String(initialVersion)} - versions are integers from 1 upwards.`, + { contentTypeId }, + ); + } + const [row] = await database .insert(translationTable) .values({ ...withCreateSlugs(parsed), itemId, languageId: target.id, + ...(initialVersion === undefined ? {} : { version: initialVersion }), }) // Targeted at the primary key only, so "this locale already has a // translation" comes back as a row this can look up and name, while a @@ -585,6 +643,7 @@ export const createContentTranslationModel = < // COALESCE, so a republish keeps the date this language first went out. // The base row's publish does the same thing to the same effect. guard: ne(statusColumn(), "published"), + requireEnabled: true, values: { publishedAt: sql`coalesce(${columns.publishedAt}, now())`, status: "published", @@ -606,6 +665,10 @@ export const createContentTranslationModel = < unpublish: async (itemId, locale, options) => await transition(itemId, locale, options ?? {}, { guard: eq(statusColumn(), "published"), + // Deliberately not `requireEnabled`: an administrator switching a + // language off next wants to take its pages down, and refusing that + // would leave published content in a locale nobody can edit. + requireEnabled: false, // `publishedAt` survives on purpose: it records when this language was // first published, which stays true after it is taken down again. values: { status: "draft" }, diff --git a/packages/vitnode/src/content/server/translation-publication-routes.test.ts b/packages/vitnode/src/content/server/translation-publication-routes.test.ts new file mode 100644 index 000000000..b52c4b2a0 --- /dev/null +++ b/packages/vitnode/src/content/server/translation-publication-routes.test.ts @@ -0,0 +1,247 @@ +// @vitest-environment node +import type { MiddlewareHandler } from "hono"; + +import { OpenAPIHono } from "@hono/zod-openapi"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { testStrictLocalizedPageContentType } from "@/tests/content-fixtures"; + +import { createContentModel } from "./model"; +import { buildContentTranslationRoutes } from "./translation-routes"; + +/** + * A localized content type with `publication` and **without** `editorial`. + * + * A legal combination since Stage 5B, and the one that used to have nowhere to + * go: the translation table grew `status` and `publishedAt`, and the routes that + * move them were gated on `editorial` - so the columns existed and nothing + * generated could change them. Publication is independent of revision history, + * and these tests are what says so. + */ +const page = createContentModel(testStrictLocalizedPageContentType); +const PLUGIN_ID = "@vitnode/example"; + +const emitted = vi.fn((_name: string, _payload: unknown) => ({ + failures: [], + listeners: 0, +})); +const permissionChecks: { module: string; permission: string }[] = []; + +vi.mock("../../api/lib/check-staff-permission", () => ({ + assertStaffPermission: async ( + _c: unknown, + args: { module: string; permission: string }, + ) => { + permissionChecks.push({ + module: args.module, + permission: args.permission, + }); + + return await Promise.resolve(); + }, +})); + +const adminUser = { + avatarColor: "000000", + birthday: null, + createdAt: new Date(), + email: "test@test.com", + emailVerified: true, + id: 1, + language: "en", + name: "Test", + nameCode: "test", + newsletter: false, + roleId: 1, +}; + +const row = (overrides: Record = {}) => ({ + createdAt: new Date("2026-01-01T00:00:00Z"), + itemId: 7, + languageId: 2, + locale: "pl", + publishedAt: null, + status: "draft", + updatedAt: new Date("2026-01-01T00:00:00Z"), + values: { featured: false, slug: "witaj", title: "Witaj" }, + version: 1, + ...overrides, +}); + +const harness = () => { + const translations = { + create: vi.fn(), + delete: vi.fn(), + exists: vi.fn(), + findByLanguageId: vi.fn(), + findByLocale: vi.fn(), + findManyForItem: vi.fn(), + publish: vi.fn(), + resolveDefaultLanguage: vi.fn(), + resolveLanguage: vi.fn(), + unpublish: vi.fn(), + update: vi.fn(), + }; + + permissionChecks.length = 0; + vi.spyOn(page, "translationService", "get").mockReturnValue( + () => translations, + ); + + const app = new OpenAPIHono(); + const context: MiddlewareHandler = async (c, next) => { + c.set("admin", { user: adminUser }); + c.set("events", { emit: emitted } as never); + await next(); + }; + app.use("*", context); + + for (const { handler, route } of buildContentTranslationRoutes(page, { + pluginId: PLUGIN_ID, + })) { + app.openapi(route, handler); + } + + return { app, translations }; +}; + +const post = async (app: OpenAPIHono, path: string) => + await app.request(path, { + body: JSON.stringify({ expectedVersion: 1 }), + headers: { "content-type": "application/json" }, + method: "post", + }); + +beforeEach(() => { + vi.restoreAllMocks(); + emitted.mockClear(); +}); + +describe("route registration without editorial", () => { + const paths = () => + buildContentTranslationRoutes(page, { pluginId: PLUGIN_ID }).map( + entry => `${entry.route.method.toUpperCase()} ${entry.route.path}`, + ); + + it("generates publish and unpublish from `publication` alone", () => { + expect(paths()).toEqual( + expect.arrayContaining([ + "POST /{id}/translations/{locale}/publish", + "POST /{id}/translations/{locale}/unpublish", + ]), + ); + }); + + it("generates no history routes", () => { + // Publication moves a status; editorial records what the values were. A + // content type that asked for the first must not be given the second. + expect(paths().some(path => path.includes("revisions"))).toBe(false); + }); + + it("still requires `can_publish` for the transition", async () => { + const { app, translations } = harness(); + translations.publish.mockResolvedValue({ + changed: true, + row: row({ status: "published", version: 2 }), + version: 2, + }); + + await post(app, "/7/translations/pl/publish"); + + expect(permissionChecks).toEqual([ + { module: "test_strict_localized_pages", permission: "can_publish" }, + ]); + }); +}); + +describe("publishing a translation without editorial", () => { + it("moves the status through the repository and announces it once", async () => { + const { app, translations } = harness(); + translations.publish.mockResolvedValue({ + changed: true, + row: row({ status: "published", version: 2 }), + version: 2, + }); + + const response = await post(app, "/7/translations/pl/publish"); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + changed: true, + row: { status: "published", version: 2 }, + }); + expect(translations.publish).toHaveBeenCalledWith(7, "pl", { + expectedVersion: 1, + }); + expect(emitted).toHaveBeenCalledTimes(1); + expect(emitted.mock.calls[0][0]).toBe( + "content.test.strict-localized-page.translation_published", + ); + }); + + it("carries no revision id in the event, because there is no history", async () => { + const { app, translations } = harness(); + translations.publish.mockResolvedValue({ + changed: true, + row: row({ status: "published", version: 2 }), + version: 2, + }); + + await post(app, "/7/translations/pl/publish"); + + // Absent rather than null: a listener checks `"revisionId" in payload` + // before acting on one, and this content type never writes any. + expect(emitted.mock.calls[0][1]).not.toHaveProperty("revisionId"); + expect(emitted.mock.calls[0][1]).toMatchObject({ + contentId: 7, + locale: "pl", + version: 2, + }); + }); + + it("is a true no-op when the translation is already published", async () => { + const { app, translations } = harness(); + translations.publish.mockResolvedValue({ + changed: false, + row: row({ status: "published", version: 2 }), + version: 2, + }); + + const response = await post(app, "/7/translations/pl/publish"); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ changed: false }); + // No version bump, and therefore no event: a double-clicked button must not + // look like two publications. + expect(emitted).not.toHaveBeenCalled(); + }); + + it("unpublishes and announces that too", async () => { + const { app, translations } = harness(); + translations.unpublish.mockResolvedValue({ + changed: true, + row: row({ publishedAt: new Date("2026-01-01T00:00:00Z"), version: 3 }), + version: 3, + }); + + const response = await post(app, "/7/translations/pl/unpublish"); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + changed: true, + row: { status: "draft" }, + }); + expect(emitted).toHaveBeenCalledTimes(1); + expect(emitted.mock.calls[0][0]).toBe( + "content.test.strict-localized-page.translation_unpublished", + ); + }); + + it("is 404 when the locale has no translation", async () => { + const { app, translations } = harness(); + translations.publish.mockResolvedValue(null); + + expect((await post(app, "/7/translations/de/publish")).status).toBe(404); + expect(emitted).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/vitnode/src/content/server/translation-routes.ts b/packages/vitnode/src/content/server/translation-routes.ts index eb6806f65..a924f0951 100644 --- a/packages/vitnode/src/content/server/translation-routes.ts +++ b/packages/vitnode/src/content/server/translation-routes.ts @@ -175,11 +175,12 @@ export const buildContentTranslationRoutes = < /** * Turns a bare repository result into the outcome the effects expect. * - * The path a localized content type **without** `editorial` takes: there is no - * history to write, so there is no revision id - but the event still fires, - * because `translation_created` is gated on localization and not on editorial. - * With `editorial` the service produces a richer outcome itself and this is not - * used. + * The path a localized content type **without** `editorial` takes, for every + * mutation including publish and unpublish: there is no history to write, so + * there is no revision id - but the event still fires, because + * `translation_published` and friends are gated on localization and publication, + * not on editorial. With `editorial` the service produces a richer outcome + * itself and this is not used. */ const plainOutcome = ( operation: ContentTranslationEditorialOutcome["operation"], @@ -435,6 +436,15 @@ export const buildContentTranslationRoutes = < // Lifecycle: only with `publication`, which is what a translation status is // subordinate to. Without it the columns do not exist and there is nothing to // move. + // + // Publication is independent of `editorial`, and these routes are gated on it + // alone: a translation's status column exists because `publication` put it + // there, so requiring history in order to *move* it would give a content type + // lifecycle columns and no generated way to reach them. With `editorial` the + // transition additionally captures a revision, in the same transaction as the + // write; without it, it does not. Everything after the write - the event, the + // search document, the outcome the AdminCP invalidates from - is the same on + // both paths, which is why they meet again at `announce`. // ------------------------------------------------------------------------- const transitionRoute = (action: "publish" | "unpublish") => @@ -474,10 +484,22 @@ export const buildContentTranslationRoutes = < const outcome = await withTranslationHttpErrors( "update", async () => - await editorial(c)[action](id, target, { - actor: resolveContentActor(c), - expectedVersion, - }), + buildEditorial + ? await editorial(c)[action](id, target, { + actor: resolveContentActor(c), + expectedVersion, + }) + : await (async () => { + const result = await translations(c)[action](id, target, { + expectedVersion, + }); + + return result + ? plainOutcome(action, result.row, { + changed: result.changed, + }) + : null; + })(), { contentTypeId: definition.id, itemId: id, locale: target }, ); if (!outcome) { @@ -835,7 +857,7 @@ export const buildContentTranslationRoutes = < create, update, remove, - ...(publication && editorialEnabled + ...(publication ? [transitionRoute("publish"), transitionRoute("unpublish")] : []), ...(editorialEnabled ? [revisionList, revisionDetail, restore] : []), diff --git a/plugins/example/src/content/localized-article.ts b/plugins/example/src/content/localized-article.ts index a11a482d0..1a3f63a08 100644 --- a/plugins/example/src/content/localized-article.ts +++ b/plugins/example/src/content/localized-article.ts @@ -11,12 +11,12 @@ import { defineContentType, field } from "@vitnode/core/content"; * history, and a unique `(languageId, slug)` index so `/en/hello` and `/pl/hello` * can both exist while a second English `hello` is a 409. * - From Stage 5C it is public as well: `publicApi` exposes the localized `title`, + * From Stage 5C it is public as well: `publicApi` exposes the localized `title`, * `slug` and `body` alongside the shared `featured`, and a public read resolves one * language - explicitly, negotiated or the default - with `fallback: "default"` - * serving English to a locale that has no translation of its own. `search` is the - * one thing still refused alongside `localization`; per-locale search documents - * land in Stage 5D. + * serving English to a locale that has no translation of its own. Stage 5D adds + * `search`, which indexes one document per published translation rather than one + * per record. */ export const localizedArticleContentType = defineContentType({ id: "example.localized-article", diff --git a/plugins/example/src/database/postgres.test.ts b/plugins/example/src/database/postgres.test.ts index 79b07fb4d..ed362b710 100644 --- a/plugins/example/src/database/postgres.test.ts +++ b/plugins/example/src/database/postgres.test.ts @@ -2692,9 +2692,17 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { await sql`DELETE FROM "core_languages" WHERE "code" = 'pl'`, ); - // `23503`, not a silent cascade: deleting a language must not quietly + // Refused, not silently cascaded: deleting a language must not quietly // delete every article written in it. - expect(code).toBe("23503"); + // + // Postgres 18 reports an explicit `ON DELETE RESTRICT` as `23001` + // (restrict_violation) where earlier majors reported the generic + // `23503` (foreign_key_violation), so the version decides which one is + // correct rather than the assertion accepting either - "one of these + // two" would still pass if a future major stopped refusing at all. + expect(code).toBe(serverMajor >= 18 ? "23001" : "23503"); + // The rows are what actually matter: the code says how it was refused, + // this says that nothing was lost. expect(await rowsFor(row.id)).toHaveLength(2); }); @@ -2961,10 +2969,12 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { ).rejects.toThrow(/version 1, not 9/); }); - it("still publishes into a locale the app has switched off", async () => { - // `de` is `enabled: false` in this app's config. Taking existing content - // in it out of circulation - and putting it back - has to stay possible; - // what is refused is *growing* content there, which is a create. + it("refuses to publish into a locale the app has switched off", async () => { + // `de` is `enabled: false` in this app's config, and one rule covers a + // disabled language in both directions: nothing new goes *into* it - + // create, update and publish all refuse - and everything can still come + // *out* of it. Publishing into a locale nothing renders would put a page + // on the internet that the app has no route for. const itemId = await twoLocales("Lifecycle Disabled"); await sql` INSERT INTO "example_localized_articles_translations" @@ -2972,9 +2982,63 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { VALUES (${itemId}, 3, 'Deutsch', 'deutsch', 'Deutscher Text') `; - const result = await translations().publish(itemId, "de"); + await expect( + translations().publish(itemId, "de"), + ).rejects.toMatchObject({ reason: "disabled" }); + expect(await statusOf(itemId, 3)).toMatchObject({ status: "draft" }); + }); + + it("still unpublishes and deletes a locale the app has switched off", async () => { + // The other half of the rule, and the reason it is the other half: an + // administrator who has just disabled a language wants to take down what + // is already published in it. Refusing would strand those pages. + const itemId = await twoLocales("Lifecycle Disabled Down"); + await sql` + INSERT INTO "example_localized_articles_translations" + ("itemId", "languageId", "title", "slug", "body", "status", "publishedAt") + VALUES (${itemId}, 3, 'Deutsch', 'deutsch-down', 'Deutscher Text', 'published', now()) + `; + + await expect( + translations().unpublish(itemId, "de"), + ).resolves.toMatchObject({ changed: true }); + expect(await statusOf(itemId, 3)).toMatchObject({ status: "draft" }); - expect(result).toMatchObject({ changed: true }); + await expect( + translations().delete(itemId, "de", { expectedVersion: 2 }), + ).resolves.toMatchObject({ locale: "de" }); + }); + + /** + * The path a content type with `publication` and no `editorial` takes. + * + * The generated publish route calls the repository directly when there is + * no history to write, so this is that orchestration in SQL: the status + * moves, the version moves, an already-published translation is a true + * no-op, and nothing lands in `core_content_revisions`. + */ + it("runs the whole lifecycle without writing any history", async () => { + const itemId = await twoLocales("Lifecycle No History"); + + const published = await translations().publish(itemId, "pl"); + expect(published).toMatchObject({ changed: true, version: 2 }); + + // Idempotent: no second version, and therefore nothing for an event or a + // search write to be triggered by either. + const again = await translations().publish(itemId, "pl"); + expect(again).toMatchObject({ changed: false, version: 2 }); + + const down = await translations().unpublish(itemId, "pl"); + expect(down).toMatchObject({ changed: true, version: 3 }); + expect(await statusOf(itemId, 2)).toMatchObject({ status: "draft" }); + + const [{ count }] = await sql<{ count: number }[]>` + SELECT count(*)::int AS count + FROM "core_content_revisions" + WHERE "contentTypeId" = 'example.localized-article' + AND "itemId" = ${itemId} + `; + expect(count).toBe(0); }); it("carries the lifecycle in the metadata list", async () => { @@ -3293,6 +3357,125 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { }), ).rejects.toThrow(/version 2, not 1/); }); + + /** + * A translation row is deleted physically; its history is not. + * + * So a locale can be recreated on top of revisions that already exist, and + * the version a fresh row starts at cannot be 1: the locale-scoped unique + * index on `(contentTypeId, itemId, languageId, version)` would reject the + * new `create` revision against the old one, and the write would fail with a + * `23505` a translator has no way to act on. The new row picks up where the + * old one left off instead. + */ + it("keeps one increasing history across a delete and a recreate", async () => { + const itemId = await guide("Recreated"); + + const created = await editorial().create( + itemId, + "pl", + { body: "Tresc", title: "Polski" }, + { actor: ACTOR }, + ); + expect(created.version).toBe(1); + + const updated = await editorial().update( + itemId, + "pl", + { title: "Polski Nowy" }, + { actor: ACTOR, expectedVersion: 1 }, + ); + expect(updated?.version).toBe(2); + + // The row held 2; the delete revision records 3, so the number the row + // last held is not reused by the thing that removed it. + const deleted = await editorial().delete(itemId, "pl", { + actor: ACTOR, + expectedVersion: 2, + }); + expect(deleted?.version).toBe(3); + expect((await rowsFor(itemId)).map(item => item.languageId)).toEqual([ + 1, + ]); + + // 4, not 1. This is the write that used to fail. + const recreated = await editorial().create( + itemId, + "pl", + { body: "Znowu", title: "Polski Znowu" }, + { actor: ACTOR }, + ); + expect(recreated.version).toBe(4); + + const again = await editorial().update( + itemId, + "pl", + { title: "Polski Trzeci" }, + { actor: ACTOR, expectedVersion: 4 }, + ); + expect(again?.version).toBe(5); + + // Nothing was pruned to make room, and nothing collided. + expect( + (await revisionsFor(itemId, 2)).map(row => [ + row.version, + row.operation, + ]), + ).toEqual([ + [1, "create"], + [2, "update"], + [3, "delete"], + [4, "create"], + [5, "update"], + ]); + + // And the AdminCP, which reads newest first, sees the same five. + const history = await editorial().listRevisions(itemId, "pl"); + expect(history.edges.map(edge => edge.version)).toEqual([ + 5, 4, 3, 2, 1, + ]); + + // The row itself is at 5, so the next optimistic write asks for 5. + expect( + (await rowsFor(itemId)).find(item => item.languageId === 2)?.version, + ).toBe(5); + }); + + it("leaves another locale's counter alone when one is recreated", async () => { + const itemId = await guide("Recreated Independently"); + + // English is at 1 from the atomic create. Polish runs to 3 and is + // deleted, so its next life starts at 4 - a shared counter would have + // started it at 5 and left a hole nothing explains. + await editorial().create( + itemId, + "pl", + { body: "Tresc", title: "Polski Nieza" }, + { actor: ACTOR }, + ); + await editorial().update( + itemId, + "pl", + { title: "Polski Nieza Nowy" }, + { actor: ACTOR, expectedVersion: 1 }, + ); + await editorial().delete(itemId, "pl", { + actor: ACTOR, + expectedVersion: 2, + }); + + const recreated = await editorial().create( + itemId, + "pl", + { body: "Znowu", title: "Polski Nieza Znowu" }, + { actor: ACTOR }, + ); + + expect(recreated.version).toBe(4); + expect((await revisionsFor(itemId, 1)).map(row => row.version)).toEqual( + [1], + ); + }); }); /** @@ -3892,6 +4075,79 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { }), ]); }); + + it("removes only the deleted translation's document", async () => { + // The blocker this pair guards: a delete cannot enumerate translations + // (the row it would read is gone), so it has to read the locale off the + // input instead. Reading nothing meant deleting every language, and the + // English document would disappear because somebody removed the Polish + // copy. + const itemId = await seed({ + pl: { published: true, title: "Do Usuniecia" }, + title: "Keep English", + }); + const { deleted, indexed, searchContext } = engine(); + + const outcome = await editorial().delete(itemId, "pl", { + actor: ACTOR, + expectedVersion: 2, + }); + if (!outcome) throw new Error("Expected an outcome."); + + const base = await localizedArticleContent + .service(context) + .findById(itemId); + + await syncContentLocalizedSearch( + searchContext, + localizedArticleContent, + { + locale: outcome.locale, + operation: "delete", + pluginId: CONFIG_PLUGIN.pluginId, + row: base as object, + }, + ); + + expect(deleted).toEqual([{ itemId, languageCode: "pl" }]); + expect(indexed).toEqual([]); + + // And the English translation really is still there to be indexed, so + // the next rebuild puts its document back exactly where it was. + const rebuilt = engine(); + await rebuild(rebuilt.searchContext); + expect(rebuilt.indexed.map(document => document.languageCode)).toEqual([ + "en", + ]); + }); + + it("removes every language when the record is deleted", async () => { + const itemId = await seed({ + pl: { published: true, title: "Cala Usunieta" }, + title: "Delete Whole", + }); + const { deleted, searchContext } = engine(); + + const removed = await localizedArticleContent + .service(context) + .delete(itemId); + if (!removed) throw new Error("Expected a deleted row."); + + await syncContentLocalizedSearch( + searchContext, + localizedArticleContent, + { + operation: "delete", + pluginId: CONFIG_PLUGIN.pluginId, + row: removed, + }, + ); + + // One call with no language: every `(itemType, itemId, *)` document + // goes, which is what deleting the record means - and there is nothing + // left to enumerate them from anyway. + expect(deleted).toEqual([{ itemId, languageCode: undefined }]); + }); }); /** Which languages a record is publicly reachable in, evaluated in SQL. */